Skip to content

Commit f811b39

Browse files
adonovangopherbot
authored andcommitted
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>
1 parent 1b3cb5c commit f811b39

File tree

12 files changed

+160
-3
lines changed

12 files changed

+160
-3
lines changed

go/analysis/analysistest/analysistest.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -574,11 +574,12 @@ func check(t Testing, gopath string, act *checker.Action) {
574574
// TODO(adonovan): we may need to handle //line directives.
575575
files := act.Package.OtherFiles
576576

577-
// Hack: these two analyzers need to extract expectations from
577+
// Hack: these analyzers need to extract expectations from
578578
// all configurations, so include the files are usually
579579
// ignored. (This was previously a hack in the respective
580580
// analyzers' tests.)
581-
if act.Analyzer.Name == "buildtag" || act.Analyzer.Name == "directive" {
581+
switch act.Analyzer.Name {
582+
case "buildtag", "directive", "plusbuild":
582583
files = slices.Concat(files, act.Package.IgnoredFiles)
583584
}
584585

go/analysis/passes/modernize/doc.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,22 @@ Replacing `omitempty` with `omitzero` is a change in behavior. The
205205
original code would always encode the struct field, whereas the
206206
modified code will omit it if it is a zero-value.
207207
208+
# Analyzer plusbuild
209+
210+
plusbuild: remove obsolete //+build comments
211+
212+
The plusbuild analyzer suggests a fix to remove obsolete build tags
213+
of the form:
214+
215+
//+build linux,amd64
216+
217+
in files that also contain a Go 1.18-style tag such as:
218+
219+
//go:build linux && amd64
220+
221+
(It does not check that the old and new tags are consistent;
222+
that is the job of the 'buildtag' analyzer in the vet suite.)
223+
208224
# Analyzer rangeint
209225
210226
rangeint: replace 3-clause for loops with for-range over integers

go/analysis/passes/modernize/modernize.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ var Suite = []*analysis.Analyzer{
4141
MinMaxAnalyzer,
4242
NewExprAnalyzer,
4343
OmitZeroAnalyzer,
44+
plusBuildAnalyzer,
4445
RangeIntAnalyzer,
4546
ReflectTypeForAnalyzer,
4647
SlicesContainsAnalyzer,

go/analysis/passes/modernize/modernize_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ func TestRangeInt(t *testing.T) {
6161
RunWithSuggestedFixes(t, TestData(), modernize.RangeIntAnalyzer, "rangeint")
6262
}
6363

64+
func TestPlusBuild(t *testing.T) {
65+
// This test has a dedicated hack in the analysistest package:
66+
// Because it cares about IgnoredFiles, which most analyzers
67+
// ignore, the test framework will consider expectations in
68+
// ignore files too, but only for this analyzer.
69+
RunWithSuggestedFixes(t, TestData(), goplsexport.PlusBuildModernizer, "plusbuild")
70+
}
71+
6472
func TestReflectTypeFor(t *testing.T) {
6573
testenv.NeedsGo1Point(t, 25) // requires go1.25 types.Var.Kind
6674
RunWithSuggestedFixes(t, TestData(), modernize.ReflectTypeForAnalyzer, "reflecttypefor")
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package modernize
6+
7+
import (
8+
"go/ast"
9+
"go/parser"
10+
"strings"
11+
12+
"golang.org/x/tools/go/analysis"
13+
"golang.org/x/tools/internal/analysisinternal"
14+
"golang.org/x/tools/internal/goplsexport"
15+
)
16+
17+
var plusBuildAnalyzer = &analysis.Analyzer{
18+
Name: "plusbuild",
19+
Doc: analysisinternal.MustExtractDoc(doc, "plusbuild"),
20+
URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild",
21+
Run: plusbuild,
22+
}
23+
24+
func init() {
25+
// Export to gopls until this is a published modernizer.
26+
goplsexport.PlusBuildModernizer = plusBuildAnalyzer
27+
}
28+
29+
func plusbuild(pass *analysis.Pass) (any, error) {
30+
check := func(f *ast.File) {
31+
if !fileUses(pass.TypesInfo, f, "go1.18") {
32+
return
33+
}
34+
35+
// When gofmt sees a +build comment, it adds a
36+
// preceding equivalent //go:build directive, so in
37+
// formatted files we can assume that a +build line is
38+
// part of a comment group that starts with a
39+
// //go:build line and is followed by a blank line.
40+
//
41+
// While we cannot delete comments from an AST and
42+
// expect consistent output in general, this specific
43+
// case--deleting only some lines from a comment
44+
// block--does format correctly.
45+
for _, g := range f.Comments {
46+
sawGoBuild := false
47+
for _, c := range g.List {
48+
if sawGoBuild && strings.HasPrefix(c.Text, "// +build ") {
49+
pass.Report(analysis.Diagnostic{
50+
Pos: c.Pos(),
51+
End: c.End(),
52+
Message: "+build line is no longer needed",
53+
SuggestedFixes: []analysis.SuggestedFix{{
54+
Message: "Remove obsolete +build line",
55+
TextEdits: []analysis.TextEdit{{
56+
Pos: c.Pos(),
57+
End: c.End(),
58+
}},
59+
}},
60+
})
61+
break
62+
}
63+
if strings.HasPrefix(c.Text, "//go:build ") {
64+
sawGoBuild = true
65+
}
66+
}
67+
}
68+
}
69+
70+
for _, f := range pass.Files {
71+
check(f)
72+
}
73+
for _, name := range pass.IgnoredFiles {
74+
if strings.HasSuffix(name, ".go") {
75+
f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments|parser.SkipObjectResolution)
76+
if err != nil {
77+
continue // parse error: ignore
78+
}
79+
check(f)
80+
}
81+
}
82+
return nil, nil
83+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// want +3 `.build line is no longer needed`
2+
3+
//go:build linux && amd64
4+
// +build linux,amd64
5+
6+
package plusbuild
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// want +3 `.build line is no longer needed`
2+
3+
//go:build linux && amd64
4+
5+
package plusbuild
6+
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// This file ensures that the package is non-empty
2+
// in every build configuration.
3+
4+
package plusbuild

gopls/doc/analyzers.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3602,6 +3602,24 @@ Default: on.
36023602

36033603
Package documentation: [omitzero](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#omitzero)
36043604

3605+
<a id='plusbuild'></a>
3606+
## `plusbuild`: remove obsolete //+build comments
3607+
3608+
The plusbuild analyzer suggests a fix to remove obsolete build tags of the form:
3609+
3610+
//+build linux,amd64
3611+
3612+
in files that also contain a Go 1.18-style tag such as:
3613+
3614+
//go:build linux && amd64
3615+
3616+
(It does not check that the old and new tags are consistent; that is the job of the 'buildtag' analyzer in the vet suite.)
3617+
3618+
3619+
Default: on.
3620+
3621+
Package documentation: [plusbuild](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild)
3622+
36053623
<a id='printf'></a>
36063624
## `printf`: check consistency of Printf format strings and arguments
36073625

gopls/internal/doc/api.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,12 @@
15801580
"Default": "true",
15811581
"Status": ""
15821582
},
1583+
{
1584+
"Name": "\"plusbuild\"",
1585+
"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.)",
1586+
"Default": "true",
1587+
"Status": ""
1588+
},
15831589
{
15841590
"Name": "\"printf\"",
15851591
"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.",
@@ -3466,6 +3472,12 @@
34663472
"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#omitzero",
34673473
"Default": true
34683474
},
3475+
{
3476+
"Name": "plusbuild",
3477+
"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.)",
3478+
"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#plusbuild",
3479+
"Default": true
3480+
},
34693481
{
34703482
"Name": "printf",
34713483
"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.",

0 commit comments

Comments
 (0)