go/analysis/passes/buildtag: revert "suggest fix to remove +build lines"
This CL reverts commit 517957c, CL 694415, because it makes
go vet and go test report errors for non-problems in existing code.
Bundling the fixer into the vet check was the wrong approach;
instead we need a separate fixer, e.g. as part of modernize.
Change-Id: I9379571439c6c855668374cc8978cef7a9fbc32b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/711320
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go
index 91aac67..6e32f29 100644
--- a/go/analysis/passes/buildtag/buildtag.go
+++ b/go/analysis/passes/buildtag/buildtag.go
@@ -15,8 +15,6 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
- "golang.org/x/tools/internal/analysisinternal"
- "golang.org/x/tools/internal/versions"
)
const Doc = "check //go:build and // +build directives"
@@ -57,6 +55,7 @@
func checkGoFile(pass *analysis.Pass, f *ast.File) {
var check checker
check.init(pass)
+ defer check.finish()
for _, group := range f.Comments {
// A +build comment is ignored after or adjoining the package declaration.
@@ -78,27 +77,6 @@
check.comment(c.Slash, c.Text)
}
}
-
- check.finish()
-
- // For Go 1.18+ files, offer a fix to remove the +build lines
- // if they passed all consistency checks.
- if check.crossCheck && !versions.Before(pass.TypesInfo.FileVersions[f], "go1.18") {
- for _, rng := range check.plusBuildRanges {
- check.pass.Report(analysis.Diagnostic{
- Pos: rng.Pos(),
- End: rng.End(),
- Message: "+build line is no longer needed",
- SuggestedFixes: []analysis.SuggestedFix{{
- Message: "Remove obsolete +build line",
- TextEdits: []analysis.TextEdit{{
- Pos: rng.Pos(),
- End: rng.End(),
- }},
- }},
- })
- }
- }
}
func checkOtherFile(pass *analysis.Pass, filename string) error {
@@ -118,15 +96,15 @@
}
type checker struct {
- pass *analysis.Pass
- plusBuildOK bool // "+build" lines still OK
- goBuildOK bool // "go:build" lines still OK
- crossCheck bool // cross-check go:build and +build lines when done reading file
- inStar bool // currently in a /* */ comment
- goBuildPos token.Pos // position of first go:build line found
- plusBuildRanges []analysis.Range // range of each "+build" line found
- goBuild constraint.Expr // go:build constraint found
- plusBuild constraint.Expr // AND of +build constraints found
+ pass *analysis.Pass
+ plusBuildOK bool // "+build" lines still OK
+ goBuildOK bool // "go:build" lines still OK
+ crossCheck bool // cross-check go:build and +build lines when done reading file
+ inStar bool // currently in a /* */ comment
+ goBuildPos token.Pos // position of first go:build line found
+ plusBuildPos token.Pos // position of first "+build" line found
+ goBuild constraint.Expr // go:build constraint found
+ plusBuild constraint.Expr // AND of +build constraints found
}
func (check *checker) init(pass *analysis.Pass) {
@@ -294,8 +272,6 @@
}
func (check *checker) plusBuildLine(pos token.Pos, line string) {
- plusBuildRange := analysisinternal.Range(pos, pos+token.Pos(len(line)))
-
line = strings.TrimSpace(line)
if !constraint.IsPlusBuild(line) {
// Comment with +build but not at beginning.
@@ -310,7 +286,9 @@
check.crossCheck = false
}
- check.plusBuildRanges = append(check.plusBuildRanges, plusBuildRange)
+ if check.plusBuildPos == token.NoPos {
+ check.plusBuildPos = pos
+ }
// testing hack: stop at // ERROR
if i := strings.Index(line, " // ERROR "); i >= 0 {
@@ -358,19 +336,19 @@
}
func (check *checker) finish() {
- if !check.crossCheck || len(check.plusBuildRanges) == 0 || check.goBuildPos == token.NoPos {
+ if !check.crossCheck || check.plusBuildPos == token.NoPos || check.goBuildPos == token.NoPos {
return
}
// Have both //go:build and // +build,
// with no errors found (crossCheck still true).
// Check they match.
+ var want constraint.Expr
lines, err := constraint.PlusBuildLines(check.goBuild)
if err != nil {
check.pass.Reportf(check.goBuildPos, "%v", err)
return
}
- var want constraint.Expr
for _, line := range lines {
y, err := constraint.Parse(line)
if err != nil {
@@ -385,8 +363,7 @@
}
}
if want.String() != check.plusBuild.String() {
- check.pass.ReportRangef(check.plusBuildRanges[0], "+build lines do not match //go:build condition")
- check.crossCheck = false // don't offer fix to remove +build
+ check.pass.Reportf(check.plusBuildPos, "+build lines do not match //go:build condition")
return
}
}
diff --git a/go/analysis/passes/buildtag/buildtag_test.go b/go/analysis/passes/buildtag/buildtag_test.go
index 190e7bb..9f0b9f5 100644
--- a/go/analysis/passes/buildtag/buildtag_test.go
+++ b/go/analysis/passes/buildtag/buildtag_test.go
@@ -16,5 +16,5 @@
// Because it cares about IgnoredFiles, which most analyzers
// ignore, the test framework will consider expectations in
// ignore files too, but only for this analyzer.
- analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), buildtag.Analyzer, "a", "b")
+ analysistest.Run(t, analysistest.TestData(), buildtag.Analyzer, "a", "b")
}
diff --git a/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go b/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go
index 9553458..2651130 100644
--- a/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go
+++ b/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go
@@ -2,12 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-//go:build go1.18 && !(bad || worse)
-//want +1 `.build line is no longer needed`
-// +build go1.18
-//want +1 `.build line is no longer needed`
+//go:build !(bad || worse)
// +build !bad
-//want +1 `.build line is no longer needed`
// +build !worse
package a
diff --git a/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go.golden b/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go.golden
deleted file mode 100644
index 44e7eb2..0000000
--- a/go/analysis/passes/buildtag/testdata/src/a/buildtag4.go.golden
+++ /dev/null
@@ -1,12 +0,0 @@
-// 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.
-
-//go:build go1.18 && !(bad || worse)
-//want +1 `.build line is no longer needed`
-
-//want +1 `.build line is no longer needed`
-
-//want +1 `.build line is no longer needed`
-
-package a
diff --git a/go/analysis/passes/buildtag/testdata/src/b/vers.go b/go/analysis/passes/buildtag/testdata/src/b/vers.go
index 0f8d5d4..71cf71d 100644
--- a/go/analysis/passes/buildtag/testdata/src/b/vers.go
+++ b/go/analysis/passes/buildtag/testdata/src/b/vers.go
@@ -2,9 +2,9 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
+// want +3 `invalid go version \"go1.20.1\" in build constraint`
// want +1 `invalid go version \"go1.20.1\" in build constraint`
//go:build go1.20.1
-// want +1 `[+]build line is no longer needed` `invalid go version \"go1.20.1\" in build constraint`
// +build go1.20.1
package b
diff --git a/go/analysis/passes/buildtag/testdata/src/b/vers.go.golden b/go/analysis/passes/buildtag/testdata/src/b/vers.go.golden
deleted file mode 100644
index b86d0d8..0000000
--- a/go/analysis/passes/buildtag/testdata/src/b/vers.go.golden
+++ /dev/null
@@ -1,9 +0,0 @@
-// Copyright 2024 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 `invalid go version \"go1.20.1\" in build constraint`
-//go:build go1.20.1
-// want +1 `[+]build line is no longer needed` `invalid go version \"go1.20.1\" in build constraint`
-
-package b
diff --git a/gopls/internal/test/integration/workspace/standalone_test.go b/gopls/internal/test/integration/workspace/standalone_test.go
index 9a4f3b7..6cd8e7f 100644
--- a/gopls/internal/test/integration/workspace/standalone_test.go
+++ b/gopls/internal/test/integration/workspace/standalone_test.go
@@ -29,6 +29,7 @@
}
-- lib/ignore.go --
//go:build ignore
+// +build ignore
package main
@@ -163,6 +164,7 @@
package lib // without this package, files are loaded as command-line-arguments
-- ignore.go --
//go:build ignore
+// +build ignore
package main
@@ -171,6 +173,7 @@
func main() {}
-- standalone.go --
//go:build standalone
+// +build standalone
package main
