Skip to content

Commit 30a2a1c

Browse files
authored
Remove shadow analyzer from TOOLS_NOGO (#4344)
**What type of PR is this?** Other **What does this PR do? Why is it needed?** The shadow analyzer issues many false positives which are not in keeping with Go best practices. Even the upstream documentation for this analyzer admits this and claims that it is experimental. It should not be included in the default list of analyzers provided by nogo (users can still opt-in to it if they want it). **Which issues(s) does this PR fix?** Fixes #4340 **Other notes for review**
1 parent cd29704 commit 30a2a1c

File tree

3 files changed

+18
-17
lines changed

3 files changed

+18
-17
lines changed

go/def.bzl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ _TOOLS_NOGO = [
103103
"@org_golang_x_tools//go/analysis/passes/nilness:go_default_library",
104104
"@org_golang_x_tools//go/analysis/passes/pkgfact:go_default_library",
105105
"@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
106-
"@org_golang_x_tools//go/analysis/passes/shadow:go_default_library",
106+
# shadow analyzer is too noisy, see #4340
107+
# "@org_golang_x_tools//go/analysis/passes/shadow:go_default_library",
107108
"@org_golang_x_tools//go/analysis/passes/shift:go_default_library",
108109
"@org_golang_x_tools//go/analysis/passes/sortslice:go_default_library",
109110
"@org_golang_x_tools//go/analysis/passes/stdmethods:go_default_library",

tests/core/nogo/bzlmod/includes_excludes_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ go_library(
4343
-- lib.go --
4444
package lib
4545
46-
func shadowed() string {
46+
func useless() string {
4747
foo := "original"
4848
if foo == "original" {
49-
foo := "shadow"
49+
foo = foo
5050
return foo
5151
}
5252
return foo
@@ -64,10 +64,10 @@ go_library(
6464
-- go/lib.go --
6565
package lib
6666
67-
func shadowed() string {
67+
func useless() string {
6868
foo := "original"
6969
if foo == "original" {
70-
foo := "shadow"
70+
foo = foo
7171
return foo
7272
}
7373
return foo
@@ -85,10 +85,10 @@ go_library(
8585
-- go/third_party/lib.go --
8686
package lib
8787
88-
func shadowed() string {
88+
func useless() string {
8989
foo := "original"
9090
if foo == "original" {
91-
foo := "shadow"
91+
foo = foo
9292
return foo
9393
}
9494
return foo
@@ -114,8 +114,8 @@ func TestNotIncluded(t *testing.T) {
114114
func TestIncluded(t *testing.T) {
115115
if err := bazel_testing.RunBazel("build", "//go:lib"); err == nil {
116116
t.Fatal("Expected build to fail")
117-
} else if !strings.Contains(err.Error(), "lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)") {
118-
t.Fatalf("Expected error to contain \"lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)\", got %s", err)
117+
} else if !strings.Contains(err.Error(), "lib.go:6:3: self-assignment of foo to foo (assign)") {
118+
t.Fatalf("Expected error to contain \"lib.go:6:3: self-assignment of foo to foo (assign)\", got %s", err)
119119
}
120120
}
121121

tests/core/nogo/includes_excludes/includes_excludes_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ go_library(
4545
-- lib.go --
4646
package lib
4747
48-
func shadowed() string {
48+
func useless() string {
4949
foo := "original"
5050
if foo == "original" {
51-
foo := "shadow"
51+
foo = foo
5252
return foo
5353
}
5454
return foo
@@ -66,10 +66,10 @@ go_library(
6666
-- go/lib.go --
6767
package lib
6868
69-
func shadowed() string {
69+
func useless() string {
7070
foo := "original"
7171
if foo == "original" {
72-
foo := "shadow"
72+
foo = foo
7373
return foo
7474
}
7575
return foo
@@ -87,10 +87,10 @@ go_library(
8787
-- go/third_party/lib.go --
8888
package lib
8989
90-
func shadowed() string {
90+
func useless() string {
9191
foo := "original"
9292
if foo == "original" {
93-
foo := "shadow"
93+
foo = foo
9494
return foo
9595
}
9696
return foo
@@ -108,8 +108,8 @@ func TestNotIncluded(t *testing.T) {
108108
func TestIncluded(t *testing.T) {
109109
if err := bazel_testing.RunBazel("build", "//go:lib"); err == nil {
110110
t.Fatal("Expected build to fail")
111-
} else if !strings.Contains(err.Error(), "lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)") {
112-
t.Fatalf("Expected error to contain \"lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)\", got %s", err)
111+
} else if !strings.Contains(err.Error(), "lib.go:6:3: self-assignment of foo to foo (assign)") {
112+
t.Fatalf("Expected error to contain \"lib.go:6:3: self-assignment of foo to foo (assign)\", got %s", err)
113113
}
114114
}
115115

0 commit comments

Comments
 (0)