diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index 8c9977355c..8c3c2e7ab9 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -95,12 +95,13 @@ type Pass struct { Analyzer *Analyzer // the identity of the current analyzer // syntax and type information - Fset *token.FileSet // file position information - Files []*ast.File // the abstract syntax tree of each file - OtherFiles []string // names of non-Go files of this package - 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 + Fset *token.FileSet // file position information + Files []*ast.File // the abstract syntax tree of each file + OtherFiles []string // names of non-Go files of this package + IgnoredFiles []string // names of ignored source files in this package + 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 // 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 bb79cde893..ae5630239c 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -355,7 +355,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis } } - // Extract 'want' comments from Go files. + // Extract 'want' comments from parsed Go files. for _, f := range pass.Files { for _, cgroup := range f.Comments { for _, c := range cgroup.List { @@ -398,6 +398,16 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis linenum := 0 for _, line := range strings.Split(string(data), "\n") { linenum++ + + // Hack: treat a comment of the form "//...// want..." + // or "/*...// want... */ + // as if it starts at 'want'. + // This allows us to add comments on comments, + // as required when testing the buildtag analyzer. + if i := strings.Index(line, "// want"); i >= 0 { + line = line[i:] + } + if i := strings.Index(line, "//"); i >= 0 { line = line[i+len("//"):] processComment(filename, linenum, line) diff --git a/go/analysis/doc.go b/go/analysis/doc.go index fb17a0e415..9fa3302dfb 100644 --- a/go/analysis/doc.go +++ b/go/analysis/doc.go @@ -121,13 +121,14 @@ package being analyzed, and provides operations to the Run function for reporting diagnostics and other information back to the driver. type Pass struct { - Fset *token.FileSet - Files []*ast.File - OtherFiles []string - Pkg *types.Package - TypesInfo *types.Info - ResultOf map[*Analyzer]interface{} - Report func(Diagnostic) + Fset *token.FileSet + Files []*ast.File + OtherFiles []string + IgnoredFiles []string + Pkg *types.Package + TypesInfo *types.Info + ResultOf map[*Analyzer]interface{} + Report func(Diagnostic) ... } @@ -139,6 +140,12 @@ files such as assembly that are part of this package. See the "asmdecl" or "buildtags" analyzers for examples of loading non-Go files and reporting diagnostics against them. +The IgnoredFiles field provides the names, but not the contents, +of ignored Go and non-Go source files that are not part of this package +with the current build configuration but may be part of other build +configurations. See the "buildtags" analyzer for an example of loading +and checking IgnoredFiles. + The ResultOf field provides the results computed by the analyzers required by this one, as expressed in its Analyzer.Requires field. The driver runs the required analyzers first and makes their results diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 5ccfb16374..34f5b47d4c 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -639,6 +639,7 @@ func (act *action) execOnce() { 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, diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go index 78176f1011..841b928578 100644 --- a/go/analysis/passes/buildtag/buildtag.go +++ b/go/analysis/passes/buildtag/buildtag.go @@ -9,6 +9,7 @@ import ( "bytes" "fmt" "go/ast" + "go/parser" "strings" "unicode" @@ -33,6 +34,20 @@ func runBuildTag(pass *analysis.Pass) (interface{}, error) { 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. + return nil, nil + } + checkGoFile(pass, f) + } else { + if err := checkOtherFile(pass, name); err != nil { + return nil, err + } + } + } return nil, nil } @@ -132,13 +147,6 @@ func checkLine(line string, pastCutoff bool) error { } func checkArguments(fields []string) error { - // The original version of this checker in vet could examine - // files with malformed build tags that would cause the file to - // be always ignored by "go build". However, drivers for the new - // analysis API will analyze only the files selected to form a - // package, so these checks will never fire. - // TODO(adonovan): rethink this. - for _, arg := range fields[1:] { for _, elem := range strings.Split(arg, ",") { if strings.HasPrefix(elem, "!!") { diff --git a/go/analysis/passes/buildtag/buildtag_test.go b/go/analysis/passes/buildtag/buildtag_test.go index e95cc1a251..110343ceac 100644 --- a/go/analysis/passes/buildtag/buildtag_test.go +++ b/go/analysis/passes/buildtag/buildtag_test.go @@ -7,11 +7,28 @@ package buildtag_test import ( "testing" + "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/go/analysis/passes/buildtag" ) func Test(t *testing.T) { - testdata := analysistest.TestData() - analysistest.Run(t, testdata, buildtag.Analyzer, "a") + analyzer := *buildtag.Analyzer + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + defer func() { + // The buildtag 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 buildtag.Analyzer.Run(pass) + } + analysistest.Run(t, analysistest.TestData(), &analyzer, "a") } diff --git a/go/analysis/passes/buildtag/testdata/src/a/buildtag2.go b/go/analysis/passes/buildtag/testdata/src/a/buildtag2.go new file mode 100644 index 0000000000..3b71ca5694 --- /dev/null +++ b/go/analysis/passes/buildtag/testdata/src/a/buildtag2.go @@ -0,0 +1,9 @@ +// Copyright 2020 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 no + +package a + +// +build toolate // want "build comment must appear before package clause and be followed by a blank line$" diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go index 2ed274949b..713e1380ef 100644 --- a/go/analysis/unitchecker/unitchecker.go +++ b/go/analysis/unitchecker/unitchecker.go @@ -63,6 +63,7 @@ type Config struct { ImportPath string GoFiles []string NonGoFiles []string + IgnoredFiles []string ImportMap map[string]string PackageFile map[string]string Standard map[string]bool @@ -333,6 +334,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re Fset: fset, Files: files, OtherFiles: cfg.NonGoFiles, + IgnoredFiles: cfg.IgnoredFiles, Pkg: pkg, TypesInfo: info, TypesSizes: tc.Sizes, diff --git a/go/packages/golist.go b/go/packages/golist.go index bc04503c10..d7dac20d92 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -381,32 +381,34 @@ func (state *golistState) adhocPackage(pattern, query string) (*driverResponse, // Fields must match go list; // see $GOROOT/src/cmd/go/internal/load/pkg.go. type jsonPackage struct { - ImportPath string - Dir string - Name string - Export string - GoFiles []string - CompiledGoFiles []string - CFiles []string - CgoFiles []string - CXXFiles []string - MFiles []string - HFiles []string - FFiles []string - SFiles []string - SwigFiles []string - SwigCXXFiles []string - SysoFiles []string - Imports []string - ImportMap map[string]string - Deps []string - Module *Module - TestGoFiles []string - TestImports []string - XTestGoFiles []string - XTestImports []string - ForTest string // q in a "p [q.test]" package, else "" - DepOnly bool + ImportPath string + Dir string + Name string + Export string + GoFiles []string + CompiledGoFiles []string + IgnoredGoFiles []string + IgnoredOtherFiles []string + CFiles []string + CgoFiles []string + CXXFiles []string + MFiles []string + HFiles []string + FFiles []string + SFiles []string + SwigFiles []string + SwigCXXFiles []string + SysoFiles []string + Imports []string + ImportMap map[string]string + Deps []string + Module *Module + TestGoFiles []string + TestImports []string + XTestGoFiles []string + XTestImports []string + ForTest string // q in a "p [q.test]" package, else "" + DepOnly bool Error *jsonPackageError } @@ -558,6 +560,7 @@ func (state *golistState) createDriverResponse(words ...string) (*driverResponse GoFiles: absJoin(p.Dir, p.GoFiles, p.CgoFiles), CompiledGoFiles: absJoin(p.Dir, p.CompiledGoFiles), OtherFiles: absJoin(p.Dir, otherFiles(p)...), + IgnoredFiles: absJoin(p.Dir, p.IgnoredGoFiles, p.IgnoredOtherFiles), forTest: p.ForTest, Module: p.Module, } diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go index 885aec3ec3..de2c1dc579 100644 --- a/go/packages/golist_overlay.go +++ b/go/packages/golist_overlay.go @@ -1,3 +1,7 @@ +// 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 packages import ( diff --git a/go/packages/packages.go b/go/packages/packages.go index 04053f1e7d..970c480de1 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -289,6 +289,11 @@ type Package struct { // including assembly, C, C++, Fortran, Objective-C, SWIG, and so on. OtherFiles []string + // IgnoredFiles lists source files that are not part of the package + // using the current build configuration but that might be part of + // the package using other build configurations. + IgnoredFiles []string + // ExportFile is the absolute path to a file containing type // information for the package as provided by the build system. ExportFile string @@ -404,6 +409,7 @@ type flatPackage struct { GoFiles []string `json:",omitempty"` CompiledGoFiles []string `json:",omitempty"` OtherFiles []string `json:",omitempty"` + IgnoredFiles []string `json:",omitempty"` ExportFile string `json:",omitempty"` Imports map[string]string `json:",omitempty"` } @@ -426,6 +432,7 @@ func (p *Package) MarshalJSON() ([]byte, error) { GoFiles: p.GoFiles, CompiledGoFiles: p.CompiledGoFiles, OtherFiles: p.OtherFiles, + IgnoredFiles: p.IgnoredFiles, ExportFile: p.ExportFile, } if len(p.Imports) > 0 { @@ -712,7 +719,8 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) { result[i] = lpkg.Package } for i := range ld.pkgs { - // Clear all unrequested fields, for extra de-Hyrum-ization. + // Clear all unrequested fields, + // to catch programs that use more than they request. if ld.requestedMode&NeedName == 0 { ld.pkgs[i].Name = "" ld.pkgs[i].PkgPath = "" @@ -720,6 +728,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) { if ld.requestedMode&NeedFiles == 0 { ld.pkgs[i].GoFiles = nil ld.pkgs[i].OtherFiles = nil + ld.pkgs[i].IgnoredFiles = nil } if ld.requestedMode&NeedCompiledGoFiles == 0 { ld.pkgs[i].CompiledGoFiles = nil diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 4015b07fc8..a5351ab3e6 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -185,19 +185,20 @@ func testLoadImportsGraph(t *testing.T, exporter packagestest.Exporter) { // Check node information: kind, name, srcs. for _, test := range []struct { - id string - wantName string - wantKind string - wantSrcs string + id string + wantName string + wantKind string + wantSrcs string + wantIgnored string }{ - {"golang.org/fake/a", "a", "package", "a.go"}, - {"golang.org/fake/b", "b", "package", "b.go"}, - {"golang.org/fake/c", "c", "package", "c.go"}, // c2.go is ignored - {"golang.org/fake/e", "main", "command", "e.go e2.go"}, - {"container/list", "list", "package", "list.go"}, - {"golang.org/fake/subdir/d", "d", "package", "d.go"}, - {"golang.org/fake/subdir/d.test", "main", "command", "0.go"}, - {"unsafe", "unsafe", "package", ""}, + {"golang.org/fake/a", "a", "package", "a.go", ""}, + {"golang.org/fake/b", "b", "package", "b.go", ""}, + {"golang.org/fake/c", "c", "package", "c.go", "c2.go"}, // c2.go is ignored + {"golang.org/fake/e", "main", "command", "e.go e2.go", ""}, + {"container/list", "list", "package", "list.go", ""}, + {"golang.org/fake/subdir/d", "d", "package", "d.go", ""}, + {"golang.org/fake/subdir/d.test", "main", "command", "0.go", ""}, + {"unsafe", "unsafe", "package", "", ""}, } { p, ok := all[test.id] if !ok { @@ -222,6 +223,9 @@ func testLoadImportsGraph(t *testing.T, exporter packagestest.Exporter) { if srcs := strings.Join(srcs(p), " "); srcs != test.wantSrcs { t.Errorf("%s.Srcs = [%s], want [%s]", test.id, srcs, test.wantSrcs) } + if ignored := strings.Join(cleanPaths(p.IgnoredFiles), " "); ignored != test.wantIgnored { + t.Errorf("%s.Srcs = [%s], want [%s]", test.id, ignored, test.wantIgnored) + } } // Test an ad-hoc package, analogous to "go run hello.go". @@ -1092,6 +1096,9 @@ func testAbsoluteFilenames(t *testing.T, exporter packagestest.Exporter) { for _, filename := range pkg.OtherFiles { checkFile(filename) } + for _, filename := range pkg.IgnoredFiles { + checkFile(filename) + } } } } @@ -1248,6 +1255,7 @@ func testJSON(t *testing.T, exporter packagestest.Exporter) { pkg.GoFiles = cleanPaths(pkg.GoFiles) pkg.CompiledGoFiles = cleanPaths(pkg.CompiledGoFiles) pkg.OtherFiles = cleanPaths(pkg.OtherFiles) + pkg.IgnoredFiles = cleanPaths(pkg.IgnoredFiles) if err := enc.Encode(pkg); err != nil { t.Fatal(err) } @@ -2629,7 +2637,7 @@ func errorMessages(errors []packages.Error) []string { } func srcs(p *packages.Package) []string { - return cleanPaths(append(p.GoFiles, p.OtherFiles...)) + return cleanPaths(append(p.GoFiles[:len(p.GoFiles):len(p.GoFiles)], p.OtherFiles...)) } // cleanPaths attempts to reduce path names to stable forms