go/analysis/passes/modernize: plusbuild: remove +build comments
This CL defines the plusbuild analyzer, which reports each
obsolete "// +build" comment and suggests a fix to remove it.
This logic, added to cmd/fix in CL 240611, was removed in
CL 706758 at the same time as CL 694415 added similar logic
to the existing cmd/vet "buildtag" analyzer.
However, that approach caused go vet and go test to report
diagnostics for non-problems in existing code,
so it was reverted in CL 711320. We now take the approach
of defining this dedicated modernizer.
Updates golang/go#73605
Change-Id: I93708f0dcc9b38d3df282891aaa29569087111de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/711340
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index 63ae278..520542b 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -574,11 +574,12 @@
// TODO(adonovan): we may need to handle //line directives.
files := act.Package.OtherFiles
- // Hack: these two analyzers need to extract expectations from
+ // Hack: these analyzers need to extract expectations from
// all configurations, so include the files are usually
// ignored. (This was previously a hack in the respective
// analyzers' tests.)
- if act.Analyzer.Name == "buildtag" || act.Analyzer.Name == "directive" {
+ switch act.Analyzer.Name {
+ case "buildtag", "directive", "plusbuild":
files = slices.Concat(files, act.Package.IgnoredFiles)
}
diff --git a/go/analysis/passes/modernize/doc.go b/go/analysis/passes/modernize/doc.go
index 5d8f93c..bc143d7 100644
--- a/go/analysis/passes/modernize/doc.go
+++ b/go/analysis/passes/modernize/doc.go
@@ -205,6 +205,22 @@
original code would always encode the struct field, whereas the
modified code will omit it if it is a zero-value.
+# Analyzer plusbuild
+
+plusbuild: remove obsolete //+build comments
+
+The plusbuild analyzer suggests a fix to remove obsolete build tags
+of the form:
+
+ //+build linux,amd64
+
+in files that also contain a Go 1.18-style tag such as:
+
+ //go:build linux && amd64
+
+(It does not check that the old and new tags are consistent;
+that is the job of the 'buildtag' analyzer in the vet suite.)
+
# Analyzer rangeint
rangeint: replace 3-clause for loops with for-range over integers
diff --git a/go/analysis/passes/modernize/modernize.go b/go/analysis/passes/modernize/modernize.go
index d1aba46..df23fc2 100644
--- a/go/analysis/passes/modernize/modernize.go
+++ b/go/analysis/passes/modernize/modernize.go
@@ -41,6 +41,7 @@
MinMaxAnalyzer,
NewExprAnalyzer,
OmitZeroAnalyzer,
+ plusBuildAnalyzer,
RangeIntAnalyzer,
ReflectTypeForAnalyzer,
SlicesContainsAnalyzer,
diff --git a/go/analysis/passes/modernize/modernize_test.go b/go/analysis/passes/modernize/modernize_test.go
index 64bf60c..4dcab68 100644
--- a/go/analysis/passes/modernize/modernize_test.go
+++ b/go/analysis/passes/modernize/modernize_test.go
@@ -61,6 +61,14 @@
RunWithSuggestedFixes(t, TestData(), modernize.RangeIntAnalyzer, "rangeint")
}
+func TestPlusBuild(t *testing.T) {
+ // This test has a dedicated hack in the analysistest package:
+ // Because it cares about IgnoredFiles, which most analyzers
+ // ignore, the test framework will consider expectations in
+ // ignore files too, but only for this analyzer.
+ RunWithSuggestedFixes(t, TestData(), goplsexport.PlusBuildModernizer, "plusbuild")
+}
+
func TestReflectTypeFor(t *testing.T) {
testenv.NeedsGo1Point(t, 25) // requires go1.25 types.Var.Kind
RunWithSuggestedFixes(t, TestData(), modernize.ReflectTypeForAnalyzer, "reflecttypefor")
diff --git a/go/analysis/passes/modernize/plusbuild.go b/go/analysis/passes/modernize/plusbuild.go
new file mode 100644
index 0000000..e8af807
--- /dev/null
+++ b/go/analysis/passes/modernize/plusbuild.go
@@ -0,0 +1,83 @@
+// Copyright 2025 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 modernize
+
+import (
+ "go/ast"
+ "go/parser"
+ "strings"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/internal/analysisinternal"
+ "golang.org/x/tools/internal/goplsexport"
+)
+
+var plusBuildAnalyzer = &analysis.Analyzer{
+ Name: "plusbuild",
+ Doc: analysisinternal.MustExtractDoc(doc, "plusbuild"),
+ URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild",
+ Run: plusbuild,
+}
+
+func init() {
+ // Export to gopls until this is a published modernizer.
+ goplsexport.PlusBuildModernizer = plusBuildAnalyzer
+}
+
+func plusbuild(pass *analysis.Pass) (any, error) {
+ check := func(f *ast.File) {
+ if !fileUses(pass.TypesInfo, f, "go1.18") {
+ return
+ }
+
+ // When gofmt sees a +build comment, it adds a
+ // preceding equivalent //go:build directive, so in
+ // formatted files we can assume that a +build line is
+ // part of a comment group that starts with a
+ // //go:build line and is followed by a blank line.
+ //
+ // While we cannot delete comments from an AST and
+ // expect consistent output in general, this specific
+ // case--deleting only some lines from a comment
+ // block--does format correctly.
+ for _, g := range f.Comments {
+ sawGoBuild := false
+ for _, c := range g.List {
+ if sawGoBuild && strings.HasPrefix(c.Text, "// +build ") {
+ pass.Report(analysis.Diagnostic{
+ Pos: c.Pos(),
+ End: c.End(),
+ Message: "+build line is no longer needed",
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: "Remove obsolete +build line",
+ TextEdits: []analysis.TextEdit{{
+ Pos: c.Pos(),
+ End: c.End(),
+ }},
+ }},
+ })
+ break
+ }
+ if strings.HasPrefix(c.Text, "//go:build ") {
+ sawGoBuild = true
+ }
+ }
+ }
+ }
+
+ for _, f := range pass.Files {
+ check(f)
+ }
+ for _, name := range pass.IgnoredFiles {
+ if strings.HasSuffix(name, ".go") {
+ f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments|parser.SkipObjectResolution)
+ if err != nil {
+ continue // parse error: ignore
+ }
+ check(f)
+ }
+ }
+ return nil, nil
+}
diff --git a/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go
new file mode 100644
index 0000000..58fe8ec
--- /dev/null
+++ b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go
@@ -0,0 +1,6 @@
+// want +3 `.build line is no longer needed`
+
+//go:build linux && amd64
+// +build linux,amd64
+
+package plusbuild
diff --git a/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go.golden b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go.golden
new file mode 100644
index 0000000..da9e766
--- /dev/null
+++ b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild.go.golden
@@ -0,0 +1,6 @@
+// want +3 `.build line is no longer needed`
+
+//go:build linux && amd64
+
+package plusbuild
+
diff --git a/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild2.go b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild2.go
new file mode 100644
index 0000000..43adfb7
--- /dev/null
+++ b/go/analysis/passes/modernize/testdata/src/plusbuild/plusbuild2.go
@@ -0,0 +1,4 @@
+// This file ensures that the package is non-empty
+// in every build configuration.
+
+package plusbuild
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 39ca9bf..e7afa2f 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -3602,6 +3602,24 @@
Package documentation: [omitzero](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#omitzero)
+<a id='plusbuild'></a>
+## `plusbuild`: remove obsolete //+build comments
+
+The plusbuild analyzer suggests a fix to remove obsolete build tags of the form:
+
+ //+build linux,amd64
+
+in files that also contain a Go 1.18-style tag such as:
+
+ //go:build linux && amd64
+
+(It does not check that the old and new tags are consistent; that is the job of the 'buildtag' analyzer in the vet suite.)
+
+
+Default: on.
+
+Package documentation: [plusbuild](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild)
+
<a id='printf'></a>
## `printf`: check consistency of Printf format strings and arguments
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index 4627fb3..4bb9023 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -1581,6 +1581,12 @@
"Status": ""
},
{
+ "Name": "\"plusbuild\"",
+ "Doc": "remove obsolete //+build comments\n\nThe plusbuild analyzer suggests a fix to remove obsolete build tags\nof the form:\n\n\t//+build linux,amd64\n\nin files that also contain a Go 1.18-style tag such as:\n\n\t//go:build linux \u0026\u0026 amd64\n\n(It does not check that the old and new tags are consistent;\nthat is the job of the 'buildtag' analyzer in the vet suite.)",
+ "Default": "true",
+ "Status": ""
+ },
+ {
"Name": "\"printf\"",
"Doc": "check consistency of Printf format strings and arguments\n\nThe check applies to calls of the formatting functions such as\n[fmt.Printf] and [fmt.Sprintf], as well as any detected wrappers of\nthose functions such as [log.Printf]. It reports a variety of\nmistakes such as syntax errors in the format string and mismatches\n(of number and type) between the verbs and their arguments.\n\nSee the documentation of the fmt package for the complete set of\nformat operators and their operand types.",
"Default": "true",
@@ -3467,6 +3473,12 @@
"Default": true
},
{
+ "Name": "plusbuild",
+ "Doc": "remove obsolete //+build comments\n\nThe plusbuild analyzer suggests a fix to remove obsolete build tags\nof the form:\n\n\t//+build linux,amd64\n\nin files that also contain a Go 1.18-style tag such as:\n\n\t//go:build linux \u0026\u0026 amd64\n\n(It does not check that the old and new tags are consistent;\nthat is the job of the 'buildtag' analyzer in the vet suite.)",
+ "URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild",
+ "Default": true
+ },
+ {
"Name": "printf",
"Doc": "check consistency of Printf format strings and arguments\n\nThe check applies to calls of the formatting functions such as\n[fmt.Printf] and [fmt.Sprintf], as well as any detected wrappers of\nthose functions such as [log.Printf]. It reports a variety of\nmistakes such as syntax errors in the format string and mismatches\n(of number and type) between the verbs and their arguments.\n\nSee the documentation of the fmt package for the complete set of\nformat operators and their operand types.",
"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf",
diff --git a/gopls/internal/settings/analysis.go b/gopls/internal/settings/analysis.go
index 97fae1f..78fa872 100644
--- a/gopls/internal/settings/analysis.go
+++ b/gopls/internal/settings/analysis.go
@@ -254,6 +254,7 @@
{analyzer: goplsexport.ErrorsAsTypeModernizer, severity: protocol.SeverityHint},
{analyzer: modernize.FmtAppendfAnalyzer, severity: protocol.SeverityHint},
{analyzer: modernize.ForVarAnalyzer, severity: protocol.SeverityHint},
+ {analyzer: goplsexport.PlusBuildModernizer, severity: protocol.SeverityHint},
{analyzer: goplsexport.StdIteratorsModernizer, severity: protocol.SeverityHint},
{analyzer: modernize.MapsLoopAnalyzer, severity: protocol.SeverityHint},
{analyzer: modernize.MinMaxAnalyzer, severity: protocol.SeverityHint},
diff --git a/internal/goplsexport/export.go b/internal/goplsexport/export.go
index d7c2b9f..2764a97 100644
--- a/internal/goplsexport/export.go
+++ b/internal/goplsexport/export.go
@@ -10,5 +10,6 @@
var (
ErrorsAsTypeModernizer *analysis.Analyzer // = modernize.errorsastypeAnalyzer
- StdIteratorsModernizer *analysis.Analyzer // = modernize.stditeratorsAnalyer
+ StdIteratorsModernizer *analysis.Analyzer // = modernize.stditeratorsAnalyzer
+ PlusBuildModernizer *analysis.Analyzer // = modernize.plusbuildAnalyzer
)
