diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index aa27c1b9df..df79a4419b 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -284,7 +284,7 @@ func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Res testenv.NeedsGoPackages(t) } - pkgs, err := loadPackages(dir, patterns...) + pkgs, err := loadPackages(a, dir, patterns...) if err != nil { t.Errorf("loading %s: %v", patterns, err) return nil @@ -308,7 +308,7 @@ type Result = checker.TestAnalyzerResult // dependencies) from dir, which is the root of a GOPATH-style project // tree. It returns an error if any package had an error, or the pattern // matched no packages. -func loadPackages(dir string, patterns ...string) ([]*packages.Package, error) { +func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*packages.Package, error) { // packages.Load loads the real standard library, not a minimal // fake version, which would be more efficient, especially if we // have many small tests that import, say, net/http. @@ -327,9 +327,13 @@ func loadPackages(dir string, patterns ...string) ([]*packages.Package, error) { return nil, err } - // Print errors but do not stop: - // some Analyzers may be disposed to RunDespiteErrors. - packages.PrintErrors(pkgs) + // Do NOT print errors if the analyzer will continue running. + // It is incredibly confusing for tests to be printing to stderr + // willy-nilly instead of their test logs, especially when the + // errors are expected and are going to be fixed. + if !a.RunDespiteErrors { + packages.PrintErrors(pkgs) + } if len(pkgs) == 0 { return nil, fmt.Errorf("no packages matched %s", patterns) @@ -441,7 +445,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis want[k] = expects return } - unmatched = append(unmatched, fmt.Sprintf("%q", exp.rx)) + unmatched = append(unmatched, fmt.Sprintf("%#q", exp.rx)) } } if unmatched == nil { @@ -505,7 +509,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis var surplus []string for key, expects := range want { for _, exp := range expects { - err := fmt.Sprintf("%s:%d: no %s was reported matching %q", key.file, key.line, exp.kind, exp.rx) + err := fmt.Sprintf("%s:%d: no %s was reported matching %#q", key.file, key.line, exp.kind, exp.rx) surplus = append(surplus, err) } } diff --git a/go/analysis/analysistest/analysistest_test.go b/go/analysis/analysistest/analysistest_test.go index cb9bdd2fd1..8c7ff7363c 100644 --- a/go/analysis/analysistest/analysistest_test.go +++ b/go/analysis/analysistest/analysistest_test.go @@ -134,19 +134,19 @@ func println(...interface{}) { println_TEST_() } // want println:"found" "call o `a/b.go:6: in 'want' comment: got String after foo, want ':'`, `a/b.go:7: in 'want' comment: got EOF, want regular expression`, `a/b.go:8: in 'want' comment: invalid char escape`, - `a/b.go:11:9: diagnostic "call of println(...)" does not match pattern "wrong expectation text"`, + "a/b.go:11:9: diagnostic \"call of println(...)\" does not match pattern `wrong expectation text`", `a/b.go:14:9: unexpected diagnostic: call of println(...)`, - `a/b.go:11: no diagnostic was reported matching "wrong expectation text"`, - `a/b.go:17: no diagnostic was reported matching "unsatisfied expectation"`, + "a/b.go:11: no diagnostic was reported matching `wrong expectation text`", + "a/b.go:17: no diagnostic was reported matching `unsatisfied expectation`", // duplicate copies of each message from the test package (see issue #40574) `a/b.go:5: in 'want' comment: unexpected ":"`, `a/b.go:6: in 'want' comment: got String after foo, want ':'`, `a/b.go:7: in 'want' comment: got EOF, want regular expression`, `a/b.go:8: in 'want' comment: invalid char escape`, - `a/b.go:11:9: diagnostic "call of println(...)" does not match pattern "wrong expectation text"`, + "a/b.go:11:9: diagnostic \"call of println(...)\" does not match pattern `wrong expectation text`", `a/b.go:14:9: unexpected diagnostic: call of println(...)`, - `a/b.go:11: no diagnostic was reported matching "wrong expectation text"`, - `a/b.go:17: no diagnostic was reported matching "unsatisfied expectation"`, + "a/b.go:11: no diagnostic was reported matching `wrong expectation text`", + "a/b.go:17: no diagnostic was reported matching `unsatisfied expectation`", } if !reflect.DeepEqual(got, want) { t.Errorf("got:\n%s\nwant:\n%s", diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index ddb8ffc376..e79f01a1ce 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -612,10 +612,11 @@ will turn into ## **noresultvalues** -suggested fixes for "no result values expected" +suggested fixes for unexpected return values This checker provides suggested fixes for type errors of the -type "no result values expected". For example: +type "no result values expected" or "too many return values". +For example: func z() { return nil } will turn into func z() { return } diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues.go b/internal/lsp/analysis/noresultvalues/noresultvalues.go index 0e6b26f8bd..b9f21f3135 100644 --- a/internal/lsp/analysis/noresultvalues/noresultvalues.go +++ b/internal/lsp/analysis/noresultvalues/noresultvalues.go @@ -10,6 +10,7 @@ import ( "bytes" "go/ast" "go/format" + "strings" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -17,10 +18,11 @@ import ( "golang.org/x/tools/internal/analysisinternal" ) -const Doc = `suggested fixes for "no result values expected" +const Doc = `suggested fixes for unexpected return values This checker provides suggested fixes for type errors of the -type "no result values expected". For example: +type "no result values expected" or "too many return values". +For example: func z() { return nil } will turn into func z() { return } @@ -83,5 +85,6 @@ func run(pass *analysis.Pass) (interface{}, error) { } func FixesError(msg string) bool { - return msg == "no result values expected" + return msg == "no result values expected" || + strings.HasPrefix(msg, "too many return values") && strings.Contains(msg, "want ()") } diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go index 30265a42f2..3daa7f7c76 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go +++ b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go @@ -4,6 +4,6 @@ package noresultvalues -func x() { return nil } // want "no result values expected" +func x() { return nil } // want `no result values expected|too many return values` -func y() { return nil, "hello" } // want "no result values expected" +func y() { return nil, "hello" } // want `no result values expected|too many return values` diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden index 6b29cefa36..5e93aa4135 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden +++ b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden @@ -4,6 +4,6 @@ package noresultvalues -func x() { return } // want "no result values expected" +func x() { return } // want `no result values expected|too many return values` -func y() { return } // want "no result values expected" +func y() { return } // want `no result values expected|too many return values` diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go index d1b2980103..f8aa43665c 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go +++ b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go @@ -2,5 +2,5 @@ package noresult func hello[T any]() { var z T - return z // want "no result values expected" + return z // want `no result values expected|too many return values` } diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden index c5f56a9274..963e3f4e1a 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden +++ b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden @@ -2,5 +2,5 @@ package noresult func hello[T any]() { var z T - return // want "no result values expected" + return // want `no result values expected|too many return values` } diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 50c55199ec..8249fb471b 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -420,7 +420,7 @@ var GeneratedAPIJSON = &APIJSON{ }, { Name: "\"noresultvalues\"", - Doc: "suggested fixes for \"no result values expected\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\". For example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n", + Doc: "suggested fixes for unexpected return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\" or \"too many return values\".\nFor example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n", Default: "true", }, { @@ -932,7 +932,7 @@ var GeneratedAPIJSON = &APIJSON{ }, { Name: "noresultvalues", - Doc: "suggested fixes for \"no result values expected\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\". For example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n", + Doc: "suggested fixes for unexpected return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\" or \"too many return values\".\nFor example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n", Default: true, }, {