diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index d11505a165..44ada22a03 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -11,8 +11,6 @@ import ( "go/token" "go/types" "reflect" - - "golang.org/x/tools/internal/analysisinternal" ) // An Analyzer describes an analysis function and its options. @@ -48,6 +46,7 @@ type Analyzer struct { // RunDespiteErrors allows the driver to invoke // the Run method of this analyzer even on a // package that contains parse or type errors. + // The Pass.TypeErrors field may consequently be non-empty. RunDespiteErrors bool // Requires is a set of analyzers that must run successfully @@ -75,17 +74,6 @@ type Analyzer struct { func (a *Analyzer) String() string { return a.Name } -func init() { - // Set the analysisinternal functions to be able to pass type errors - // to the Pass type without modifying the go/analysis API. - analysisinternal.SetTypeErrors = func(p interface{}, errors []types.Error) { - p.(*Pass).typeErrors = errors - } - analysisinternal.GetTypeErrors = func(p interface{}) []types.Error { - return p.(*Pass).typeErrors - } -} - // A Pass provides information to the Run function that // applies a specific analyzer to a single Go package. // @@ -106,6 +94,7 @@ type Pass struct { Pkg *types.Package // type information about the package TypesInfo *types.Info // type information about the syntax trees TypesSizes types.Sizes // function for computing sizes of types + TypeErrors []types.Error // type errors (only if Analyzer.RunDespiteErrors) // Report reports a Diagnostic, a finding about a specific location // in the analyzed source code such as a potential mistake. diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 2972e44155..cf76bdebe7 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -26,7 +26,6 @@ import ( "runtime/pprof" "runtime/trace" "sort" - "strconv" "strings" "sync" "time" @@ -34,7 +33,6 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/internal/analysisflags" "golang.org/x/tools/go/packages" - "golang.org/x/tools/internal/analysisinternal" ) var ( @@ -699,14 +697,16 @@ func (act *action) execOnce() { // Run the analysis. pass := &analysis.Pass{ - Analyzer: act.a, - Fset: act.pkg.Fset, - Files: act.pkg.Syntax, - OtherFiles: act.pkg.OtherFiles, - IgnoredFiles: act.pkg.IgnoredFiles, - Pkg: act.pkg.Types, - TypesInfo: act.pkg.TypesInfo, - TypesSizes: act.pkg.TypesSizes, + Analyzer: act.a, + Fset: act.pkg.Fset, + Files: act.pkg.Syntax, + OtherFiles: act.pkg.OtherFiles, + IgnoredFiles: act.pkg.IgnoredFiles, + Pkg: act.pkg.Types, + TypesInfo: act.pkg.TypesInfo, + TypesSizes: act.pkg.TypesSizes, + TypeErrors: act.pkg.TypeErrors, + ResultOf: inputs, Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, ImportObjectFact: act.importObjectFact, @@ -718,72 +718,6 @@ func (act *action) execOnce() { } act.pass = pass - var errors []types.Error - // Get any type errors that are attributed to the pkg. - // This is necessary to test analyzers that provide - // suggested fixes for compiler/type errors. - // TODO(adonovan): eliminate this hack; - // see https://github.com/golang/go/issues/54619. - for _, err := range act.pkg.Errors { - if err.Kind != packages.TypeError { - continue - } - - // Parse err.Pos, a string of form: "file:line:col" or "file:line" or "" or "-" - // The filename may have a single ASCII letter Windows drive prefix such as "C:" - var file string - var line, col int - var convErr error - words := strings.Split(err.Pos, ":") - if runtime.GOOS == "windows" && - len(words) > 2 && - len(words[0]) == 1 && - ('A' <= words[0][0] && words[0][0] <= 'Z' || - 'a' <= words[0][0] && words[0][0] <= 'z') { - words[1] = words[0] + ":" + words[1] - words = words[1:] - } - switch len(words) { - case 2: - // file:line - file = words[0] - line, convErr = strconv.Atoi(words[1]) - case 3: - // file:line:col - file = words[0] - line, convErr = strconv.Atoi(words[1]) - if convErr == nil { - col, convErr = strconv.Atoi(words[2]) - } - default: - continue - } - if convErr != nil { - continue - } - - // Extract the token positions from the error string. - // (This is guesswork: Fset may contain all manner - // of stale files with the same name.) - offset := -1 - act.pkg.Fset.Iterate(func(f *token.File) bool { - if f.Name() != file { - return true - } - offset = int(f.LineStart(line)) + col - 1 - return false - }) - if offset == -1 { - continue - } - errors = append(errors, types.Error{ - Fset: act.pkg.Fset, - Msg: err.Msg, - Pos: token.Pos(offset), - }) - } - analysisinternal.SetTypeErrors(pass, errors) - var err error if act.pkg.IllTyped && !pass.Analyzer.RunDespiteErrors { err = fmt.Errorf("analysis skipped due to errors in package") diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go index d9c8f11cdd..6e6907d261 100644 --- a/go/analysis/unitchecker/unitchecker.go +++ b/go/analysis/unitchecker/unitchecker.go @@ -340,6 +340,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re Pkg: pkg, TypesInfo: info, TypesSizes: tc.Sizes, + TypeErrors: nil, // unitchecker doesn't RunDespiteErrors ResultOf: inputs, Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, ImportObjectFact: facts.ImportObjectFact, diff --git a/go/packages/packages.go b/go/packages/packages.go index 54d880d206..046212347c 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -303,6 +303,9 @@ type Package struct { // of the package, or while parsing or type-checking its files. Errors []Error + // TypeErrors contains the subset of errors produced during type checking. + TypeErrors []types.Error + // GoFiles lists the absolute file paths of the package's Go source files. GoFiles []string @@ -911,6 +914,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { case types.Error: // from type checker + lpkg.TypeErrors = append(lpkg.TypeErrors, err) errs = append(errs, Error{ Pos: err.Fset.Position(err.Pos).String(), Msg: err.Msg, diff --git a/gopls/internal/lsp/analysis/fillreturns/fillreturns.go b/gopls/internal/lsp/analysis/fillreturns/fillreturns.go index 4415ddd66e..c8146df2dd 100644 --- a/gopls/internal/lsp/analysis/fillreturns/fillreturns.go +++ b/gopls/internal/lsp/analysis/fillreturns/fillreturns.go @@ -52,9 +52,8 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, fmt.Errorf("nil TypeInfo") } - errors := analysisinternal.GetTypeErrors(pass) outer: - for _, typeErr := range errors { + for _, typeErr := range pass.TypeErrors { // Filter out the errors that are not relevant to this analyzer. if !FixesError(typeErr) { continue diff --git a/gopls/internal/lsp/analysis/nonewvars/nonewvars.go b/gopls/internal/lsp/analysis/nonewvars/nonewvars.go index e7fa430cc5..2d2044eb39 100644 --- a/gopls/internal/lsp/analysis/nonewvars/nonewvars.go +++ b/gopls/internal/lsp/analysis/nonewvars/nonewvars.go @@ -30,7 +30,7 @@ will turn into ` var Analyzer = &analysis.Analyzer{ - Name: string(analysisinternal.NoNewVars), + Name: "nonewvars", Doc: Doc, Requires: []*analysis.Analyzer{inspect.Analyzer}, Run: run, @@ -39,7 +39,6 @@ var Analyzer = &analysis.Analyzer{ func run(pass *analysis.Pass) (interface{}, error) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - errors := analysisinternal.GetTypeErrors(pass) nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)} inspect.Preorder(nodeFilter, func(n ast.Node) { @@ -60,7 +59,7 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - for _, err := range errors { + for _, err := range pass.TypeErrors { if !FixesError(err.Msg) { continue } diff --git a/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go index b9f21f3135..b3fd84bcc9 100644 --- a/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go +++ b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go @@ -29,7 +29,7 @@ will turn into ` var Analyzer = &analysis.Analyzer{ - Name: string(analysisinternal.NoResultValues), + Name: "noresultvalues", Doc: Doc, Requires: []*analysis.Analyzer{inspect.Analyzer}, Run: run, @@ -38,7 +38,6 @@ var Analyzer = &analysis.Analyzer{ func run(pass *analysis.Pass) (interface{}, error) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - errors := analysisinternal.GetTypeErrors(pass) nodeFilter := []ast.Node{(*ast.ReturnStmt)(nil)} inspect.Preorder(nodeFilter, func(n ast.Node) { @@ -55,7 +54,7 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - for _, err := range errors { + for _, err := range pass.TypeErrors { if !FixesError(err.Msg) { continue } diff --git a/gopls/internal/lsp/analysis/stubmethods/stubmethods.go b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go index f9dc69a965..effeb8a405 100644 --- a/gopls/internal/lsp/analysis/stubmethods/stubmethods.go +++ b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go @@ -35,7 +35,7 @@ var Analyzer = &analysis.Analyzer{ } func run(pass *analysis.Pass) (interface{}, error) { - for _, err := range analysisinternal.GetTypeErrors(pass) { + for _, err := range pass.TypeErrors { ifaceErr := strings.Contains(err.Msg, "missing method") || strings.HasPrefix(err.Msg, "cannot convert") if !ifaceErr { continue diff --git a/gopls/internal/lsp/analysis/undeclaredname/undeclared.go b/gopls/internal/lsp/analysis/undeclaredname/undeclared.go index fd8d35eb1b..742a4d265d 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/undeclared.go +++ b/gopls/internal/lsp/analysis/undeclaredname/undeclared.go @@ -38,7 +38,7 @@ func <>(inferred parameters) { ` var Analyzer = &analysis.Analyzer{ - Name: string(analysisinternal.UndeclaredName), + Name: "undeclaredname", Doc: Doc, Requires: []*analysis.Analyzer{}, Run: run, @@ -49,7 +49,7 @@ var Analyzer = &analysis.Analyzer{ var undeclaredNamePrefixes = []string{"undeclared name: ", "undefined: "} func run(pass *analysis.Pass) (interface{}, error) { - for _, err := range analysisinternal.GetTypeErrors(pass) { + for _, err := range pass.TypeErrors { runForError(pass, err) } return nil, nil diff --git a/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go index 134cbb2c43..904016be71 100644 --- a/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go +++ b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go @@ -16,7 +16,6 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/ast/astutil" - "golang.org/x/tools/internal/analysisinternal" ) const Doc = `check for unused variables @@ -36,7 +35,7 @@ var Analyzer = &analysis.Analyzer{ var unusedVariableSuffixes = []string{" declared and not used", " declared but not used"} func run(pass *analysis.Pass) (interface{}, error) { - for _, typeErr := range analysisinternal.GetTypeErrors(pass) { + for _, typeErr := range pass.TypeErrors { for _, suffix := range unusedVariableSuffixes { if strings.HasSuffix(typeErr.Msg, suffix) { varName := strings.TrimSuffix(typeErr.Msg, suffix) diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go index 43f1afbe89..e2c56a53c0 100644 --- a/gopls/internal/lsp/cache/analysis.go +++ b/gopls/internal/lsp/cache/analysis.go @@ -17,7 +17,6 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/span" - "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/bug" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/event/tag" @@ -291,6 +290,7 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a Pkg: pkg.GetTypes(), TypesInfo: pkg.GetTypesInfo(), TypesSizes: pkg.GetTypesSizes(), + TypeErrors: pkg.typeErrors, ResultOf: inputs, Report: func(d analysis.Diagnostic) { // Prefix the diagnostic category with the analyzer's name. @@ -351,7 +351,6 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a return facts }, } - analysisinternal.SetTypeErrors(pass, pkg.typeErrors) if (pkg.HasListOrParseErrors() || pkg.HasTypeErrors()) && !analyzer.RunDespiteErrors { return nil, fmt.Errorf("skipping analysis %s because package %s contains errors", analyzer.Name, pkg.ID()) diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go index 6fceef5e72..d15f0eb7ab 100644 --- a/internal/analysisinternal/analysis.go +++ b/internal/analysisinternal/analysis.go @@ -2,7 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Package analysisinternal exposes internal-only fields from go/analysis. +// Package analysisinternal provides gopls' internal analyses with a +// number of helper functions that operate on typed syntax trees. package analysisinternal import ( @@ -18,11 +19,6 @@ import ( // in Go 1.18+. var DiagnoseFuzzTests bool = false -var ( - GetTypeErrors func(p interface{}) []types.Error - SetTypeErrors func(p interface{}, errors []types.Error) -) - func TypeErrorEndPos(fset *token.FileSet, src []byte, start token.Pos) token.Pos { // Get the end position for the type error. offset, end := fset.PositionFor(start, false).Offset, start @@ -210,14 +206,6 @@ func TypeExpr(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr { } } -type TypeErrorPass string - -const ( - NoNewVars TypeErrorPass = "nonewvars" - NoResultValues TypeErrorPass = "noresultvalues" - UndeclaredName TypeErrorPass = "undeclaredname" -) - // StmtToInsertVarBefore returns the ast.Stmt before which we can safely insert a new variable. // Some examples: //