aboutsummaryrefslogtreecommitdiff
path: root/go/analysis
diff options
context:
space:
mode:
authorAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-03-16 14:15:45 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-03-16 14:15:45 +0000
commitcd3c7908c2ca750b27d330b4d257edc6818c4a5d (patch)
tree194d7b0e539d014393564a256bec571e18d6533a /go/analysis
parent3225eca48f7ce16eb31b2dd5a170806c1214a49e (diff)
parent09c5a32afc5b66f28f166a68afe1fc71afbf9b73 (diff)
downloadgolang-x-tools-build-tools-release.tar.gz
Snap for 9757917 from 09c5a32afc5b66f28f166a68afe1fc71afbf9b73 to build-tools-releasebuild-tools-release
Change-Id: If48e809642d94de846f47e34b88e446095e21aa5
Diffstat (limited to 'go/analysis')
-rw-r--r--go/analysis/analysis.go15
-rw-r--r--go/analysis/analysistest/analysistest.go96
-rw-r--r--go/analysis/diagnostic.go2
-rw-r--r--go/analysis/doc.go44
-rw-r--r--go/analysis/internal/analysisflags/flags.go75
-rw-r--r--go/analysis/internal/analysisflags/flags_test.go7
-rw-r--r--go/analysis/internal/checker/checker.go296
-rw-r--r--go/analysis/internal/checker/checker_test.go90
-rw-r--r--go/analysis/internal/checker/fix_test.go309
-rw-r--r--go/analysis/internal/checker/start_test.go85
-rw-r--r--go/analysis/internal/facts/facts.go323
-rw-r--r--go/analysis/internal/facts/facts_test.go384
-rw-r--r--go/analysis/internal/facts/imports.go119
-rw-r--r--go/analysis/passes/asmdecl/arches_go118.go12
-rw-r--r--go/analysis/passes/asmdecl/arches_go119.go14
-rw-r--r--go/analysis/passes/asmdecl/asmdecl.go5
-rw-r--r--go/analysis/passes/asmdecl/asmdecl_test.go19
-rw-r--r--go/analysis/passes/asmdecl/testdata/src/a/asm10.s192
-rw-r--r--go/analysis/passes/asmdecl/testdata/src/a/asm11.s13
-rw-r--r--go/analysis/passes/assign/assign.go15
-rw-r--r--go/analysis/passes/assign/testdata/src/a/a.go28
-rw-r--r--go/analysis/passes/assign/testdata/src/a/a.go.golden28
-rw-r--r--go/analysis/passes/bools/bools.go12
-rw-r--r--go/analysis/passes/buildssa/buildssa_test.go37
-rw-r--r--go/analysis/passes/buildssa/testdata/src/b/b.go20
-rw-r--r--go/analysis/passes/buildssa/testdata/src/c/c.go24
-rw-r--r--go/analysis/passes/buildtag/buildtag.go2
-rw-r--r--go/analysis/passes/buildtag/buildtag_old.go2
-rw-r--r--go/analysis/passes/cgocall/cgocall.go16
-rw-r--r--go/analysis/passes/composite/composite.go41
-rw-r--r--go/analysis/passes/composite/composite_test.go2
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a.go17
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a.go.golden144
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden16
-rw-r--r--go/analysis/passes/composite/testdata/src/typeparams/typeparams.go10
-rw-r--r--go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden27
-rw-r--r--go/analysis/passes/copylock/copylock.go2
-rw-r--r--go/analysis/passes/copylock/testdata/src/a/copylock.go42
-rw-r--r--go/analysis/passes/copylock/testdata/src/a/copylock_func.go2
-rw-r--r--go/analysis/passes/directive/directive.go216
-rw-r--r--go/analysis/passes/directive/directive_test.go39
-rw-r--r--go/analysis/passes/directive/testdata/src/a/badspace.go11
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced.go10
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced.s19
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced_test.go10
-rw-r--r--go/analysis/passes/directive/testdata/src/a/p.go11
-rw-r--r--go/analysis/passes/errorsas/errorsas.go28
-rw-r--r--go/analysis/passes/errorsas/testdata/src/a/a.go4
-rw-r--r--go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go2
-rw-r--r--go/analysis/passes/fieldalignment/fieldalignment.go7
-rw-r--r--go/analysis/passes/httpresponse/httpresponse.go27
-rw-r--r--go/analysis/passes/httpresponse/httpresponse_test.go3
-rw-r--r--go/analysis/passes/httpresponse/testdata/src/a/a.go27
-rw-r--r--go/analysis/passes/ifaceassert/parameterized.go1
-rw-r--r--go/analysis/passes/inspect/inspect.go15
-rw-r--r--go/analysis/passes/loopclosure/loopclosure.go408
-rw-r--r--go/analysis/passes/loopclosure/loopclosure_test.go4
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/a/a.go131
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/a/b.go9
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go202
-rw-r--r--go/analysis/passes/nilness/nilness.go18
-rw-r--r--go/analysis/passes/nilness/nilness_test.go17
-rw-r--r--go/analysis/passes/nilness/testdata/src/a/a.go41
-rw-r--r--go/analysis/passes/nilness/testdata/src/c/c.go14
-rw-r--r--go/analysis/passes/nilness/testdata/src/d/d.go55
-rw-r--r--go/analysis/passes/pkgfact/pkgfact.go8
-rw-r--r--go/analysis/passes/printf/printf.go27
-rw-r--r--go/analysis/passes/printf/testdata/src/a/a.go3
-rw-r--r--go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go22
-rw-r--r--go/analysis/passes/printf/types.go10
-rw-r--r--go/analysis/passes/shadow/shadow.go1
-rw-r--r--go/analysis/passes/sigchanyzer/sigchanyzer.go2
-rw-r--r--go/analysis/passes/sortslice/analyzer.go9
-rw-r--r--go/analysis/passes/sortslice/testdata/src/a/a.go24
-rw-r--r--go/analysis/passes/stdmethods/stdmethods.go13
-rw-r--r--go/analysis/passes/stdmethods/testdata/src/a/a.go14
-rw-r--r--go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go4
-rw-r--r--go/analysis/passes/tests/testdata/src/a/go118_test.go5
-rw-r--r--go/analysis/passes/tests/tests.go30
-rw-r--r--go/analysis/passes/timeformat/testdata/src/a/a.go50
-rw-r--r--go/analysis/passes/timeformat/testdata/src/a/a.go.golden50
-rw-r--r--go/analysis/passes/timeformat/testdata/src/b/b.go11
-rw-r--r--go/analysis/passes/timeformat/timeformat.go129
-rw-r--r--go/analysis/passes/timeformat/timeformat_test.go17
-rw-r--r--go/analysis/passes/unusedwrite/unusedwrite.go53
-rw-r--r--go/analysis/singlechecker/singlechecker.go15
-rw-r--r--go/analysis/unitchecker/main.go4
-rw-r--r--go/analysis/unitchecker/unitchecker.go30
-rw-r--r--go/analysis/unitchecker/unitchecker_test.go81
-rw-r--r--go/analysis/validate.go5
-rw-r--r--go/analysis/validate_test.go34
91 files changed, 3221 insertions, 1346 deletions
diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go
index d11505a16..44ada22a0 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/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index df79a4419..be016e7e9 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -19,14 +19,13 @@ import (
"sort"
"strconv"
"strings"
+ "testing"
"text/scanner"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/internal/checker"
"golang.org/x/tools/go/packages"
- "golang.org/x/tools/internal/lsp/diff"
- "golang.org/x/tools/internal/lsp/diff/myers"
- "golang.org/x/tools/internal/span"
+ "golang.org/x/tools/internal/diff"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/txtar"
)
@@ -81,23 +80,24 @@ type Testing interface {
// Each section in the archive corresponds to a single message.
//
// A golden file using txtar may look like this:
-// -- turn into single negation --
-// package pkg
//
-// func fn(b1, b2 bool) {
-// if !b1 { // want `negating a boolean twice`
-// println()
-// }
-// }
+// -- turn into single negation --
+// package pkg
//
-// -- remove double negation --
-// package pkg
+// func fn(b1, b2 bool) {
+// if !b1 { // want `negating a boolean twice`
+// println()
+// }
+// }
//
-// func fn(b1, b2 bool) {
-// if b1 { // want `negating a boolean twice`
-// println()
-// }
-// }
+// -- remove double negation --
+// package pkg
+//
+// func fn(b1, b2 bool) {
+// if b1 { // want `negating a boolean twice`
+// println()
+// }
+// }
func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
r := Run(t, dir, a, patterns...)
@@ -113,7 +113,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
// should match up.
for _, act := range r {
// file -> message -> edits
- fileEdits := make(map[*token.File]map[string][]diff.TextEdit)
+ fileEdits := make(map[*token.File]map[string][]diff.Edit)
fileContents := make(map[*token.File][]byte)
// Validate edits, prepare the fileEdits map and read the file contents.
@@ -141,17 +141,13 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
}
fileContents[file] = contents
}
- spn, err := span.NewRange(act.Pass.Fset, edit.Pos, edit.End).Span()
- if err != nil {
- t.Errorf("error converting edit to span %s: %v", file.Name(), err)
- }
-
if _, ok := fileEdits[file]; !ok {
- fileEdits[file] = make(map[string][]diff.TextEdit)
+ fileEdits[file] = make(map[string][]diff.Edit)
}
- fileEdits[file][sf.Message] = append(fileEdits[file][sf.Message], diff.TextEdit{
- Span: spn,
- NewText: string(edit.NewText),
+ fileEdits[file][sf.Message] = append(fileEdits[file][sf.Message], diff.Edit{
+ Start: file.Offset(edit.Pos),
+ End: file.Offset(edit.End),
+ New: string(edit.NewText),
})
}
}
@@ -188,23 +184,24 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
for _, vf := range ar.Files {
if vf.Name == sf {
found = true
- out := diff.ApplyEdits(string(orig), edits)
+ out, err := diff.ApplyBytes(orig, edits)
+ if err != nil {
+ t.Errorf("%s: error applying fixes: %v", file.Name(), err)
+ continue
+ }
// the file may contain multiple trailing
// newlines if the user places empty lines
// between files in the archive. normalize
// this to a single newline.
want := string(bytes.TrimRight(vf.Data, "\n")) + "\n"
- formatted, err := format.Source([]byte(out))
+ formatted, err := format.Source(out)
if err != nil {
t.Errorf("%s: error formatting edited source: %v\n%s", file.Name(), err, out)
continue
}
- if want != string(formatted) {
- d, err := myers.ComputeEdits("", want, string(formatted))
- if err != nil {
- t.Errorf("failed to compute suggested fix diff: %v", err)
- }
- t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), diff.ToUnified(fmt.Sprintf("%s.golden [%s]", file.Name(), sf), "actual", want, d))
+ if got := string(formatted); got != want {
+ unified := diff.Unified(fmt.Sprintf("%s.golden [%s]", file.Name(), sf), "actual", want, got)
+ t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), unified)
}
break
}
@@ -216,25 +213,26 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
} else {
// all suggested fixes are represented by a single file
- var catchallEdits []diff.TextEdit
+ var catchallEdits []diff.Edit
for _, edits := range fixes {
catchallEdits = append(catchallEdits, edits...)
}
- out := diff.ApplyEdits(string(orig), catchallEdits)
+ out, err := diff.ApplyBytes(orig, catchallEdits)
+ if err != nil {
+ t.Errorf("%s: error applying fixes: %v", file.Name(), err)
+ continue
+ }
want := string(ar.Comment)
- formatted, err := format.Source([]byte(out))
+ formatted, err := format.Source(out)
if err != nil {
t.Errorf("%s: error formatting resulting source: %v\n%s", file.Name(), err, out)
continue
}
- if want != string(formatted) {
- d, err := myers.ComputeEdits("", want, string(formatted))
- if err != nil {
- t.Errorf("%s: failed to compute suggested fix diff: %s", file.Name(), err)
- }
- t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), diff.ToUnified(file.Name()+".golden", "actual", want, d))
+ if got := string(formatted); got != want {
+ unified := diff.Unified(file.Name()+".golden", "actual", want, got)
+ t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), unified)
}
}
}
@@ -248,7 +246,8 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
// directory using golang.org/x/tools/go/packages, runs the analysis on
// them, and checks that each analysis emits the expected diagnostics
// and facts specified by the contents of '// want ...' comments in the
-// package's source files.
+// package's source files. It treats a comment of the form
+// "//...// want..." or "/*...// want... */" as if it starts at 'want'
//
// An expectation of a Diagnostic is specified by a string literal
// containing a regular expression that must match the diagnostic
@@ -280,7 +279,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
// attempted, even if unsuccessful. It is safe for a test to ignore all
// the results, but a test may use it to perform additional checks.
func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
- if t, ok := t.(testenv.Testing); ok {
+ if t, ok := t.(testing.TB); ok {
testenv.NeedsGoPackages(t)
}
@@ -316,8 +315,11 @@ func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*pack
// a list of packages we generate and then do the parsing and
// typechecking, though this feature seems to be a recurring need.
+ mode := packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedImports |
+ packages.NeedTypes | packages.NeedTypesSizes | packages.NeedSyntax | packages.NeedTypesInfo |
+ packages.NeedDeps
cfg := &packages.Config{
- Mode: packages.LoadAllSyntax,
+ Mode: mode,
Dir: dir,
Tests: true,
Env: append(os.Environ(), "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off"),
diff --git a/go/analysis/diagnostic.go b/go/analysis/diagnostic.go
index cd462a0cb..5cdcf46d2 100644
--- a/go/analysis/diagnostic.go
+++ b/go/analysis/diagnostic.go
@@ -37,7 +37,7 @@ type Diagnostic struct {
// declaration.
type RelatedInformation struct {
Pos token.Pos
- End token.Pos
+ End token.Pos // optional
Message string
}
diff --git a/go/analysis/doc.go b/go/analysis/doc.go
index 94a3bd5d0..c5429c9e2 100644
--- a/go/analysis/doc.go
+++ b/go/analysis/doc.go
@@ -3,12 +3,10 @@
// license that can be found in the LICENSE file.
/*
-
Package analysis defines the interface between a modular static
analysis and an analysis driver program.
-
-Background
+# Background
A static analysis is a function that inspects a package of Go code and
reports a set of diagnostics (typically mistakes in the code), and
@@ -32,8 +30,7 @@ frameworks, code review tools, code-base indexers (such as SourceGraph),
documentation viewers (such as godoc), batch pipelines for large code
bases, and so on.
-
-Analyzer
+# Analyzer
The primary type in the API is Analyzer. An Analyzer statically
describes an analysis function: its name, documentation, flags,
@@ -115,8 +112,7 @@ Finally, the Run field contains a function to be called by the driver to
execute the analysis on a single package. The driver passes it an
instance of the Pass type.
-
-Pass
+# Pass
A Pass describes a single unit of work: the application of a particular
Analyzer to a particular package of Go code.
@@ -181,14 +177,14 @@ Diagnostic is defined as:
The optional Category field is a short identifier that classifies the
kind of message when an analysis produces several kinds of diagnostic.
-Many analyses want to associate diagnostics with a severity level.
-Because Diagnostic does not have a severity level field, an Analyzer's
-diagnostics effectively all have the same severity level. To separate which
-diagnostics are high severity and which are low severity, expose multiple
-Analyzers instead. Analyzers should also be separated when their
-diagnostics belong in different groups, or could be tagged differently
-before being shown to the end user. Analyzers should document their severity
-level to help downstream tools surface diagnostics properly.
+The Diagnostic struct does not have a field to indicate its severity
+because opinions about the relative importance of Analyzers and their
+diagnostics vary widely among users. The design of this framework does
+not hold each Analyzer responsible for identifying the severity of its
+diagnostics. Instead, we expect that drivers will allow the user to
+customize the filtering and prioritization of diagnostics based on the
+producing Analyzer and optional Category, according to the user's
+preferences.
Most Analyzers inspect typed Go syntax trees, but a few, such as asmdecl
and buildtag, inspect the raw text of Go source files or even non-Go
@@ -202,8 +198,7 @@ raw text file, use the following sequence:
...
pass.Reportf(tf.LineStart(line), "oops")
-
-Modular analysis with Facts
+# Modular analysis with Facts
To improve efficiency and scalability, large programs are routinely
built using separate compilation: units of the program are compiled
@@ -246,6 +241,12 @@ Consequently, Facts must be serializable. The API requires that drivers
use the gob encoding, an efficient, robust, self-describing binary
protocol. A fact type may implement the GobEncoder/GobDecoder interfaces
if the default encoding is unsuitable. Facts should be stateless.
+Because serialized facts may appear within build outputs, the gob encoding
+of a fact must be deterministic, to avoid spurious cache misses in
+build systems that use content-addressable caches.
+The driver makes a single call to the gob encoder for all facts
+exported by a given analysis pass, so that the topology of
+shared data structures referenced by multiple facts is preserved.
The Pass type has functions to import and export facts,
associated either with an object or with a package:
@@ -280,8 +281,7 @@ this fact is built in to the analyzer so that it correctly checks
calls to log.Printf even when run in a driver that does not apply
it to standard packages. We would like to remove this limitation in future.
-
-Testing an Analyzer
+# Testing an Analyzer
The analysistest subpackage provides utilities for testing an Analyzer.
In a few lines of code, it is possible to run an analyzer on a package
@@ -289,8 +289,7 @@ of testdata files and check that it reported all the expected
diagnostics and facts (and no more). Expectations are expressed using
"// want ..." comments in the input code.
-
-Standalone commands
+# Standalone commands
Analyzers are provided in the form of packages that a driver program is
expected to import. The vet command imports a set of several analyzers,
@@ -301,7 +300,7 @@ singlechecker and multichecker subpackages.
The singlechecker package provides the main function for a command that
runs one analyzer. By convention, each analyzer such as
-go/passes/findcall should be accompanied by a singlechecker-based
+go/analysis/passes/findcall should be accompanied by a singlechecker-based
command such as go/analysis/passes/findcall/cmd/findcall, defined in its
entirety as:
@@ -316,6 +315,5 @@ entirety as:
A tool that provides multiple analyzers can use multichecker in a
similar way, giving it the list of Analyzers.
-
*/
package analysis
diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go
index 4b7be2d1f..e127a42b9 100644
--- a/go/analysis/internal/analysisflags/flags.go
+++ b/go/analysis/internal/analysisflags/flags.go
@@ -206,7 +206,7 @@ func (versionFlag) Get() interface{} { return nil }
func (versionFlag) String() string { return "" }
func (versionFlag) Set(s string) error {
if s != "full" {
- log.Fatalf("unsupported flag value: -V=%s", s)
+ log.Fatalf("unsupported flag value: -V=%s (use -V=full)", s)
}
// This replicates the minimal subset of
@@ -218,7 +218,10 @@ func (versionFlag) Set(s string) error {
// Formats:
// $progname version devel ... buildID=...
// $progname version go1.9.1
- progname := os.Args[0]
+ progname, err := os.Executable()
+ if err != nil {
+ return err
+ }
f, err := os.Open(progname)
if err != nil {
log.Fatal(err)
@@ -339,9 +342,38 @@ func PrintPlain(fset *token.FileSet, diag analysis.Diagnostic) {
}
// A JSONTree is a mapping from package ID to analysis name to result.
-// Each result is either a jsonError or a list of jsonDiagnostic.
+// Each result is either a jsonError or a list of JSONDiagnostic.
type JSONTree map[string]map[string]interface{}
+// A TextEdit describes the replacement of a portion of a file.
+// Start and End are zero-based half-open indices into the original byte
+// sequence of the file, and New is the new text.
+type JSONTextEdit struct {
+ Filename string `json:"filename"`
+ Start int `json:"start"`
+ End int `json:"end"`
+ New string `json:"new"`
+}
+
+// A JSONSuggestedFix describes an edit that should be applied as a whole or not
+// at all. It might contain multiple TextEdits/text_edits if the SuggestedFix
+// consists of multiple non-contiguous edits.
+type JSONSuggestedFix struct {
+ Message string `json:"message"`
+ Edits []JSONTextEdit `json:"edits"`
+}
+
+// A JSONDiagnostic can be used to encode and decode analysis.Diagnostics to and
+// from JSON.
+// TODO(matloob): Should the JSON diagnostics contain ranges?
+// If so, how should they be formatted?
+type JSONDiagnostic struct {
+ Category string `json:"category,omitempty"`
+ Posn string `json:"posn"`
+ Message string `json:"message"`
+ SuggestedFixes []JSONSuggestedFix `json:"suggested_fixes,omitempty"`
+}
+
// Add adds the result of analysis 'name' on package 'id'.
// The result is either a list of diagnostics or an error.
func (tree JSONTree) Add(fset *token.FileSet, id, name string, diags []analysis.Diagnostic, err error) {
@@ -352,20 +384,31 @@ func (tree JSONTree) Add(fset *token.FileSet, id, name string, diags []analysis.
}
v = jsonError{err.Error()}
} else if len(diags) > 0 {
- type jsonDiagnostic struct {
- Category string `json:"category,omitempty"`
- Posn string `json:"posn"`
- Message string `json:"message"`
- }
- var diagnostics []jsonDiagnostic
- // TODO(matloob): Should the JSON diagnostics contain ranges?
- // If so, how should they be formatted?
+ diagnostics := make([]JSONDiagnostic, 0, len(diags))
for _, f := range diags {
- diagnostics = append(diagnostics, jsonDiagnostic{
- Category: f.Category,
- Posn: fset.Position(f.Pos).String(),
- Message: f.Message,
- })
+ var fixes []JSONSuggestedFix
+ for _, fix := range f.SuggestedFixes {
+ var edits []JSONTextEdit
+ for _, edit := range fix.TextEdits {
+ edits = append(edits, JSONTextEdit{
+ Filename: fset.Position(edit.Pos).Filename,
+ Start: fset.Position(edit.Pos).Offset,
+ End: fset.Position(edit.End).Offset,
+ New: string(edit.NewText),
+ })
+ }
+ fixes = append(fixes, JSONSuggestedFix{
+ Message: fix.Message,
+ Edits: edits,
+ })
+ }
+ jdiag := JSONDiagnostic{
+ Category: f.Category,
+ Posn: fset.Position(f.Pos).String(),
+ Message: f.Message,
+ SuggestedFixes: fixes,
+ }
+ diagnostics = append(diagnostics, jdiag)
}
v = diagnostics
}
diff --git a/go/analysis/internal/analysisflags/flags_test.go b/go/analysis/internal/analysisflags/flags_test.go
index 1f055dde7..b5cfb3d44 100644
--- a/go/analysis/internal/analysisflags/flags_test.go
+++ b/go/analysis/internal/analysisflags/flags_test.go
@@ -42,7 +42,7 @@ func TestExec(t *testing.T) {
for _, test := range []struct {
flags string
- want string
+ want string // output should contain want
}{
{"", "[a1 a2 a3]"},
{"-a1=0", "[a2 a3]"},
@@ -50,6 +50,7 @@ func TestExec(t *testing.T) {
{"-a1", "[a1]"},
{"-a1=1 -a3=1", "[a1 a3]"},
{"-a1=1 -a3=0", "[a1]"},
+ {"-V=full", "analysisflags.test version devel"},
} {
cmd := exec.Command(progname, "-test.run=TestExec")
cmd.Env = append(os.Environ(), "ANALYSISFLAGS_CHILD=1", "FLAGS="+test.flags)
@@ -60,8 +61,8 @@ func TestExec(t *testing.T) {
}
got := strings.TrimSpace(string(output))
- if got != test.want {
- t.Errorf("got %s, want %s", got, test.want)
+ if !strings.Contains(got, test.want) {
+ t.Errorf("got %q, does not contain %q", got, test.want)
}
}
}
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index e405a2ae1..5346acd76 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -15,7 +15,6 @@ import (
"flag"
"fmt"
"go/format"
- "go/parser"
"go/token"
"go/types"
"io/ioutil"
@@ -33,8 +32,8 @@ 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"
- "golang.org/x/tools/internal/span"
+ "golang.org/x/tools/internal/diff"
+ "golang.org/x/tools/internal/robustio"
)
var (
@@ -51,6 +50,9 @@ var (
// Log files for optional performance tracing.
CPUProfile, MemProfile, Trace string
+ // IncludeTests indicates whether test files should be analyzed too.
+ IncludeTests = true
+
// Fix determines whether to apply all suggested fixes.
Fix bool
)
@@ -65,6 +67,7 @@ func RegisterFlags() {
flag.StringVar(&CPUProfile, "cpuprofile", "", "write CPU profile to this file")
flag.StringVar(&MemProfile, "memprofile", "", "write memory profile to this file")
flag.StringVar(&Trace, "trace", "", "write trace log to this file")
+ flag.BoolVar(&IncludeTests, "test", IncludeTests, "indicates whether test files should be analyzed, too")
flag.BoolVar(&Fix, "fix", false, "apply all suggested fixes")
}
@@ -143,7 +146,11 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) {
roots := analyze(initial, analyzers)
if Fix {
- applyFixes(roots)
+ if err := applyFixes(roots); err != nil {
+ // Fail when applying fixes failed.
+ log.Print(err)
+ return 1
+ }
}
return printDiagnostics(roots)
}
@@ -163,7 +170,7 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) {
}
conf := packages.Config{
Mode: mode,
- Tests: true,
+ Tests: IncludeTests,
}
initial, err := packages.Load(&conf, patterns...)
if err == nil {
@@ -301,7 +308,10 @@ func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action
return roots
}
-func applyFixes(roots []*action) {
+func applyFixes(roots []*action) error {
+ // visit all of the actions and accumulate the suggested edits.
+ paths := make(map[robustio.FileID]string)
+ editsByAction := make(map[robustio.FileID]map[*action][]diff.Edit)
visited := make(map[*action]bool)
var apply func(*action) error
var visitAll func(actions []*action) error
@@ -309,7 +319,9 @@ func applyFixes(roots []*action) {
for _, act := range actions {
if !visited[act] {
visited[act] = true
- visitAll(act.deps)
+ if err := visitAll(act.deps); err != nil {
+ return err
+ }
if err := apply(act); err != nil {
return err
}
@@ -318,116 +330,167 @@ func applyFixes(roots []*action) {
return nil
}
- // TODO(matloob): Is this tree business too complicated? (After all this is Go!)
- // Just create a set (map) of edits, sort by pos and call it a day?
- type offsetedit struct {
- start, end int
- newText []byte
- } // TextEdit using byteOffsets instead of pos
- type node struct {
- edit offsetedit
- left, right *node
- }
-
- var insert func(tree **node, edit offsetedit) error
- insert = func(treeptr **node, edit offsetedit) error {
- if *treeptr == nil {
- *treeptr = &node{edit, nil, nil}
- return nil
- }
- tree := *treeptr
- if edit.end <= tree.edit.start {
- return insert(&tree.left, edit)
- } else if edit.start >= tree.edit.end {
- return insert(&tree.right, edit)
- }
-
- // Overlapping text edit.
- return fmt.Errorf("analyses applying overlapping text edits affecting pos range (%v, %v) and (%v, %v)",
- edit.start, edit.end, tree.edit.start, tree.edit.end)
-
- }
-
- editsForFile := make(map[*token.File]*node)
-
apply = func(act *action) error {
+ editsForTokenFile := make(map[*token.File][]diff.Edit)
for _, diag := range act.diagnostics {
for _, sf := range diag.SuggestedFixes {
for _, edit := range sf.TextEdits {
// Validate the edit.
+ // Any error here indicates a bug in the analyzer.
+ file := act.pkg.Fset.File(edit.Pos)
+ if file == nil {
+ return fmt.Errorf("analysis %q suggests invalid fix: missing file info for pos (%v)",
+ act.a.Name, edit.Pos)
+ }
if edit.Pos > edit.End {
- return fmt.Errorf(
- "diagnostic for analysis %v contains Suggested Fix with malformed edit: pos (%v) > end (%v)",
+ return fmt.Errorf("analysis %q suggests invalid fix: pos (%v) > end (%v)",
act.a.Name, edit.Pos, edit.End)
}
- file, endfile := act.pkg.Fset.File(edit.Pos), act.pkg.Fset.File(edit.End)
- if file == nil || endfile == nil || file != endfile {
- return (fmt.Errorf(
- "diagnostic for analysis %v contains Suggested Fix with malformed spanning files %v and %v",
- act.a.Name, file.Name(), endfile.Name()))
+ if eof := token.Pos(file.Base() + file.Size()); edit.End > eof {
+ return fmt.Errorf("analysis %q suggests invalid fix: end (%v) past end of file (%v)",
+ act.a.Name, edit.End, eof)
}
- start, end := file.Offset(edit.Pos), file.Offset(edit.End)
-
- // TODO(matloob): Validate that edits do not affect other packages.
- root := editsForFile[file]
- if err := insert(&root, offsetedit{start, end, edit.NewText}); err != nil {
- return err
- }
- editsForFile[file] = root // In case the root changed
+ edit := diff.Edit{Start: file.Offset(edit.Pos), End: file.Offset(edit.End), New: string(edit.NewText)}
+ editsForTokenFile[file] = append(editsForTokenFile[file], edit)
}
}
}
+
+ for f, edits := range editsForTokenFile {
+ id, _, err := robustio.GetFileID(f.Name())
+ if err != nil {
+ return err
+ }
+ if _, hasId := paths[id]; !hasId {
+ paths[id] = f.Name()
+ editsByAction[id] = make(map[*action][]diff.Edit)
+ }
+ editsByAction[id][act] = edits
+ }
return nil
}
- visitAll(roots)
+ if err := visitAll(roots); err != nil {
+ return err
+ }
- fset := token.NewFileSet() // Shared by parse calls below
- // Now we've got a set of valid edits for each file. Get the new file contents.
- for f, tree := range editsForFile {
- contents, err := ioutil.ReadFile(f.Name())
- if err != nil {
- log.Fatal(err)
+ // Validate and group the edits to each actual file.
+ editsByPath := make(map[string][]diff.Edit)
+ for id, actToEdits := range editsByAction {
+ path := paths[id]
+ actions := make([]*action, 0, len(actToEdits))
+ for act := range actToEdits {
+ actions = append(actions, act)
}
- cur := 0 // current position in the file
-
- var out bytes.Buffer
-
- var recurse func(*node)
- recurse = func(node *node) {
- if node.left != nil {
- recurse(node.left)
+ // Does any action create conflicting edits?
+ for _, act := range actions {
+ edits := actToEdits[act]
+ if _, invalid := validateEdits(edits); invalid > 0 {
+ name, x, y := act.a.Name, edits[invalid-1], edits[invalid]
+ return diff3Conflict(path, name, name, []diff.Edit{x}, []diff.Edit{y})
}
+ }
- edit := node.edit
- if edit.start > cur {
- out.Write(contents[cur:edit.start])
- out.Write(edit.newText)
+ // Does any pair of different actions create edits that conflict?
+ for j := range actions {
+ for k := range actions[:j] {
+ x, y := actions[j], actions[k]
+ if x.a.Name > y.a.Name {
+ x, y = y, x
+ }
+ xedits, yedits := actToEdits[x], actToEdits[y]
+ combined := append(xedits, yedits...)
+ if _, invalid := validateEdits(combined); invalid > 0 {
+ // TODO: consider applying each action's consistent list of edits entirely,
+ // and then using a three-way merge (such as GNU diff3) on the resulting
+ // files to report more precisely the parts that actually conflict.
+ return diff3Conflict(path, x.a.Name, y.a.Name, xedits, yedits)
+ }
}
- cur = edit.end
+ }
- if node.right != nil {
- recurse(node.right)
- }
+ var edits []diff.Edit
+ for act := range actToEdits {
+ edits = append(edits, actToEdits[act]...)
}
- recurse(tree)
- // Write out the rest of the file.
- if cur < len(contents) {
- out.Write(contents[cur:])
+ editsByPath[path], _ = validateEdits(edits) // remove duplicates. already validated.
+ }
+
+ // Now we've got a set of valid edits for each file. Apply them.
+ for path, edits := range editsByPath {
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
+ }
+
+ out, err := diff.ApplyBytes(contents, edits)
+ if err != nil {
+ return err
}
// Try to format the file.
- ff, err := parser.ParseFile(fset, f.Name(), out.Bytes(), parser.ParseComments)
- if err == nil {
- var buf bytes.Buffer
- if err = format.Node(&buf, fset, ff); err == nil {
- out = buf
+ if formatted, err := format.Source(out); err == nil {
+ out = formatted
+ }
+
+ if err := ioutil.WriteFile(path, out, 0644); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// validateEdits returns a list of edits that is sorted and
+// contains no duplicate edits. Returns the index of some
+// overlapping adjacent edits if there is one and <0 if the
+// edits are valid.
+func validateEdits(edits []diff.Edit) ([]diff.Edit, int) {
+ if len(edits) == 0 {
+ return nil, -1
+ }
+ equivalent := func(x, y diff.Edit) bool {
+ return x.Start == y.Start && x.End == y.End && x.New == y.New
+ }
+ diff.SortEdits(edits)
+ unique := []diff.Edit{edits[0]}
+ invalid := -1
+ for i := 1; i < len(edits); i++ {
+ prev, cur := edits[i-1], edits[i]
+ // We skip over equivalent edits without considering them
+ // an error. This handles identical edits coming from the
+ // multiple ways of loading a package into a
+ // *go/packages.Packages for testing, e.g. packages "p" and "p [p.test]".
+ if !equivalent(prev, cur) {
+ unique = append(unique, cur)
+ if prev.End > cur.Start {
+ invalid = i
}
}
+ }
+ return unique, invalid
+}
- ioutil.WriteFile(f.Name(), out.Bytes(), 0644)
+// diff3Conflict returns an error describing two conflicting sets of
+// edits on a file at path.
+func diff3Conflict(path string, xlabel, ylabel string, xedits, yedits []diff.Edit) error {
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
}
+ oldlabel, old := "base", string(contents)
+
+ xdiff, err := diff.ToUnified(oldlabel, xlabel, old, xedits)
+ if err != nil {
+ return err
+ }
+ ydiff, err := diff.ToUnified(oldlabel, ylabel, old, yedits)
+ if err != nil {
+ return err
+ }
+
+ return fmt.Errorf("conflicting edits from %s and %s on %s\nfirst edits:\n%s\nsecond edits:\n%s",
+ xlabel, ylabel, path, xdiff, ydiff)
}
// printDiagnostics prints the diagnostics for the root packages in either
@@ -574,7 +637,6 @@ type action struct {
deps []*action
objectFacts map[objectFactKey]analysis.Fact
packageFacts map[packageFactKey]analysis.Fact
- inputs map[*analysis.Analyzer]interface{}
result interface{}
diagnostics []analysis.Diagnostic
err error
@@ -672,14 +734,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,
@@ -691,36 +755,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.
- for _, err := range act.pkg.Errors {
- if err.Kind != packages.TypeError {
- continue
- }
- // err.Pos is a string of form: "file:line:col" or "file:line" or "" or "-"
- spn := span.Parse(err.Pos)
- // Extract the token positions from the error string.
- line, col, offset := spn.Start().Line(), spn.Start().Column(), -1
- act.pkg.Fset.Iterate(func(f *token.File) bool {
- if f.Name() != spn.URI().Filename() {
- 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")
@@ -762,7 +796,7 @@ func inheritFacts(act, dep *action) {
if serialize {
encodedFact, err := codeFact(fact)
if err != nil {
- log.Panicf("internal error: encoding of %T fact failed in %v", fact, act)
+ log.Panicf("internal error: encoding of %T fact failed in %v: %v", fact, act, err)
}
fact = encodedFact
}
@@ -826,7 +860,7 @@ func codeFact(fact analysis.Fact) (analysis.Fact, error) {
// exportedFrom reports whether obj may be visible to a package that imports pkg.
// This includes not just the exported members of pkg, but also unexported
-// constants, types, fields, and methods, perhaps belonging to oether packages,
+// constants, types, fields, and methods, perhaps belonging to other packages,
// that find there way into the API.
// This is an overapproximation of the more accurate approach used by
// gc export data, which walks the type graph, but it's much simpler.
@@ -890,7 +924,7 @@ func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) {
func (act *action) allObjectFacts() []analysis.ObjectFact {
facts := make([]analysis.ObjectFact, 0, len(act.objectFacts))
for k := range act.objectFacts {
- facts = append(facts, analysis.ObjectFact{k.obj, act.objectFacts[k]})
+ facts = append(facts, analysis.ObjectFact{Object: k.obj, Fact: act.objectFacts[k]})
}
return facts
}
@@ -932,11 +966,11 @@ func factType(fact analysis.Fact) reflect.Type {
return t
}
-// allObjectFacts implements Pass.AllObjectFacts.
+// allPackageFacts implements Pass.AllPackageFacts.
func (act *action) allPackageFacts() []analysis.PackageFact {
facts := make([]analysis.PackageFact, 0, len(act.packageFacts))
for k := range act.packageFacts {
- facts = append(facts, analysis.PackageFact{k.pkg, act.packageFacts[k]})
+ facts = append(facts, analysis.PackageFact{Package: k.pkg, Fact: act.packageFacts[k]})
}
return facts
}
diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go
index eee211c21..34acae81e 100644
--- a/go/analysis/internal/checker/checker_test.go
+++ b/go/analysis/internal/checker/checker_test.go
@@ -19,14 +19,9 @@ import (
"golang.org/x/tools/internal/testenv"
)
-var from, to string
-
func TestApplyFixes(t *testing.T) {
testenv.NeedsGoPackages(t)
- from = "bar"
- to = "baz"
-
files := map[string]string{
"rename/test.go": `package rename
@@ -74,26 +69,55 @@ var analyzer = &analysis.Analyzer{
Run: run,
}
+var other = &analysis.Analyzer{ // like analyzer but with a different Name.
+ Name: "other",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
func run(pass *analysis.Pass) (interface{}, error) {
+ const (
+ from = "bar"
+ to = "baz"
+ conflict = "conflict" // add conflicting edits to package conflict.
+ duplicate = "duplicate" // add duplicate edits to package conflict.
+ other = "other" // add conflicting edits to package other from different analyzers.
+ )
+
+ if pass.Analyzer.Name == other {
+ if pass.Pkg.Name() != other {
+ return nil, nil // only apply Analyzer other to packages named other
+ }
+ }
+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{(*ast.Ident)(nil)}
inspect.Preorder(nodeFilter, func(n ast.Node) {
ident := n.(*ast.Ident)
if ident.Name == from {
msg := fmt.Sprintf("renaming %q to %q", from, to)
+ edits := []analysis.TextEdit{
+ {Pos: ident.Pos(), End: ident.End(), NewText: []byte(to)},
+ }
+ switch pass.Pkg.Name() {
+ case conflict:
+ edits = append(edits, []analysis.TextEdit{
+ {Pos: ident.Pos() - 1, End: ident.End(), NewText: []byte(to)},
+ {Pos: ident.Pos(), End: ident.End() - 1, NewText: []byte(to)},
+ {Pos: ident.Pos(), End: ident.End(), NewText: []byte("lorem ipsum")},
+ }...)
+ case duplicate:
+ edits = append(edits, edits...)
+ case other:
+ if pass.Analyzer.Name == other {
+ edits[0].Pos = edits[0].Pos + 1 // shift by one to mismatch analyzer and other
+ }
+ }
pass.Report(analysis.Diagnostic{
- Pos: ident.Pos(),
- End: ident.End(),
- Message: msg,
- SuggestedFixes: []analysis.SuggestedFix{{
- Message: msg,
- TextEdits: []analysis.TextEdit{{
- Pos: ident.Pos(),
- End: ident.End(),
- NewText: []byte(to),
- }},
- }},
- })
+ Pos: ident.Pos(),
+ End: ident.End(),
+ Message: msg,
+ SuggestedFixes: []analysis.SuggestedFix{{Message: msg, TextEdits: edits}}})
}
})
@@ -129,6 +153,18 @@ func Foo(s string) int {
RunDespiteErrors: true,
}
+ // A no-op analyzer that should finish regardless of
+ // parse or type errors in the code.
+ noopWithFact := &analysis.Analyzer{
+ Name: "noopfact",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: func(pass *analysis.Pass) (interface{}, error) {
+ return nil, nil
+ },
+ RunDespiteErrors: true,
+ FactTypes: []analysis.Fact{&EmptyFact{}},
+ }
+
for _, test := range []struct {
name string
pattern []string
@@ -137,7 +173,17 @@ func Foo(s string) int {
}{
// parse/type errors
{name: "skip-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{analyzer}, code: 1},
- {name: "despite-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noop}, code: 0},
+ // RunDespiteErrors allows a driver to run an Analyzer even after parse/type errors.
+ //
+ // The noop analyzer doesn't use facts, so the driver loads only the root
+ // package from source. For the rest, it asks 'go list' for export data,
+ // which fails because the compiler encounters the type error. Since the
+ // errors come from 'go list', the driver doesn't run the analyzer.
+ {name: "despite-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noop}, code: 1},
+ // The noopfact analyzer does use facts, so the driver loads source for
+ // all dependencies, does type checking itself, recognizes the error as a
+ // type error, and runs the analyzer.
+ {name: "despite-error-fact", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noopWithFact}, code: 0},
// combination of parse/type errors and no errors
{name: "despite-error-and-no-error", pattern: []string{"file=" + path, "sort"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 1},
// non-existing package error
@@ -151,6 +197,10 @@ func Foo(s string) int {
// no errors
{name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 0},
} {
+ if test.name == "despite-error" && testenv.Go1Point() < 20 {
+ // The behavior in the comment on the despite-error test only occurs for Go 1.20+.
+ continue
+ }
if got := checker.Run(test.pattern, test.analyzers); got != test.code {
t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code)
}
@@ -158,3 +208,7 @@ func Foo(s string) int {
defer cleanup()
}
+
+type EmptyFact struct{}
+
+func (f *EmptyFact) AFact() {}
diff --git a/go/analysis/internal/checker/fix_test.go b/go/analysis/internal/checker/fix_test.go
new file mode 100644
index 000000000..3ea92b38c
--- /dev/null
+++ b/go/analysis/internal/checker/fix_test.go
@@ -0,0 +1,309 @@
+// Copyright 2022 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 checker_test
+
+import (
+ "flag"
+ "io/ioutil"
+ "os"
+ "os/exec"
+ "path"
+ "regexp"
+ "runtime"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/internal/checker"
+ "golang.org/x/tools/internal/testenv"
+)
+
+func main() {
+ checker.Fix = true
+ patterns := flag.Args()
+
+ code := checker.Run(patterns, []*analysis.Analyzer{analyzer, other})
+ os.Exit(code)
+}
+
+// TestFixes ensures that checker.Run applies fixes correctly.
+// This test fork/execs the main function above.
+func TestFixes(t *testing.T) {
+ oses := map[string]bool{"darwin": true, "linux": true}
+ if !oses[runtime.GOOS] {
+ t.Skipf("skipping fork/exec test on this platform")
+ }
+
+ if os.Getenv("TESTFIXES_CHILD") == "1" {
+ // child process
+
+ // replace [progname -test.run=TestFixes -- ...]
+ // by [progname ...]
+ os.Args = os.Args[2:]
+ os.Args[0] = "vet"
+ main()
+ panic("unreachable")
+ }
+
+ testenv.NeedsTool(t, "go")
+
+ files := map[string]string{
+ "rename/foo.go": `package rename
+
+func Foo() {
+ bar := 12
+ _ = bar
+}
+
+// the end
+`,
+ "rename/intestfile_test.go": `package rename
+
+func InTestFile() {
+ bar := 13
+ _ = bar
+}
+
+// the end
+`,
+ "rename/foo_test.go": `package rename_test
+
+func Foo() {
+ bar := 14
+ _ = bar
+}
+
+// the end
+`,
+ "duplicate/dup.go": `package duplicate
+
+func Foo() {
+ bar := 14
+ _ = bar
+}
+
+// the end
+`,
+ }
+ fixed := map[string]string{
+ "rename/foo.go": `package rename
+
+func Foo() {
+ baz := 12
+ _ = baz
+}
+
+// the end
+`,
+ "rename/intestfile_test.go": `package rename
+
+func InTestFile() {
+ baz := 13
+ _ = baz
+}
+
+// the end
+`,
+ "rename/foo_test.go": `package rename_test
+
+func Foo() {
+ baz := 14
+ _ = baz
+}
+
+// the end
+`,
+ "duplicate/dup.go": `package duplicate
+
+func Foo() {
+ baz := 14
+ _ = baz
+}
+
+// the end
+`,
+ }
+ dir, cleanup, err := analysistest.WriteFiles(files)
+ if err != nil {
+ t.Fatalf("Creating test files failed with %s", err)
+ }
+ defer cleanup()
+
+ args := []string{"-test.run=TestFixes", "--", "rename", "duplicate"}
+ cmd := exec.Command(os.Args[0], args...)
+ cmd.Env = append(os.Environ(), "TESTFIXES_CHILD=1", "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off")
+
+ out, err := cmd.CombinedOutput()
+ if len(out) > 0 {
+ t.Logf("%s: out=<<%s>>", args, out)
+ }
+ var exitcode int
+ if err, ok := err.(*exec.ExitError); ok {
+ exitcode = err.ExitCode() // requires go1.12
+ }
+
+ const diagnosticsExitCode = 3
+ if exitcode != diagnosticsExitCode {
+ t.Errorf("%s: exited %d, want %d", args, exitcode, diagnosticsExitCode)
+ }
+
+ for name, want := range fixed {
+ path := path.Join(dir, "src", name)
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ t.Errorf("error reading %s: %v", path, err)
+ }
+ if got := string(contents); got != want {
+ t.Errorf("contents of %s file did not match expectations. got=%s, want=%s", path, got, want)
+ }
+ }
+}
+
+// TestConflict ensures that checker.Run detects conflicts correctly.
+// This test fork/execs the main function above.
+func TestConflict(t *testing.T) {
+ oses := map[string]bool{"darwin": true, "linux": true}
+ if !oses[runtime.GOOS] {
+ t.Skipf("skipping fork/exec test on this platform")
+ }
+
+ if os.Getenv("TESTCONFLICT_CHILD") == "1" {
+ // child process
+
+ // replace [progname -test.run=TestConflict -- ...]
+ // by [progname ...]
+ os.Args = os.Args[2:]
+ os.Args[0] = "vet"
+ main()
+ panic("unreachable")
+ }
+
+ testenv.NeedsTool(t, "go")
+
+ files := map[string]string{
+ "conflict/foo.go": `package conflict
+
+func Foo() {
+ bar := 12
+ _ = bar
+}
+
+// the end
+`,
+ }
+ dir, cleanup, err := analysistest.WriteFiles(files)
+ if err != nil {
+ t.Fatalf("Creating test files failed with %s", err)
+ }
+ defer cleanup()
+
+ args := []string{"-test.run=TestConflict", "--", "conflict"}
+ cmd := exec.Command(os.Args[0], args...)
+ cmd.Env = append(os.Environ(), "TESTCONFLICT_CHILD=1", "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off")
+
+ out, err := cmd.CombinedOutput()
+ var exitcode int
+ if err, ok := err.(*exec.ExitError); ok {
+ exitcode = err.ExitCode() // requires go1.12
+ }
+ const errExitCode = 1
+ if exitcode != errExitCode {
+ t.Errorf("%s: exited %d, want %d", args, exitcode, errExitCode)
+ }
+
+ pattern := `conflicting edits from rename and rename on /.*/conflict/foo.go`
+ matched, err := regexp.Match(pattern, out)
+ if err != nil {
+ t.Errorf("error matching pattern %s: %v", pattern, err)
+ } else if !matched {
+ t.Errorf("%s: output was=<<%s>>. Expected it to match <<%s>>", args, out, pattern)
+ }
+
+ // No files updated
+ for name, want := range files {
+ path := path.Join(dir, "src", name)
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ t.Errorf("error reading %s: %v", path, err)
+ }
+ if got := string(contents); got != want {
+ t.Errorf("contents of %s file updated. got=%s, want=%s", path, got, want)
+ }
+ }
+}
+
+// TestOther ensures that checker.Run reports conflicts from
+// distinct actions correctly.
+// This test fork/execs the main function above.
+func TestOther(t *testing.T) {
+ oses := map[string]bool{"darwin": true, "linux": true}
+ if !oses[runtime.GOOS] {
+ t.Skipf("skipping fork/exec test on this platform")
+ }
+
+ if os.Getenv("TESTOTHER_CHILD") == "1" {
+ // child process
+
+ // replace [progname -test.run=TestOther -- ...]
+ // by [progname ...]
+ os.Args = os.Args[2:]
+ os.Args[0] = "vet"
+ main()
+ panic("unreachable")
+ }
+
+ testenv.NeedsTool(t, "go")
+
+ files := map[string]string{
+ "other/foo.go": `package other
+
+func Foo() {
+ bar := 12
+ _ = bar
+}
+
+// the end
+`,
+ }
+ dir, cleanup, err := analysistest.WriteFiles(files)
+ if err != nil {
+ t.Fatalf("Creating test files failed with %s", err)
+ }
+ defer cleanup()
+
+ args := []string{"-test.run=TestOther", "--", "other"}
+ cmd := exec.Command(os.Args[0], args...)
+ cmd.Env = append(os.Environ(), "TESTOTHER_CHILD=1", "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off")
+
+ out, err := cmd.CombinedOutput()
+ var exitcode int
+ if err, ok := err.(*exec.ExitError); ok {
+ exitcode = err.ExitCode() // requires go1.12
+ }
+ const errExitCode = 1
+ if exitcode != errExitCode {
+ t.Errorf("%s: exited %d, want %d", args, exitcode, errExitCode)
+ }
+
+ pattern := `conflicting edits from other and rename on /.*/other/foo.go`
+ matched, err := regexp.Match(pattern, out)
+ if err != nil {
+ t.Errorf("error matching pattern %s: %v", pattern, err)
+ } else if !matched {
+ t.Errorf("%s: output was=<<%s>>. Expected it to match <<%s>>", args, out, pattern)
+ }
+
+ // No files updated
+ for name, want := range files {
+ path := path.Join(dir, "src", name)
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ t.Errorf("error reading %s: %v", path, err)
+ }
+ if got := string(contents); got != want {
+ t.Errorf("contents of %s file updated. got=%s, want=%s", path, got, want)
+ }
+ }
+}
diff --git a/go/analysis/internal/checker/start_test.go b/go/analysis/internal/checker/start_test.go
new file mode 100644
index 000000000..ede21159b
--- /dev/null
+++ b/go/analysis/internal/checker/start_test.go
@@ -0,0 +1,85 @@
+// Copyright 2022 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 checker_test
+
+import (
+ "go/ast"
+ "io/ioutil"
+ "path/filepath"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/internal/checker"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/internal/testenv"
+)
+
+// TestStartFixes make sure modifying the first character
+// of the file takes effect.
+func TestStartFixes(t *testing.T) {
+ testenv.NeedsGoPackages(t)
+
+ files := map[string]string{
+ "comment/doc.go": `/* Package comment */
+package comment
+`}
+
+ want := `// Package comment
+package comment
+`
+
+ testdata, cleanup, err := analysistest.WriteFiles(files)
+ if err != nil {
+ t.Fatal(err)
+ }
+ path := filepath.Join(testdata, "src/comment/doc.go")
+ checker.Fix = true
+ checker.Run([]string{"file=" + path}, []*analysis.Analyzer{commentAnalyzer})
+
+ contents, err := ioutil.ReadFile(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ got := string(contents)
+ if got != want {
+ t.Errorf("contents of rewritten file\ngot: %s\nwant: %s", got, want)
+ }
+
+ defer cleanup()
+}
+
+var commentAnalyzer = &analysis.Analyzer{
+ Name: "comment",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: commentRun,
+}
+
+func commentRun(pass *analysis.Pass) (interface{}, error) {
+ const (
+ from = "/* Package comment */"
+ to = "// Package comment"
+ )
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+ inspect.Preorder(nil, func(n ast.Node) {
+ if n, ok := n.(*ast.Comment); ok && n.Text == from {
+ pass.Report(analysis.Diagnostic{
+ Pos: n.Pos(),
+ End: n.End(),
+ SuggestedFixes: []analysis.SuggestedFix{{
+ TextEdits: []analysis.TextEdit{{
+ Pos: n.Pos(),
+ End: n.End(),
+ NewText: []byte(to),
+ }},
+ }},
+ })
+ }
+ })
+
+ return nil, nil
+}
diff --git a/go/analysis/internal/facts/facts.go b/go/analysis/internal/facts/facts.go
deleted file mode 100644
index 1fb69c615..000000000
--- a/go/analysis/internal/facts/facts.go
+++ /dev/null
@@ -1,323 +0,0 @@
-// Copyright 2018 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 facts defines a serializable set of analysis.Fact.
-//
-// It provides a partial implementation of the Fact-related parts of the
-// analysis.Pass interface for use in analysis drivers such as "go vet"
-// and other build systems.
-//
-// The serial format is unspecified and may change, so the same version
-// of this package must be used for reading and writing serialized facts.
-//
-// The handling of facts in the analysis system parallels the handling
-// of type information in the compiler: during compilation of package P,
-// the compiler emits an export data file that describes the type of
-// every object (named thing) defined in package P, plus every object
-// indirectly reachable from one of those objects. Thus the downstream
-// compiler of package Q need only load one export data file per direct
-// import of Q, and it will learn everything about the API of package P
-// and everything it needs to know about the API of P's dependencies.
-//
-// Similarly, analysis of package P emits a fact set containing facts
-// about all objects exported from P, plus additional facts about only
-// those objects of P's dependencies that are reachable from the API of
-// package P; the downstream analysis of Q need only load one fact set
-// per direct import of Q.
-//
-// The notion of "exportedness" that matters here is that of the
-// compiler. According to the language spec, a method pkg.T.f is
-// unexported simply because its name starts with lowercase. But the
-// compiler must nonetheless export f so that downstream compilations can
-// accurately ascertain whether pkg.T implements an interface pkg.I
-// defined as interface{f()}. Exported thus means "described in export
-// data".
-//
-package facts
-
-import (
- "bytes"
- "encoding/gob"
- "fmt"
- "go/types"
- "io/ioutil"
- "log"
- "reflect"
- "sort"
- "sync"
-
- "golang.org/x/tools/go/analysis"
- "golang.org/x/tools/go/types/objectpath"
-)
-
-const debug = false
-
-// A Set is a set of analysis.Facts.
-//
-// Decode creates a Set of facts by reading from the imports of a given
-// package, and Encode writes out the set. Between these operation,
-// the Import and Export methods will query and update the set.
-//
-// All of Set's methods except String are safe to call concurrently.
-type Set struct {
- pkg *types.Package
- mu sync.Mutex
- m map[key]analysis.Fact
-}
-
-type key struct {
- pkg *types.Package
- obj types.Object // (object facts only)
- t reflect.Type
-}
-
-// ImportObjectFact implements analysis.Pass.ImportObjectFact.
-func (s *Set) ImportObjectFact(obj types.Object, ptr analysis.Fact) bool {
- if obj == nil {
- panic("nil object")
- }
- key := key{pkg: obj.Pkg(), obj: obj, t: reflect.TypeOf(ptr)}
- s.mu.Lock()
- defer s.mu.Unlock()
- if v, ok := s.m[key]; ok {
- reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem())
- return true
- }
- return false
-}
-
-// ExportObjectFact implements analysis.Pass.ExportObjectFact.
-func (s *Set) ExportObjectFact(obj types.Object, fact analysis.Fact) {
- if obj.Pkg() != s.pkg {
- log.Panicf("in package %s: ExportObjectFact(%s, %T): can't set fact on object belonging another package",
- s.pkg, obj, fact)
- }
- key := key{pkg: obj.Pkg(), obj: obj, t: reflect.TypeOf(fact)}
- s.mu.Lock()
- s.m[key] = fact // clobber any existing entry
- s.mu.Unlock()
-}
-
-func (s *Set) AllObjectFacts(filter map[reflect.Type]bool) []analysis.ObjectFact {
- var facts []analysis.ObjectFact
- s.mu.Lock()
- for k, v := range s.m {
- if k.obj != nil && filter[k.t] {
- facts = append(facts, analysis.ObjectFact{Object: k.obj, Fact: v})
- }
- }
- s.mu.Unlock()
- return facts
-}
-
-// ImportPackageFact implements analysis.Pass.ImportPackageFact.
-func (s *Set) ImportPackageFact(pkg *types.Package, ptr analysis.Fact) bool {
- if pkg == nil {
- panic("nil package")
- }
- key := key{pkg: pkg, t: reflect.TypeOf(ptr)}
- s.mu.Lock()
- defer s.mu.Unlock()
- if v, ok := s.m[key]; ok {
- reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem())
- return true
- }
- return false
-}
-
-// ExportPackageFact implements analysis.Pass.ExportPackageFact.
-func (s *Set) ExportPackageFact(fact analysis.Fact) {
- key := key{pkg: s.pkg, t: reflect.TypeOf(fact)}
- s.mu.Lock()
- s.m[key] = fact // clobber any existing entry
- s.mu.Unlock()
-}
-
-func (s *Set) AllPackageFacts(filter map[reflect.Type]bool) []analysis.PackageFact {
- var facts []analysis.PackageFact
- s.mu.Lock()
- for k, v := range s.m {
- if k.obj == nil && filter[k.t] {
- facts = append(facts, analysis.PackageFact{Package: k.pkg, Fact: v})
- }
- }
- s.mu.Unlock()
- return facts
-}
-
-// gobFact is the Gob declaration of a serialized fact.
-type gobFact struct {
- PkgPath string // path of package
- Object objectpath.Path // optional path of object relative to package itself
- Fact analysis.Fact // type and value of user-defined Fact
-}
-
-// Decode decodes all the facts relevant to the analysis of package pkg.
-// The read function reads serialized fact data from an external source
-// for one of of pkg's direct imports. The empty file is a valid
-// encoding of an empty fact set.
-//
-// It is the caller's responsibility to call gob.Register on all
-// necessary fact types.
-func Decode(pkg *types.Package, read func(packagePath string) ([]byte, error)) (*Set, error) {
- // Compute the import map for this package.
- // See the package doc comment.
- packages := importMap(pkg.Imports())
-
- // Read facts from imported packages.
- // Facts may describe indirectly imported packages, or their objects.
- m := make(map[key]analysis.Fact) // one big bucket
- for _, imp := range pkg.Imports() {
- logf := func(format string, args ...interface{}) {
- if debug {
- prefix := fmt.Sprintf("in %s, importing %s: ",
- pkg.Path(), imp.Path())
- log.Print(prefix, fmt.Sprintf(format, args...))
- }
- }
-
- // Read the gob-encoded facts.
- data, err := read(imp.Path())
- if err != nil {
- return nil, fmt.Errorf("in %s, can't import facts for package %q: %v",
- pkg.Path(), imp.Path(), err)
- }
- if len(data) == 0 {
- continue // no facts
- }
- var gobFacts []gobFact
- if err := gob.NewDecoder(bytes.NewReader(data)).Decode(&gobFacts); err != nil {
- return nil, fmt.Errorf("decoding facts for %q: %v", imp.Path(), err)
- }
- if debug {
- logf("decoded %d facts: %v", len(gobFacts), gobFacts)
- }
-
- // Parse each one into a key and a Fact.
- for _, f := range gobFacts {
- factPkg := packages[f.PkgPath]
- if factPkg == nil {
- // Fact relates to a dependency that was
- // unused in this translation unit. Skip.
- logf("no package %q; discarding %v", f.PkgPath, f.Fact)
- continue
- }
- key := key{pkg: factPkg, t: reflect.TypeOf(f.Fact)}
- if f.Object != "" {
- // object fact
- obj, err := objectpath.Object(factPkg, f.Object)
- if err != nil {
- // (most likely due to unexported object)
- // TODO(adonovan): audit for other possibilities.
- logf("no object for path: %v; discarding %s", err, f.Fact)
- continue
- }
- key.obj = obj
- logf("read %T fact %s for %v", f.Fact, f.Fact, key.obj)
- } else {
- // package fact
- logf("read %T fact %s for %v", f.Fact, f.Fact, factPkg)
- }
- m[key] = f.Fact
- }
- }
-
- return &Set{pkg: pkg, m: m}, nil
-}
-
-// Encode encodes a set of facts to a memory buffer.
-//
-// It may fail if one of the Facts could not be gob-encoded, but this is
-// a sign of a bug in an Analyzer.
-func (s *Set) Encode() []byte {
-
- // TODO(adonovan): opt: use a more efficient encoding
- // that avoids repeating PkgPath for each fact.
-
- // Gather all facts, including those from imported packages.
- var gobFacts []gobFact
-
- s.mu.Lock()
- for k, fact := range s.m {
- if debug {
- log.Printf("%v => %s\n", k, fact)
- }
- var object objectpath.Path
- if k.obj != nil {
- path, err := objectpath.For(k.obj)
- if err != nil {
- if debug {
- log.Printf("discarding fact %s about %s\n", fact, k.obj)
- }
- continue // object not accessible from package API; discard fact
- }
- object = path
- }
- gobFacts = append(gobFacts, gobFact{
- PkgPath: k.pkg.Path(),
- Object: object,
- Fact: fact,
- })
- }
- s.mu.Unlock()
-
- // Sort facts by (package, object, type) for determinism.
- sort.Slice(gobFacts, func(i, j int) bool {
- x, y := gobFacts[i], gobFacts[j]
- if x.PkgPath != y.PkgPath {
- return x.PkgPath < y.PkgPath
- }
- if x.Object != y.Object {
- return x.Object < y.Object
- }
- tx := reflect.TypeOf(x.Fact)
- ty := reflect.TypeOf(y.Fact)
- if tx != ty {
- return tx.String() < ty.String()
- }
- return false // equal
- })
-
- var buf bytes.Buffer
- if len(gobFacts) > 0 {
- if err := gob.NewEncoder(&buf).Encode(gobFacts); err != nil {
- // Fact encoding should never fail. Identify the culprit.
- for _, gf := range gobFacts {
- if err := gob.NewEncoder(ioutil.Discard).Encode(gf); err != nil {
- fact := gf.Fact
- pkgpath := reflect.TypeOf(fact).Elem().PkgPath()
- log.Panicf("internal error: gob encoding of analysis fact %s failed: %v; please report a bug against fact %T in package %q",
- fact, err, fact, pkgpath)
- }
- }
- }
- }
-
- if debug {
- log.Printf("package %q: encode %d facts, %d bytes\n",
- s.pkg.Path(), len(gobFacts), buf.Len())
- }
-
- return buf.Bytes()
-}
-
-// String is provided only for debugging, and must not be called
-// concurrent with any Import/Export method.
-func (s *Set) String() string {
- var buf bytes.Buffer
- buf.WriteString("{")
- for k, f := range s.m {
- if buf.Len() > 1 {
- buf.WriteString(", ")
- }
- if k.obj != nil {
- buf.WriteString(k.obj.String())
- } else {
- buf.WriteString(k.pkg.Path())
- }
- fmt.Fprintf(&buf, ": %v", f)
- }
- buf.WriteString("}")
- return buf.String()
-}
diff --git a/go/analysis/internal/facts/facts_test.go b/go/analysis/internal/facts/facts_test.go
deleted file mode 100644
index 13c358230..000000000
--- a/go/analysis/internal/facts/facts_test.go
+++ /dev/null
@@ -1,384 +0,0 @@
-// Copyright 2018 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 facts_test
-
-import (
- "encoding/gob"
- "fmt"
- "go/token"
- "go/types"
- "os"
- "reflect"
- "testing"
-
- "golang.org/x/tools/go/analysis/analysistest"
- "golang.org/x/tools/go/analysis/internal/facts"
- "golang.org/x/tools/go/packages"
- "golang.org/x/tools/internal/testenv"
- "golang.org/x/tools/internal/typeparams"
-)
-
-type myFact struct {
- S string
-}
-
-func (f *myFact) String() string { return fmt.Sprintf("myFact(%s)", f.S) }
-func (f *myFact) AFact() {}
-
-func init() {
- gob.Register(new(myFact))
-}
-
-func TestEncodeDecode(t *testing.T) {
- tests := []struct {
- name string
- typeparams bool // requires typeparams to be enabled
- files map[string]string
- plookups []pkgLookups // see testEncodeDecode for details
- }{
- {
- name: "loading-order",
- // c -> b -> a, a2
- // c does not directly depend on a, but it indirectly uses a.T.
- //
- // Package a2 is never loaded directly so it is incomplete.
- //
- // We use only types in this example because we rely on
- // types.Eval to resolve the lookup expressions, and it only
- // works for types. This is a definite gap in the typechecker API.
- files: map[string]string{
- "a/a.go": `package a; type A int; type T int`,
- "a2/a.go": `package a2; type A2 int; type Unneeded int`,
- "b/b.go": `package b; import ("a"; "a2"); type B chan a2.A2; type F func() a.T`,
- "c/c.go": `package c; import "b"; type C []b.B`,
- },
- // In the following table, we analyze packages (a, b, c) in order,
- // look up various objects accessible within each package,
- // and see if they have a fact. The "analysis" exports a fact
- // for every object at package level.
- //
- // Note: Loop iterations are not independent test cases;
- // order matters, as we populate factmap.
- plookups: []pkgLookups{
- {"a", []lookup{
- {"A", "myFact(a.A)"},
- }},
- {"b", []lookup{
- {"a.A", "myFact(a.A)"},
- {"a.T", "myFact(a.T)"},
- {"B", "myFact(b.B)"},
- {"F", "myFact(b.F)"},
- {"F(nil)()", "myFact(a.T)"}, // (result type of b.F)
- }},
- {"c", []lookup{
- {"b.B", "myFact(b.B)"},
- {"b.F", "myFact(b.F)"},
- //{"b.F(nil)()", "myFact(a.T)"}, // no fact; TODO(adonovan): investigate
- {"C", "myFact(c.C)"},
- {"C{}[0]", "myFact(b.B)"},
- {"<-(C{}[0])", "no fact"}, // object but no fact (we never "analyze" a2)
- }},
- },
- },
- {
- name: "globals",
- files: map[string]string{
- "a/a.go": `package a;
- type T1 int
- type T2 int
- type T3 int
- type T4 int
- type T5 int
- type K int; type V string
- `,
- "b/b.go": `package b
- import "a"
- var (
- G1 []a.T1
- G2 [7]a.T2
- G3 chan a.T3
- G4 *a.T4
- G5 struct{ F a.T5 }
- G6 map[a.K]a.V
- )
- `,
- "c/c.go": `package c; import "b";
- var (
- v1 = b.G1
- v2 = b.G2
- v3 = b.G3
- v4 = b.G4
- v5 = b.G5
- v6 = b.G6
- )
- `,
- },
- plookups: []pkgLookups{
- {"a", []lookup{}},
- {"b", []lookup{}},
- {"c", []lookup{
- {"v1[0]", "myFact(a.T1)"},
- {"v2[0]", "myFact(a.T2)"},
- {"<-v3", "myFact(a.T3)"},
- {"*v4", "myFact(a.T4)"},
- {"v5.F", "myFact(a.T5)"},
- {"v6[0]", "myFact(a.V)"},
- }},
- },
- },
- {
- name: "typeparams",
- typeparams: true,
- files: map[string]string{
- "a/a.go": `package a
- type T1 int
- type T2 int
- type T3 interface{Foo()}
- type T4 int
- type T5 int
- type T6 interface{Foo()}
- `,
- "b/b.go": `package b
- import "a"
- type N1[T a.T1|int8] func() T
- type N2[T any] struct{ F T }
- type N3[T a.T3] func() T
- type N4[T a.T4|int8] func() T
- type N5[T interface{Bar() a.T5} ] func() T
-
- type t5 struct{}; func (t5) Bar() a.T5
-
- var G1 N1[a.T1]
- var G2 func() N2[a.T2]
- var G3 N3[a.T3]
- var G4 N4[a.T4]
- var G5 N5[t5]
-
- func F6[T a.T6]() T { var x T; return x }
- `,
- "c/c.go": `package c; import "b";
- var (
- v1 = b.G1
- v2 = b.G2
- v3 = b.G3
- v4 = b.G4
- v5 = b.G5
- v6 = b.F6[t6]
- )
-
- type t6 struct{}; func (t6) Foo() {}
- `,
- },
- plookups: []pkgLookups{
- {"a", []lookup{}},
- {"b", []lookup{}},
- {"c", []lookup{
- {"v1", "myFact(b.N1)"},
- {"v1()", "myFact(a.T1)"},
- {"v2()", "myFact(b.N2)"},
- {"v2().F", "myFact(a.T2)"},
- {"v3", "myFact(b.N3)"},
- {"v4", "myFact(b.N4)"},
- {"v4()", "myFact(a.T4)"},
- {"v5", "myFact(b.N5)"},
- {"v5()", "myFact(b.t5)"},
- {"v6()", "myFact(c.t6)"},
- }},
- },
- },
- }
-
- for i := range tests {
- test := tests[i]
- t.Run(test.name, func(t *testing.T) {
- t.Parallel()
- if test.typeparams && !typeparams.Enabled {
- t.Skip("type parameters are not enabled")
- }
- testEncodeDecode(t, test.files, test.plookups)
- })
- }
-}
-
-type lookup struct {
- objexpr string
- want string
-}
-
-type pkgLookups struct {
- path string
- lookups []lookup
-}
-
-// testEncodeDecode tests fact encoding and decoding and simulates how package facts
-// are passed during analysis. It operates on a group of Go file contents. Then
-// for each <package, []lookup> in tests it does the following:
-// 1) loads and type checks the package,
-// 2) calls facts.Decode to loads the facts exported by its imports,
-// 3) exports a myFact Fact for all of package level objects,
-// 4) For each lookup for the current package:
-// 4.a) lookup the types.Object for an Go source expression in the curent package
-// (or confirms one is not expected want=="no object"),
-// 4.b) finds a Fact for the object (or confirms one is not expected want=="no fact"),
-// 4.c) compares the content of the Fact to want.
-// 5) encodes the Facts of the package.
-//
-// Note: tests are not independent test cases; order matters (as does a package being
-// skipped). It changes what Facts can be imported.
-//
-// Failures are reported on t.
-func testEncodeDecode(t *testing.T, files map[string]string, tests []pkgLookups) {
- dir, cleanup, err := analysistest.WriteFiles(files)
- if err != nil {
- t.Fatal(err)
- }
- defer cleanup()
-
- // factmap represents the passing of encoded facts from one
- // package to another. In practice one would use the file system.
- factmap := make(map[string][]byte)
- read := func(path string) ([]byte, error) { return factmap[path], nil }
-
- // Analyze packages in order, look up various objects accessible within
- // each package, and see if they have a fact. The "analysis" exports a
- // fact for every object at package level.
- //
- // Note: Loop iterations are not independent test cases;
- // order matters, as we populate factmap.
- for _, test := range tests {
- // load package
- pkg, err := load(t, dir, test.path)
- if err != nil {
- t.Fatal(err)
- }
-
- // decode
- facts, err := facts.Decode(pkg, read)
- if err != nil {
- t.Fatalf("Decode failed: %v", err)
- }
- t.Logf("decode %s facts = %v", pkg.Path(), facts) // show all facts
-
- // export
- // (one fact for each package-level object)
- for _, name := range pkg.Scope().Names() {
- obj := pkg.Scope().Lookup(name)
- fact := &myFact{obj.Pkg().Name() + "." + obj.Name()}
- facts.ExportObjectFact(obj, fact)
- }
- t.Logf("exported %s facts = %v", pkg.Path(), facts) // show all facts
-
- // import
- // (after export, because an analyzer may import its own facts)
- for _, lookup := range test.lookups {
- fact := new(myFact)
- var got string
- if obj := find(pkg, lookup.objexpr); obj == nil {
- got = "no object"
- } else if facts.ImportObjectFact(obj, fact) {
- got = fact.String()
- } else {
- got = "no fact"
- }
- if got != lookup.want {
- t.Errorf("in %s, ImportObjectFact(%s, %T) = %s, want %s",
- pkg.Path(), lookup.objexpr, fact, got, lookup.want)
- }
- }
-
- // encode
- factmap[pkg.Path()] = facts.Encode()
- }
-}
-
-func find(p *types.Package, expr string) types.Object {
- // types.Eval only allows us to compute a TypeName object for an expression.
- // TODO(adonovan): support other expressions that denote an object:
- // - an identifier (or qualified ident) for a func, const, or var
- // - new(T).f for a field or method
- // I've added CheckExpr in https://go-review.googlesource.com/c/go/+/144677.
- // If that becomes available, use it.
-
- // Choose an arbitrary position within the (single-file) package
- // so that we are within the scope of its import declarations.
- somepos := p.Scope().Lookup(p.Scope().Names()[0]).Pos()
- tv, err := types.Eval(token.NewFileSet(), p, somepos, expr)
- if err != nil {
- return nil
- }
- if n, ok := tv.Type.(*types.Named); ok {
- return n.Obj()
- }
- return nil
-}
-
-func load(t *testing.T, dir string, path string) (*types.Package, error) {
- cfg := &packages.Config{
- Mode: packages.LoadSyntax,
- Dir: dir,
- Env: append(os.Environ(), "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off"),
- }
- testenv.NeedsGoPackagesEnv(t, cfg.Env)
- pkgs, err := packages.Load(cfg, path)
- if err != nil {
- return nil, err
- }
- if packages.PrintErrors(pkgs) > 0 {
- return nil, fmt.Errorf("packages had errors")
- }
- if len(pkgs) == 0 {
- return nil, fmt.Errorf("no package matched %s", path)
- }
- return pkgs[0].Types, nil
-}
-
-type otherFact struct {
- S string
-}
-
-func (f *otherFact) String() string { return fmt.Sprintf("otherFact(%s)", f.S) }
-func (f *otherFact) AFact() {}
-
-func TestFactFilter(t *testing.T) {
- files := map[string]string{
- "a/a.go": `package a; type A int`,
- }
- dir, cleanup, err := analysistest.WriteFiles(files)
- if err != nil {
- t.Fatal(err)
- }
- defer cleanup()
-
- pkg, err := load(t, dir, "a")
- if err != nil {
- t.Fatal(err)
- }
-
- obj := pkg.Scope().Lookup("A")
- s, err := facts.Decode(pkg, func(string) ([]byte, error) { return nil, nil })
- if err != nil {
- t.Fatal(err)
- }
- s.ExportObjectFact(obj, &myFact{"good object fact"})
- s.ExportPackageFact(&myFact{"good package fact"})
- s.ExportObjectFact(obj, &otherFact{"bad object fact"})
- s.ExportPackageFact(&otherFact{"bad package fact"})
-
- filter := map[reflect.Type]bool{
- reflect.TypeOf(&myFact{}): true,
- }
-
- pkgFacts := s.AllPackageFacts(filter)
- wantPkgFacts := `[{package a ("a") myFact(good package fact)}]`
- if got := fmt.Sprintf("%v", pkgFacts); got != wantPkgFacts {
- t.Errorf("AllPackageFacts: got %v, want %v", got, wantPkgFacts)
- }
-
- objFacts := s.AllObjectFacts(filter)
- wantObjFacts := "[{type a.A int myFact(good object fact)}]"
- if got := fmt.Sprintf("%v", objFacts); got != wantObjFacts {
- t.Errorf("AllObjectFacts: got %v, want %v", got, wantObjFacts)
- }
-}
diff --git a/go/analysis/internal/facts/imports.go b/go/analysis/internal/facts/imports.go
deleted file mode 100644
index ade0cc6fa..000000000
--- a/go/analysis/internal/facts/imports.go
+++ /dev/null
@@ -1,119 +0,0 @@
-// Copyright 2018 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 facts
-
-import (
- "go/types"
-
- "golang.org/x/tools/internal/typeparams"
-)
-
-// importMap computes the import map for a package by traversing the
-// entire exported API each of its imports.
-//
-// This is a workaround for the fact that we cannot access the map used
-// internally by the types.Importer returned by go/importer. The entries
-// in this map are the packages and objects that may be relevant to the
-// current analysis unit.
-//
-// Packages in the map that are only indirectly imported may be
-// incomplete (!pkg.Complete()).
-//
-func importMap(imports []*types.Package) map[string]*types.Package {
- objects := make(map[types.Object]bool)
- packages := make(map[string]*types.Package)
-
- var addObj func(obj types.Object) bool
- var addType func(T types.Type)
-
- addObj = func(obj types.Object) bool {
- if !objects[obj] {
- objects[obj] = true
- addType(obj.Type())
- if pkg := obj.Pkg(); pkg != nil {
- packages[pkg.Path()] = pkg
- }
- return true
- }
- return false
- }
-
- addType = func(T types.Type) {
- switch T := T.(type) {
- case *types.Basic:
- // nop
- case *types.Named:
- if addObj(T.Obj()) {
- // TODO(taking): Investigate why the Underlying type is not added here.
- for i := 0; i < T.NumMethods(); i++ {
- addObj(T.Method(i))
- }
- if tparams := typeparams.ForNamed(T); tparams != nil {
- for i := 0; i < tparams.Len(); i++ {
- addType(tparams.At(i))
- }
- }
- if targs := typeparams.NamedTypeArgs(T); targs != nil {
- for i := 0; i < targs.Len(); i++ {
- addType(targs.At(i))
- }
- }
- }
- case *types.Pointer:
- addType(T.Elem())
- case *types.Slice:
- addType(T.Elem())
- case *types.Array:
- addType(T.Elem())
- case *types.Chan:
- addType(T.Elem())
- case *types.Map:
- addType(T.Key())
- addType(T.Elem())
- case *types.Signature:
- addType(T.Params())
- addType(T.Results())
- if tparams := typeparams.ForSignature(T); tparams != nil {
- for i := 0; i < tparams.Len(); i++ {
- addType(tparams.At(i))
- }
- }
- case *types.Struct:
- for i := 0; i < T.NumFields(); i++ {
- addObj(T.Field(i))
- }
- case *types.Tuple:
- for i := 0; i < T.Len(); i++ {
- addObj(T.At(i))
- }
- case *types.Interface:
- for i := 0; i < T.NumMethods(); i++ {
- addObj(T.Method(i))
- }
- for i := 0; i < T.NumEmbeddeds(); i++ {
- addType(T.EmbeddedType(i)) // walk Embedded for implicits
- }
- case *typeparams.Union:
- for i := 0; i < T.Len(); i++ {
- addType(T.Term(i).Type())
- }
- case *typeparams.TypeParam:
- if addObj(T.Obj()) {
- addType(T.Constraint())
- }
- }
- }
-
- for _, imp := range imports {
- packages[imp.Path()] = imp
-
- scope := imp.Scope()
- for _, name := range scope.Names() {
- addObj(scope.Lookup(name))
- }
- }
-
- return packages
-}
diff --git a/go/analysis/passes/asmdecl/arches_go118.go b/go/analysis/passes/asmdecl/arches_go118.go
new file mode 100644
index 000000000..d8211afdc
--- /dev/null
+++ b/go/analysis/passes/asmdecl/arches_go118.go
@@ -0,0 +1,12 @@
+// Copyright 2022 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.
+
+//go:build !go1.19
+// +build !go1.19
+
+package asmdecl
+
+func additionalArches() []*asmArch {
+ return nil
+}
diff --git a/go/analysis/passes/asmdecl/arches_go119.go b/go/analysis/passes/asmdecl/arches_go119.go
new file mode 100644
index 000000000..3018383e7
--- /dev/null
+++ b/go/analysis/passes/asmdecl/arches_go119.go
@@ -0,0 +1,14 @@
+// Copyright 2022 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.
+
+//go:build go1.19
+// +build go1.19
+
+package asmdecl
+
+var asmArchLoong64 = asmArch{name: "loong64", bigEndian: false, stack: "R3", lr: true}
+
+func additionalArches() []*asmArch {
+ return []*asmArch{&asmArchLoong64}
+}
diff --git a/go/analysis/passes/asmdecl/asmdecl.go b/go/analysis/passes/asmdecl/asmdecl.go
index b05ed5c15..7288559fc 100644
--- a/go/analysis/passes/asmdecl/asmdecl.go
+++ b/go/analysis/passes/asmdecl/asmdecl.go
@@ -92,7 +92,7 @@ var (
asmArchMips64LE = asmArch{name: "mips64le", bigEndian: false, stack: "R29", lr: true}
asmArchPpc64 = asmArch{name: "ppc64", bigEndian: true, stack: "R1", lr: true, retRegs: []string{"R3", "F1"}}
asmArchPpc64LE = asmArch{name: "ppc64le", bigEndian: false, stack: "R1", lr: true, retRegs: []string{"R3", "F1"}}
- asmArchRISCV64 = asmArch{name: "riscv64", bigEndian: false, stack: "SP", lr: true}
+ asmArchRISCV64 = asmArch{name: "riscv64", bigEndian: false, stack: "SP", lr: true, retRegs: []string{"X10", "F10"}}
asmArchS390X = asmArch{name: "s390x", bigEndian: true, stack: "R15", lr: true}
asmArchWasm = asmArch{name: "wasm", bigEndian: false, stack: "SP", lr: false}
@@ -114,6 +114,7 @@ var (
)
func init() {
+ arches = append(arches, additionalArches()...)
for _, arch := range arches {
arch.sizes = types.SizesFor("gc", arch.name)
if arch.sizes == nil {
@@ -731,7 +732,7 @@ func asmCheckVar(badf func(string, ...interface{}), fn *asmFunc, line, expr stri
src = 8
}
}
- case "mips", "mipsle", "mips64", "mips64le":
+ case "loong64", "mips", "mipsle", "mips64", "mips64le":
switch op {
case "MOVB", "MOVBU":
src = 1
diff --git a/go/analysis/passes/asmdecl/asmdecl_test.go b/go/analysis/passes/asmdecl/asmdecl_test.go
index f88c188b2..50938a075 100644
--- a/go/analysis/passes/asmdecl/asmdecl_test.go
+++ b/go/analysis/passes/asmdecl/asmdecl_test.go
@@ -14,14 +14,17 @@ import (
)
var goosarches = []string{
- "linux/amd64", // asm1.s, asm4.s
- "linux/386", // asm2.s
- "linux/arm", // asm3.s
- "linux/mips64", // asm5.s
- "linux/s390x", // asm6.s
- "linux/ppc64", // asm7.s
- "linux/mips", // asm8.s,
- "js/wasm", // asm9.s
+ "linux/amd64", // asm1.s, asm4.s
+ "linux/386", // asm2.s
+ "linux/arm", // asm3.s
+ // TODO: skip test on loong64 until go toolchain supported loong64.
+ // "linux/loong64", // asm10.s
+ "linux/mips64", // asm5.s
+ "linux/s390x", // asm6.s
+ "linux/ppc64", // asm7.s
+ "linux/mips", // asm8.s,
+ "js/wasm", // asm9.s
+ "linux/riscv64", // asm11.s
}
func Test(t *testing.T) {
diff --git a/go/analysis/passes/asmdecl/testdata/src/a/asm10.s b/go/analysis/passes/asmdecl/testdata/src/a/asm10.s
new file mode 100644
index 000000000..f0045882a
--- /dev/null
+++ b/go/analysis/passes/asmdecl/testdata/src/a/asm10.s
@@ -0,0 +1,192 @@
+// Copyright 2022 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.
+
+//go:build loong64
+
+TEXT ·arg1(SB),0,$0-2
+ MOVB x+0(FP), R19
+ MOVBU y+1(FP), R18
+ MOVH x+0(FP), R19 // want `\[loong64\] arg1: invalid MOVH of x\+0\(FP\); int8 is 1-byte value`
+ MOVHU y+1(FP), R19 // want `invalid MOVHU of y\+1\(FP\); uint8 is 1-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int8 is 1-byte value`
+ MOVWU y+1(FP), R19 // want `invalid MOVWU of y\+1\(FP\); uint8 is 1-byte value`
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int8 is 1-byte value`
+ MOVV y+1(FP), R19 // want `invalid MOVV of y\+1\(FP\); uint8 is 1-byte value`
+ MOVB x+1(FP), R19 // want `invalid offset x\+1\(FP\); expected x\+0\(FP\)`
+ MOVBU y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+1\(FP\)`
+ MOVB 16(R3), R19 // want `16\(R3\) should be x\+0\(FP\)`
+ MOVB 17(R3), R19 // want `17\(R3\) should be y\+1\(FP\)`
+ MOVB 18(R3), R19 // want `use of 18\(R3\) points beyond argument frame`
+ RET
+
+TEXT ·arg2(SB),0,$0-4
+ MOVBU x+0(FP), R19 // want `arg2: invalid MOVBU of x\+0\(FP\); int16 is 2-byte value`
+ MOVB y+2(FP), R19 // want `invalid MOVB of y\+2\(FP\); uint16 is 2-byte value`
+ MOVHU x+0(FP), R19
+ MOVH y+2(FP), R18
+ MOVWU x+0(FP), R19 // want `invalid MOVWU of x\+0\(FP\); int16 is 2-byte value`
+ MOVW y+2(FP), R19 // want `invalid MOVW of y\+2\(FP\); uint16 is 2-byte value`
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int16 is 2-byte value`
+ MOVV y+2(FP), R19 // want `invalid MOVV of y\+2\(FP\); uint16 is 2-byte value`
+ MOVHU x+2(FP), R19 // want `invalid offset x\+2\(FP\); expected x\+0\(FP\)`
+ MOVH y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+2\(FP\)`
+ RET
+
+TEXT ·arg4(SB),0,$0-2 // want `arg4: wrong argument size 2; expected \$\.\.\.-8`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int32 is 4-byte value`
+ MOVB y+4(FP), R18 // want `invalid MOVB of y\+4\(FP\); uint32 is 4-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int32 is 4-byte value`
+ MOVH y+4(FP), R19 // want `invalid MOVH of y\+4\(FP\); uint32 is 4-byte value`
+ MOVW x+0(FP), R19
+ MOVW y+4(FP), R19
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int32 is 4-byte value`
+ MOVV y+4(FP), R19 // want `invalid MOVV of y\+4\(FP\); uint32 is 4-byte value`
+ MOVW x+4(FP), R19 // want `invalid offset x\+4\(FP\); expected x\+0\(FP\)`
+ MOVW y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+4\(FP\)`
+ RET
+
+TEXT ·arg8(SB),7,$0-2 // want `wrong argument size 2; expected \$\.\.\.-16`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int64 is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); uint64 is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int64 is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); uint64 is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int64 is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); uint64 is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ RET
+
+TEXT ·argint(SB),0,$0-2 // want `wrong argument size 2; expected \$\.\.\.-16`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); uint is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); uint is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); uint is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ RET
+
+TEXT ·argptr(SB),7,$0-2 // want `wrong argument size 2; expected \$\.\.\.-40`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); \*byte is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); \*byte is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); \*byte is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); \*byte is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); \*byte is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); \*byte is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ MOVW c+16(FP), R19 // want `invalid MOVW of c\+16\(FP\); chan int is 8-byte value`
+ MOVW m+24(FP), R19 // want `invalid MOVW of m\+24\(FP\); map\[int\]int is 8-byte value`
+ MOVW f+32(FP), R19 // want `invalid MOVW of f\+32\(FP\); func\(\) is 8-byte value`
+ RET
+
+TEXT ·argstring(SB),0,$32 // want `wrong argument size 0; expected \$\.\.\.-32`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); string base is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); string base is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_base+0(FP), R19 // want `invalid MOVH of x_base\+0\(FP\); string base is 8-byte value`
+ MOVW x_base+0(FP), R19 // want `invalid MOVW of x_base\+0\(FP\); string base is 8-byte value`
+ MOVV x_base+0(FP), R19
+ MOVH x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVW x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVV x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVH x_len+8(FP), R19 // want `invalid MOVH of x_len\+8\(FP\); string len is 8-byte value`
+ MOVW x_len+8(FP), R19 // want `invalid MOVW of x_len\+8\(FP\); string len is 8-byte value`
+ MOVV x_len+8(FP), R19
+ MOVV y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+16\(FP\)`
+ MOVV y_len+8(FP), R19 // want `invalid offset y_len\+8\(FP\); expected y_len\+24\(FP\)`
+ RET
+
+TEXT ·argslice(SB),0,$48 // want `wrong argument size 0; expected \$\.\.\.-48`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); slice base is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); slice base is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_base+0(FP), R19 // want `invalid MOVH of x_base\+0\(FP\); slice base is 8-byte value`
+ MOVW x_base+0(FP), R19 // want `invalid MOVW of x_base\+0\(FP\); slice base is 8-byte value`
+ MOVV x_base+0(FP), R19
+ MOVH x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVW x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVV x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVH x_len+8(FP), R19 // want `invalid MOVH of x_len\+8\(FP\); slice len is 8-byte value`
+ MOVW x_len+8(FP), R19 // want `invalid MOVW of x_len\+8\(FP\); slice len is 8-byte value`
+ MOVV x_len+8(FP), R19
+ MOVH x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVW x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVV x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVH x_cap+16(FP), R19 // want `invalid MOVH of x_cap\+16\(FP\); slice cap is 8-byte value`
+ MOVW x_cap+16(FP), R19 // want `invalid MOVW of x_cap\+16\(FP\); slice cap is 8-byte value`
+ MOVV x_cap+16(FP), R19
+ MOVV y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+24\(FP\)`
+ MOVV y_len+8(FP), R19 // want `invalid offset y_len\+8\(FP\); expected y_len\+32\(FP\)`
+ MOVV y_cap+16(FP), R19 // want `invalid offset y_cap\+16\(FP\); expected y_cap\+40\(FP\)`
+ RET
+
+TEXT ·argiface(SB),0,$0-32
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); interface type is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); interface type is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_type+0(FP), R19 // want `invalid MOVH of x_type\+0\(FP\); interface type is 8-byte value`
+ MOVW x_type+0(FP), R19 // want `invalid MOVW of x_type\+0\(FP\); interface type is 8-byte value`
+ MOVV x_type+0(FP), R19
+ MOVV x_itable+0(FP), R19 // want `unknown variable x_itable; offset 0 is x_type\+0\(FP\)`
+ MOVV x_itable+1(FP), R19 // want `unknown variable x_itable; offset 1 is x_type\+0\(FP\)`
+ MOVH x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVW x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVV x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVH x_data+8(FP), R19 // want `invalid MOVH of x_data\+8\(FP\); interface data is 8-byte value`
+ MOVW x_data+8(FP), R19 // want `invalid MOVW of x_data\+8\(FP\); interface data is 8-byte value`
+ MOVV x_data+8(FP), R19
+ MOVH y+16(FP), R19 // want `invalid MOVH of y\+16\(FP\); interface itable is 8-byte value`
+ MOVW y+16(FP), R19 // want `invalid MOVW of y\+16\(FP\); interface itable is 8-byte value`
+ MOVV y+16(FP), R19
+ MOVH y_itable+16(FP), R19 // want `invalid MOVH of y_itable\+16\(FP\); interface itable is 8-byte value`
+ MOVW y_itable+16(FP), R19 // want `invalid MOVW of y_itable\+16\(FP\); interface itable is 8-byte value`
+ MOVV y_itable+16(FP), R19
+ MOVV y_type+16(FP), R19 // want `unknown variable y_type; offset 16 is y_itable\+16\(FP\)`
+ MOVH y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVW y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVV y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVH y_data+24(FP), R19 // want `invalid MOVH of y_data\+24\(FP\); interface data is 8-byte value`
+ MOVW y_data+24(FP), R19 // want `invalid MOVW of y_data\+24\(FP\); interface data is 8-byte value`
+ MOVV y_data+24(FP), R19
+ RET
+
+TEXT ·returnint(SB),0,$0-8
+ MOVB R19, ret+0(FP) // want `invalid MOVB of ret\+0\(FP\); int is 8-byte value`
+ MOVH R19, ret+0(FP) // want `invalid MOVH of ret\+0\(FP\); int is 8-byte value`
+ MOVW R19, ret+0(FP) // want `invalid MOVW of ret\+0\(FP\); int is 8-byte value`
+ MOVV R19, ret+0(FP)
+ MOVV R19, ret+1(FP) // want `invalid offset ret\+1\(FP\); expected ret\+0\(FP\)`
+ MOVV R19, r+0(FP) // want `unknown variable r; offset 0 is ret\+0\(FP\)`
+ RET
+
+TEXT ·returnbyte(SB),0,$0-9
+ MOVV x+0(FP), R19
+ MOVB R19, ret+8(FP)
+ MOVH R19, ret+8(FP) // want `invalid MOVH of ret\+8\(FP\); byte is 1-byte value`
+ MOVW R19, ret+8(FP) // want `invalid MOVW of ret\+8\(FP\); byte is 1-byte value`
+ MOVV R19, ret+8(FP) // want `invalid MOVV of ret\+8\(FP\); byte is 1-byte value`
+ MOVB R19, ret+7(FP) // want `invalid offset ret\+7\(FP\); expected ret\+8\(FP\)`
+ RET
+
+TEXT ·returnnamed(SB),0,$0-41
+ MOVB x+0(FP), R19
+ MOVV R19, r1+8(FP)
+ MOVH R19, r2+16(FP)
+ MOVV R19, r3+24(FP)
+ MOVV R19, r3_base+24(FP)
+ MOVV R19, r3_len+32(FP)
+ MOVB R19, r4+40(FP)
+ MOVW R19, r1+8(FP) // want `invalid MOVW of r1\+8\(FP\); int is 8-byte value`
+ RET
+
+TEXT ·returnintmissing(SB),0,$0-8
+ RET // want `RET without writing to 8-byte ret\+0\(FP\)`
diff --git a/go/analysis/passes/asmdecl/testdata/src/a/asm11.s b/go/analysis/passes/asmdecl/testdata/src/a/asm11.s
new file mode 100644
index 000000000..e81e8ee17
--- /dev/null
+++ b/go/analysis/passes/asmdecl/testdata/src/a/asm11.s
@@ -0,0 +1,13 @@
+// Copyright 2022 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.
+
+// +build riscv64
+
+// writing to result in ABIInternal function
+TEXT ·returnABIInternal<ABIInternal>(SB), NOSPLIT, $8
+ MOV $123, X10
+ RET
+TEXT ·returnmissingABIInternal<ABIInternal>(SB), NOSPLIT, $8
+ MOV $123, X20
+ RET // want `RET without writing to result register`
diff --git a/go/analysis/passes/assign/assign.go b/go/analysis/passes/assign/assign.go
index 3586638ef..89146b733 100644
--- a/go/analysis/passes/assign/assign.go
+++ b/go/analysis/passes/assign/assign.go
@@ -12,6 +12,7 @@ import (
"fmt"
"go/ast"
"go/token"
+ "go/types"
"reflect"
"golang.org/x/tools/go/analysis"
@@ -51,7 +52,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
for i, lhs := range stmt.Lhs {
rhs := stmt.Rhs[i]
if analysisutil.HasSideEffects(pass.TypesInfo, lhs) ||
- analysisutil.HasSideEffects(pass.TypesInfo, rhs) {
+ analysisutil.HasSideEffects(pass.TypesInfo, rhs) ||
+ isMapIndex(pass.TypesInfo, lhs) {
continue // expressions may not be equal
}
if reflect.TypeOf(lhs) != reflect.TypeOf(rhs) {
@@ -74,3 +76,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}
+
+// isMapIndex returns true if e is a map index expression.
+func isMapIndex(info *types.Info, e ast.Expr) bool {
+ if idx, ok := analysisutil.Unparen(e).(*ast.IndexExpr); ok {
+ if typ := info.Types[idx.X].Type; typ != nil {
+ _, ok := typ.Underlying().(*types.Map)
+ return ok
+ }
+ }
+ return false
+}
diff --git a/go/analysis/passes/assign/testdata/src/a/a.go b/go/analysis/passes/assign/testdata/src/a/a.go
index eaec634d1..f9663120b 100644
--- a/go/analysis/passes/assign/testdata/src/a/a.go
+++ b/go/analysis/passes/assign/testdata/src/a/a.go
@@ -29,3 +29,31 @@ func (s *ST) SetX(x int, ch chan int) {
}
func num() int { return 2 }
+
+func Index() {
+ s := []int{1}
+ s[0] = s[0] // want "self-assignment"
+
+ var a [5]int
+ a[0] = a[0] // want "self-assignment"
+
+ pa := &[2]int{1, 2}
+ pa[1] = pa[1] // want "self-assignment"
+
+ var pss *struct { // report self assignment despite nil dereference
+ s []int
+ }
+ pss.s[0] = pss.s[0] // want "self-assignment"
+
+ m := map[int]string{1: "a"}
+ m[0] = m[0] // bail on map self-assignments due to side effects
+ m[1] = m[1] // not modeling what elements must be in the map
+ (m[2]) = (m[2]) // even with parens
+ type Map map[string]bool
+ named := make(Map)
+ named["s"] = named["s"] // even on named maps.
+ var psm *struct {
+ m map[string]int
+ }
+ psm.m["key"] = psm.m["key"] // handles dereferences
+}
diff --git a/go/analysis/passes/assign/testdata/src/a/a.go.golden b/go/analysis/passes/assign/testdata/src/a/a.go.golden
index 6c91d3666..f45b7f208 100644
--- a/go/analysis/passes/assign/testdata/src/a/a.go.golden
+++ b/go/analysis/passes/assign/testdata/src/a/a.go.golden
@@ -29,3 +29,31 @@ func (s *ST) SetX(x int, ch chan int) {
}
func num() int { return 2 }
+
+func Index() {
+ s := []int{1}
+ // want "self-assignment"
+
+ var a [5]int
+ // want "self-assignment"
+
+ pa := &[2]int{1, 2}
+ // want "self-assignment"
+
+ var pss *struct { // report self assignment despite nil dereference
+ s []int
+ }
+ // want "self-assignment"
+
+ m := map[int]string{1: "a"}
+ m[0] = m[0] // bail on map self-assignments due to side effects
+ m[1] = m[1] // not modeling what elements must be in the map
+ (m[2]) = (m[2]) // even with parens
+ type Map map[string]bool
+ named := make(Map)
+ named["s"] = named["s"] // even on named maps.
+ var psm *struct {
+ m map[string]int
+ }
+ psm.m["key"] = psm.m["key"] // handles dereferences
+}
diff --git a/go/analysis/passes/bools/bools.go b/go/analysis/passes/bools/bools.go
index 5ae47d894..0d8b0bf4f 100644
--- a/go/analysis/passes/bools/bools.go
+++ b/go/analysis/passes/bools/bools.go
@@ -94,8 +94,10 @@ func (op boolOp) commutativeSets(info *types.Info, e *ast.BinaryExpr, seen map[*
}
// checkRedundant checks for expressions of the form
-// e && e
-// e || e
+//
+// e && e
+// e || e
+//
// Exprs must contain only side effect free expressions.
func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
seen := make(map[string]bool)
@@ -110,8 +112,10 @@ func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
}
// checkSuspect checks for expressions of the form
-// x != c1 || x != c2
-// x == c1 && x == c2
+//
+// x != c1 || x != c2
+// x == c1 && x == c2
+//
// where c1 and c2 are constant expressions.
// If c1 and c2 are the same then it's redundant;
// if c1 and c2 are different then it's always true or always false.
diff --git a/go/analysis/passes/buildssa/buildssa_test.go b/go/analysis/passes/buildssa/buildssa_test.go
index 0b381500b..52f7e7aa6 100644
--- a/go/analysis/passes/buildssa/buildssa_test.go
+++ b/go/analysis/passes/buildssa/buildssa_test.go
@@ -11,6 +11,7 @@ import (
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/buildssa"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
@@ -27,3 +28,39 @@ func Test(t *testing.T) {
}
}
}
+
+func TestGenericDecls(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestGenericDecls requires type parameters.")
+ }
+ testdata := analysistest.TestData()
+ result := analysistest.Run(t, testdata, buildssa.Analyzer, "b")[0].Result
+
+ ssainfo := result.(*buildssa.SSA)
+ got := fmt.Sprint(ssainfo.SrcFuncs)
+ want := `[(*b.Pointer[T]).Load b.Load b.LoadPointer]`
+ if got != want {
+ t.Errorf("SSA.SrcFuncs = %s, want %s", got, want)
+ for _, f := range ssainfo.SrcFuncs {
+ f.WriteTo(os.Stderr)
+ }
+ }
+}
+
+func TestImporting(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestImporting depends on testdata/b/b/go which uses type parameters.")
+ }
+ testdata := analysistest.TestData()
+ result := analysistest.Run(t, testdata, buildssa.Analyzer, "c")[0].Result
+
+ ssainfo := result.(*buildssa.SSA)
+ got := fmt.Sprint(ssainfo.SrcFuncs)
+ want := `[c.A c.B]`
+ if got != want {
+ t.Errorf("SSA.SrcFuncs = %s, want %s", got, want)
+ for _, f := range ssainfo.SrcFuncs {
+ f.WriteTo(os.Stderr)
+ }
+ }
+}
diff --git a/go/analysis/passes/buildssa/testdata/src/b/b.go b/go/analysis/passes/buildssa/testdata/src/b/b.go
new file mode 100644
index 000000000..dd029cf60
--- /dev/null
+++ b/go/analysis/passes/buildssa/testdata/src/b/b.go
@@ -0,0 +1,20 @@
+// Package b contains declarations of generic functions.
+package b
+
+import "unsafe"
+
+type Pointer[T any] struct {
+ v unsafe.Pointer
+}
+
+func (x *Pointer[T]) Load() *T {
+ return (*T)(LoadPointer(&x.v))
+}
+
+func Load[T any](x *Pointer[T]) *T {
+ return x.Load()
+}
+
+func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer)
+
+var G Pointer[int]
diff --git a/go/analysis/passes/buildssa/testdata/src/c/c.go b/go/analysis/passes/buildssa/testdata/src/c/c.go
new file mode 100644
index 000000000..387a3b0ed
--- /dev/null
+++ b/go/analysis/passes/buildssa/testdata/src/c/c.go
@@ -0,0 +1,24 @@
+// Package c is to test buildssa importing packages.
+package c
+
+import (
+ "a"
+ "b"
+ "unsafe"
+)
+
+func A() {
+ _ = a.Fib(10)
+}
+
+func B() {
+ var x int
+ ptr := unsafe.Pointer(&x)
+ _ = b.LoadPointer(&ptr)
+
+ m := b.G.Load()
+ f := b.Load(&b.G)
+ if f != m {
+ panic("loads of b.G are expected to be indentical")
+ }
+}
diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go
index c4407ad91..775e507a3 100644
--- a/go/analysis/passes/buildtag/buildtag.go
+++ b/go/analysis/passes/buildtag/buildtag.go
@@ -20,7 +20,7 @@ import (
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check //go:build and // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/buildtag/buildtag_old.go b/go/analysis/passes/buildtag/buildtag_old.go
index e9234925f..0001ba536 100644
--- a/go/analysis/passes/buildtag/buildtag_old.go
+++ b/go/analysis/passes/buildtag/buildtag_old.go
@@ -22,7 +22,7 @@ import (
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/cgocall/cgocall.go b/go/analysis/passes/cgocall/cgocall.go
index 5768d0b9b..b61ee5c3d 100644
--- a/go/analysis/passes/cgocall/cgocall.go
+++ b/go/analysis/passes/cgocall/cgocall.go
@@ -122,8 +122,8 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// For example, for each raw cgo source file in the original package,
// such as this one:
//
-// package p
-// import "C"
+// package p
+// import "C"
// import "fmt"
// type T int
// const k = 3
@@ -147,9 +147,9 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// the receiver into the first parameter;
// and all functions are renamed to "_".
//
-// package p
-// import . "·this·" // declares T, k, x, y, f, g, T.f
-// import "C"
+// package p
+// import . "·this·" // declares T, k, x, y, f, g, T.f
+// import "C"
// import "fmt"
// const _ = 3
// var _, _ = fmt.Println()
@@ -169,7 +169,6 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// C.f would resolve to "·this·"._C_func_f, for example. But we have
// limited ourselves here to preserving function bodies and initializer
// expressions since that is all that the cgocall analyzer needs.
-//
func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*ast.File, info *types.Info, sizes types.Sizes) ([]*ast.File, *types.Info, error) {
const thispkg = "·this·"
@@ -284,8 +283,9 @@ func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*a
// cgoBaseType tries to look through type conversions involving
// unsafe.Pointer to find the real type. It converts:
-// unsafe.Pointer(x) => x
-// *(*unsafe.Pointer)(unsafe.Pointer(&x)) => x
+//
+// unsafe.Pointer(x) => x
+// *(*unsafe.Pointer)(unsafe.Pointer(&x)) => x
func cgoBaseType(info *types.Info, arg ast.Expr) types.Type {
switch arg := arg.(type) {
case *ast.CallExpr:
diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go
index d3670aca9..64e184d34 100644
--- a/go/analysis/passes/composite/composite.go
+++ b/go/analysis/passes/composite/composite.go
@@ -7,6 +7,7 @@
package composite
import (
+ "fmt"
"go/ast"
"go/types"
"strings"
@@ -83,7 +84,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
for _, typ := range structuralTypes {
under := deref(typ.Underlying())
- if _, ok := under.(*types.Struct); !ok {
+ strct, ok := under.(*types.Struct)
+ if !ok {
// skip non-struct composite literals
continue
}
@@ -92,20 +94,47 @@ func run(pass *analysis.Pass) (interface{}, error) {
continue
}
- // check if the CompositeLit contains an unkeyed field
+ // check if the struct contains an unkeyed field
allKeyValue := true
- for _, e := range cl.Elts {
+ var suggestedFixAvailable = len(cl.Elts) == strct.NumFields()
+ var missingKeys []analysis.TextEdit
+ for i, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
- break
+ if i >= strct.NumFields() {
+ break
+ }
+ field := strct.Field(i)
+ if !field.Exported() {
+ // Adding unexported field names for structs not defined
+ // locally will not work.
+ suggestedFixAvailable = false
+ break
+ }
+ missingKeys = append(missingKeys, analysis.TextEdit{
+ Pos: e.Pos(),
+ End: e.Pos(),
+ NewText: []byte(fmt.Sprintf("%s: ", field.Name())),
+ })
}
}
if allKeyValue {
- // all the composite literal fields are keyed
+ // all the struct fields are keyed
continue
}
- pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
+ diag := analysis.Diagnostic{
+ Pos: cl.Pos(),
+ End: cl.End(),
+ Message: fmt.Sprintf("%s struct literal uses unkeyed fields", typeName),
+ }
+ if suggestedFixAvailable {
+ diag.SuggestedFixes = []analysis.SuggestedFix{{
+ Message: "Add field names to struct literal",
+ TextEdits: missingKeys,
+ }}
+ }
+ pass.Report(diag)
return
}
})
diff --git a/go/analysis/passes/composite/composite_test.go b/go/analysis/passes/composite/composite_test.go
index 952de8bfd..7afaaa7ff 100644
--- a/go/analysis/passes/composite/composite_test.go
+++ b/go/analysis/passes/composite/composite_test.go
@@ -18,5 +18,5 @@ func Test(t *testing.T) {
if typeparams.Enabled {
pkgs = append(pkgs, "typeparams")
}
- analysistest.Run(t, testdata, composite.Analyzer, pkgs...)
+ analysistest.RunWithSuggestedFixes(t, testdata, composite.Analyzer, pkgs...)
}
diff --git a/go/analysis/passes/composite/testdata/src/a/a.go b/go/analysis/passes/composite/testdata/src/a/a.go
index 3a5bc203b..cd69d3951 100644
--- a/go/analysis/passes/composite/testdata/src/a/a.go
+++ b/go/analysis/passes/composite/testdata/src/a/a.go
@@ -11,6 +11,7 @@ import (
"go/scanner"
"go/token"
"image"
+ "sync"
"unicode"
)
@@ -79,6 +80,18 @@ var badStructLiteral = flag.Flag{ // want "unkeyed fields"
nil, // Value
"DefValue",
}
+var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+ "DefValue",
+ "Extra Field",
+}
+var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+}
var delta [3]rune
@@ -100,6 +113,10 @@ var badScannerErrorList = scanner.ErrorList{
&scanner.Error{token.Position{}, "foobar"}, // want "unkeyed fields"
}
+// sync.Mutex has unexported fields. We expect a diagnostic but no
+// suggested fix.
+var mu = sync.Mutex{0, 0} // want "unkeyed fields"
+
// Check whitelisted structs: if vet is run with --compositewhitelist=false,
// this line triggers an error.
var whitelistedPoint = image.Point{1, 2}
diff --git a/go/analysis/passes/composite/testdata/src/a/a.go.golden b/go/analysis/passes/composite/testdata/src/a/a.go.golden
new file mode 100644
index 000000000..fe73a2e0a
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/a/a.go.golden
@@ -0,0 +1,144 @@
+// Copyright 2012 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 the test for untagged struct literals.
+
+package a
+
+import (
+ "flag"
+ "go/scanner"
+ "go/token"
+ "image"
+ "sync"
+ "unicode"
+)
+
+var Okay1 = []string{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay2 = map[string]bool{
+ "Name": true,
+ "Usage": true,
+ "DefValue": true,
+}
+
+var Okay3 = struct {
+ X string
+ Y string
+ Z string
+}{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay4 = []struct {
+ A int
+ B int
+}{
+ {1, 2},
+ {3, 4},
+}
+
+type MyStruct struct {
+ X string
+ Y string
+ Z string
+}
+
+var Okay5 = &MyStruct{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay6 = []MyStruct{
+ {"foo", "bar", "baz"},
+ {"aa", "bb", "cc"},
+}
+
+var Okay7 = []*MyStruct{
+ {"foo", "bar", "baz"},
+ {"aa", "bb", "cc"},
+}
+
+// Testing is awkward because we need to reference things from a separate package
+// to trigger the warnings.
+
+var goodStructLiteral = flag.Flag{
+ Name: "Name",
+ Usage: "Usage",
+}
+var badStructLiteral = flag.Flag{ // want "unkeyed fields"
+ Name: "Name",
+ Usage: "Usage",
+ Value: nil, // Value
+ DefValue: "DefValue",
+}
+var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+ "DefValue",
+ "Extra Field",
+}
+var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+}
+
+var delta [3]rune
+
+// SpecialCase is a named slice of CaseRange to test issue 9171.
+var goodNamedSliceLiteral = unicode.SpecialCase{
+ {Lo: 1, Hi: 2, Delta: delta},
+ unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta},
+}
+var badNamedSliceLiteral = unicode.SpecialCase{
+ {Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+ unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+}
+
+// ErrorList is a named slice, so no warnings should be emitted.
+var goodScannerErrorList = scanner.ErrorList{
+ &scanner.Error{Msg: "foobar"},
+}
+var badScannerErrorList = scanner.ErrorList{
+ &scanner.Error{Pos: token.Position{}, Msg: "foobar"}, // want "unkeyed fields"
+}
+
+// sync.Mutex has unexported fields. We expect a diagnostic but no
+// suggested fix.
+var mu = sync.Mutex{0, 0} // want "unkeyed fields"
+
+// Check whitelisted structs: if vet is run with --compositewhitelist=false,
+// this line triggers an error.
+var whitelistedPoint = image.Point{1, 2}
+
+// Do not check type from unknown package.
+// See issue 15408.
+var unknownPkgVar = unicode.NoSuchType{"foo", "bar"}
+
+// A named pointer slice of CaseRange to test issue 23539. In
+// particular, we're interested in how some slice elements omit their
+// type.
+var goodNamedPointerSliceLiteral = []*unicode.CaseRange{
+ {Lo: 1, Hi: 2},
+ &unicode.CaseRange{Lo: 1, Hi: 2},
+}
+var badNamedPointerSliceLiteral = []*unicode.CaseRange{
+ {Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+ &unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+}
+
+// unicode.Range16 is whitelisted, so there'll be no vet error
+var range16 = unicode.Range16{0xfdd0, 0xfdef, 1}
+
+// unicode.Range32 is whitelisted, so there'll be no vet error
+var range32 = unicode.Range32{0x1fffe, 0x1ffff, 1}
diff --git a/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden b/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
new file mode 100644
index 000000000..20b652e88
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
@@ -0,0 +1,16 @@
+// Copyright 2022 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.
+
+//go:build go1.18
+// +build go1.18
+
+package a
+
+import "testing"
+
+var fuzzTargets = []testing.InternalFuzzTarget{
+ {"Fuzz", Fuzz},
+}
+
+func Fuzz(f *testing.F) {}
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
index dd5d57efe..f9a5e1fb1 100644
--- a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
@@ -6,7 +6,7 @@ package typeparams
import "typeparams/lib"
-type localStruct struct { F int }
+type localStruct struct{ F int }
func F[
T1 ~struct{ f int },
@@ -20,8 +20,8 @@ func F[
_ = T1{2}
_ = T2a{2}
_ = T2b{2} // want "unkeyed fields"
- _ = T3{1,2}
- _ = T4{1,2}
- _ = T5{1:2}
- _ = T6{1:2}
+ _ = T3{1, 2}
+ _ = T4{1, 2}
+ _ = T5{1: 2}
+ _ = T6{1: 2}
}
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
new file mode 100644
index 000000000..66cd9158c
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
@@ -0,0 +1,27 @@
+// Copyright 2021 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 typeparams
+
+import "typeparams/lib"
+
+type localStruct struct{ F int }
+
+func F[
+ T1 ~struct{ f int },
+ T2a localStruct,
+ T2b lib.Struct,
+ T3 ~[]int,
+ T4 lib.Slice,
+ T5 ~map[int]int,
+ T6 lib.Map,
+]() {
+ _ = T1{2}
+ _ = T2a{2}
+ _ = T2b{F: 2} // want "unkeyed fields"
+ _ = T3{1, 2}
+ _ = T4{1, 2}
+ _ = T5{1: 2}
+ _ = T6{1: 2}
+}
diff --git a/go/analysis/passes/copylock/copylock.go b/go/analysis/passes/copylock/copylock.go
index 350dc4e0f..8cc93e94d 100644
--- a/go/analysis/passes/copylock/copylock.go
+++ b/go/analysis/passes/copylock/copylock.go
@@ -128,7 +128,7 @@ func checkCopyLocksCallExpr(pass *analysis.Pass, ce *ast.CallExpr) {
}
if fun, ok := pass.TypesInfo.Uses[id].(*types.Builtin); ok {
switch fun.Name() {
- case "new", "len", "cap", "Sizeof":
+ case "new", "len", "cap", "Sizeof", "Offsetof", "Alignof":
return
}
}
diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock.go b/go/analysis/passes/copylock/testdata/src/a/copylock.go
index e528280ab..4ab66dca1 100644
--- a/go/analysis/passes/copylock/testdata/src/a/copylock.go
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock.go
@@ -50,27 +50,27 @@ func BadFunc() {
var t Tlock
var tp *Tlock
tp = &t
- *tp = t // want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync.Mutex`
- t = *tp // want "assignment copies lock value to t: a.Tlock contains sync.Once contains sync.Mutex"
+ *tp = t // want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync\b.*`
+ t = *tp // want `assignment copies lock value to t: a.Tlock contains sync.Once contains sync\b.*`
y := *x // want "assignment copies lock value to y: sync.Mutex"
- var z = t // want "variable declaration copies lock value to z: a.Tlock contains sync.Once contains sync.Mutex"
+ var z = t // want `variable declaration copies lock value to z: a.Tlock contains sync.Once contains sync\b.*`
w := struct{ L sync.Mutex }{
L: *x, // want `literal copies lock value from \*x: sync.Mutex`
}
var q = map[int]Tlock{
- 1: t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
- 2: *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ 1: t, // want `literal copies lock value from t: a.Tlock contains sync.Once contains sync\b.*`
+ 2: *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync\b.*`
}
yy := []Tlock{
- t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
- *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ t, // want `literal copies lock value from t: a.Tlock contains sync.Once contains sync\b.*`
+ *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync\b.*`
}
// override 'new' keyword
new := func(interface{}) {}
- new(t) // want "call of new copies lock value: a.Tlock contains sync.Once contains sync.Mutex"
+ new(t) // want `call of new copies lock value: a.Tlock contains sync.Once contains sync\b.*`
// copy of array of locks
var muA [5]sync.Mutex
@@ -124,6 +124,26 @@ func SizeofMutex() {
Sizeof(mu) // want "call of Sizeof copies lock value: sync.Mutex"
}
+func OffsetofMutex() {
+ type T struct {
+ f int
+ mu sync.Mutex
+ }
+ unsafe.Offsetof(T{}.mu) // OK
+ unsafe := struct{ Offsetof func(interface{}) }{}
+ unsafe.Offsetof(T{}.mu) // want "call of unsafe.Offsetof copies lock value: sync.Mutex"
+}
+
+func AlignofMutex() {
+ type T struct {
+ f int
+ mu sync.Mutex
+ }
+ unsafe.Alignof(T{}.mu) // OK
+ unsafe := struct{ Alignof func(interface{}) }{}
+ unsafe.Alignof(T{}.mu) // want "call of unsafe.Alignof copies lock value: sync.Mutex"
+}
+
// SyncTypesCheck checks copying of sync.* types except sync.Mutex
func SyncTypesCheck() {
// sync.RWMutex copying
@@ -173,9 +193,9 @@ func SyncTypesCheck() {
var onceX sync.Once
var onceXX = sync.Once{}
onceX1 := new(sync.Once)
- onceY := onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- onceY = onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- var onceYY = onceX // want "variable declaration copies lock value to onceYY: sync.Once contains sync.Mutex"
+ onceY := onceX // want `assignment copies lock value to onceY: sync.Once contains sync\b.*`
+ onceY = onceX // want `assignment copies lock value to onceY: sync.Once contains sync\b.*`
+ var onceYY = onceX // want `variable declaration copies lock value to onceYY: sync.Once contains sync\b.*`
onceP := &onceX
onceZ := &sync.Once{}
}
diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock_func.go b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
index 801bc6f24..0d3168f1e 100644
--- a/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
@@ -126,7 +126,7 @@ func AcceptedCases() {
// sync.Mutex gets called out, but without any reference to the sync.Once.
type LocalOnce sync.Once
-func (LocalOnce) Bad() {} // want "Bad passes lock by value: a.LocalOnce contains sync.Mutex"
+func (LocalOnce) Bad() {} // want `Bad passes lock by value: a.LocalOnce contains sync.\b.*`
// False negative:
// LocalMutex doesn't have a Lock method.
diff --git a/go/analysis/passes/directive/directive.go b/go/analysis/passes/directive/directive.go
new file mode 100644
index 000000000..76d852cd0
--- /dev/null
+++ b/go/analysis/passes/directive/directive.go
@@ -0,0 +1,216 @@
+// Copyright 2023 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 directive defines an Analyzer that checks known Go toolchain directives.
+package directive
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "strings"
+ "unicode"
+ "unicode/utf8"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+)
+
+const Doc = `check Go toolchain directives such as //go:debug
+
+This analyzer checks for problems with known Go toolchain directives
+in all Go source files in a package directory, even those excluded by
+//go:build constraints, and all non-Go source files too.
+
+For //go:debug (see https://go.dev/doc/godebug), the analyzer checks
+that the directives are placed only in Go source files, only above the
+package comment, and only in package main or *_test.go files.
+
+Support for other known directives may be added in the future.
+
+This analyzer does not check //go:build, which is handled by the
+buildtag analyzer.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "directive",
+ Doc: Doc,
+ Run: runDirective,
+}
+
+func runDirective(pass *analysis.Pass) (interface{}, error) {
+ for _, f := range pass.Files {
+ checkGoFile(pass, f)
+ }
+ for _, name := range pass.OtherFiles {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ for _, name := range pass.IgnoredFiles {
+ if strings.HasSuffix(name, ".go") {
+ f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments)
+ if err != nil {
+ // Not valid Go source code - not our job to diagnose, so ignore.
+ continue
+ }
+ checkGoFile(pass, f)
+ } else {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ }
+ return nil, nil
+}
+
+func checkGoFile(pass *analysis.Pass, f *ast.File) {
+ check := newChecker(pass, pass.Fset.File(f.Package).Name(), f)
+
+ for _, group := range f.Comments {
+ // A +build comment is ignored after or adjoining the package declaration.
+ if group.End()+1 >= f.Package {
+ check.inHeader = false
+ }
+ // A //go:build comment is ignored after the package declaration
+ // (but adjoining it is OK, in contrast to +build comments).
+ if group.Pos() >= f.Package {
+ check.inHeader = false
+ }
+
+ // Check each line of a //-comment.
+ for _, c := range group.List {
+ check.comment(c.Slash, c.Text)
+ }
+ }
+}
+
+func checkOtherFile(pass *analysis.Pass, filename string) error {
+ // We cannot use the Go parser, since is not a Go source file.
+ // Read the raw bytes instead.
+ content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+ if err != nil {
+ return err
+ }
+
+ check := newChecker(pass, filename, nil)
+ check.nonGoFile(token.Pos(tf.Base()), string(content))
+ return nil
+}
+
+type checker struct {
+ pass *analysis.Pass
+ filename string
+ file *ast.File // nil for non-Go file
+ inHeader bool // in file header (before package declaration)
+ inStar bool // currently in a /* */ comment
+}
+
+func newChecker(pass *analysis.Pass, filename string, file *ast.File) *checker {
+ return &checker{
+ pass: pass,
+ filename: filename,
+ file: file,
+ inHeader: true,
+ }
+}
+
+func (check *checker) nonGoFile(pos token.Pos, fullText string) {
+ // Process each line.
+ text := fullText
+ inStar := false
+ for text != "" {
+ offset := len(fullText) - len(text)
+ var line string
+ line, text, _ = stringsCut(text, "\n")
+
+ if !inStar && strings.HasPrefix(line, "//") {
+ check.comment(pos+token.Pos(offset), line)
+ continue
+ }
+
+ // Skip over, cut out any /* */ comments,
+ // to avoid being confused by a commented-out // comment.
+ for {
+ line = strings.TrimSpace(line)
+ if inStar {
+ var ok bool
+ _, line, ok = stringsCut(line, "*/")
+ if !ok {
+ break
+ }
+ inStar = false
+ continue
+ }
+ line, inStar = stringsCutPrefix(line, "/*")
+ if !inStar {
+ break
+ }
+ }
+ if line != "" {
+ // Found non-comment non-blank line.
+ // Ends space for valid //go:build comments,
+ // but also ends the fraction of the file we can
+ // reliably parse. From this point on we might
+ // incorrectly flag "comments" inside multiline
+ // string constants or anything else (this might
+ // not even be a Go program). So stop.
+ break
+ }
+ }
+}
+
+func (check *checker) comment(pos token.Pos, line string) {
+ if !strings.HasPrefix(line, "//go:") {
+ return
+ }
+ // testing hack: stop at // ERROR
+ if i := strings.Index(line, " // ERROR "); i >= 0 {
+ line = line[:i]
+ }
+
+ verb := line
+ if i := strings.IndexFunc(verb, unicode.IsSpace); i >= 0 {
+ verb = verb[:i]
+ if line[i] != ' ' && line[i] != '\t' && line[i] != '\n' {
+ r, _ := utf8.DecodeRuneInString(line[i:])
+ check.pass.Reportf(pos, "invalid space %#q in %s directive", r, verb)
+ }
+ }
+
+ switch verb {
+ default:
+ // TODO: Use the go language version for the file.
+ // If that version is not newer than us, then we can
+ // report unknown directives.
+
+ case "//go:build":
+ // Ignore. The buildtag analyzer reports misplaced comments.
+
+ case "//go:debug":
+ if check.file == nil {
+ check.pass.Reportf(pos, "//go:debug directive only valid in Go source files")
+ } else if check.file.Name.Name != "main" && !strings.HasSuffix(check.filename, "_test.go") {
+ check.pass.Reportf(pos, "//go:debug directive only valid in package main or test")
+ } else if !check.inHeader {
+ check.pass.Reportf(pos, "//go:debug directive only valid before package declaration")
+ }
+ }
+}
+
+// Go 1.18 strings.Cut.
+func stringsCut(s, sep string) (before, after string, found bool) {
+ if i := strings.Index(s, sep); i >= 0 {
+ return s[:i], s[i+len(sep):], true
+ }
+ return s, "", false
+}
+
+// Go 1.20 strings.CutPrefix.
+func stringsCutPrefix(s, prefix string) (after string, found bool) {
+ if !strings.HasPrefix(s, prefix) {
+ return s, false
+ }
+ return s[len(prefix):], true
+}
diff --git a/go/analysis/passes/directive/directive_test.go b/go/analysis/passes/directive/directive_test.go
new file mode 100644
index 000000000..a526c0d74
--- /dev/null
+++ b/go/analysis/passes/directive/directive_test.go
@@ -0,0 +1,39 @@
+// Copyright 2023 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 directive_test
+
+import (
+ "runtime"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/directive"
+)
+
+func Test(t *testing.T) {
+ if strings.HasPrefix(runtime.Version(), "go1.") && runtime.Version() < "go1.16" {
+ t.Skipf("skipping on %v", runtime.Version())
+ }
+ analyzer := *directive.Analyzer
+ analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
+ defer func() {
+ // The directive pass is unusual in that it checks the IgnoredFiles.
+ // After analysis, add IgnoredFiles to OtherFiles so that
+ // the test harness checks for expected diagnostics in those.
+ // (The test harness shouldn't do this by default because most
+ // passes can't do anything with the IgnoredFiles without type
+ // information, which is unavailable because they are ignored.)
+ var files []string
+ files = append(files, pass.OtherFiles...)
+ files = append(files, pass.IgnoredFiles...)
+ pass.OtherFiles = files
+ }()
+
+ return directive.Analyzer.Run(pass)
+ }
+ analysistest.Run(t, analysistest.TestData(), &analyzer, "a")
+}
diff --git a/go/analysis/passes/directive/testdata/src/a/badspace.go b/go/analysis/passes/directive/testdata/src/a/badspace.go
new file mode 100644
index 000000000..113139960
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/badspace.go
@@ -0,0 +1,11 @@
+// Copyright 2023 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.
+
+//go:build ignore
+
+// want +1 `invalid space '\\u00a0' in //go:debug directive`
+//go:debug 00a0
+
+package main
+
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.go b/go/analysis/passes/directive/testdata/src/a/misplaced.go
new file mode 100644
index 000000000..db30ceb47
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.go
@@ -0,0 +1,10 @@
+// Copyright 2023 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.
+
+//go:build ignore
+
+package main
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.s b/go/analysis/passes/directive/testdata/src/a/misplaced.s
new file mode 100644
index 000000000..9e26dbc52
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.s
@@ -0,0 +1,19 @@
+// Copyright 2023 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.
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+/*
+can skip over comments
+//go:debug doesn't matter here
+*/
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+package a
+
+// no error here because we can't parse this far
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced_test.go b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
new file mode 100644
index 000000000..6b4527a35
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
@@ -0,0 +1,10 @@
+// Copyright 2023 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.
+
+//go:debug panicnil=1
+
+package p_test
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/p.go b/go/analysis/passes/directive/testdata/src/a/p.go
new file mode 100644
index 000000000..e1e3e6552
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/p.go
@@ -0,0 +1,11 @@
+// Copyright 2023 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.
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
+
+package p
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/errorsas/errorsas.go b/go/analysis/passes/errorsas/errorsas.go
index 384f02557..96adad3ee 100644
--- a/go/analysis/passes/errorsas/errorsas.go
+++ b/go/analysis/passes/errorsas/errorsas.go
@@ -7,6 +7,7 @@
package errorsas
import (
+ "errors"
"go/ast"
"go/types"
@@ -50,26 +51,39 @@ func run(pass *analysis.Pass) (interface{}, error) {
if len(call.Args) < 2 {
return // not enough arguments, e.g. called with return values of another function
}
- if fn.FullName() == "errors.As" && !pointerToInterfaceOrError(pass, call.Args[1]) {
- pass.ReportRangef(call, "second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ if fn.FullName() != "errors.As" {
+ return
+ }
+ if err := checkAsTarget(pass, call.Args[1]); err != nil {
+ pass.ReportRangef(call, "%v", err)
}
})
return nil, nil
}
-var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
+var errorType = types.Universe.Lookup("error").Type()
// pointerToInterfaceOrError reports whether the type of e is a pointer to an interface or a type implementing error,
// or is the empty interface.
-func pointerToInterfaceOrError(pass *analysis.Pass, e ast.Expr) bool {
+
+// checkAsTarget reports an error if the second argument to errors.As is invalid.
+func checkAsTarget(pass *analysis.Pass, e ast.Expr) error {
t := pass.TypesInfo.Types[e].Type
if it, ok := t.Underlying().(*types.Interface); ok && it.NumMethods() == 0 {
- return true
+ // A target of interface{} is always allowed, since it often indicates
+ // a value forwarded from another source.
+ return nil
}
pt, ok := t.Underlying().(*types.Pointer)
if !ok {
- return false
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ }
+ if pt.Elem() == errorType {
+ return errors.New("second argument to errors.As should not be *error")
}
_, ok = pt.Elem().Underlying().(*types.Interface)
- return ok || types.Implements(pt.Elem(), errorType)
+ if ok || types.Implements(pt.Elem(), errorType.Underlying().(*types.Interface)) {
+ return nil
+ }
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
}
diff --git a/go/analysis/passes/errorsas/testdata/src/a/a.go b/go/analysis/passes/errorsas/testdata/src/a/a.go
index c987a8a65..7a9ae8976 100644
--- a/go/analysis/passes/errorsas/testdata/src/a/a.go
+++ b/go/analysis/passes/errorsas/testdata/src/a/a.go
@@ -28,10 +28,10 @@ func _() {
f iface
ei interface{}
)
- errors.As(nil, &e) // *error
+ errors.As(nil, &e) // want `second argument to errors.As should not be \*error`
errors.As(nil, &m) // *T where T implemements error
errors.As(nil, &f) // *interface
- errors.As(nil, perr()) // *error, via a call
+ errors.As(nil, perr()) // want `second argument to errors.As should not be \*error`
errors.As(nil, ei) // empty interface
errors.As(nil, nil) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
diff --git a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
index 5b9ec457c..4f7ae8491 100644
--- a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
@@ -28,7 +28,7 @@ func _[E error](e E) {
errors.As(nil, &e)
errors.As(nil, &m) // *T where T implemements error
errors.As(nil, &tw.t) // *T where T implements error
- errors.As(nil, perr[error]()) // *error, via a call
+ errors.As(nil, perr[error]()) // want `second argument to errors.As should not be \*error`
errors.As(nil, e) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
errors.As(nil, m) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
diff --git a/go/analysis/passes/fieldalignment/fieldalignment.go b/go/analysis/passes/fieldalignment/fieldalignment.go
index 78afe94ab..aff663046 100644
--- a/go/analysis/passes/fieldalignment/fieldalignment.go
+++ b/go/analysis/passes/fieldalignment/fieldalignment.go
@@ -23,7 +23,7 @@ import (
const Doc = `find structs that would use less memory if their fields were sorted
This analyzer find structs that can be rearranged to use less memory, and provides
-a suggested edit with the optimal order.
+a suggested edit with the most compact order.
Note that there are two different diagnostics reported. One checks struct size,
and the other reports "pointer bytes" used. Pointer bytes is how many bytes of the
@@ -41,6 +41,11 @@ has 24 pointer bytes because it has to scan further through the *uint32.
struct { string; uint32 }
has 8 because it can stop immediately after the string pointer.
+
+Be aware that the most compact order is not always the most efficient.
+In rare cases it may cause two variables each updated by its own goroutine
+to occupy the same CPU cache line, inducing a form of memory contention
+known as "false sharing" that slows down both goroutines.
`
var Analyzer = &analysis.Analyzer{
diff --git a/go/analysis/passes/httpresponse/httpresponse.go b/go/analysis/passes/httpresponse/httpresponse.go
index fd9e2af2b..3b9168c6c 100644
--- a/go/analysis/passes/httpresponse/httpresponse.go
+++ b/go/analysis/passes/httpresponse/httpresponse.go
@@ -62,15 +62,23 @@ func run(pass *analysis.Pass) (interface{}, error) {
// Find the innermost containing block, and get the list
// of statements starting with the one containing call.
- stmts := restOfBlock(stack)
+ stmts, ncalls := restOfBlock(stack)
if len(stmts) < 2 {
- return true // the call to the http function is the last statement of the block.
+ // The call to the http function is the last statement of the block.
+ return true
+ }
+
+ // Skip cases in which the call is wrapped by another (#52661).
+ // Example: resp, err := checkError(http.Get(url))
+ if ncalls > 1 {
+ return true
}
asg, ok := stmts[0].(*ast.AssignStmt)
if !ok {
return true // the first statement is not assignment.
}
+
resp := rootIdent(asg.Lhs[0])
if resp == nil {
return true // could not find the http.Response in the assignment.
@@ -130,20 +138,25 @@ func isHTTPFuncOrMethodOnClient(info *types.Info, expr *ast.CallExpr) bool {
}
// restOfBlock, given a traversal stack, finds the innermost containing
-// block and returns the suffix of its statements starting with the
-// current node (the last element of stack).
-func restOfBlock(stack []ast.Node) []ast.Stmt {
+// block and returns the suffix of its statements starting with the current
+// node, along with the number of call expressions encountered.
+func restOfBlock(stack []ast.Node) ([]ast.Stmt, int) {
+ var ncalls int
for i := len(stack) - 1; i >= 0; i-- {
if b, ok := stack[i].(*ast.BlockStmt); ok {
for j, v := range b.List {
if v == stack[i+1] {
- return b.List[j:]
+ return b.List[j:], ncalls
}
}
break
}
+
+ if _, ok := stack[i].(*ast.CallExpr); ok {
+ ncalls++
+ }
}
- return nil
+ return nil, 0
}
// rootIdent finds the root identifier x in a chain of selections x.y.z, or nil if not found.
diff --git a/go/analysis/passes/httpresponse/httpresponse_test.go b/go/analysis/passes/httpresponse/httpresponse_test.go
index 14e166789..34dc78ce2 100644
--- a/go/analysis/passes/httpresponse/httpresponse_test.go
+++ b/go/analysis/passes/httpresponse/httpresponse_test.go
@@ -5,10 +5,11 @@
package httpresponse_test
import (
+ "testing"
+
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/httpresponse"
"golang.org/x/tools/internal/typeparams"
- "testing"
)
func Test(t *testing.T) {
diff --git a/go/analysis/passes/httpresponse/testdata/src/a/a.go b/go/analysis/passes/httpresponse/testdata/src/a/a.go
index df7703f41..de4121270 100644
--- a/go/analysis/passes/httpresponse/testdata/src/a/a.go
+++ b/go/analysis/passes/httpresponse/testdata/src/a/a.go
@@ -83,3 +83,30 @@ func badClientDo() {
log.Fatal(err)
}
}
+
+func goodUnwrapResp() {
+ unwrapResp := func(resp *http.Response, err error) *http.Response {
+ if err != nil {
+ panic(err)
+ }
+ return resp
+ }
+ resp := unwrapResp(http.Get("https://golang.org"))
+ // It is ok to call defer here immediately as err has
+ // been checked in unwrapResp (see #52661).
+ defer resp.Body.Close()
+}
+
+func badUnwrapResp() {
+ unwrapResp := func(resp *http.Response, err error) string {
+ if err != nil {
+ panic(err)
+ }
+ return "https://golang.org/" + resp.Status
+ }
+ resp, err := http.Get(unwrapResp(http.Get("https://golang.org")))
+ defer resp.Body.Close() // want "using resp before checking for errors"
+ if err != nil {
+ log.Fatal(err)
+ }
+}
diff --git a/go/analysis/passes/ifaceassert/parameterized.go b/go/analysis/passes/ifaceassert/parameterized.go
index 1285ecf13..b35f62dc7 100644
--- a/go/analysis/passes/ifaceassert/parameterized.go
+++ b/go/analysis/passes/ifaceassert/parameterized.go
@@ -1,6 +1,7 @@
// Copyright 2022 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 ifaceassert
import (
diff --git a/go/analysis/passes/inspect/inspect.go b/go/analysis/passes/inspect/inspect.go
index 4bb652a72..165c70cbd 100644
--- a/go/analysis/passes/inspect/inspect.go
+++ b/go/analysis/passes/inspect/inspect.go
@@ -19,14 +19,13 @@
// Requires: []*analysis.Analyzer{inspect.Analyzer},
// }
//
-// func run(pass *analysis.Pass) (interface{}, error) {
-// inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-// inspect.Preorder(nil, func(n ast.Node) {
-// ...
-// })
-// return nil
-// }
-//
+// func run(pass *analysis.Pass) (interface{}, error) {
+// inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+// inspect.Preorder(nil, func(n ast.Node) {
+// ...
+// })
+// return nil, nil
+// }
package inspect
import (
diff --git a/go/analysis/passes/loopclosure/loopclosure.go b/go/analysis/passes/loopclosure/loopclosure.go
index 3ea91574d..ae5b4151d 100644
--- a/go/analysis/passes/loopclosure/loopclosure.go
+++ b/go/analysis/passes/loopclosure/loopclosure.go
@@ -18,19 +18,60 @@ import (
const Doc = `check references to loop variables from within nested functions
-This analyzer checks for references to loop variables from within a
-function literal inside the loop body. It checks only instances where
-the function literal is called in a defer or go statement that is the
-last statement in the loop body, as otherwise we would need whole
-program analysis.
+This analyzer reports places where a function literal references the
+iteration variable of an enclosing loop, and the loop calls the function
+in such a way (e.g. with go or defer) that it may outlive the loop
+iteration and possibly observe the wrong value of the variable.
-For example:
+In this example, all the deferred functions run after the loop has
+completed, so all observe the final value of v.
- for i, v := range s {
- go func() {
- println(i, v) // not what you might expect
- }()
- }
+ for _, v := range list {
+ defer func() {
+ use(v) // incorrect
+ }()
+ }
+
+One fix is to create a new variable for each iteration of the loop:
+
+ for _, v := range list {
+ v := v // new var per iteration
+ defer func() {
+ use(v) // ok
+ }()
+ }
+
+The next example uses a go statement and has a similar problem.
+In addition, it has a data race because the loop updates v
+concurrent with the goroutines accessing it.
+
+ for _, v := range elem {
+ go func() {
+ use(v) // incorrect, and a data race
+ }()
+ }
+
+A fix is the same as before. The checker also reports problems
+in goroutines started by golang.org/x/sync/errgroup.Group.
+A hard-to-spot variant of this form is common in parallel tests:
+
+ func Test(t *testing.T) {
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ use(test) // incorrect, and a data race
+ })
+ }
+ }
+
+The t.Parallel() call causes the rest of the function to execute
+concurrent with the loop.
+
+The analyzer reports references only in the last statement,
+as it is not deep enough to understand the effects of subsequent
+statements that might render the reference benign.
+("Last statement" is defined recursively in compound
+statements such as if, switch, and select.)
See: https://golang.org/doc/go_faq.html#closures_and_goroutines`
@@ -50,10 +91,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
// Find the variables updated by the loop statement.
- var vars []*ast.Ident
+ var vars []types.Object
addVar := func(expr ast.Expr) {
- if id, ok := expr.(*ast.Ident); ok {
- vars = append(vars, id)
+ if id, _ := expr.(*ast.Ident); id != nil {
+ if obj := pass.TypesInfo.ObjectOf(id); obj != nil {
+ vars = append(vars, obj)
+ }
}
}
var body *ast.BlockStmt
@@ -79,87 +122,312 @@ func run(pass *analysis.Pass) (interface{}, error) {
return
}
- // Inspect a go or defer statement
- // if it's the last one in the loop body.
- // (We give up if there are following statements,
- // because it's hard to prove go isn't followed by wait,
- // or defer by return.)
- if len(body.List) == 0 {
- return
- }
- // The function invoked in the last return statement.
- var fun ast.Expr
- switch s := body.List[len(body.List)-1].(type) {
- case *ast.GoStmt:
- fun = s.Call.Fun
- case *ast.DeferStmt:
- fun = s.Call.Fun
- case *ast.ExprStmt: // check for errgroup.Group.Go()
- if call, ok := s.X.(*ast.CallExpr); ok {
- fun = goInvokes(pass.TypesInfo, call)
- }
- }
- lit, ok := fun.(*ast.FuncLit)
- if !ok {
- return
- }
- ast.Inspect(lit.Body, func(n ast.Node) bool {
- id, ok := n.(*ast.Ident)
- if !ok || id.Obj == nil {
- return true
+ // Inspect statements to find function literals that may be run outside of
+ // the current loop iteration.
+ //
+ // For go, defer, and errgroup.Group.Go, we ignore all but the last
+ // statement, because it's hard to prove go isn't followed by wait, or
+ // defer by return. "Last" is defined recursively.
+ //
+ // TODO: consider allowing the "last" go/defer/Go statement to be followed by
+ // N "trivial" statements, possibly under a recursive definition of "trivial"
+ // so that that checker could, for example, conclude that a go statement is
+ // followed by an if statement made of only trivial statements and trivial expressions,
+ // and hence the go statement could still be checked.
+ forEachLastStmt(body.List, func(last ast.Stmt) {
+ var stmts []ast.Stmt
+ switch s := last.(type) {
+ case *ast.GoStmt:
+ stmts = litStmts(s.Call.Fun)
+ case *ast.DeferStmt:
+ stmts = litStmts(s.Call.Fun)
+ case *ast.ExprStmt: // check for errgroup.Group.Go
+ if call, ok := s.X.(*ast.CallExpr); ok {
+ stmts = litStmts(goInvoke(pass.TypesInfo, call))
+ }
}
- if pass.TypesInfo.Types[id].Type == nil {
- // Not referring to a variable (e.g. struct field name)
- return true
+ for _, stmt := range stmts {
+ reportCaptured(pass, vars, stmt)
}
- for _, v := range vars {
- if v.Obj == id.Obj {
- pass.ReportRangef(id, "loop variable %s captured by func literal",
- id.Name)
+ })
+
+ // Also check for testing.T.Run (with T.Parallel).
+ // We consider every t.Run statement in the loop body, because there is
+ // no commonly used mechanism for synchronizing parallel subtests.
+ // It is of course theoretically possible to synchronize parallel subtests,
+ // though such a pattern is likely to be exceedingly rare as it would be
+ // fighting against the test runner.
+ for _, s := range body.List {
+ switch s := s.(type) {
+ case *ast.ExprStmt:
+ if call, ok := s.X.(*ast.CallExpr); ok {
+ for _, stmt := range parallelSubtest(pass.TypesInfo, call) {
+ reportCaptured(pass, vars, stmt)
+ }
+
}
}
- return true
- })
+ }
})
return nil, nil
}
-// goInvokes returns a function expression that would be called asynchronously
+// reportCaptured reports a diagnostic stating a loop variable
+// has been captured by a func literal if checkStmt has escaping
+// references to vars. vars is expected to be variables updated by a loop statement,
+// and checkStmt is expected to be a statements from the body of a func literal in the loop.
+func reportCaptured(pass *analysis.Pass, vars []types.Object, checkStmt ast.Stmt) {
+ ast.Inspect(checkStmt, func(n ast.Node) bool {
+ id, ok := n.(*ast.Ident)
+ if !ok {
+ return true
+ }
+ obj := pass.TypesInfo.Uses[id]
+ if obj == nil {
+ return true
+ }
+ for _, v := range vars {
+ if v == obj {
+ pass.ReportRangef(id, "loop variable %s captured by func literal", id.Name)
+ }
+ }
+ return true
+ })
+}
+
+// forEachLastStmt calls onLast on each "last" statement in a list of statements.
+// "Last" is defined recursively so, for example, if the last statement is
+// a switch statement, then each switch case is also visited to examine
+// its last statements.
+func forEachLastStmt(stmts []ast.Stmt, onLast func(last ast.Stmt)) {
+ if len(stmts) == 0 {
+ return
+ }
+
+ s := stmts[len(stmts)-1]
+ switch s := s.(type) {
+ case *ast.IfStmt:
+ loop:
+ for {
+ forEachLastStmt(s.Body.List, onLast)
+ switch e := s.Else.(type) {
+ case *ast.BlockStmt:
+ forEachLastStmt(e.List, onLast)
+ break loop
+ case *ast.IfStmt:
+ s = e
+ case nil:
+ break loop
+ }
+ }
+ case *ast.ForStmt:
+ forEachLastStmt(s.Body.List, onLast)
+ case *ast.RangeStmt:
+ forEachLastStmt(s.Body.List, onLast)
+ case *ast.SwitchStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CaseClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ case *ast.TypeSwitchStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CaseClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ case *ast.SelectStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CommClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ default:
+ onLast(s)
+ }
+}
+
+// litStmts returns all statements from the function body of a function
+// literal.
+//
+// If fun is not a function literal, it returns nil.
+func litStmts(fun ast.Expr) []ast.Stmt {
+ lit, _ := fun.(*ast.FuncLit)
+ if lit == nil {
+ return nil
+ }
+ return lit.Body.List
+}
+
+// goInvoke returns a function expression that would be called asynchronously
// (but not awaited) in another goroutine as a consequence of the call.
// For example, given the g.Go call below, it returns the function literal expression.
//
-// import "sync/errgroup"
-// var g errgroup.Group
-// g.Go(func() error { ... })
+// import "sync/errgroup"
+// var g errgroup.Group
+// g.Go(func() error { ... })
//
// Currently only "golang.org/x/sync/errgroup.Group()" is considered.
-func goInvokes(info *types.Info, call *ast.CallExpr) ast.Expr {
- f := typeutil.StaticCallee(info, call)
- // Note: Currently only supports: golang.org/x/sync/errgroup.Go.
- if f == nil || f.Name() != "Go" {
+func goInvoke(info *types.Info, call *ast.CallExpr) ast.Expr {
+ if !isMethodCall(info, call, "golang.org/x/sync/errgroup", "Group", "Go") {
return nil
}
- recv := f.Type().(*types.Signature).Recv()
- if recv == nil {
+ return call.Args[0]
+}
+
+// parallelSubtest returns statements that can be easily proven to execute
+// concurrently via the go test runner, as t.Run has been invoked with a
+// function literal that calls t.Parallel.
+//
+// In practice, users rely on the fact that statements before the call to
+// t.Parallel are synchronous. For example by declaring test := test inside the
+// function literal, but before the call to t.Parallel.
+//
+// Therefore, we only flag references in statements that are obviously
+// dominated by a call to t.Parallel. As a simple heuristic, we only consider
+// statements following the final labeled statement in the function body, to
+// avoid scenarios where a jump would cause either the call to t.Parallel or
+// the problematic reference to be skipped.
+//
+// import "testing"
+//
+// func TestFoo(t *testing.T) {
+// tests := []int{0, 1, 2}
+// for i, test := range tests {
+// t.Run("subtest", func(t *testing.T) {
+// println(i, test) // OK
+// t.Parallel()
+// println(i, test) // Not OK
+// })
+// }
+// }
+func parallelSubtest(info *types.Info, call *ast.CallExpr) []ast.Stmt {
+ if !isMethodCall(info, call, "testing", "T", "Run") {
return nil
}
- rtype, ok := recv.Type().(*types.Pointer)
- if !ok {
+
+ if len(call.Args) != 2 {
+ // Ignore calls such as t.Run(fn()).
return nil
}
- named, ok := rtype.Elem().(*types.Named)
- if !ok {
+
+ lit, _ := call.Args[1].(*ast.FuncLit)
+ if lit == nil {
return nil
}
- if named.Obj().Name() != "Group" {
+
+ // Capture the *testing.T object for the first argument to the function
+ // literal.
+ if len(lit.Type.Params.List[0].Names) == 0 {
+ return nil
+ }
+
+ tObj := info.Defs[lit.Type.Params.List[0].Names[0]]
+ if tObj == nil {
return nil
}
+
+ // Match statements that occur after a call to t.Parallel following the final
+ // labeled statement in the function body.
+ //
+ // We iterate over lit.Body.List to have a simple, fast and "frequent enough"
+ // dominance relationship for t.Parallel(): lit.Body.List[i] dominates
+ // lit.Body.List[j] for i < j unless there is a jump.
+ var stmts []ast.Stmt
+ afterParallel := false
+ for _, stmt := range lit.Body.List {
+ stmt, labeled := unlabel(stmt)
+ if labeled {
+ // Reset: naively we don't know if a jump could have caused the
+ // previously considered statements to be skipped.
+ stmts = nil
+ afterParallel = false
+ }
+
+ if afterParallel {
+ stmts = append(stmts, stmt)
+ continue
+ }
+
+ // Check if stmt is a call to t.Parallel(), for the correct t.
+ exprStmt, ok := stmt.(*ast.ExprStmt)
+ if !ok {
+ continue
+ }
+ expr := exprStmt.X
+ if isMethodCall(info, expr, "testing", "T", "Parallel") {
+ call, _ := expr.(*ast.CallExpr)
+ if call == nil {
+ continue
+ }
+ x, _ := call.Fun.(*ast.SelectorExpr)
+ if x == nil {
+ continue
+ }
+ id, _ := x.X.(*ast.Ident)
+ if id == nil {
+ continue
+ }
+ if info.Uses[id] == tObj {
+ afterParallel = true
+ }
+ }
+ }
+
+ return stmts
+}
+
+// unlabel returns the inner statement for the possibly labeled statement stmt,
+// stripping any (possibly nested) *ast.LabeledStmt wrapper.
+//
+// The second result reports whether stmt was an *ast.LabeledStmt.
+func unlabel(stmt ast.Stmt) (ast.Stmt, bool) {
+ labeled := false
+ for {
+ labelStmt, ok := stmt.(*ast.LabeledStmt)
+ if !ok {
+ return stmt, labeled
+ }
+ labeled = true
+ stmt = labelStmt.Stmt
+ }
+}
+
+// isMethodCall reports whether expr is a method call of
+// <pkgPath>.<typeName>.<method>.
+func isMethodCall(info *types.Info, expr ast.Expr, pkgPath, typeName, method string) bool {
+ call, ok := expr.(*ast.CallExpr)
+ if !ok {
+ return false
+ }
+
+ // Check that we are calling a method <method>
+ f := typeutil.StaticCallee(info, call)
+ if f == nil || f.Name() != method {
+ return false
+ }
+ recv := f.Type().(*types.Signature).Recv()
+ if recv == nil {
+ return false
+ }
+
+ // Check that the receiver is a <pkgPath>.<typeName> or
+ // *<pkgPath>.<typeName>.
+ rtype := recv.Type()
+ if ptr, ok := recv.Type().(*types.Pointer); ok {
+ rtype = ptr.Elem()
+ }
+ named, ok := rtype.(*types.Named)
+ if !ok {
+ return false
+ }
+ if named.Obj().Name() != typeName {
+ return false
+ }
pkg := f.Pkg()
if pkg == nil {
- return nil
+ return false
}
- if pkg.Path() != "golang.org/x/sync/errgroup" {
- return nil
+ if pkg.Path() != pkgPath {
+ return false
}
- return call.Args[0]
+
+ return true
}
diff --git a/go/analysis/passes/loopclosure/loopclosure_test.go b/go/analysis/passes/loopclosure/loopclosure_test.go
index 1498838d7..55fb2a4a3 100644
--- a/go/analysis/passes/loopclosure/loopclosure_test.go
+++ b/go/analysis/passes/loopclosure/loopclosure_test.go
@@ -5,16 +5,16 @@
package loopclosure_test
import (
- "golang.org/x/tools/internal/typeparams"
"testing"
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/loopclosure"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
- tests := []string{"a", "golang.org/..."}
+ tests := []string{"a", "golang.org/...", "subtests"}
if typeparams.Enabled {
tests = append(tests, "typeparams")
}
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/a.go b/go/analysis/passes/loopclosure/testdata/src/a/a.go
index 2c8e2e6c4..7a7f05f66 100644
--- a/go/analysis/passes/loopclosure/testdata/src/a/a.go
+++ b/go/analysis/passes/loopclosure/testdata/src/a/a.go
@@ -6,7 +6,13 @@
package testdata
-import "golang.org/x/sync/errgroup"
+import (
+ "sync"
+
+ "golang.org/x/sync/errgroup"
+)
+
+var A int
func _() {
var s []int
@@ -49,6 +55,19 @@ func _() {
println(i, v)
}()
}
+
+ // iteration variable declared outside the loop
+ for A = range s {
+ go func() {
+ println(A) // want "loop variable A captured by func literal"
+ }()
+ }
+ // iteration variable declared in a different file
+ for B = range s {
+ go func() {
+ println(B) // want "loop variable B captured by func literal"
+ }()
+ }
// If the key of the range statement is not an identifier
// the code should not panic (it used to).
var x [2]int
@@ -91,9 +110,73 @@ func _() {
}
}
-// Group is used to test that loopclosure does not match on any type named "Group".
-// The checker only matches on methods "(*...errgroup.Group).Go".
-type Group struct{};
+// Cases that rely on recursively checking for last statements.
+func _() {
+
+ for i := range "outer" {
+ for j := range "inner" {
+ if j < 1 {
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ } else if j < 2 {
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ } else {
+ go func() {
+ print(i)
+ }()
+ println("we don't catch the error above because of this statement")
+ }
+ }
+ }
+
+ for i := 0; i < 10; i++ {
+ for j := 0; j < 10; j++ {
+ if j < 1 {
+ switch j {
+ case 0:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ } else if j < 2 {
+ var a interface{} = j
+ switch a.(type) {
+ case int:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ } else {
+ ch := make(chan string)
+ select {
+ case <-ch:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ }
+ }
+ }
+}
+
+// Group is used to test that loopclosure only matches Group.Go when Group is
+// from the golang.org/x/sync/errgroup package.
+type Group struct{}
func (g *Group) Go(func() error) {}
@@ -108,6 +191,21 @@ func _() {
return nil
})
}
+
+ for i, v := range s {
+ if i > 0 {
+ g.Go(func() error {
+ print(i) // want "loop variable i captured by func literal"
+ return nil
+ })
+ } else {
+ g.Go(func() error {
+ print(v) // want "loop variable v captured by func literal"
+ return nil
+ })
+ }
+ }
+
// Do not match other Group.Go cases
g1 := new(Group)
for i, v := range s {
@@ -118,3 +216,28 @@ func _() {
})
}
}
+
+// Real-world example from #16520, slightly simplified
+func _() {
+ var nodes []interface{}
+
+ critical := new(errgroup.Group)
+ others := sync.WaitGroup{}
+
+ isCritical := func(node interface{}) bool { return false }
+ run := func(node interface{}) error { return nil }
+
+ for _, node := range nodes {
+ if isCritical(node) {
+ critical.Go(func() error {
+ return run(node) // want "loop variable node captured by func literal"
+ })
+ } else {
+ others.Add(1)
+ go func() {
+ _ = run(node) // want "loop variable node captured by func literal"
+ others.Done()
+ }()
+ }
+ }
+}
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/b.go b/go/analysis/passes/loopclosure/testdata/src/a/b.go
new file mode 100644
index 000000000..d4e5da418
--- /dev/null
+++ b/go/analysis/passes/loopclosure/testdata/src/a/b.go
@@ -0,0 +1,9 @@
+// Copyright 2022 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 testdata
+
+// B is declared in a separate file to test that object resolution spans the
+// entire package.
+var B int
diff --git a/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
new file mode 100644
index 000000000..50283ec61
--- /dev/null
+++ b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
@@ -0,0 +1,202 @@
+// Copyright 2022 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 tests that the loopclosure analyzer detects leaked
+// references via parallel subtests.
+
+package subtests
+
+import (
+ "testing"
+)
+
+// T is used to test that loopclosure only matches T.Run when T is from the
+// testing package.
+type T struct{}
+
+// Run should not match testing.T.Run. Note that the second argument is
+// intentionally a *testing.T, not a *T, so that we can check both
+// testing.T.Parallel inside a T.Run, and a T.Parallel inside a testing.T.Run.
+func (t *T) Run(string, func(*testing.T)) {
+}
+
+func (t *T) Parallel() {}
+
+func _(t *testing.T) {
+ for i, test := range []int{1, 2, 3} {
+ // Check that parallel subtests are identified.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that serial tests are OK.
+ t.Run("", func(t *testing.T) {
+ println(i)
+ println(test)
+ })
+
+ // Check that the location of t.Parallel matters.
+ t.Run("", func(t *testing.T) {
+ println(i)
+ println(test)
+ t.Parallel()
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that *testing.T value matters.
+ t.Run("", func(t *testing.T) {
+ var x testing.T
+ x.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that shadowing the loop variables within the test literal is OK if
+ // it occurs before t.Parallel().
+ t.Run("", func(t *testing.T) {
+ i := i
+ test := test
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that shadowing the loop variables within the test literal is Not
+ // OK if it occurs after t.Parallel().
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ i := i // want "loop variable i captured by func literal"
+ test := test // want "loop variable test captured by func literal"
+ println(i) // OK
+ println(test) // OK
+ })
+
+ // Check uses in nested blocks.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ {
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ }
+ })
+
+ // Check that we catch uses in nested subtests.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ t.Run("", func(t *testing.T) {
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+ })
+
+ // Check that there is no diagnostic if t is not a *testing.T.
+ t.Run("", func(_ *testing.T) {
+ t := &T{}
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that there is no diagnostic when a jump to a label may have caused
+ // the call to t.Parallel to have been skipped.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test
+ }
+ t.Parallel()
+ Test:
+ println(i)
+ println(test)
+ })
+
+ // Check that there is no diagnostic when a jump to a label may have caused
+ // the loop variable reference to be skipped, but there is a diagnostic
+ // when both the call to t.Parallel and the loop variable reference occur
+ // after the final label in the block.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test
+ }
+ t.Parallel()
+ println(i) // maybe OK
+ Test:
+ t.Parallel()
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that multiple labels are handled.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test1
+ } else {
+ goto Test2
+ }
+ Test1:
+ Test2:
+ t.Parallel()
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that we do not have problems when t.Run has a single argument.
+ fn := func() (string, func(t *testing.T)) { return "", nil }
+ t.Run(fn())
+ }
+}
+
+// Check that there is no diagnostic when loop variables are shadowed within
+// the loop body.
+func _(t *testing.T) {
+ for i, test := range []int{1, 2, 3} {
+ i := i
+ test := test
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+ }
+}
+
+// Check that t.Run must be *testing.T.Run.
+func _(t *T) {
+ for i, test := range []int{1, 2, 3} {
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+ }
+}
+
+// Check that the top-level must be parallel in order to cause a diagnostic.
+//
+// From https://pkg.go.dev/testing:
+//
+// "Run does not return until parallel subtests have completed, providing a
+// way to clean up after a group of parallel tests"
+func _(t *testing.T) {
+ for _, test := range []int{1, 2, 3} {
+ // In this subtest, a/b must complete before the synchronous subtest "a"
+ // completes, so the reference to test does not escape the current loop
+ // iteration.
+ t.Run("a", func(s *testing.T) {
+ s.Run("b", func(u *testing.T) {
+ u.Parallel()
+ println(test)
+ })
+ })
+
+ // In this subtest, c executes concurrently, so the reference to test may
+ // escape the current loop iteration.
+ t.Run("c", func(s *testing.T) {
+ s.Parallel()
+ s.Run("d", func(u *testing.T) {
+ println(test) // want "loop variable test captured by func literal"
+ })
+ })
+ }
+}
diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go
index 8fd8cd000..6849c33cc 100644
--- a/go/analysis/passes/nilness/nilness.go
+++ b/go/analysis/passes/nilness/nilness.go
@@ -15,6 +15,7 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/buildssa"
"golang.org/x/tools/go/ssa"
+ "golang.org/x/tools/internal/typeparams"
)
const Doc = `check for redundant or impossible nil comparisons
@@ -102,8 +103,11 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
for _, instr := range b.Instrs {
switch instr := instr.(type) {
case ssa.CallInstruction:
- notNil(stack, instr, instr.Common().Value,
- instr.Common().Description())
+ // A nil receiver may be okay for type params.
+ cc := instr.Common()
+ if !(cc.IsInvoke() && typeparams.IsTypeParam(cc.Value.Type())) {
+ notNil(stack, instr, cc.Value, cc.Description())
+ }
case *ssa.FieldAddr:
notNil(stack, instr, instr.X, "field selection")
case *ssa.IndexAddr:
@@ -250,7 +254,7 @@ func (n nilness) String() string { return nilnessStrings[n+1] }
// or unknown given the dominating stack of facts.
func nilnessOf(stack []fact, v ssa.Value) nilness {
switch v := v.(type) {
- // unwrap ChangeInterface values recursively, to detect if underlying
+ // unwrap ChangeInterface and Slice values recursively, to detect if underlying
// values have any facts recorded or are otherwise known with regard to nilness.
//
// This work must be in addition to expanding facts about
@@ -264,6 +268,10 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
if underlying := nilnessOf(stack, v.X); underlying != unknown {
return underlying
}
+ case *ssa.Slice:
+ if underlying := nilnessOf(stack, v.X); underlying != unknown {
+ return underlying
+ }
case *ssa.SliceToArrayPointer:
nn := nilnessOf(stack, v.X)
if slice2ArrayPtrLen(v) > 0 {
@@ -302,9 +310,9 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
return isnonnil
case *ssa.Const:
if v.IsNil() {
- return isnil
+ return isnil // nil or zero value of a pointer-like type
} else {
- return isnonnil
+ return unknown // non-pointer
}
}
diff --git a/go/analysis/passes/nilness/nilness_test.go b/go/analysis/passes/nilness/nilness_test.go
index b258c1efb..99c4dfbac 100644
--- a/go/analysis/passes/nilness/nilness_test.go
+++ b/go/analysis/passes/nilness/nilness_test.go
@@ -9,9 +9,26 @@ import (
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/nilness"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, nilness.Analyzer, "a")
}
+
+func TestInstantiated(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestInstantiated requires type parameters")
+ }
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, nilness.Analyzer, "c")
+}
+
+func TestTypeSet(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestTypeSet requires type parameters")
+ }
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, nilness.Analyzer, "d")
+}
diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go
index f4d8f455e..0629e08d8 100644
--- a/go/analysis/passes/nilness/testdata/src/a/a.go
+++ b/go/analysis/passes/nilness/testdata/src/a/a.go
@@ -130,7 +130,6 @@ func f9(x interface {
b()
c()
}) {
-
x.b() // we don't catch this panic because we don't have any facts yet
xx := interface {
a()
@@ -155,11 +154,27 @@ func f9(x interface {
}
}
+func f10() {
+ s0 := make([]string, 0)
+ if s0 == nil { // want "impossible condition: non-nil == nil"
+ print(0)
+ }
+
+ var s1 []string
+ if s1 == nil { // want "tautological condition: nil == nil"
+ print(0)
+ }
+ s2 := s1[:][:]
+ if s2 == nil { // want "tautological condition: nil == nil"
+ print(0)
+ }
+}
+
func unknown() bool {
return false
}
-func f10(a interface{}) {
+func f11(a interface{}) {
switch a.(type) {
case nil:
return
@@ -170,7 +185,7 @@ func f10(a interface{}) {
}
}
-func f11(a interface{}) {
+func f12(a interface{}) {
switch a {
case nil:
return
@@ -181,3 +196,23 @@ func f11(a interface{}) {
return
}
}
+
+type Y struct {
+ innerY
+}
+
+type innerY struct {
+ value int
+}
+
+func f13() {
+ var d *Y
+ print(d.value) // want "nil dereference in field selection"
+}
+
+func f14() {
+ var x struct{ f string }
+ if x == struct{ f string }{} { // we don't catch this tautology as we restrict to reference types
+ print(x)
+ }
+}
diff --git a/go/analysis/passes/nilness/testdata/src/c/c.go b/go/analysis/passes/nilness/testdata/src/c/c.go
new file mode 100644
index 000000000..c9a05a714
--- /dev/null
+++ b/go/analysis/passes/nilness/testdata/src/c/c.go
@@ -0,0 +1,14 @@
+package c
+
+func instantiated[X any](x *X) int {
+ if x == nil {
+ print(*x) // want "nil dereference in load"
+ }
+ return 1
+}
+
+var g int
+
+func init() {
+ g = instantiated[int](&g)
+}
diff --git a/go/analysis/passes/nilness/testdata/src/d/d.go b/go/analysis/passes/nilness/testdata/src/d/d.go
new file mode 100644
index 000000000..72bd1c872
--- /dev/null
+++ b/go/analysis/passes/nilness/testdata/src/d/d.go
@@ -0,0 +1,55 @@
+package d
+
+type message interface{ PR() }
+
+func noparam() {
+ var messageT message
+ messageT.PR() // want "nil dereference in dynamic method call"
+}
+
+func paramNonnil[T message]() {
+ var messageT T
+ messageT.PR() // cannot conclude messageT is nil.
+}
+
+func instance() {
+ // buildssa.BuilderMode does not include InstantiateGenerics.
+ paramNonnil[message]() // no warning is expected as param[message] id not built.
+}
+
+func param[T interface {
+ message
+ ~*int | ~chan int
+}]() {
+ var messageT T // messageT is nil.
+ messageT.PR() // nil receiver may be okay. See param[nilMsg].
+}
+
+type nilMsg chan int
+
+func (m nilMsg) PR() {
+ if m == nil {
+ print("not an error")
+ }
+}
+
+var G func() = param[nilMsg] // no warning
+
+func allNillable[T ~*int | ~chan int]() {
+ var x, y T // both are nillable and are nil.
+ if x != y { // want "impossible condition: nil != nil"
+ print("unreachable")
+ }
+}
+
+func notAll[T ~*int | ~chan int | ~int]() {
+ var x, y T // neither are nillable due to ~int
+ if x != y { // no warning
+ print("unreachable")
+ }
+}
+
+func noninvoke[T ~func()]() {
+ var x T
+ x() // want "nil dereference in dynamic function call"
+}
diff --git a/go/analysis/passes/pkgfact/pkgfact.go b/go/analysis/passes/pkgfact/pkgfact.go
index 2262fc4f1..f4f5616e5 100644
--- a/go/analysis/passes/pkgfact/pkgfact.go
+++ b/go/analysis/passes/pkgfact/pkgfact.go
@@ -10,14 +10,14 @@
// Each key/value pair comes from a top-level constant declaration
// whose name starts and ends with "_". For example:
//
-// package p
+// package p
//
-// const _greeting_ = "hello"
-// const _audience_ = "world"
+// const _greeting_ = "hello"
+// const _audience_ = "world"
//
// the pkgfact analysis output for package p would be:
//
-// {"greeting": "hello", "audience": "world"}.
+// {"greeting": "hello", "audience": "world"}.
//
// In addition, the analysis reports a diagnostic at each import
// showing which key/value pairs it contributes.
diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go
index dee37d78a..daaf709a4 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -342,7 +342,6 @@ func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, k
// not do so with gccgo, and nor do some other build systems.
// TODO(adonovan): eliminate the redundant facts once this restriction
// is lifted.
-//
var isPrint = stringSet{
"fmt.Errorf": true,
"fmt.Fprint": true,
@@ -584,7 +583,6 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.F
argNum := firstArg
maxArgNum := firstArg
anyIndex := false
- anyW := false
for i, w := 0, 0; i < len(format); i += w {
w = 1
if format[i] != '%' {
@@ -607,11 +605,6 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.F
pass.Reportf(call.Pos(), "%s does not support error-wrapping directive %%w", state.name)
return
}
- if anyW {
- pass.Reportf(call.Pos(), "%s call has more than one error-wrapping directive %%w", state.name)
- return
- }
- anyW = true
}
if len(state.argNums) > 0 {
// Continue with the next sequential argument.
@@ -673,12 +666,13 @@ func (s *formatState) parseIndex() bool {
s.scanNum()
ok := true
if s.nbytes == len(s.format) || s.nbytes == start || s.format[s.nbytes] != ']' {
- ok = false
- s.nbytes = strings.Index(s.format, "]")
+ ok = false // syntax error is either missing "]" or invalid index.
+ s.nbytes = strings.Index(s.format[start:], "]")
if s.nbytes < 0 {
s.pass.ReportRangef(s.call, "%s format %s is missing closing ]", s.name, s.format)
return false
}
+ s.nbytes = s.nbytes + start
}
arg32, err := strconv.ParseInt(s.format[start:s.nbytes], 10, 32)
if err != nil || !ok || arg32 <= 0 || arg32 > int64(len(s.call.Args)-s.firstArg) {
@@ -931,9 +925,9 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
// recursiveStringer reports whether the argument e is a potential
// recursive call to stringer or is an error, such as t and &t in these examples:
//
-// func (t *T) String() string { printf("%s", t) }
-// func (t T) Error() string { printf("%s", t) }
-// func (t T) String() string { printf("%s", &t) }
+// func (t *T) String() string { printf("%s", t) }
+// func (t T) Error() string { printf("%s", t) }
+// func (t T) String() string { printf("%s", &t) }
func recursiveStringer(pass *analysis.Pass, e ast.Expr) (string, bool) {
typ := pass.TypesInfo.Types[e].Type
@@ -951,11 +945,16 @@ func recursiveStringer(pass *analysis.Pass, e ast.Expr) (string, bool) {
return "", false
}
+ // inScope returns true if e is in the scope of f.
+ inScope := func(e ast.Expr, f *types.Func) bool {
+ return f.Scope() != nil && f.Scope().Contains(e.Pos())
+ }
+
// Is the expression e within the body of that String or Error method?
var method *types.Func
- if strOk && strMethod.Pkg() == pass.Pkg && strMethod.Scope().Contains(e.Pos()) {
+ if strOk && strMethod.Pkg() == pass.Pkg && inScope(e, strMethod) {
method = strMethod
- } else if errOk && errMethod.Pkg() == pass.Pkg && errMethod.Scope().Contains(e.Pos()) {
+ } else if errOk && errMethod.Pkg() == pass.Pkg && inScope(e, errMethod) {
method = errMethod
} else {
return "", false
diff --git a/go/analysis/passes/printf/testdata/src/a/a.go b/go/analysis/passes/printf/testdata/src/a/a.go
index 5eca3172d..0c4d11bf0 100644
--- a/go/analysis/passes/printf/testdata/src/a/a.go
+++ b/go/analysis/passes/printf/testdata/src/a/a.go
@@ -217,6 +217,7 @@ func PrintfTests() {
Printf("%[2]*.[1]*[3]d x", 2, "hi", 4) // want `a.Printf format %\[2]\*\.\[1\]\*\[3\]d uses non-int \x22hi\x22 as argument of \*`
Printf("%[0]s x", "arg1") // want `a.Printf format has invalid argument index \[0\]`
Printf("%[0]d x", 1) // want `a.Printf format has invalid argument index \[0\]`
+ Printf("%[3]*.[2*[1]f", 1, 2, 3) // want `a.Printf format has invalid argument index \[2\*\[1\]`
// Something that satisfies the error interface.
var e error
fmt.Println(e.Error()) // ok
@@ -341,7 +342,7 @@ func PrintfTests() {
_ = fmt.Errorf("%[2]w %[1]s", "x", err) // OK
_ = fmt.Errorf("%[2]w %[1]s", e, "x") // want `fmt.Errorf format %\[2\]w has arg "x" of wrong type string`
_ = fmt.Errorf("%w", "x") // want `fmt.Errorf format %w has arg "x" of wrong type string`
- _ = fmt.Errorf("%w %w", err, err) // want `fmt.Errorf call has more than one error-wrapping directive %w`
+ _ = fmt.Errorf("%w %w", err, err) // OK
_ = fmt.Errorf("%w", interface{}(nil)) // want `fmt.Errorf format %w has arg interface{}\(nil\) of wrong type interface{}`
_ = fmt.Errorf("%w", errorTestOK(0)) // concrete value implements error
_ = fmt.Errorf("%w", errSubset) // interface value implements error
diff --git a/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go b/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
index 76a9a205a..c4d7e530d 100644
--- a/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
+++ b/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
@@ -121,3 +121,25 @@ func TestTermReduction[T1 interface{ ~int | string }, T2 interface {
fmt.Printf("%d", t2)
fmt.Printf("%s", t2) // want "wrong type.*contains typeparams.myInt"
}
+
+type U[T any] struct{}
+
+func (u U[T]) String() string {
+ fmt.Println(u) // want `fmt.Println arg u causes recursive call to \(typeparams.U\[T\]\).String method`
+ return ""
+}
+
+type S[T comparable] struct {
+ t T
+}
+
+func (s S[T]) String() T {
+ fmt.Println(s) // Not flagged. We currently do not consider String() T to implement fmt.Stringer (see #55928).
+ return s.t
+}
+
+func TestInstanceStringer() {
+ // Tests String method with nil Scope (#55350)
+ fmt.Println(&S[string]{})
+ fmt.Println(&U[string]{})
+}
diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go
index 270e917c8..7cbb0bdbf 100644
--- a/go/analysis/passes/printf/types.go
+++ b/go/analysis/passes/printf/types.go
@@ -299,13 +299,3 @@ func isConvertibleToString(typ types.Type) bool {
return false
}
-
-// hasBasicType reports whether x's type is a types.Basic with the given kind.
-func hasBasicType(pass *analysis.Pass, x ast.Expr, kind types.BasicKind) bool {
- t := pass.TypesInfo.Types[x].Type
- if t != nil {
- t = t.Underlying()
- }
- b, ok := t.(*types.Basic)
- return ok && b.Kind() == kind
-}
diff --git a/go/analysis/passes/shadow/shadow.go b/go/analysis/passes/shadow/shadow.go
index b160dcf5b..a19cecd14 100644
--- a/go/analysis/passes/shadow/shadow.go
+++ b/go/analysis/passes/shadow/shadow.go
@@ -120,7 +120,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
// the block, we should complain about it but don't.
// - A variable declared inside a function literal can falsely be identified
// as shadowing a variable in the outer function.
-//
type span struct {
min token.Pos
max token.Pos
diff --git a/go/analysis/passes/sigchanyzer/sigchanyzer.go b/go/analysis/passes/sigchanyzer/sigchanyzer.go
index 0d6c8ebf1..c490a84ea 100644
--- a/go/analysis/passes/sigchanyzer/sigchanyzer.go
+++ b/go/analysis/passes/sigchanyzer/sigchanyzer.go
@@ -50,7 +50,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
case *ast.CallExpr:
// Only signal.Notify(make(chan os.Signal), os.Interrupt) is safe,
- // conservatively treate others as not safe, see golang/go#45043
+ // conservatively treat others as not safe, see golang/go#45043
if isBuiltinMake(pass.TypesInfo, arg) {
return
}
diff --git a/go/analysis/passes/sortslice/analyzer.go b/go/analysis/passes/sortslice/analyzer.go
index 5eb957a18..f85837d66 100644
--- a/go/analysis/passes/sortslice/analyzer.go
+++ b/go/analysis/passes/sortslice/analyzer.go
@@ -52,11 +52,20 @@ func run(pass *analysis.Pass) (interface{}, error) {
arg := call.Args[0]
typ := pass.TypesInfo.Types[arg].Type
+
+ if tuple, ok := typ.(*types.Tuple); ok {
+ typ = tuple.At(0).Type() // special case for Slice(f(...))
+ }
+
switch typ.Underlying().(type) {
case *types.Slice, *types.Interface:
return
}
+ // Restore typ to the original type, we may unwrap the tuple above,
+ // typ might not be the type of arg.
+ typ = pass.TypesInfo.Types[arg].Type
+
var fixes []analysis.SuggestedFix
switch v := typ.Underlying().(type) {
case *types.Array:
diff --git a/go/analysis/passes/sortslice/testdata/src/a/a.go b/go/analysis/passes/sortslice/testdata/src/a/a.go
index bc6cc16e9..c6aca8df1 100644
--- a/go/analysis/passes/sortslice/testdata/src/a/a.go
+++ b/go/analysis/passes/sortslice/testdata/src/a/a.go
@@ -6,8 +6,8 @@ import "sort"
func IncorrectSort() {
i := 5
sortFn := func(i, j int) bool { return false }
- sort.Slice(i, sortFn) // want "sort.Slice's argument must be a slice; is called with int"
- sort.SliceStable(i, sortFn) // want "sort.SliceStable's argument must be a slice; is called with int"
+ sort.Slice(i, sortFn) // want "sort.Slice's argument must be a slice; is called with int"
+ sort.SliceStable(i, sortFn) // want "sort.SliceStable's argument must be a slice; is called with int"
sort.SliceIsSorted(i, sortFn) // want "sort.SliceIsSorted's argument must be a slice; is called with int"
}
@@ -62,3 +62,23 @@ func UnderlyingSlice() {
sort.SliceStable(s, sortFn)
sort.SliceIsSorted(s, sortFn)
}
+
+// FunctionResultsAsArguments passes a function which returns two values
+// that satisfy sort.Slice signature. It should not produce a diagnostic.
+func FunctionResultsAsArguments() {
+ s := []string{"a", "z", "ooo"}
+ sort.Slice(less(s))
+ sort.Slice(lessPtr(s)) // want `sort.Slice's argument must be a slice; is called with \(\*\[\]string,.*`
+}
+
+func less(s []string) ([]string, func(i, j int) bool) {
+ return s, func(i, j int) bool {
+ return s[i] < s[j]
+ }
+}
+
+func lessPtr(s []string) (*[]string, func(i, j int) bool) {
+ return &s, func(i, j int) bool {
+ return s[i] < s[j]
+ }
+}
diff --git a/go/analysis/passes/stdmethods/stdmethods.go b/go/analysis/passes/stdmethods/stdmethods.go
index cc9497179..41f455d10 100644
--- a/go/analysis/passes/stdmethods/stdmethods.go
+++ b/go/analysis/passes/stdmethods/stdmethods.go
@@ -134,6 +134,19 @@ func canonicalMethod(pass *analysis.Pass, id *ast.Ident) {
}
}
+ // Special case: Unwrap has two possible signatures.
+ // Check for Unwrap() []error here.
+ if id.Name == "Unwrap" {
+ if args.Len() == 0 && results.Len() == 1 {
+ t := typeString(results.At(0).Type())
+ if t == "error" || t == "[]error" {
+ return
+ }
+ }
+ pass.ReportRangef(id, "method Unwrap() should have signature Unwrap() error or Unwrap() []error")
+ return
+ }
+
// Do the =s (if any) all match?
if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") {
return
diff --git a/go/analysis/passes/stdmethods/testdata/src/a/a.go b/go/analysis/passes/stdmethods/testdata/src/a/a.go
index c95cf5d2b..2b01f4693 100644
--- a/go/analysis/passes/stdmethods/testdata/src/a/a.go
+++ b/go/analysis/passes/stdmethods/testdata/src/a/a.go
@@ -49,7 +49,7 @@ func (E) Error() string { return "" } // E implements error.
func (E) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (E) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type F int
@@ -57,8 +57,18 @@ func (F) Error() string { return "" } // Both F and *F implement error.
func (*F) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (*F) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type G int
func (G) As(interface{}) bool // ok
+
+type W int
+
+func (W) Error() string { return "" }
+func (W) Unwrap() error { return nil } // ok
+
+type M int
+
+func (M) Error() string { return "" }
+func (M) Unwrap() []error { return nil } // ok
diff --git a/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go b/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
index 72df30d49..3d4146e9b 100644
--- a/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
@@ -30,7 +30,7 @@ func (E[_]) Error() string { return "" } // E implements error.
func (E[P]) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (E[_]) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (E[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (E[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type F[P any] int
@@ -38,4 +38,4 @@ func (F[_]) Error() string { return "" } // Both F and *F implement error.
func (*F[_]) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (*F[_]) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (*F[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (*F[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
diff --git a/go/analysis/passes/tests/testdata/src/a/go118_test.go b/go/analysis/passes/tests/testdata/src/a/go118_test.go
index dc898daca..e2bc3f3a0 100644
--- a/go/analysis/passes/tests/testdata/src/a/go118_test.go
+++ b/go/analysis/passes/tests/testdata/src/a/go118_test.go
@@ -94,3 +94,8 @@ func FuzzObjectMethod(f *testing.F) {
}
f.Fuzz(obj.myVar) // ok
}
+
+// Test for golang/go#56505: checking fuzz arguments should not panic on *error.
+func FuzzIssue56505(f *testing.F) {
+ f.Fuzz(func(e *error) {}) // want "the first parameter of a fuzz target must be \\*testing.T"
+}
diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go
index ffa5205dd..935aad00c 100644
--- a/go/analysis/passes/tests/tests.go
+++ b/go/analysis/passes/tests/tests.go
@@ -84,7 +84,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}
-// Checks the contents of a fuzz function.
+// checkFuzz checks the contents of a fuzz function.
func checkFuzz(pass *analysis.Pass, fn *ast.FuncDecl) {
params := checkFuzzCall(pass, fn)
if params != nil {
@@ -92,15 +92,17 @@ func checkFuzz(pass *analysis.Pass, fn *ast.FuncDecl) {
}
}
-// Check the arguments of f.Fuzz() calls :
-// 1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz().
-// 2. The called function in f.Fuzz(func(){}) should not return result.
-// 3. First argument of func() should be of type *testing.T
-// 4. Second argument onwards should be of type []byte, string, bool, byte,
-// rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16,
-// uint32, uint64
-// 5. func() must not call any *F methods, e.g. (*F).Log, (*F).Error, (*F).Skip
-// The only *F methods that are allowed in the (*F).Fuzz function are (*F).Failed and (*F).Name.
+// checkFuzzCall checks the arguments of f.Fuzz() calls:
+//
+// 1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz().
+// 2. The called function in f.Fuzz(func(){}) should not return result.
+// 3. First argument of func() should be of type *testing.T
+// 4. Second argument onwards should be of type []byte, string, bool, byte,
+// rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16,
+// uint32, uint64
+// 5. func() must not call any *F methods, e.g. (*F).Log, (*F).Error, (*F).Skip
+// The only *F methods that are allowed in the (*F).Fuzz function are (*F).Failed and (*F).Name.
+//
// Returns the list of parameters to the fuzz function, if they are valid fuzz parameters.
func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple) {
ast.Inspect(fn, func(n ast.Node) bool {
@@ -160,7 +162,7 @@ func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple)
return params
}
-// Check that the arguments of f.Add() calls have the same number and type of arguments as
+// checkAddCalls checks that the arguments of f.Add calls have the same number and type of arguments as
// the signature of the function passed to (*testing.F).Fuzz
func checkAddCalls(pass *analysis.Pass, fn *ast.FuncDecl, params *types.Tuple) {
ast.Inspect(fn, func(n ast.Node) bool {
@@ -267,7 +269,9 @@ func isTestingType(typ types.Type, testingType string) bool {
if !ok {
return false
}
- return named.Obj().Pkg().Path() == "testing" && named.Obj().Name() == testingType
+ obj := named.Obj()
+ // obj.Pkg is nil for the error type.
+ return obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == "testing" && obj.Name() == testingType
}
// Validate that fuzz target function's arguments are of accepted types.
@@ -473,10 +477,12 @@ func checkTest(pass *analysis.Pass, fn *ast.FuncDecl, prefix string) {
if tparams := typeparams.ForFuncType(fn.Type); tparams != nil && len(tparams.List) > 0 {
// Note: cmd/go/internal/load also errors about TestXXX and BenchmarkXXX functions with type parameters.
// We have currently decided to also warn before compilation/package loading. This can help users in IDEs.
+ // TODO(adonovan): use ReportRangef(tparams).
pass.Reportf(fn.Pos(), "%s has type parameters: it will not be run by go test as a %sXXX function", fn.Name.Name, prefix)
}
if !isTestSuffix(fn.Name.Name[len(prefix):]) {
+ // TODO(adonovan): use ReportRangef(fn.Name).
pass.Reportf(fn.Pos(), "%s has malformed name: first letter after '%s' must not be lowercase", fn.Name.Name, prefix)
}
}
diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go b/go/analysis/passes/timeformat/testdata/src/a/a.go
new file mode 100644
index 000000000..98481446e
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/a/a.go
@@ -0,0 +1,50 @@
+// Copyright 2022 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 tests for the timeformat checker.
+
+package a
+
+import (
+ "time"
+
+ "b"
+)
+
+func hasError() {
+ a, _ := time.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02`
+ a.Format(`2006-02-01`) // want `2006-02-01 should be 2006-01-02`
+ a.Format("2006-02-01 15:04:05") // want `2006-02-01 should be 2006-01-02`
+
+ const c = "2006-02-01"
+ a.Format(c) // want `2006-02-01 should be 2006-01-02`
+}
+
+func notHasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00")
+ a.Format("2006-01-02")
+
+ const c = "2006-01-02"
+ a.Format(c)
+
+ v := "2006-02-01"
+ a.Format(v) // Allowed though variables.
+
+ m := map[string]string{
+ "y": "2006-02-01",
+ }
+ a.Format(m["y"])
+
+ s := []string{"2006-02-01"}
+ a.Format(s[0])
+
+ a.Format(badFormat())
+
+ o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00")
+ o.Format("2006-02-01")
+}
+
+func badFormat() string {
+ return "2006-02-01"
+}
diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go.golden b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden
new file mode 100644
index 000000000..9eccded63
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden
@@ -0,0 +1,50 @@
+// Copyright 2022 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 tests for the timeformat checker.
+
+package a
+
+import (
+ "time"
+
+ "b"
+)
+
+func hasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02`
+ a.Format(`2006-01-02`) // want `2006-02-01 should be 2006-01-02`
+ a.Format("2006-01-02 15:04:05") // want `2006-02-01 should be 2006-01-02`
+
+ const c = "2006-02-01"
+ a.Format(c) // want `2006-02-01 should be 2006-01-02`
+}
+
+func notHasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00")
+ a.Format("2006-01-02")
+
+ const c = "2006-01-02"
+ a.Format(c)
+
+ v := "2006-02-01"
+ a.Format(v) // Allowed though variables.
+
+ m := map[string]string{
+ "y": "2006-02-01",
+ }
+ a.Format(m["y"])
+
+ s := []string{"2006-02-01"}
+ a.Format(s[0])
+
+ a.Format(badFormat())
+
+ o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00")
+ o.Format("2006-02-01")
+}
+
+func badFormat() string {
+ return "2006-02-01"
+}
diff --git a/go/analysis/passes/timeformat/testdata/src/b/b.go b/go/analysis/passes/timeformat/testdata/src/b/b.go
new file mode 100644
index 000000000..de5690863
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/b/b.go
@@ -0,0 +1,11 @@
+package b
+
+type B struct {
+}
+
+func Parse(string, string) B {
+ return B{}
+}
+
+func (b B) Format(string) {
+}
diff --git a/go/analysis/passes/timeformat/timeformat.go b/go/analysis/passes/timeformat/timeformat.go
new file mode 100644
index 000000000..acb198f95
--- /dev/null
+++ b/go/analysis/passes/timeformat/timeformat.go
@@ -0,0 +1,129 @@
+// Copyright 2022 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 timeformat defines an Analyzer that checks for the use
+// of time.Format or time.Parse calls with a bad format.
+package timeformat
+
+import (
+ "go/ast"
+ "go/constant"
+ "go/token"
+ "go/types"
+ "strings"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/go/types/typeutil"
+)
+
+const badFormat = "2006-02-01"
+const goodFormat = "2006-01-02"
+
+const Doc = `check for calls of (time.Time).Format or time.Parse with 2006-02-01
+
+The timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm)
+format. Internationally, "yyyy-dd-mm" does not occur in common calendar date
+standards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "timeformat",
+ Doc: Doc,
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+ nodeFilter := []ast.Node{
+ (*ast.CallExpr)(nil),
+ }
+ inspect.Preorder(nodeFilter, func(n ast.Node) {
+ call := n.(*ast.CallExpr)
+ fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func)
+ if !ok {
+ return
+ }
+ if !isTimeDotFormat(fn) && !isTimeDotParse(fn) {
+ return
+ }
+ if len(call.Args) > 0 {
+ arg := call.Args[0]
+ badAt := badFormatAt(pass.TypesInfo, arg)
+
+ if badAt > -1 {
+ // Check if it's a literal string, otherwise we can't suggest a fix.
+ if _, ok := arg.(*ast.BasicLit); ok {
+ pos := int(arg.Pos()) + badAt + 1 // +1 to skip the " or `
+ end := pos + len(badFormat)
+
+ pass.Report(analysis.Diagnostic{
+ Pos: token.Pos(pos),
+ End: token.Pos(end),
+ Message: badFormat + " should be " + goodFormat,
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: "Replace " + badFormat + " with " + goodFormat,
+ TextEdits: []analysis.TextEdit{{
+ Pos: token.Pos(pos),
+ End: token.Pos(end),
+ NewText: []byte(goodFormat),
+ }},
+ }},
+ })
+ } else {
+ pass.Reportf(arg.Pos(), badFormat+" should be "+goodFormat)
+ }
+ }
+ }
+ })
+ return nil, nil
+}
+
+func isTimeDotFormat(f *types.Func) bool {
+ if f.Name() != "Format" || f.Pkg().Path() != "time" {
+ return false
+ }
+ sig, ok := f.Type().(*types.Signature)
+ if !ok {
+ return false
+ }
+ // Verify that the receiver is time.Time.
+ recv := sig.Recv()
+ if recv == nil {
+ return false
+ }
+ named, ok := recv.Type().(*types.Named)
+ return ok && named.Obj().Name() == "Time"
+}
+
+func isTimeDotParse(f *types.Func) bool {
+ if f.Name() != "Parse" || f.Pkg().Path() != "time" {
+ return false
+ }
+ // Verify that there is no receiver.
+ sig, ok := f.Type().(*types.Signature)
+ return ok && sig.Recv() == nil
+}
+
+// badFormatAt return the start of a bad format in e or -1 if no bad format is found.
+func badFormatAt(info *types.Info, e ast.Expr) int {
+ tv, ok := info.Types[e]
+ if !ok { // no type info, assume good
+ return -1
+ }
+
+ t, ok := tv.Type.(*types.Basic)
+ if !ok || t.Info()&types.IsString == 0 {
+ return -1
+ }
+
+ if tv.Value == nil {
+ return -1
+ }
+
+ return strings.Index(constant.StringVal(tv.Value), badFormat)
+}
diff --git a/go/analysis/passes/timeformat/timeformat_test.go b/go/analysis/passes/timeformat/timeformat_test.go
new file mode 100644
index 000000000..86bbe1bb3
--- /dev/null
+++ b/go/analysis/passes/timeformat/timeformat_test.go
@@ -0,0 +1,17 @@
+// Copyright 2022 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 timeformat_test
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/timeformat"
+)
+
+func Test(t *testing.T) {
+ testdata := analysistest.TestData()
+ analysistest.RunWithSuggestedFixes(t, testdata, timeformat.Analyzer, "a")
+}
diff --git a/go/analysis/passes/unusedwrite/unusedwrite.go b/go/analysis/passes/unusedwrite/unusedwrite.go
index 37a0e784b..9cc45e0a3 100644
--- a/go/analysis/passes/unusedwrite/unusedwrite.go
+++ b/go/analysis/passes/unusedwrite/unusedwrite.go
@@ -41,7 +41,7 @@ Another example is about non-pointer receiver:
`
// Analyzer reports instances of writes to struct fields and arrays
-//that are never read.
+// that are never read.
var Analyzer = &analysis.Analyzer{
Name: "unusedwrite",
Doc: Doc,
@@ -50,40 +50,49 @@ var Analyzer = &analysis.Analyzer{
}
func run(pass *analysis.Pass) (interface{}, error) {
- // Check the writes to struct and array objects.
- checkStore := func(store *ssa.Store) {
- // Consider field/index writes to an object whose elements are copied and not shared.
- // MapUpdate is excluded since only the reference of the map is copied.
- switch addr := store.Addr.(type) {
- case *ssa.FieldAddr:
- if isDeadStore(store, addr.X, addr) {
- // Report the bug.
+ ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+ for _, fn := range ssainput.SrcFuncs {
+ // TODO(taking): Iterate over fn._Instantiations() once exported. If so, have 1 report per Pos().
+ reports := checkStores(fn)
+ for _, store := range reports {
+ switch addr := store.Addr.(type) {
+ case *ssa.FieldAddr:
pass.Reportf(store.Pos(),
"unused write to field %s",
getFieldName(addr.X.Type(), addr.Field))
- }
- case *ssa.IndexAddr:
- if isDeadStore(store, addr.X, addr) {
- // Report the bug.
+ case *ssa.IndexAddr:
pass.Reportf(store.Pos(),
"unused write to array index %s", addr.Index)
}
}
}
+ return nil, nil
+}
- ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
- for _, fn := range ssainput.SrcFuncs {
- // Visit each block. No need to visit fn.Recover.
- for _, blk := range fn.Blocks {
- for _, instr := range blk.Instrs {
- // Identify writes.
- if store, ok := instr.(*ssa.Store); ok {
- checkStore(store)
+// checkStores returns *Stores in fn whose address is written to but never used.
+func checkStores(fn *ssa.Function) []*ssa.Store {
+ var reports []*ssa.Store
+ // Visit each block. No need to visit fn.Recover.
+ for _, blk := range fn.Blocks {
+ for _, instr := range blk.Instrs {
+ // Identify writes.
+ if store, ok := instr.(*ssa.Store); ok {
+ // Consider field/index writes to an object whose elements are copied and not shared.
+ // MapUpdate is excluded since only the reference of the map is copied.
+ switch addr := store.Addr.(type) {
+ case *ssa.FieldAddr:
+ if isDeadStore(store, addr.X, addr) {
+ reports = append(reports, store)
+ }
+ case *ssa.IndexAddr:
+ if isDeadStore(store, addr.X, addr) {
+ reports = append(reports, store)
+ }
}
}
}
}
- return nil, nil
+ return reports
}
// isDeadStore determines whether a field/index write to an object is dead.
diff --git a/go/analysis/singlechecker/singlechecker.go b/go/analysis/singlechecker/singlechecker.go
index 28530777b..91044ca08 100644
--- a/go/analysis/singlechecker/singlechecker.go
+++ b/go/analysis/singlechecker/singlechecker.go
@@ -11,16 +11,15 @@
// all that is needed to define a standalone tool is a file,
// example.org/findbadness/cmd/findbadness/main.go, containing:
//
-// // The findbadness command runs an analysis.
-// package main
+// // The findbadness command runs an analysis.
+// package main
//
-// import (
-// "example.org/findbadness"
-// "golang.org/x/tools/go/analysis/singlechecker"
-// )
-//
-// func main() { singlechecker.Main(findbadness.Analyzer) }
+// import (
+// "example.org/findbadness"
+// "golang.org/x/tools/go/analysis/singlechecker"
+// )
//
+// func main() { singlechecker.Main(findbadness.Analyzer) }
package singlechecker
import (
diff --git a/go/analysis/unitchecker/main.go b/go/analysis/unitchecker/main.go
index 23acb7ed0..a054a2dce 100644
--- a/go/analysis/unitchecker/main.go
+++ b/go/analysis/unitchecker/main.go
@@ -10,8 +10,8 @@
// It serves as a model for the behavior of the cmd/vet tool in $GOROOT.
// Being based on the unitchecker driver, it must be run by go vet:
//
-// $ go build -o unitchecker main.go
-// $ go vet -vettool=unitchecker my/project/...
+// $ go build -o unitchecker main.go
+// $ go vet -vettool=unitchecker my/project/...
//
// For a checker also capable of running standalone, use multichecker.
package main
diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go
index b539866dd..37693564e 100644
--- a/go/analysis/unitchecker/unitchecker.go
+++ b/go/analysis/unitchecker/unitchecker.go
@@ -6,13 +6,13 @@
// driver that analyzes a single compilation unit during a build.
// It is invoked by a build system such as "go vet":
//
-// $ go vet -vettool=$(which vet)
+// $ go vet -vettool=$(which vet)
//
// It supports the following command-line protocol:
//
-// -V=full describe executable (to the build tool)
-// -flags describe flags (to the build tool)
-// foo.cfg description of compilation unit (from the build tool)
+// -V=full describe executable (to the build tool)
+// -flags describe flags (to the build tool)
+// foo.cfg description of compilation unit (from the build tool)
//
// This package does not depend on go/packages.
// If you need a standalone tool, use multichecker,
@@ -50,7 +50,7 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/internal/analysisflags"
- "golang.org/x/tools/go/analysis/internal/facts"
+ "golang.org/x/tools/internal/facts"
"golang.org/x/tools/internal/typeparams"
)
@@ -79,11 +79,10 @@ type Config struct {
//
// The protocol required by 'go vet -vettool=...' is that the tool must support:
//
-// -flags describe flags in JSON
-// -V=full describe executable for build caching
-// foo.cfg perform separate modular analyze on the single
-// unit described by a JSON config file foo.cfg.
-//
+// -flags describe flags in JSON
+// -V=full describe executable for build caching
+// foo.cfg perform separate modular analyze on the single
+// unit described by a JSON config file foo.cfg.
func Main(analyzers ...*analysis.Analyzer) {
progname := filepath.Base(os.Args[0])
log.SetFlags(0)
@@ -250,6 +249,10 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
// In VetxOnly mode, analyzers are only for their facts,
// so we can skip any analysis that neither produces facts
// nor depends on any analysis that produces facts.
+ //
+ // TODO(adonovan): fix: the command (and logic!) here are backwards.
+ // It should say "...nor is required by any...". (Issue 443099)
+ //
// Also build a map to hold working state and result.
type action struct {
once sync.Once
@@ -288,13 +291,13 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
analyzers = filtered
// Read facts from imported packages.
- read := func(path string) ([]byte, error) {
- if vetx, ok := cfg.PackageVetx[path]; ok {
+ read := func(imp *types.Package) ([]byte, error) {
+ if vetx, ok := cfg.PackageVetx[imp.Path()]; ok {
return ioutil.ReadFile(vetx)
}
return nil, nil // no .vetx file, no facts
}
- facts, err := facts.Decode(pkg, read)
+ facts, err := facts.NewDecoder(pkg).Decode(read)
if err != nil {
return nil, err
}
@@ -341,6 +344,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/analysis/unitchecker/unitchecker_test.go b/go/analysis/unitchecker/unitchecker_test.go
index 7e5b848de..197abd9a1 100644
--- a/go/analysis/unitchecker/unitchecker_test.go
+++ b/go/analysis/unitchecker/unitchecker_test.go
@@ -20,6 +20,7 @@ import (
"strings"
"testing"
+ "golang.org/x/tools/go/analysis/passes/assign"
"golang.org/x/tools/go/analysis/passes/findcall"
"golang.org/x/tools/go/analysis/passes/printf"
"golang.org/x/tools/go/analysis/unitchecker"
@@ -41,6 +42,7 @@ func main() {
unitchecker.Main(
findcall.Analyzer,
printf.Analyzer,
+ assign.Analyzer,
)
}
@@ -75,6 +77,13 @@ func _() {
func MyFunc123() {}
`,
+ "c/c.go": `package c
+
+func _() {
+ i := 5
+ i = i
+}
+`,
}}})
defer exported.Cleanup()
@@ -85,29 +94,71 @@ func MyFunc123() {}
([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?b/b.go:6:13: call of MyFunc123\(...\)
([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?b/b.go:7:11: call of MyFunc123\(...\)
`
+ const wantC = `# golang.org/fake/c
+([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?c/c.go:5:5: self-assignment of i to i
+`
const wantAJSON = `# golang.org/fake/a
\{
"golang.org/fake/a": \{
"findcall": \[
\{
"posn": "([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?a/a.go:4:11",
- "message": "call of MyFunc123\(...\)"
+ "message": "call of MyFunc123\(...\)",
+ "suggested_fixes": \[
+ \{
+ "message": "Add '_TEST_'",
+ "edits": \[
+ \{
+ "filename": "([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?a/a.go",
+ "start": 32,
+ "end": 32,
+ "new": "_TEST_"
+ \}
+ \]
+ \}
+ \]
+ \}
+ \]
+ \}
+\}
+`
+ const wantCJSON = `# golang.org/fake/c
+\{
+ "golang.org/fake/c": \{
+ "assign": \[
+ \{
+ "posn": "([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?c/c.go:5:5",
+ "message": "self-assignment of i to i",
+ "suggested_fixes": \[
+ \{
+ "message": "Remove",
+ "edits": \[
+ \{
+ "filename": "([/._\-a-zA-Z0-9]+[\\/]fake[\\/])?c/c.go",
+ "start": 37,
+ "end": 42,
+ "new": ""
+ \}
+ \]
+ \}
+ \]
\}
\]
\}
\}
`
-
for _, test := range []struct {
- args string
- wantOut string
- wantExit int
+ args string
+ wantOut string
+ wantExitError bool
}{
- {args: "golang.org/fake/a", wantOut: wantA, wantExit: 2},
- {args: "golang.org/fake/b", wantOut: wantB, wantExit: 2},
- {args: "golang.org/fake/a golang.org/fake/b", wantOut: wantA + wantB, wantExit: 2},
- {args: "-json golang.org/fake/a", wantOut: wantAJSON, wantExit: 0},
- {args: "-c=0 golang.org/fake/a", wantOut: wantA + "4 MyFunc123\\(\\)\n", wantExit: 2},
+ {args: "golang.org/fake/a", wantOut: wantA, wantExitError: true},
+ {args: "golang.org/fake/b", wantOut: wantB, wantExitError: true},
+ {args: "golang.org/fake/c", wantOut: wantC, wantExitError: true},
+ {args: "golang.org/fake/a golang.org/fake/b", wantOut: wantA + wantB, wantExitError: true},
+ {args: "-json golang.org/fake/a", wantOut: wantAJSON, wantExitError: false},
+ {args: "-json golang.org/fake/c", wantOut: wantCJSON, wantExitError: false},
+ {args: "-c=0 golang.org/fake/a", wantOut: wantA + "4 MyFunc123\\(\\)\n", wantExitError: true},
} {
cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "-findcall.name=MyFunc123")
cmd.Args = append(cmd.Args, strings.Fields(test.args)...)
@@ -119,13 +170,17 @@ func MyFunc123() {}
if exitErr, ok := err.(*exec.ExitError); ok {
exitcode = exitErr.ExitCode()
}
- if exitcode != test.wantExit {
- t.Errorf("%s: got exit code %d, want %d", test.args, exitcode, test.wantExit)
+ if (exitcode != 0) != test.wantExitError {
+ want := "zero"
+ if test.wantExitError {
+ want = "nonzero"
+ }
+ t.Errorf("%s: got exit code %d, want %s", test.args, exitcode, want)
}
matched, err := regexp.Match(test.wantOut, out)
if err != nil {
- t.Fatal(err)
+ t.Fatalf("regexp.Match(<<%s>>): %v", test.wantOut, err)
}
if !matched {
t.Errorf("%s: got <<%s>>, want match of regexp <<%s>>", test.args, out, test.wantOut)
diff --git a/go/analysis/validate.go b/go/analysis/validate.go
index 23e57bf02..9da5692af 100644
--- a/go/analysis/validate.go
+++ b/go/analysis/validate.go
@@ -14,6 +14,8 @@ import (
// Validate reports an error if any of the analyzers are misconfigured.
// Checks include:
// that the name is a valid identifier;
+// that the Doc is not empty;
+// that the Run is non-nil;
// that the Requires graph is acyclic;
// that analyzer fact types are unique;
// that each fact type is a pointer.
@@ -46,6 +48,9 @@ func Validate(analyzers []*Analyzer) error {
return fmt.Errorf("analyzer %q is undocumented", a)
}
+ if a.Run == nil {
+ return fmt.Errorf("analyzer %q has nil Run", a)
+ }
// fact types
for _, f := range a.FactTypes {
if f == nil {
diff --git a/go/analysis/validate_test.go b/go/analysis/validate_test.go
index 1116034f7..7f4ee2c05 100644
--- a/go/analysis/validate_test.go
+++ b/go/analysis/validate_test.go
@@ -11,33 +11,43 @@ import (
func TestValidate(t *testing.T) {
var (
+ run = func(p *Pass) (interface{}, error) {
+ return nil, nil
+ }
dependsOnSelf = &Analyzer{
Name: "dependsOnSelf",
Doc: "this analyzer depends on itself",
+ Run: run,
}
inCycleA = &Analyzer{
Name: "inCycleA",
Doc: "this analyzer depends on inCycleB",
+ Run: run,
}
inCycleB = &Analyzer{
Name: "inCycleB",
Doc: "this analyzer depends on inCycleA and notInCycleA",
+ Run: run,
}
pointsToCycle = &Analyzer{
Name: "pointsToCycle",
Doc: "this analyzer depends on inCycleA",
+ Run: run,
}
notInCycleA = &Analyzer{
Name: "notInCycleA",
Doc: "this analyzer depends on notInCycleB and notInCycleC",
+ Run: run,
}
notInCycleB = &Analyzer{
Name: "notInCycleB",
Doc: "this analyzer depends on notInCycleC",
+ Run: run,
}
notInCycleC = &Analyzer{
Name: "notInCycleC",
Doc: "this analyzer has no dependencies",
+ Run: run,
}
)
@@ -116,3 +126,27 @@ func TestCycleInRequiresGraphErrorMessage(t *testing.T) {
t.Errorf("error string %s does not contain expected substring %q", errMsg, wantSubstring)
}
}
+
+func TestValidateEmptyDoc(t *testing.T) {
+ withoutDoc := &Analyzer{
+ Name: "withoutDoc",
+ Run: func(p *Pass) (interface{}, error) {
+ return nil, nil
+ },
+ }
+ err := Validate([]*Analyzer{withoutDoc})
+ if err == nil || !strings.Contains(err.Error(), "is undocumented") {
+ t.Errorf("got unexpected error while validating analyzers withoutDoc: %v", err)
+ }
+}
+
+func TestValidateNoRun(t *testing.T) {
+ withoutRun := &Analyzer{
+ Name: "withoutRun",
+ Doc: "this analyzer has no Run",
+ }
+ err := Validate([]*Analyzer{withoutRun})
+ if err == nil || !strings.Contains(err.Error(), "has nil Run") {
+ t.Errorf("got unexpected error while validating analyzers withoutRun: %v", err)
+ }
+}