From 6550bcc7c43308ced61adf2ee7d6d4ae8e74dfee Mon Sep 17 00:00:00 2001 From: Boris Kreitchman Date: Fri, 1 Aug 2025 17:32:29 +0300 Subject: [PATCH 1/2] go/analysis/passes/shadow: add filename if shadowed variable is in other file --- go/analysis/passes/shadow/shadow.go | 23 +++++++++++++++++-- go/analysis/passes/shadow/shadow_test.go | 5 ++++ .../testdata/src/crossfile/crossfile.go | 15 ++++++++++++ .../shadow/testdata/src/crossfile/other.go | 7 ++++++ 4 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 go/analysis/passes/shadow/testdata/src/crossfile/crossfile.go create mode 100644 go/analysis/passes/shadow/testdata/src/crossfile/other.go diff --git a/go/analysis/passes/shadow/shadow.go b/go/analysis/passes/shadow/shadow.go index 8f768bb76c5..afc14f3aab0 100644 --- a/go/analysis/passes/shadow/shadow.go +++ b/go/analysis/passes/shadow/shadow.go @@ -6,9 +6,11 @@ package shadow import ( _ "embed" + "fmt" "go/ast" "go/token" "go/types" + "path/filepath" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -262,7 +264,24 @@ func checkShadowing(pass *analysis.Pass, spans map[types.Object]span, ident *ast } // Don't complain if the types differ: that implies the programmer really wants two different things. if types.Identical(obj.Type(), shadowed.Type()) { - line := pass.Fset.Position(shadowed.Pos()).Line - pass.ReportRangef(ident, "declaration of %q shadows declaration at line %d", obj.Name(), line) + shadowedPos := pass.Fset.Position(shadowed.Pos()) + + // Build the message, adding filename only if in a different file + message := fmt.Sprintf("declaration of %q shadows declaration at line %d", obj.Name(), shadowedPos.Line) + currentFile := pass.Fset.Position(ident.Pos()).Filename + if shadowedPos.Filename != currentFile { + message += fmt.Sprintf(" in %s", filepath.Base(shadowedPos.Filename)) + } + + pass.Report(analysis.Diagnostic{ + Pos: ident.Pos(), + End: ident.End(), + Message: message, + Related: []analysis.RelatedInformation{{ + Pos: shadowed.Pos(), + End: shadowed.Pos() + token.Pos(len(shadowed.Name())), + Message: fmt.Sprintf("shadowed symbol %q declared here", obj.Name()), + }}, + }) } } diff --git a/go/analysis/passes/shadow/shadow_test.go b/go/analysis/passes/shadow/shadow_test.go index 4fcdc922ee5..4fb532f2aae 100644 --- a/go/analysis/passes/shadow/shadow_test.go +++ b/go/analysis/passes/shadow/shadow_test.go @@ -15,3 +15,8 @@ func Test(t *testing.T) { testdata := analysistest.TestData() analysistest.Run(t, testdata, shadow.Analyzer, "a") } + +func TestCrossFile(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, shadow.Analyzer, "crossfile") +} diff --git a/go/analysis/passes/shadow/testdata/src/crossfile/crossfile.go b/go/analysis/passes/shadow/testdata/src/crossfile/crossfile.go new file mode 100644 index 00000000000..e821012787d --- /dev/null +++ b/go/analysis/passes/shadow/testdata/src/crossfile/crossfile.go @@ -0,0 +1,15 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains a test for the shadowed variable checker with cross-file reference. + +package crossfile + +func ShadowGlobal() { + { + global := 1 // want "declaration of .global. shadows declaration at line 7 in other.go" + _ = global + } + _ = global +} diff --git a/go/analysis/passes/shadow/testdata/src/crossfile/other.go b/go/analysis/passes/shadow/testdata/src/crossfile/other.go new file mode 100644 index 00000000000..108060a5437 --- /dev/null +++ b/go/analysis/passes/shadow/testdata/src/crossfile/other.go @@ -0,0 +1,7 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package crossfile + +var global int From 28b865ab55cacab5fdff8fe64ac9ff4bf7cc7999 Mon Sep 17 00:00:00 2001 From: Boris Kreitchman Date: Sat, 2 Aug 2025 12:24:10 +0300 Subject: [PATCH 2/2] fix cross-file --- go/analysis/passes/shadow/shadow.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/go/analysis/passes/shadow/shadow.go b/go/analysis/passes/shadow/shadow.go index afc14f3aab0..2f935b9e91d 100644 --- a/go/analysis/passes/shadow/shadow.go +++ b/go/analysis/passes/shadow/shadow.go @@ -245,6 +245,10 @@ func checkShadowing(pass *analysis.Pass, spans map[types.Object]span, ident *ast if shadowed.Parent() == types.Universe { return } + + shadowedPos := pass.Fset.Position(shadowed.Pos()) + identPos := pass.Fset.Position(ident.Pos()) + if strict { // The shadowed identifier must appear before this one to be an instance of shadowing. if shadowed.Pos() > ident.Pos() { @@ -252,24 +256,23 @@ func checkShadowing(pass *analysis.Pass, spans map[types.Object]span, ident *ast } } else { // Don't complain if the span of validity of the shadowed identifier doesn't include - // the shadowing identifier. + // the shadowing identifier, except for cross-file shadowing where file processing + // order affects span checks. span, ok := spans[shadowed] if !ok { pass.ReportRangef(ident, "internal error: no range for %q", ident.Name) return } - if !span.contains(ident.Pos()) { + + if shadowedPos.Filename == identPos.Filename && !span.contains(ident.Pos()) { return } } // Don't complain if the types differ: that implies the programmer really wants two different things. if types.Identical(obj.Type(), shadowed.Type()) { - shadowedPos := pass.Fset.Position(shadowed.Pos()) - // Build the message, adding filename only if in a different file message := fmt.Sprintf("declaration of %q shadows declaration at line %d", obj.Name(), shadowedPos.Line) - currentFile := pass.Fset.Position(ident.Pos()).Filename - if shadowedPos.Filename != currentFile { + if shadowedPos.Filename != identPos.Filename { message += fmt.Sprintf(" in %s", filepath.Base(shadowedPos.Filename)) }