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

go/analysis: add IgnoredFiles, check ignored files in buildtag check 

This CL fixes a long-standing TODO from the original conversion
of go vet to the go/analysis framework: apply the buildtag check
to all source files, not just the ones selected for the current GOOS/GOARCH.

Note that this only triggers when IgnoredFiles is passed from the
go command, which only happens in the current Go 1.16 development tree.

Change-Id: I37692f0bbe7b2b6e96f016e3c4827ebed1eeafa4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261725
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Russ Cox 2020-10-12 23:13:33 -04:00
parent 67cabf80d0
commit d88ec18c69
12 changed files with 142 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
go/analysis/analysis.go
View File

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

12
go/analysis/analysistest/analysistest.go
View File

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

21
go/analysis/doc.go
View File

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

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

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

22
go/analysis/passes/buildtag/buildtag.go
View File

@ -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, "!!") {

21
go/analysis/passes/buildtag/buildtag_test.go
View File

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

9
go/analysis/passes/buildtag/testdata/src/a/buildtag2.go vendored Normal file
View File

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

2
go/analysis/unitchecker/unitchecker.go
View File

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

55
go/packages/golist.go
View File

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

4
go/packages/golist_overlay.go
View File

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

11
go/packages/packages.go
View File

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

34
go/packages/packages_test.go
View File

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