mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

go/analysis: add Pass.TypeErrors field 

The result of type checking is a package (types.Package), type
annotations on syntax trees (types.Info), and zero or more type
errors (types.Error).  Hitherto we had assumed that analyzers don't
need access to type errors, but in fact it turns out to be useful:
for example, analyzers can look at broken code and suggest quick
fixes to repair the mistakes.

This change adds a Pass.TypeErrors field that holds the errors
produced during type checking. It may be non-nil only when the
Analyzer.RunDespiteErrors flag was enabled. Similarly, it adds
a TypeErrors field to go/packages.Package.
(The existing Packages.Error field loses important details.)

Gopls was already using analyzers in this way, (ab)using its privileged
position in the x/tools repo. This change removes the need for such hacks.

We expect that all analysis drivers that support RunDespiteErrors will
in due course populate the Pass.TypesErrors field. This change updates
the go/packages-based driver to do so; no changes were needed to
unitchecker since it does not support RunDespiteErrors.
In the meantime, not populating this field is not expected to cause
any compatibility problems.

Fixes golang/go#54619

Change-Id: Ia7c72242e332782e8919a4c30b2107c37bcec9ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/425095
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Tim King <taking@google.com>
This commit is contained in:
Alan Donovan 2022-08-22 15:21:17 -04:00
parent b0ad6b2e07
commit ce26db4201
12 changed files with 29 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
go/analysis/analysis.go
View File

@ -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.

86
go/analysis/internal/checker/checker.go
View File

@ -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")

1
go/analysis/unitchecker/unitchecker.go
View File

@ -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,

4
go/packages/packages.go
View File

@ -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,

3
gopls/internal/lsp/analysis/fillreturns/fillreturns.go
View File

@ -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

5
gopls/internal/lsp/analysis/nonewvars/nonewvars.go
View File

@ -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
}

5
gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go
View File

@ -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
}

2
gopls/internal/lsp/analysis/stubmethods/stubmethods.go
View File

@ -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

4
gopls/internal/lsp/analysis/undeclaredname/undeclared.go
View File

@ -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

3
gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go
View File

@ -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)

3
gopls/internal/lsp/cache/analysis.go vendored
View File

@ -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())

16
internal/analysisinternal/analysis.go
View File

@ -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:
//