all: merge master (b5a64bb) into gopls-release-branch.0.18
Also add back the replace directive.
For golang/go#71607
Conflicts:
- gopls/go.mod
- gopls/go.sum
Merge List:
+ 2025-02-12 b5a64bbcf go/analysis/internal/checker: be silent with -fix
+ 2025-02-11 b752317a2 internal/analysisinternal: disable AddImport test without go command
+ 2025-02-11 d98774edc cmd/signature-fuzzer/internal/fuzz-generator: update to math/rand/v2
+ 2025-02-11 25932623b gopls/internal/telemetry/cmd/stacks: remove leading \b match
+ 2025-02-11 b3c5d108c gopls: record telemetry counters for settings that are used
+ 2025-02-11 d2585c467 gopls/internal/golang: folding range: remove FoldingRangeInfo
+ 2025-02-11 0d16805d3 internal/stdlib: update stdlib index for Go 1.24.0
+ 2025-02-11 027eab55a go/analysis/analysistest: RunWithSuggestedFix: 3-way merge
+ 2025-02-10 f61b225cb internal/analysisinternal: AddImport puts new import in a group
+ 2025-02-10 91bac86b5 internal/analysisinternal: add CanImport
+ 2025-02-10 94c41d32c gopls/internal/golang: add comment about SymbolKind
+ 2025-02-10 09747cdf5 go.mod: update golang.org/x dependencies
+ 2025-02-10 dc9353b60 gopls/internal/analysis/modernize: appendclipped: unclip
+ 2025-02-07 a886a1c2e internal/analysisinternal: AddImport handles dot imports
+ 2025-02-07 94c3c49c4 go/analysis/analysistest: RunWithSuggestedFix: assume valid fixes
+ 2025-02-07 5f9967d63 gopls/internal/analysis/modernize: strings.Split -> SplitSeq
+ 2025-02-07 a1eb5fda8 go/analysis/passes/framepointer: support arm64
+ 2025-02-07 9c087d9bf internal/analysis/gofix: change "forward" back to "inline"
+ 2025-02-07 82317cea8 gopls/internal/analysis/modernize: slices.Delete: import slices
Change-Id: I5e21f982437cd4288f71b00484fa589ddcab8df6
diff --git a/cmd/signature-fuzzer/internal/fuzz-generator/wraprand.go b/cmd/signature-fuzzer/internal/fuzz-generator/wraprand.go
index bba178d..f83a5f2 100644
--- a/cmd/signature-fuzzer/internal/fuzz-generator/wraprand.go
+++ b/cmd/signature-fuzzer/internal/fuzz-generator/wraprand.go
@@ -6,7 +6,7 @@
import (
"fmt"
- "math/rand"
+ "math/rand/v2"
"os"
"runtime"
"strings"
@@ -20,8 +20,7 @@
)
func NewWrapRand(seed int64, ctl int) *wraprand {
- rand.Seed(seed)
- return &wraprand{seed: seed, ctl: ctl}
+ return &wraprand{seed: seed, ctl: ctl, rand: rand.New(rand.NewPCG(0, uint64(seed)))}
}
type wraprand struct {
@@ -32,6 +31,7 @@
tag string
calls []string
ctl int
+ rand *rand.Rand
}
func (w *wraprand) captureCall(tag string, val string) {
@@ -59,7 +59,7 @@
func (w *wraprand) Intn(n int64) int64 {
w.intncalls++
- rv := rand.Int63n(n)
+ rv := w.rand.Int64N(n)
if w.ctl&RandCtlCapture != 0 {
w.captureCall("Intn", fmt.Sprintf("%d", rv))
}
@@ -68,7 +68,7 @@
func (w *wraprand) Float32() float32 {
w.f32calls++
- rv := rand.Float32()
+ rv := w.rand.Float32()
if w.ctl&RandCtlCapture != 0 {
w.captureCall("Float32", fmt.Sprintf("%f", rv))
}
@@ -77,7 +77,7 @@
func (w *wraprand) NormFloat64() float64 {
w.f64calls++
- rv := rand.NormFloat64()
+ rv := w.rand.NormFloat64()
if w.ctl&RandCtlCapture != 0 {
w.captureCall("NormFloat64", fmt.Sprintf("%f", rv))
}
@@ -85,7 +85,7 @@
}
func (w *wraprand) emitCalls(fn string) {
- outf, err := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+ outf, err := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o666)
if err != nil {
panic(err)
}
diff --git a/go.mod b/go.mod
index 0f49047..8cea866 100644
--- a/go.mod
+++ b/go.mod
@@ -5,10 +5,10 @@
require (
github.com/google/go-cmp v0.6.0
github.com/yuin/goldmark v1.4.13
- golang.org/x/mod v0.22.0
- golang.org/x/net v0.34.0
- golang.org/x/sync v0.10.0
+ golang.org/x/mod v0.23.0
+ golang.org/x/net v0.35.0
+ golang.org/x/sync v0.11.0
golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457
)
-require golang.org/x/sys v0.29.0 // indirect
+require golang.org/x/sys v0.30.0 // indirect
diff --git a/go.sum b/go.sum
index c788c5f..2d11b06 100644
--- a/go.sum
+++ b/go.sum
@@ -2,13 +2,13 @@
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
-golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
-golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
-golang.org/x/net v0.34.0 h1:Mb7Mrk043xzHgnRM88suvJFwzVrRfHEHJEl5/71CKw0=
-golang.org/x/net v0.34.0/go.mod h1:di0qlW3YNM5oh6GqDGQr92MyTozJPmybPK4Ev/Gm31k=
-golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
-golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
-golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
-golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/mod v0.23.0 h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM=
+golang.org/x/mod v0.23.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
+golang.org/x/net v0.35.0 h1:T5GQRQb2y08kTAByq9L4/bz8cipCdA8FbRTXewonqY8=
+golang.org/x/net v0.35.0/go.mod h1:EglIi67kWsHKlRzzVMUD93VMSWGFOMSZgxFjparz1Qk=
+golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w=
+golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
+golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457 h1:zf5N6UOrA487eEFacMePxjXAJctxKmyjKUsjA11Uzuk=
golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0=
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index 8c62c56..0898177 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -7,6 +7,7 @@
import (
"bytes"
+ "cmp"
"fmt"
"go/format"
"go/token"
@@ -78,16 +79,24 @@
Errorf(format string, args ...interface{})
}
-// RunWithSuggestedFixes behaves like Run, but additionally verifies suggested fixes.
-// It uses golden files placed alongside the source code under analysis:
-// suggested fixes for code in example.go will be compared against example.go.golden.
+// RunWithSuggestedFixes behaves like Run, but additionally applies
+// suggested fixes and verifies their output.
//
-// Golden files can be formatted in one of two ways: as plain Go source code, or as txtar archives.
-// In the first case, all suggested fixes will be applied to the original source, which will then be compared against the golden file.
-// In the second case, suggested fixes will be grouped by their messages, and each set of fixes will be applied and tested separately.
-// Each section in the archive corresponds to a single message.
+// It uses golden files, placed alongside each source file, to express
+// the desired output: the expected transformation of file example.go
+// is specified in file example.go.golden.
//
-// A golden file using txtar may look like this:
+// Golden files may be of two forms: a plain Go source file, or a
+// txtar archive.
+//
+// A plain Go source file indicates the expected result of applying
+// all suggested fixes to the original file.
+//
+// A txtar archive specifies, in each section, the expected result of
+// applying all suggested fixes of a given message to the original
+// file; the name of the archive section is the fix's message. In this
+// way, the various alternative fixes offered by a single diagnostic
+// can be tested independently. Here's an example:
//
// -- turn into single negation --
// package pkg
@@ -109,41 +118,28 @@
//
// # Conflicts
//
-// A single analysis pass may offer two or more suggested fixes that
-// (1) conflict but are nonetheless logically composable, (e.g.
-// because both update the import declaration), or (2) are
-// fundamentally incompatible (e.g. alternative fixes to the same
-// statement).
+// Regardless of the form of the golden file, it is possible for
+// multiple fixes to conflict, either because they overlap, or are
+// close enough together that the particular diff algorithm cannot
+// separate them.
//
-// It is up to the driver to decide how to apply such fixes. A
-// sophisticated driver could attempt to resolve conflicts of the
-// first kind, but this test driver simply reports the fact of the
-// conflict with the expectation that the user will split their tests
-// into nonconflicting parts.
+// RunWithSuggestedFixes uses a simple three-way merge to accumulate
+// fixes, similar to a git merge. The merge algorithm may be able to
+// coalesce identical edits, for example duplicate imports of the same
+// package. (Bear in mind that this is an editorial decision. In
+// general, coalescing identical edits may not be correct: consider
+// two statements that increment the same counter.)
//
-// Conflicts of the second kind can be avoided by giving the
-// alternative fixes different names (SuggestedFix.Message) and
-// defining the .golden file as a multi-section txtar file with a
-// named section for each alternative fix, as shown above.
+// If there are conflicts, the test fails. In any case, the
+// non-conflicting edits will be compared against the expected output.
+// In this situation, we recommend that you increase the textual
+// separation between conflicting parts or, if that fails, split
+// your tests into smaller parts.
//
-// Analyzers that compute fixes from a textual diff of the
-// before/after file contents (instead of directly from syntax tree
-// positions) may produce fixes that, although logically
-// non-conflicting, nonetheless conflict due to the particulars of the
-// diff algorithm. In such cases it may suffice to introduce
-// sufficient separation of the statements in the test input so that
-// the computed diffs do not overlap. If that fails, break the test
-// into smaller parts.
-//
-// TODO(adonovan): the behavior of RunWithSuggestedFixes as documented
-// above is impractical for tests that report multiple diagnostics and
-// offer multiple alternative fixes for the same diagnostic, and it is
-// inconsistent with the interpretation of multiple diagnostics
-// described at Diagnostic.SuggestedFixes.
-// We need to rethink the analyzer testing API to better support such
-// cases. In the meantime, users of RunWithSuggestedFixes testing
-// analyzers that offer alternative fixes are advised to put each fix
-// in a separate .go file in the testdata.
+// If a diagnostic offers multiple fixes for the same problem, they
+// are almost certain to conflict, so in this case you should define
+// the expected output using a multi-section txtar file as described
+// above.
func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
results := Run(t, dir, a, patterns...)
@@ -173,167 +169,165 @@
for _, result := range results {
act := result.Action
- // file -> message -> edits
- // TODO(adonovan): this mapping assumes fix.Messages are unique across analyzers.
- fileEdits := make(map[*token.File]map[string][]diff.Edit)
- fileContents := make(map[*token.File][]byte)
-
- // Validate edits, prepare the fileEdits map and read the file contents.
+ // For each fix, split its edits by file and convert to diff form.
+ var (
+ // fixEdits: message -> fixes -> filename -> edits
+ //
+ // TODO(adonovan): this mapping assumes fix.Messages
+ // are unique across analyzers, whereas they are only
+ // unique within a given Diagnostic.
+ fixEdits = make(map[string][]map[string][]diff.Edit)
+ allFilenames = make(map[string]bool)
+ )
for _, diag := range act.Diagnostics {
+ // Fixes are validated upon creation in Pass.Report.
for _, fix := range diag.SuggestedFixes {
-
// Assert that lazy fixes have a Category (#65578, #65087).
if inTools && len(fix.TextEdits) == 0 && diag.Category == "" {
t.Errorf("missing Diagnostic.Category for SuggestedFix without TextEdits (gopls requires the category for the name of the fix command")
}
- // TODO(adonovan): factor in common with go/analysis/internal/checker.validateEdits.
-
+ // Convert edits to diff form.
+ // Group fixes by message and file.
+ edits := make(map[string][]diff.Edit)
for _, edit := range fix.TextEdits {
- start, end := edit.Pos, edit.End
- if !end.IsValid() {
- end = start
- }
- // Validate the edit.
- if start > end {
- t.Errorf(
- "diagnostic for analysis %v contains Suggested Fix with malformed edit: pos (%v) > end (%v)",
- act.Analyzer.Name, start, end)
- continue
- }
- file := act.Package.Fset.File(start)
- if file == nil {
- t.Errorf("diagnostic for analysis %v contains Suggested Fix with malformed start position %v", act.Analyzer.Name, start)
- continue
- }
- endFile := act.Package.Fset.File(end)
- if endFile == nil {
- t.Errorf("diagnostic for analysis %v contains Suggested Fix with malformed end position %v", act.Analyzer.Name, end)
- continue
- }
- if file != endFile {
- t.Errorf(
- "diagnostic for analysis %v contains Suggested Fix with malformed spanning files %v and %v",
- act.Analyzer.Name, file.Name(), endFile.Name())
- continue
- }
- if _, ok := fileContents[file]; !ok {
- contents, err := os.ReadFile(file.Name())
- if err != nil {
- t.Errorf("error reading %s: %v", file.Name(), err)
- }
- fileContents[file] = contents
- }
- if _, ok := fileEdits[file]; !ok {
- fileEdits[file] = make(map[string][]diff.Edit)
- }
- fileEdits[file][fix.Message] = append(fileEdits[file][fix.Message], diff.Edit{
- Start: file.Offset(start),
- End: file.Offset(end),
+ file := act.Package.Fset.File(edit.Pos)
+ allFilenames[file.Name()] = true
+ edits[file.Name()] = append(edits[file.Name()], diff.Edit{
+ Start: file.Offset(edit.Pos),
+ End: file.Offset(edit.End),
New: string(edit.NewText),
})
}
+ fixEdits[fix.Message] = append(fixEdits[fix.Message], edits)
}
}
- for file, fixes := range fileEdits {
- // Get the original file contents.
- orig, ok := fileContents[file]
+ merge := func(file, message string, x, y []diff.Edit) []diff.Edit {
+ z, ok := diff.Merge(x, y)
if !ok {
- t.Errorf("could not find file contents for %s", file.Name())
- continue
+ t.Errorf("in file %s, conflict applying fix %q", file, message)
+ return x // discard y
}
+ return z
+ }
- // Get the golden file and read the contents.
- ar, err := txtar.ParseFile(file.Name() + ".golden")
+ // Because the checking is driven by original
+ // filenames, there is no way to express that a fix
+ // (e.g. extract declaration) creates a new file.
+ for _, filename := range sortedKeys(allFilenames) {
+ // Read the original file.
+ content, err := os.ReadFile(filename)
if err != nil {
- t.Errorf("error reading %s.golden: %v", file.Name(), err)
+ t.Errorf("error reading %s: %v", filename, err)
continue
}
- if len(ar.Files) > 0 {
- // one virtual file per kind of suggested fix
+ // check checks that the accumulated edits applied
+ // to the original content yield the wanted content.
+ check := func(prefix string, accumulated []diff.Edit, want []byte) {
+ if err := applyDiffsAndCompare(filename, content, want, accumulated); err != nil {
+ t.Errorf("%s: %s", prefix, err)
+ }
+ }
- if len(ar.Comment) != 0 {
- // we allow either just the comment, or just virtual
- // files, not both. it is not clear how "both" should
- // behave.
- t.Errorf("%s.golden has leading comment; we don't know what to do with it", file.Name())
+ // Read the golden file. It may have one of two forms:
+ // (1) A txtar archive with one section per fix title,
+ // including all fixes of just that title.
+ // (2) The expected output for file.Name after all (?) fixes are applied.
+ // This form requires that no diagnostic has multiple fixes.
+ ar, err := txtar.ParseFile(filename + ".golden")
+ if err != nil {
+ t.Errorf("error reading %s.golden: %v", filename, err)
+ continue
+ }
+ if len(ar.Files) > 0 {
+ // Form #1: one archive section per kind of suggested fix.
+ if len(ar.Comment) > 0 {
+ // Disallow the combination of comment and archive sections.
+ t.Errorf("%s.golden has leading comment; we don't know what to do with it", filename)
continue
}
- // Sort map keys for determinism in tests.
- // TODO(jba): replace with slices.Sorted(maps.Keys(fixes)) when go.mod >= 1.23.
- var keys []string
- for k := range fixes {
- keys = append(keys, k)
- }
- slices.Sort(keys)
- for _, sf := range keys {
- edits := fixes[sf]
- found := false
- for _, vf := range ar.Files {
- if vf.Name == sf {
- found = true
- // the file may contain multiple trailing
- // newlines if the user places empty lines
- // between files in the archive. normalize
- // this to a single newline.
- golden := append(bytes.TrimRight(vf.Data, "\n"), '\n')
- if err := applyDiffsAndCompare(orig, golden, edits, file.Name()); err != nil {
- t.Errorf("%s", err)
- }
- break
- }
+ // Each archive section is named for a fix.Message.
+ // Accumulate the parts of the fix that apply to the current file,
+ // using a simple three-way merge, discarding conflicts,
+ // then apply the merged edits and compare to the archive section.
+ for _, section := range ar.Files {
+ message, want := section.Name, section.Data
+ var accumulated []diff.Edit
+ for _, fix := range fixEdits[message] {
+ accumulated = merge(filename, message, accumulated, fix[filename])
}
- if !found {
- t.Errorf("no section for suggested fix %q in %s.golden", sf, file.Name())
- }
+ check(fmt.Sprintf("all fixes of message %q", message), accumulated, want)
}
+
} else {
- // all suggested fixes are represented by a single file
- // TODO(adonovan): fix: this makes no sense if len(fixes) > 1.
- // Sort map keys for determinism in tests.
- // TODO(jba): replace with slices.Sorted(maps.Keys(fixes)) when go.mod >= 1.23.
- var keys []string
- for k := range fixes {
- keys = append(keys, k)
+ // Form #2: all suggested fixes are represented by a single file.
+ want := ar.Comment
+ var accumulated []diff.Edit
+ for _, message := range sortedKeys(fixEdits) {
+ for _, fix := range fixEdits[message] {
+ accumulated = merge(filename, message, accumulated, fix[filename])
+ }
}
- slices.Sort(keys)
- var catchallEdits []diff.Edit
- for _, k := range keys {
- catchallEdits = append(catchallEdits, fixes[k]...)
- }
-
- if err := applyDiffsAndCompare(orig, ar.Comment, catchallEdits, file.Name()); err != nil {
- t.Errorf("%s", err)
- }
+ check("all fixes", accumulated, want)
}
}
}
+
return results
}
-// applyDiffsAndCompare applies edits to src and compares the results against
-// golden after formatting both. fileName is use solely for error reporting.
-func applyDiffsAndCompare(src, golden []byte, edits []diff.Edit, fileName string) error {
- out, err := diff.ApplyBytes(src, edits)
- if err != nil {
- return fmt.Errorf("%s: error applying fixes: %v (see possible explanations at RunWithSuggestedFixes)", fileName, err)
+// applyDiffsAndCompare applies edits to original and compares the results against
+// want after formatting both. fileName is use solely for error reporting.
+func applyDiffsAndCompare(filename string, original, want []byte, edits []diff.Edit) error {
+ // Relativize filename, for tidier errors.
+ if cwd, err := os.Getwd(); err == nil {
+ if rel, err := filepath.Rel(cwd, filename); err == nil {
+ filename = rel
+ }
}
- wantRaw, err := format.Source(golden)
- if err != nil {
- return fmt.Errorf("%s.golden: error formatting golden file: %v\n%s", fileName, err, out)
- }
- want := string(wantRaw)
- formatted, err := format.Source(out)
- if err != nil {
- return fmt.Errorf("%s: error formatting resulting source: %v\n%s", fileName, err, out)
+ if len(edits) == 0 {
+ return fmt.Errorf("%s: no edits", filename)
}
- if got := string(formatted); got != want {
- unified := diff.Unified(fileName+".golden", "actual", want, got)
- return fmt.Errorf("suggested fixes failed for %s:\n%s", fileName, unified)
+ fixedBytes, err := diff.ApplyBytes(original, edits)
+ if err != nil {
+ return fmt.Errorf("%s: error applying fixes: %v (see possible explanations at RunWithSuggestedFixes)", filename, err)
+ }
+ fixed, err := format.Source(fixedBytes)
+ if err != nil {
+ return fmt.Errorf("%s: error formatting resulting source: %v\n%s", filename, err, fixed)
+ }
+
+ want, err = format.Source(want)
+ if err != nil {
+ return fmt.Errorf("%s.golden: error formatting golden file: %v\n%s", filename, err, fixed)
+ }
+
+ // Keep error reporting logic below consistent with
+ // TestScript in ../internal/checker/fix_test.go!
+
+ unified := func(xlabel, ylabel string, x, y []byte) string {
+ x = append(slices.Clip(bytes.TrimSpace(x)), '\n')
+ y = append(slices.Clip(bytes.TrimSpace(y)), '\n')
+ return diff.Unified(xlabel, ylabel, string(x), string(y))
+ }
+
+ if diff := unified(filename+" (fixed)", filename+" (want)", fixed, want); diff != "" {
+ return fmt.Errorf("unexpected %s content:\n"+
+ "-- original --\n%s\n"+
+ "-- fixed --\n%s\n"+
+ "-- want --\n%s\n"+
+ "-- diff original fixed --\n%s\n"+
+ "-- diff fixed want --\n%s",
+ filename,
+ original,
+ fixed,
+ want,
+ unified(filename+" (original)", filename+" (fixed)", original, fixed),
+ diff)
}
return nil
}
@@ -774,3 +768,13 @@
prefix := gopath + string(os.PathSeparator) + "src" + string(os.PathSeparator)
return filepath.ToSlash(strings.TrimPrefix(filename, prefix))
}
+
+// TODO(adonovan): use better stuff from go1.23.
+func sortedKeys[K cmp.Ordered, V any](m map[K]V) []K {
+ keys := make([]K, 0, len(m))
+ for k := range m {
+ keys = append(keys, k)
+ }
+ slices.Sort(keys)
+ return keys
+}
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index fb3c47b..2a9ff29 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -86,7 +86,36 @@
// It provides most of the logic for the main functions of both the
// singlechecker and the multi-analysis commands.
// It returns the appropriate exit code.
-func Run(args []string, analyzers []*analysis.Analyzer) int {
+//
+// TODO(adonovan): tests should not call this function directly;
+// fiddling with global variables (flags) is error-prone and hostile
+// to parallelism. Instead, use unit tests of the actual units (e.g.
+// checker.Analyze) and integration tests (e.g. TestScript) of whole
+// executables.
+func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) {
+ // Instead of returning a code directly,
+ // call this function to monotonically increase the exit code.
+ // This allows us to keep going in the face of some errors
+ // without having to remember what code to return.
+ //
+ // TODO(adonovan): interpreting exit codes is like reading tea-leaves.
+ // Insted of wasting effort trying to encode a multidimensional result
+ // into 7 bits we should just emit structured JSON output, and
+ // an exit code of 0 or 1 for success or failure.
+ exitAtLeast := func(code int) {
+ exitcode = max(code, exitcode)
+ }
+
+ // When analysisflags is linked in (for {single,multi}checker),
+ // then the -v flag is registered for complex legacy reasons
+ // related to cmd/vet CLI.
+ // Treat it as an undocumented alias for -debug=v.
+ if v := flag.CommandLine.Lookup("v"); v != nil &&
+ v.Value.(flag.Getter).Get() == true &&
+ !strings.Contains(Debug, "v") {
+ Debug += "v"
+ }
+
if CPUProfile != "" {
f, err := os.Create(CPUProfile)
if err != nil {
@@ -142,17 +171,14 @@
initial, err := load(args, allSyntax)
if err != nil {
log.Print(err)
- return 1
+ exitAtLeast(1)
+ return
}
- // TODO(adonovan): simplify exit code logic by using a single
- // exit code variable and applying "code = max(code, X)" each
- // time an error of code X occurs.
- pkgsExitCode := 0
// Print package and module errors regardless of RunDespiteErrors.
// Do not exit if there are errors, yet.
if n := packages.PrintErrors(initial); n > 0 {
- pkgsExitCode = 1
+ exitAtLeast(1)
}
var factLog io.Writer
@@ -172,7 +198,8 @@
graph, err := checker.Analyze(analyzers, initial, opts)
if err != nil {
log.Print(err)
- return 1
+ exitAtLeast(1)
+ return
}
// Don't print the diagnostics,
@@ -181,22 +208,22 @@
if err := applyFixes(graph.Roots, Diff); err != nil {
// Fail when applying fixes failed.
log.Print(err)
- return 1
+ exitAtLeast(1)
+ return
}
- // TODO(adonovan): don't proceed to print the text or JSON output
- // if we applied fixes; stop here.
- //
- // return pkgsExitCode
+ // Don't proceed to print text/JSON,
+ // and don't report an error
+ // just because there were diagnostics.
+ return
}
// Print the results. If !RunDespiteErrors and there
// are errors in the packages, this will have 0 exit
// code. Otherwise, we prefer to return exit code
// indicating diagnostics.
- if diagExitCode := printDiagnostics(graph); diagExitCode != 0 {
- return diagExitCode // there were diagnostics
- }
- return pkgsExitCode // package errors but no diagnostics
+ exitAtLeast(printDiagnostics(graph))
+
+ return
}
// printDiagnostics prints diagnostics in text or JSON form
@@ -541,6 +568,10 @@
}
}
+ if dbg('v') {
+ log.Printf("applied %d fixes, updated %d files", len(fixes), filesUpdated)
+ }
+
return nil
}
diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go
index fcf5f66..9ec6e61 100644
--- a/go/analysis/internal/checker/checker_test.go
+++ b/go/analysis/internal/checker/checker_test.go
@@ -49,8 +49,10 @@
t.Fatal(err)
}
path := filepath.Join(testdata, "src/rename/test.go")
+
checker.Fix = true
checker.Run([]string{"file=" + path}, []*analysis.Analyzer{renameAnalyzer})
+ checker.Fix = false
contents, err := os.ReadFile(path)
if err != nil {
@@ -138,31 +140,33 @@
// package from source. For the rest, it asks 'go list' for export data,
// which fails because the compiler encounters the type error. Since the
// errors come from 'go list', the driver doesn't run the analyzer.
- {name: "despite-error", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noop}, code: 1},
+ {name: "despite-error", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noop}, code: exitCodeFailed},
// The noopfact analyzer does use facts, so the driver loads source for
// all dependencies, does type checking itself, recognizes the error as a
// type error, and runs the analyzer.
- {name: "despite-error-fact", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noopWithFact}, code: 1},
+ {name: "despite-error-fact", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noopWithFact}, code: exitCodeFailed},
// combination of parse/type errors and no errors
- {name: "despite-error-and-no-error", pattern: []string{rderrFile, "sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1},
+ {name: "despite-error-and-no-error", pattern: []string{rderrFile, "sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: exitCodeFailed},
// non-existing package error
- {name: "no-package", pattern: []string{"xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 1},
- {name: "no-package-despite-error", pattern: []string{"abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1},
- {name: "no-multi-package-despite-error", pattern: []string{"xyz", "abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1},
+ {name: "no-package", pattern: []string{"xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: exitCodeFailed},
+ {name: "no-package-despite-error", pattern: []string{"abc"}, analyzers: []*analysis.Analyzer{noop}, code: exitCodeFailed},
+ {name: "no-multi-package-despite-error", pattern: []string{"xyz", "abc"}, analyzers: []*analysis.Analyzer{noop}, code: exitCodeFailed},
// combination of type/parsing and different errors
- {name: "different-errors", pattern: []string{rderrFile, "xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1},
+ {name: "different-errors", pattern: []string{rderrFile, "xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: exitCodeFailed},
// non existing dir error
- {name: "no-match-dir", pattern: []string{"file=non/existing/dir"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1},
+ {name: "no-match-dir", pattern: []string{"file=non/existing/dir"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: exitCodeFailed},
// no errors
- {name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 0},
+ {name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: exitCodeSuccess},
// duplicate list error with no findings
- {name: "list-error", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{noop}, code: 1},
+ {name: "list-error", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{noop}, code: exitCodeFailed},
// duplicate list errors with findings (issue #67790)
- {name: "list-error-findings", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 3},
+ {name: "list-error-findings", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: exitCodeDiagnostics},
} {
- if got := checker.Run(test.pattern, test.analyzers); got != test.code {
- t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code)
- }
+ t.Run(test.name, func(t *testing.T) {
+ if got := checker.Run(test.pattern, test.analyzers); got != test.code {
+ t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code)
+ }
+ })
}
}
diff --git a/go/analysis/internal/checker/fix_test.go b/go/analysis/internal/checker/fix_test.go
index 8fb7506..68d965d 100644
--- a/go/analysis/internal/checker/fix_test.go
+++ b/go/analysis/internal/checker/fix_test.go
@@ -52,9 +52,9 @@
}
const (
- exitCodeSuccess = 0 // success (no diagnostics)
+ exitCodeSuccess = 0 // success (no diagnostics, or successful -fix)
exitCodeFailed = 1 // analysis failed to run
- exitCodeDiagnostics = 3 // diagnostics were reported
+ exitCodeDiagnostics = 3 // diagnostics were reported (and no -fix)
)
// TestReportInvalidDiagnostic tests that a call to pass.Report with
@@ -281,6 +281,9 @@
t.Logf("%s: $ %s\nstdout:\n%s\nstderr:\n%s", prefix, clean(cmd), stdout, lastStderr)
}
+ // Keep error reporting logic below consistent with
+ // applyDiffsAndCompare in ../../analysistest/analysistest.go!
+
unified := func(xlabel, ylabel string, x, y []byte) string {
x = append(slices.Clip(bytes.TrimSpace(x)), '\n')
y = append(slices.Clip(bytes.TrimSpace(y)), '\n')
diff --git a/go/analysis/internal/checker/start_test.go b/go/analysis/internal/checker/start_test.go
index 618ccd0..c78829a 100644
--- a/go/analysis/internal/checker/start_test.go
+++ b/go/analysis/internal/checker/start_test.go
@@ -40,6 +40,7 @@
path := filepath.Join(testdata, "src/comment/doc.go")
checker.Fix = true
checker.Run([]string{"file=" + path}, []*analysis.Analyzer{commentAnalyzer})
+ checker.Fix = false
contents, err := os.ReadFile(path)
if err != nil {
diff --git a/go/analysis/internal/checker/testdata/diff.txt b/go/analysis/internal/checker/testdata/diff.txt
index 5a0c9c2..f11f01a 100644
--- a/go/analysis/internal/checker/testdata/diff.txt
+++ b/go/analysis/internal/checker/testdata/diff.txt
@@ -8,8 +8,7 @@
skip GOOS=windows
checker -rename -fix -diff example.com/p
-exit 3
-stderr renaming "bar" to "baz"
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/internal/checker/testdata/fixes.txt b/go/analysis/internal/checker/testdata/fixes.txt
index 89f245f..4d906ca 100644
--- a/go/analysis/internal/checker/testdata/fixes.txt
+++ b/go/analysis/internal/checker/testdata/fixes.txt
@@ -2,9 +2,9 @@
# particular when processing duplicate fixes for overlapping packages
# in the same directory ("p", "p [p.test]", "p_test [p.test]").
-checker -rename -fix example.com/p
-exit 3
-stderr renaming "bar" to "baz"
+checker -rename -fix -v example.com/p
+stderr applied 8 fixes, updated 3 files
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/internal/checker/testdata/importdup.txt b/go/analysis/internal/checker/testdata/importdup.txt
index e178377..4c144a6 100644
--- a/go/analysis/internal/checker/testdata/importdup.txt
+++ b/go/analysis/internal/checker/testdata/importdup.txt
@@ -1,8 +1,9 @@
# Test that duplicate imports--and, more generally, duplicate
# identical insertions--are coalesced.
-checker -marker -fix example.com/a
-exit 3
+checker -marker -fix -v example.com/a
+stderr applied 2 fixes, updated 1 files
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/internal/checker/testdata/importdup2.txt b/go/analysis/internal/checker/testdata/importdup2.txt
index 118fdc0..c2da0f3 100644
--- a/go/analysis/internal/checker/testdata/importdup2.txt
+++ b/go/analysis/internal/checker/testdata/importdup2.txt
@@ -19,8 +19,9 @@
# In more complex examples, the result
# may be more subtly order-dependent.
-checker -marker -fix example.com/a example.com/b
-exit 3
+checker -marker -fix -v example.com/a example.com/b
+stderr applied 6 fixes, updated 2 files
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/internal/checker/testdata/noend.txt b/go/analysis/internal/checker/testdata/noend.txt
index 2d6be07..5ebc5e0 100644
--- a/go/analysis/internal/checker/testdata/noend.txt
+++ b/go/analysis/internal/checker/testdata/noend.txt
@@ -2,8 +2,7 @@
# interpreted as if equal to SuggestedFix.Pos (see issue #64199).
checker -noend -fix example.com/a
-exit 3
-stderr say hello
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/internal/checker/testdata/overlap.txt b/go/analysis/internal/checker/testdata/overlap.txt
index f556ef3..581f2e1 100644
--- a/go/analysis/internal/checker/testdata/overlap.txt
+++ b/go/analysis/internal/checker/testdata/overlap.txt
@@ -15,9 +15,12 @@
# (This is a pretty unlikely situation, but it corresponds
# to a historical test, TestOther, that used to check for
# a conflict, and it seemed wrong to delete it without explanation.)
+#
+# The fixes are silently and successfully applied.
-checker -rename -marker -fix example.com/a
-exit 3
+checker -rename -marker -fix -v example.com/a
+stderr applied 2 fixes, updated 1 file
+exit 0
-- go.mod --
module example.com
diff --git a/go/analysis/passes/framepointer/framepointer.go b/go/analysis/passes/framepointer/framepointer.go
index 6eff3a2..8012de9 100644
--- a/go/analysis/passes/framepointer/framepointer.go
+++ b/go/analysis/passes/framepointer/framepointer.go
@@ -10,6 +10,7 @@
"go/build"
"regexp"
"strings"
+ "unicode"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
@@ -24,15 +25,97 @@
Run: run,
}
-var (
- re = regexp.MustCompile
- asmWriteBP = re(`,\s*BP$`) // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
- asmMentionBP = re(`\bBP\b`)
- asmControlFlow = re(`^(J|RET)`)
-)
+// Per-architecture checks for instructions.
+// Assume comments, leading and trailing spaces are removed.
+type arch struct {
+ isFPWrite func(string) bool
+ isFPRead func(string) bool
+ isBranch func(string) bool
+}
+
+var re = regexp.MustCompile
+
+func hasAnyPrefix(s string, prefixes ...string) bool {
+ for _, p := range prefixes {
+ if strings.HasPrefix(s, p) {
+ return true
+ }
+ }
+ return false
+}
+
+var arches = map[string]arch{
+ "amd64": {
+ isFPWrite: re(`,\s*BP$`).MatchString, // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
+ isFPRead: re(`\bBP\b`).MatchString,
+ isBranch: func(s string) bool {
+ return hasAnyPrefix(s, "J", "RET")
+ },
+ },
+ "arm64": {
+ isFPWrite: func(s string) bool {
+ if i := strings.LastIndex(s, ","); i > 0 && strings.HasSuffix(s[i:], "R29") {
+ return true
+ }
+ if hasAnyPrefix(s, "LDP", "LDAXP", "LDXP", "CASP") {
+ // Instructions which write to a pair of registers, e.g.
+ // LDP 8(R0), (R26, R29)
+ // CASPD (R2, R3), (R2), (R26, R29)
+ lp := strings.LastIndex(s, "(")
+ rp := strings.LastIndex(s, ")")
+ if lp > -1 && lp < rp {
+ return strings.Contains(s[lp:rp], ",") && strings.Contains(s[lp:rp], "R29")
+ }
+ }
+ return false
+ },
+ isFPRead: re(`\bR29\b`).MatchString,
+ isBranch: func(s string) bool {
+ // Get just the instruction
+ if i := strings.IndexFunc(s, unicode.IsSpace); i > 0 {
+ s = s[:i]
+ }
+ return arm64Branch[s]
+ },
+ },
+}
+
+// arm64 has many control flow instructions.
+// ^(B|RET) isn't sufficient or correct (e.g. BIC, BFI aren't control flow.)
+// It's easier to explicitly enumerate them in a map than to write a regex.
+// Borrowed from Go tree, cmd/asm/internal/arch/arm64.go
+var arm64Branch = map[string]bool{
+ "B": true,
+ "BL": true,
+ "BEQ": true,
+ "BNE": true,
+ "BCS": true,
+ "BHS": true,
+ "BCC": true,
+ "BLO": true,
+ "BMI": true,
+ "BPL": true,
+ "BVS": true,
+ "BVC": true,
+ "BHI": true,
+ "BLS": true,
+ "BGE": true,
+ "BLT": true,
+ "BGT": true,
+ "BLE": true,
+ "CBZ": true,
+ "CBZW": true,
+ "CBNZ": true,
+ "CBNZW": true,
+ "JMP": true,
+ "TBNZ": true,
+ "TBZ": true,
+ "RET": true,
+}
func run(pass *analysis.Pass) (interface{}, error) {
- if build.Default.GOARCH != "amd64" { // TODO: arm64 also?
+ arch, ok := arches[build.Default.GOARCH]
+ if !ok {
return nil, nil
}
if build.Default.GOOS != "linux" && build.Default.GOOS != "darwin" {
@@ -63,6 +146,9 @@
line = line[:i]
}
line = strings.TrimSpace(line)
+ if line == "" {
+ continue
+ }
// We start checking code at a TEXT line for a frameless function.
if strings.HasPrefix(line, "TEXT") && strings.Contains(line, "(SB)") && strings.Contains(line, "$0") {
@@ -73,16 +159,12 @@
continue
}
- if asmWriteBP.MatchString(line) { // clobber of BP, function is not OK
+ if arch.isFPWrite(line) {
pass.Reportf(analysisutil.LineStart(tf, lineno), "frame pointer is clobbered before saving")
active = false
continue
}
- if asmMentionBP.MatchString(line) { // any other use of BP might be a read, so function is OK
- active = false
- continue
- }
- if asmControlFlow.MatchString(line) { // give up after any branch instruction
+ if arch.isFPRead(line) || arch.isBranch(line) {
active = false
continue
}
diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm_arm64.s b/go/analysis/passes/framepointer/testdata/src/a/asm_arm64.s
new file mode 100644
index 0000000..f2be7bd
--- /dev/null
+++ b/go/analysis/passes/framepointer/testdata/src/a/asm_arm64.s
@@ -0,0 +1,42 @@
+// 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.
+
+TEXT ·bad1(SB), 0, $0
+ MOVD $0, R29 // want `frame pointer is clobbered before saving`
+ RET
+TEXT ·bad2(SB), 0, $0
+ MOVD R1, R29 // want `frame pointer is clobbered before saving`
+ RET
+TEXT ·bad3(SB), 0, $0
+ MOVD 6(R2), R29 // want `frame pointer is clobbered before saving`
+ RET
+TEXT ·bad4(SB), 0, $0
+ LDP 0(R1), (R26, R29) // want `frame pointer is clobbered before saving`
+ RET
+TEXT ·bad5(SB), 0, $0
+ AND $0x1, R3, R29 // want `frame pointer is clobbered before saving`
+ RET
+TEXT ·good1(SB), 0, $0
+ STPW (R29, R30), -32(RSP)
+ MOVD $0, R29 // this is ok
+ LDPW 32(RSP), (R29, R30)
+ RET
+TEXT ·good2(SB), 0, $0
+ MOVD R29, R1
+ MOVD $0, R29 // this is ok
+ MOVD R1, R29
+ RET
+TEXT ·good3(SB), 0, $0
+ CMP R1, R2
+ BEQ skip
+ MOVD $0, R29 // this is ok
+skip:
+ RET
+TEXT ·good4(SB), 0, $0
+ RET
+ MOVD $0, R29 // this is ok
+ RET
+TEXT ·good5(SB), 0, $8
+ MOVD $0, R29 // this is ok
+ RET
diff --git a/go/analysis/passes/stringintconv/string.go b/go/analysis/passes/stringintconv/string.go
index 108600a..f56e6ec 100644
--- a/go/analysis/passes/stringintconv/string.go
+++ b/go/analysis/passes/stringintconv/string.go
@@ -198,14 +198,14 @@
// the type has methods, as some {String,GoString,Format}
// may change the behavior of fmt.Sprint.
if len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0 {
- fmtName, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, arg.Pos(), "fmt", "fmt")
+ _, prefix, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, "fmt", "fmt", "Sprint", arg.Pos())
if types.Identical(T0, types.Typ[types.String]) {
// string(x) -> fmt.Sprint(x)
addFix("Format the number as a decimal", append(importEdits,
analysis.TextEdit{
Pos: call.Fun.Pos(),
End: call.Fun.End(),
- NewText: []byte(fmtName + ".Sprint"),
+ NewText: []byte(prefix + "Sprint"),
}),
)
} else {
@@ -214,7 +214,7 @@
analysis.TextEdit{
Pos: call.Lparen + 1,
End: call.Lparen + 1,
- NewText: []byte(fmtName + ".Sprint("),
+ NewText: []byte(prefix + "Sprint("),
},
analysis.TextEdit{
Pos: call.Rparen,
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go
new file mode 100644
index 0000000..d89ca94
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go
@@ -0,0 +1,7 @@
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+ Println(string(x)) // want `conversion from uint64 to string yields...`
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden
new file mode 100644
index 0000000..18aec2d
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden
@@ -0,0 +1,18 @@
+-- Format the number as a decimal --
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+ Println(Sprint(x)) // want `conversion from uint64 to string yields...`
+}
+
+-- Convert a single rune to a string --
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+ Println(string(rune(x))) // want `conversion from uint64 to string yields...`
+}
+
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 8764791..68465f9 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -294,8 +294,7 @@
## `gofix`: apply fixes based on go:fix comment directives
-The gofix analyzer inlines functions that are marked for inlining
-and forwards constants that are marked for forwarding.
+The gofix analyzer inlines functions and constants that are marked for inlining.
Default: on.
@@ -498,6 +497,8 @@
added in go1.21
- replacing a 3-clause for i := 0; i < n; i++ {} loop by
for i := range n {}, added in go1.22;
+ - replacing Split in "for range strings.Split(...)" by go1.24's
+ more efficient SplitSeq;
Default: on.
diff --git a/gopls/doc/release/v0.18.0.md b/gopls/doc/release/v0.18.0.md
index 9df1793..8d641a2 100644
--- a/gopls/doc/release/v0.18.0.md
+++ b/gopls/doc/release/v0.18.0.md
@@ -73,9 +73,8 @@
## New `gofix` analyzer
-Gopls now reports when a function call should be inlined or a use of a constant
-should be forwarded.
-These diagnostics and the associated code actions are triggered by "//go:fix"
+Gopls now reports when a function call or a use of a constant should be inlined.
+These diagnostics and the associated code actions are triggered by "//go:fix inline"
directives at the function and constant definitions.
(See [the go:fix proposal](https://go.dev/issue/32816).)
@@ -90,10 +89,10 @@
If gopls sees a call to `intmath.Square` in your code, it will suggest inlining
it, and will offer a code action to do so.
-The same feature works for constants, only the directive is "//go:fix forward".
+The same feature works for constants.
With a constant definition like this:
```
-//go:fix forward
+//go:fix inline
const Ptr = Pointer
```
gopls will suggest replacing `Ptr` in your code with `Pointer`.
diff --git a/gopls/go.mod b/gopls/go.mod
index 48a4619..8362072 100644
--- a/gopls/go.mod
+++ b/gopls/go.mod
@@ -7,12 +7,12 @@
require (
github.com/google/go-cmp v0.6.0
github.com/jba/templatecheck v0.7.1
- golang.org/x/mod v0.22.0
- golang.org/x/sync v0.10.0
- golang.org/x/sys v0.29.0
+ golang.org/x/mod v0.23.0
+ golang.org/x/sync v0.11.0
+ golang.org/x/sys v0.30.0
golang.org/x/telemetry v0.0.0-20241220003058-cc96b6e0d3d9
- golang.org/x/text v0.21.0
- golang.org/x/tools v0.29.1-0.20250207173059-30439fc82a98
+ golang.org/x/text v0.22.0
+ golang.org/x/tools v0.28.0
golang.org/x/vuln v1.1.3
gopkg.in/yaml.v3 v3.0.1
honnef.co/go/tools v0.5.1
@@ -26,3 +26,5 @@
golang.org/x/exp/typeparams v0.0.0-20241210194714-1829a127f884 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
)
+
+replace golang.org/x/tools => ../
diff --git a/gopls/go.sum b/gopls/go.sum
index 7bdb154..b2b3d92 100644
--- a/gopls/go.sum
+++ b/gopls/go.sum
@@ -14,22 +14,38 @@
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
+github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
+golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
+golang.org/x/crypto v0.33.0/go.mod h1:bVdXmD7IV/4GdElGPozy6U7lWdRXA4qyRVGJV57uQ5M=
golang.org/x/exp/typeparams v0.0.0-20241210194714-1829a127f884 h1:1xaZTydL5Gsg78QharTwKfA9FY9CZ1VQj6D/AZEvHR0=
golang.org/x/exp/typeparams v0.0.0-20241210194714-1829a127f884/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
-golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
-golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
-golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
-golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
-golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
-golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/mod v0.23.0 h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM=
+golang.org/x/mod v0.23.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
+golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
+golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
+golang.org/x/net v0.35.0/go.mod h1:EglIi67kWsHKlRzzVMUD93VMSWGFOMSZgxFjparz1Qk=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w=
+golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
+golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0=
golang.org/x/telemetry v0.0.0-20241220003058-cc96b6e0d3d9 h1:L2k9GUV2TpQKVRGMjN94qfUMgUwOFimSQ6gipyJIjKw=
golang.org/x/telemetry v0.0.0-20241220003058-cc96b6e0d3d9/go.mod h1:8h4Hgq+jcTvCDv2+i7NrfWwpYHcESleo2nGHxLbFLJ4=
+golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
+golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
+golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
-golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
-golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
-golang.org/x/tools v0.29.1-0.20250207173059-30439fc82a98 h1:Hd/d+QygQ355a6wowhs7TdfG8iaFPkctgkKN/hhFRWU=
-golang.org/x/tools v0.29.1-0.20250207173059-30439fc82a98/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588=
+golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
+golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
+golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM=
+golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY=
golang.org/x/vuln v1.1.3 h1:NPGnvPOTgnjBc9HTaUx+nj+EaUYxl5SJOWqaDYGaFYw=
golang.org/x/vuln v1.1.3/go.mod h1:7Le6Fadm5FOqE9C926BCD0g12NWyhg7cxV4BwcPFuNY=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
diff --git a/gopls/internal/analysis/gofix/doc.go b/gopls/internal/analysis/gofix/doc.go
index c3c453f..a0c6a08 100644
--- a/gopls/internal/analysis/gofix/doc.go
+++ b/gopls/internal/analysis/gofix/doc.go
@@ -4,16 +4,14 @@
/*
Package gofix defines an Analyzer that inlines calls to functions
-marked with a "//go:fix inline" doc comment,
-and forwards uses of constants
-marked with a "//go:fix forward" doc comment.
+and uses of constants
+marked with a "//go:fix inline" doc comment.
# Analyzer gofix
gofix: apply fixes based on go:fix comment directives
-The gofix analyzer inlines functions that are marked for inlining
-and forwards constants that are marked for forwarding.
+The gofix analyzer inlines functions and constants that are marked for inlining.
# Functions
@@ -48,31 +46,31 @@
# Constants
-Given a constant that is marked for forwarding, like this one:
+Given a constant that is marked for inlining, like this one:
- //go:fix forward
+ //go:fix inline
const Ptr = Pointer
this analyzer will recommend that uses of Ptr should be replaced with Pointer.
-As with inlining, forwarding can be used to replace deprecated constants and
+As with functions, inlining can be used to replace deprecated constants and
constants in obsolete packages.
-A constant definition can be marked for forwarding only if it refers to another
+A constant definition can be marked for inlining only if it refers to another
named constant.
-The "//go:fix forward" comment must appear before a single const declaration on its own,
+The "//go:fix inline" comment must appear before a single const declaration on its own,
as above; before a const declaration that is part of a group, as in this case:
const (
C = 1
- //go:fix forward
+ //go:fix inline
Ptr = Pointer
)
or before a group, applying to every constant in the group:
- //go:fix forward
+ //go:fix inline
const (
Ptr = Pointer
Val = Value
diff --git a/gopls/internal/analysis/gofix/gofix.go b/gopls/internal/analysis/gofix/gofix.go
index 7021d50..1019243 100644
--- a/gopls/internal/analysis/gofix/gofix.go
+++ b/gopls/internal/analysis/gofix/gofix.go
@@ -33,7 +33,7 @@
Doc: analysisinternal.MustExtractDoc(doc, "gofix"),
URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/gofix",
Run: run,
- FactTypes: []analysis.Fact{new(goFixInlineFuncFact), new(goFixForwardConstFact)},
+ FactTypes: []analysis.Fact{new(goFixInlineFuncFact), new(goFixInlineConstFact)},
Requires: []*analysis.Analyzer{inspect.Analyzer},
}
@@ -64,19 +64,14 @@
// comment (the syntax proposed by #32816),
// and export a fact for each one.
inlinableFuncs := make(map[*types.Func]*inline.Callee) // memoization of fact import (nil => no fact)
- forwardableConsts := make(map[*types.Const]*goFixForwardConstFact)
+ inlinableConsts := make(map[*types.Const]*goFixInlineConstFact)
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{(*ast.FuncDecl)(nil), (*ast.GenDecl)(nil)}
inspect.Preorder(nodeFilter, func(n ast.Node) {
switch decl := n.(type) {
case *ast.FuncDecl:
- hasInline, hasForward := fixDirectives(decl.Doc)
- if hasForward {
- pass.Reportf(decl.Doc.Pos(), "use //go:fix inline for functions")
- return
- }
- if !hasInline {
+ if !hasFixInline(decl.Doc) {
return
}
content, err := readFile(decl)
@@ -97,20 +92,12 @@
if decl.Tok != token.CONST {
return
}
- declInline, declForward := fixDirectives(decl.Doc)
- if declInline {
- pass.Reportf(decl.Doc.Pos(), "use //go:fix forward for constants")
- return
- }
- // Accept forward directives on the entire decl as well as individual specs.
+ declInline := hasFixInline(decl.Doc)
+ // Accept inline directives on the entire decl as well as individual specs.
for _, spec := range decl.Specs {
spec := spec.(*ast.ValueSpec) // guaranteed by Tok == CONST
- specInline, specForward := fixDirectives(spec.Doc)
- if specInline {
- pass.Reportf(spec.Doc.Pos(), "use //go:fix forward for constants")
- return
- }
- if declForward || specForward {
+ specInline := hasFixInline(spec.Doc)
+ if declInline || specInline {
for i, name := range spec.Names {
if i >= len(spec.Values) {
// Possible following an iota.
@@ -120,21 +107,21 @@
var rhsID *ast.Ident
switch e := val.(type) {
case *ast.Ident:
- // Constants defined with the predeclared iota cannot be forwarded.
+ // Constants defined with the predeclared iota cannot be inlined.
if pass.TypesInfo.Uses[e] == builtinIota {
- pass.Reportf(val.Pos(), "invalid //go:fix forward directive: const value is iota")
+ pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is iota")
continue
}
rhsID = e
case *ast.SelectorExpr:
rhsID = e.Sel
default:
- pass.Reportf(val.Pos(), "invalid //go:fix forward directive: const value is not the name of another constant")
+ pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is not the name of another constant")
continue
}
lhs := pass.TypesInfo.Defs[name].(*types.Const)
rhs := pass.TypesInfo.Uses[rhsID].(*types.Const) // must be so in a well-typed program
- con := &goFixForwardConstFact{
+ con := &goFixInlineConstFact{
RHSName: rhs.Name(),
RHSPkgName: rhs.Pkg().Name(),
RHSPkgPath: rhs.Pkg().Path(),
@@ -142,7 +129,7 @@
if rhs.Pkg() == pass.Pkg {
con.rhsObj = rhs
}
- forwardableConsts[lhs] = con
+ inlinableConsts[lhs] = con
// Create a fact only if the LHS is exported and defined at top level.
// We create a fact even if the RHS is non-exported,
// so we can warn uses in other packages.
@@ -155,8 +142,8 @@
}
})
- // Pass 2. Inline each static call to an inlinable function,
- // and forward each reference to a forwardable constant.
+ // Pass 2. Inline each static call to an inlinable function
+ // and each reference to an inlinable constant.
//
// TODO(adonovan): handle multiple diffs that each add the same import.
for cur := range cursor.Root(inspect).Preorder((*ast.CallExpr)(nil), (*ast.Ident)(nil)) {
@@ -231,14 +218,14 @@
}
case *ast.Ident:
- // If the identifier is a use of a forwardable constant, suggest forwarding it.
+ // If the identifier is a use of an inlinable constant, suggest inlining it.
if con, ok := pass.TypesInfo.Uses[n].(*types.Const); ok {
- fcon, ok := forwardableConsts[con]
+ fcon, ok := inlinableConsts[con]
if !ok {
- var fact goFixForwardConstFact
+ var fact goFixInlineConstFact
if pass.ImportObjectFact(con, &fact) {
fcon = &fact
- forwardableConsts[con] = fcon
+ inlinableConsts[con] = fcon
}
}
if fcon == nil {
@@ -253,7 +240,7 @@
curFile := currentFile(cur)
// We have an identifier A here (n), possibly qualified by a package identifier (sel.X),
- // and a forwardable "const A = B" elsewhere (fcon).
+ // and an inlinable "const A = B" elsewhere (fcon).
// Consider replacing A with B.
// Check that the expression we are inlining (B) means the same thing
@@ -268,22 +255,20 @@
if obj == nil {
// Should be impossible: if code at n can refer to the LHS,
// it can refer to the RHS.
- panic(fmt.Sprintf("no object for forwardable const %s RHS %s", n.Name, fcon.RHSName))
+ panic(fmt.Sprintf("no object for inlinable const %s RHS %s", n.Name, fcon.RHSName))
}
if obj != fcon.rhsObj {
- // "B" means something different here than at the forwardable const's scope.
+ // "B" means something different here than at the inlinable const's scope.
continue
}
}
- importPrefix := ""
- var edits []analysis.TextEdit
+ var (
+ importPrefix string
+ edits []analysis.TextEdit
+ )
if fcon.RHSPkgPath != pass.Pkg.Path() {
- // TODO(jba): fix AddImport so that it returns "." if an existing dot import will work.
- // We will need to tell AddImport the name of the identifier we want to qualify (fcon.RHSName here).
- importID, eds := analysisinternal.AddImport(
- pass.TypesInfo, curFile, n.Pos(), fcon.RHSPkgPath, fcon.RHSPkgName)
- importPrefix = importID + "."
- edits = eds
+ _, importPrefix, edits = analysisinternal.AddImport(
+ pass.TypesInfo, curFile, fcon.RHSPkgName, fcon.RHSPkgPath, fcon.RHSName, n.Pos())
}
var (
pos = n.Pos()
@@ -304,9 +289,9 @@
pass.Report(analysis.Diagnostic{
Pos: pos,
End: end,
- Message: fmt.Sprintf("Constant %s should be forwarded", name),
+ Message: fmt.Sprintf("Constant %s should be inlined", name),
SuggestedFixes: []analysis.SuggestedFix{{
- Message: fmt.Sprintf("Forward constant %s", name),
+ Message: fmt.Sprintf("Inline constant %s", name),
TextEdits: edits,
}},
})
@@ -317,20 +302,15 @@
return nil, nil
}
-// fixDirectives reports the presence of "//go:fix inline" and "//go:fix forward"
-// directives in the comments.
-func fixDirectives(cg *ast.CommentGroup) (inline, forward bool) {
+// hasFixInline reports the presence of a "//go:fix inline" directive
+// in the comments.
+func hasFixInline(cg *ast.CommentGroup) bool {
for _, d := range directives(cg) {
- if d.Tool == "go" && d.Name == "fix" {
- switch d.Args {
- case "inline":
- inline = true
- case "forward":
- forward = true
- }
+ if d.Tool == "go" && d.Name == "fix" && d.Args == "inline" {
+ return true
}
}
- return
+ return false
}
// A goFixInlineFuncFact is exported for each function marked "//go:fix inline".
@@ -340,9 +320,9 @@
func (f *goFixInlineFuncFact) String() string { return "goFixInline " + f.Callee.String() }
func (*goFixInlineFuncFact) AFact() {}
-// A goFixForwardConstFact is exported for each constant marked "//go:fix forward".
-// It holds information about a forwardable constant. Gob-serializable.
-type goFixForwardConstFact struct {
+// A goFixInlineConstFact is exported for each constant marked "//go:fix inline".
+// It holds information about an inlinable constant. Gob-serializable.
+type goFixInlineConstFact struct {
// Information about "const LHSName = RHSName".
RHSName string
RHSPkgPath string
@@ -350,11 +330,11 @@
rhsObj types.Object // for current package
}
-func (c *goFixForwardConstFact) String() string {
- return fmt.Sprintf("goFixForward const %q.%s", c.RHSPkgPath, c.RHSName)
+func (c *goFixInlineConstFact) String() string {
+ return fmt.Sprintf("goFixInline const %q.%s", c.RHSPkgPath, c.RHSName)
}
-func (*goFixForwardConstFact) AFact() {}
+func (*goFixInlineConstFact) AFact() {}
func discard(string, ...any) {}
diff --git a/gopls/internal/analysis/gofix/testdata/src/a/a.go b/gopls/internal/analysis/gofix/testdata/src/a/a.go
index 009afd5..ae48674 100644
--- a/gopls/internal/analysis/gofix/testdata/src/a/a.go
+++ b/gopls/internal/analysis/gofix/testdata/src/a/a.go
@@ -18,87 +18,81 @@
//go:fix inline
func (T) Two() int { return 2 } // want Two:`goFixInline \(a.T\).Two`
-//go:fix forward // want `use //go:fix inline for functions`
-func Three() {}
-
// Constants.
const Uno = 1
-//go:fix forward
-const In1 = Uno // want In1: `goFixForward const "a".Uno`
+//go:fix inline
+const In1 = Uno // want In1: `goFixInline const "a".Uno`
const (
no1 = one
- //go:fix forward
- In2 = one // want In2: `goFixForward const "a".one`
+ //go:fix inline
+ In2 = one // want In2: `goFixInline const "a".one`
)
-//go:fix forward
+//go:fix inline
const (
in3 = one
in4 = one
- bad1 = 1 // want `invalid //go:fix forward directive: const value is not the name of another constant`
+ bad1 = 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
)
-//go:fix forward
+//go:fix inline
const in5,
in6,
bad2 = one, one,
- one + 1 // want `invalid //go:fix forward directive: const value is not the name of another constant`
+ one + 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
// Make sure we don't crash on iota consts, but still process the whole decl.
//
-//go:fix forward
+//go:fix inline
const (
- a = iota // want `invalid //go:fix forward directive: const value is iota`
+ a = iota // want `invalid //go:fix inline directive: const value is iota`
b
in7 = one
)
func _() {
- x := In1 // want `Constant In1 should be forwarded`
- x = In2 // want `Constant In2 should be forwarded`
- x = in3 // want `Constant in3 should be forwarded`
- x = in4 // want `Constant in4 should be forwarded`
- x = in5 // want `Constant in5 should be forwarded`
- x = in6 // want `Constant in6 should be forwarded`
- x = in7 // want `Constant in7 should be forwarded`
+ x := In1 // want `Constant In1 should be inlined`
+ x = In2 // want `Constant In2 should be inlined`
+ x = in3 // want `Constant in3 should be inlined`
+ x = in4 // want `Constant in4 should be inlined`
+ x = in5 // want `Constant in5 should be inlined`
+ x = in6 // want `Constant in6 should be inlined`
+ x = in7 // want `Constant in7 should be inlined`
x = no1
_ = x
- in1 := 1 // don't forward lvalues
+ in1 := 1 // don't inline lvalues
_ = in1
}
const (
x = 1
- //go:fix forward
+ //go:fix inline
in8 = x
)
func shadow() {
var x int // shadows x at package scope
- //go:fix forward
- const a = iota // want `invalid //go:fix forward directive: const value is iota`
+ //go:fix inline
+ const a = iota // want `invalid //go:fix inline directive: const value is iota`
const iota = 2
// Below this point, iota is an ordinary constant.
- //go:fix forward
+ //go:fix inline
const b = iota
- x = a // a is defined with the predeclared iota, so it cannot be forwarded
- x = b // want `Constant b should be forwarded`
+ x = a // a is defined with the predeclared iota, so it cannot be inlined
+ x = b // want `Constant b should be inlined`
- // Don't offer to forward in8, because the result, "x", would mean something different
+ // Don't offer to inline in8, because the result, "x", would mean something different
// in this scope than it does in the scope where in8 is defined.
x = in8
_ = x
}
-
-//go:fix inline // want `use //go:fix forward for constants`
-const In9 = x
diff --git a/gopls/internal/analysis/gofix/testdata/src/a/a.go.golden b/gopls/internal/analysis/gofix/testdata/src/a/a.go.golden
index decbcdd..7d75a59 100644
--- a/gopls/internal/analysis/gofix/testdata/src/a/a.go.golden
+++ b/gopls/internal/analysis/gofix/testdata/src/a/a.go.golden
@@ -18,87 +18,81 @@
//go:fix inline
func (T) Two() int { return 2 } // want Two:`goFixInline \(a.T\).Two`
-//go:fix forward // want `use //go:fix inline for functions`
-func Three() {}
-
// Constants.
const Uno = 1
-//go:fix forward
-const In1 = Uno // want In1: `goFixForward const "a".Uno`
+//go:fix inline
+const In1 = Uno // want In1: `goFixInline const "a".Uno`
const (
no1 = one
- //go:fix forward
- In2 = one // want In2: `goFixForward const "a".one`
+ //go:fix inline
+ In2 = one // want In2: `goFixInline const "a".one`
)
-//go:fix forward
+//go:fix inline
const (
in3 = one
in4 = one
- bad1 = 1 // want `invalid //go:fix forward directive: const value is not the name of another constant`
+ bad1 = 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
)
-//go:fix forward
+//go:fix inline
const in5,
in6,
bad2 = one, one,
- one + 1 // want `invalid //go:fix forward directive: const value is not the name of another constant`
+ one + 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
// Make sure we don't crash on iota consts, but still process the whole decl.
//
-//go:fix forward
+//go:fix inline
const (
- a = iota // want `invalid //go:fix forward directive: const value is iota`
+ a = iota // want `invalid //go:fix inline directive: const value is iota`
b
in7 = one
)
func _() {
- x := Uno // want `Constant In1 should be forwarded`
- x = one // want `Constant In2 should be forwarded`
- x = one // want `Constant in3 should be forwarded`
- x = one // want `Constant in4 should be forwarded`
- x = one // want `Constant in5 should be forwarded`
- x = one // want `Constant in6 should be forwarded`
- x = one // want `Constant in7 should be forwarded`
+ x := Uno // want `Constant In1 should be inlined`
+ x = one // want `Constant In2 should be inlined`
+ x = one // want `Constant in3 should be inlined`
+ x = one // want `Constant in4 should be inlined`
+ x = one // want `Constant in5 should be inlined`
+ x = one // want `Constant in6 should be inlined`
+ x = one // want `Constant in7 should be inlined`
x = no1
_ = x
- in1 := 1 // don't forward lvalues
+ in1 := 1 // don't inline lvalues
_ = in1
}
const (
x = 1
- //go:fix forward
+ //go:fix inline
in8 = x
)
func shadow() {
var x int // shadows x at package scope
- //go:fix forward
- const a = iota // want `invalid //go:fix forward directive: const value is iota`
+ //go:fix inline
+ const a = iota // want `invalid //go:fix inline directive: const value is iota`
const iota = 2
// Below this point, iota is an ordinary constant.
- //go:fix forward
+ //go:fix inline
const b = iota
- x = a // a is defined with the predeclared iota, so it cannot be forwarded
- x = iota // want `Constant b should be forwarded`
+ x = a // a is defined with the predeclared iota, so it cannot be inlined
+ x = iota // want `Constant b should be inlined`
- // Don't offer to forward in8, because the result, "x", would mean something different
+ // Don't offer to inline in8, because the result, "x", would mean something different
// in this scope than it does in the scope where in8 is defined.
x = in8
_ = x
}
-
-//go:fix inline // want `use //go:fix forward for constants`
-const In9 = x
diff --git a/gopls/internal/analysis/gofix/testdata/src/b/b.go b/gopls/internal/analysis/gofix/testdata/src/b/b.go
index 72d4748..4bf9f0d 100644
--- a/gopls/internal/analysis/gofix/testdata/src/b/b.go
+++ b/gopls/internal/analysis/gofix/testdata/src/b/b.go
@@ -9,21 +9,21 @@
new(a.T).Two() // want `Call of \(a.T\).Two should be inlined`
}
-//go:fix forward
+//go:fix inline
const in2 = a.Uno
-//go:fix forward
+//go:fix inline
const in3 = C // c.C, by dot import
func g() {
- x := a.In1 // want `Constant a\.In1 should be forwarded`
+ x := a.In1 // want `Constant a\.In1 should be inlined`
a := 1
// Although the package identifier "a" is shadowed here,
// a second import of "a" will be added with a new package identifer.
- x = in2 // want `Constant in2 should be forwarded`
+ x = in2 // want `Constant in2 should be inlined`
- x = in3 // want `Constant in3 should be forwarded`
+ x = in3 // want `Constant in3 should be inlined`
_ = a
_ = x
diff --git a/gopls/internal/analysis/gofix/testdata/src/b/b.go.golden b/gopls/internal/analysis/gofix/testdata/src/b/b.go.golden
index fdc83c5..b26a05c 100644
--- a/gopls/internal/analysis/gofix/testdata/src/b/b.go.golden
+++ b/gopls/internal/analysis/gofix/testdata/src/b/b.go.golden
@@ -2,8 +2,6 @@
import a0 "a"
-import "c"
-
import (
"a"
. "c"
@@ -15,21 +13,21 @@
_ = 2 // want `Call of \(a.T\).Two should be inlined`
}
-//go:fix forward
+//go:fix inline
const in2 = a.Uno
-//go:fix forward
+//go:fix inline
const in3 = C // c.C, by dot import
func g() {
- x := a.Uno // want `Constant a\.In1 should be forwarded`
+ x := a.Uno // want `Constant a\.In1 should be inlined`
a := 1
// Although the package identifier "a" is shadowed here,
// a second import of "a" will be added with a new package identifer.
- x = a0.Uno // want `Constant in2 should be forwarded`
+ x = a0.Uno // want `Constant in2 should be inlined`
- x = c.C // want `Constant in3 should be forwarded`
+ x = C // want `Constant in3 should be inlined`
_ = a
_ = x
diff --git a/gopls/internal/analysis/modernize/doc.go b/gopls/internal/analysis/modernize/doc.go
index 6a247fe..15aeab6 100644
--- a/gopls/internal/analysis/modernize/doc.go
+++ b/gopls/internal/analysis/modernize/doc.go
@@ -30,4 +30,6 @@
// added in go1.21
// - replacing a 3-clause for i := 0; i < n; i++ {} loop by
// for i := range n {}, added in go1.22;
+// - replacing Split in "for range strings.Split(...)" by go1.24's
+// more efficient SplitSeq;
package modernize
diff --git a/gopls/internal/analysis/modernize/maps.go b/gopls/internal/analysis/modernize/maps.go
index 7950b54..c938996 100644
--- a/gopls/internal/analysis/modernize/maps.go
+++ b/gopls/internal/analysis/modernize/maps.go
@@ -126,28 +126,33 @@
}
}
- // Choose function, report diagnostic, and suggest fix.
+ // Choose function.
+ var funcName string
+ if mrhs != nil {
+ funcName = cond(xmap, "Clone", "Collect")
+ } else {
+ funcName = cond(xmap, "Copy", "Insert")
+ }
+
+ // Report diagnostic, and suggest fix.
rng := curRange.Node()
- mapsName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "maps", "maps")
+ _, prefix, importEdits := analysisinternal.AddImport(info, file, "maps", "maps", funcName, rng.Pos())
var (
- funcName string
newText []byte
start, end token.Pos
)
if mrhs != nil {
// Replace RHS of preceding m=... assignment (and loop) with expression.
start, end = mrhs.Pos(), rng.End()
- funcName = cond(xmap, "Clone", "Collect")
- newText = fmt.Appendf(nil, "%s.%s(%s)",
- mapsName,
+ newText = fmt.Appendf(nil, "%s%s(%s)",
+ prefix,
funcName,
analysisinternal.Format(pass.Fset, x))
} else {
// Replace loop with call statement.
start, end = rng.Pos(), rng.End()
- funcName = cond(xmap, "Copy", "Insert")
- newText = fmt.Appendf(nil, "%s.%s(%s, %s)",
- mapsName,
+ newText = fmt.Appendf(nil, "%s%s(%s, %s)",
+ prefix,
funcName,
analysisinternal.Format(pass.Fset, m),
analysisinternal.Format(pass.Fset, x))
diff --git a/gopls/internal/analysis/modernize/modernize.go b/gopls/internal/analysis/modernize/modernize.go
index 96ab313..0f7b58e 100644
--- a/gopls/internal/analysis/modernize/modernize.go
+++ b/gopls/internal/analysis/modernize/modernize.go
@@ -70,6 +70,7 @@
rangeint(pass)
slicescontains(pass)
slicesdelete(pass)
+ splitseq(pass)
sortslice(pass)
testingContext(pass)
@@ -129,6 +130,7 @@
builtinAppend = types.Universe.Lookup("append")
builtinBool = types.Universe.Lookup("bool")
builtinFalse = types.Universe.Lookup("false")
+ builtinLen = types.Universe.Lookup("len")
builtinMake = types.Universe.Lookup("make")
builtinNil = types.Universe.Lookup("nil")
builtinTrue = types.Universe.Lookup("true")
diff --git a/gopls/internal/analysis/modernize/modernize_test.go b/gopls/internal/analysis/modernize/modernize_test.go
index 7e375c1..6662914 100644
--- a/gopls/internal/analysis/modernize/modernize_test.go
+++ b/gopls/internal/analysis/modernize/modernize_test.go
@@ -23,6 +23,7 @@
"rangeint",
"slicescontains",
"slicesdelete",
+ "splitseq",
"sortslice",
"testingcontext",
)
diff --git a/gopls/internal/analysis/modernize/slices.go b/gopls/internal/analysis/modernize/slices.go
index cb73f7e..bdab9de 100644
--- a/gopls/internal/analysis/modernize/slices.go
+++ b/gopls/internal/analysis/modernize/slices.go
@@ -52,18 +52,21 @@
// Only appends whose base is a clipped slice can be simplified:
// We must conservatively assume an append to an unclipped slice
// such as append(y[:0], x...) is intended to have effects on y.
- clipped, empty := isClippedSlice(info, base)
- if !clipped {
+ clipped, empty := clippedSlice(info, base)
+ if clipped == nil {
return
}
// If the (clipped) base is empty, it may be safely ignored.
- // Otherwise treat it as just another arg (the first) to Concat.
+ // Otherwise treat it (or its unclipped subexpression, if possible)
+ // as just another arg (the first) to Concat.
if !empty {
- sliceArgs = append(sliceArgs, base)
+ sliceArgs = append(sliceArgs, clipped)
}
slices.Reverse(sliceArgs)
+ // TODO(adonovan): simplify sliceArgs[0] further: slices.Clone(s) -> s
+
// Concat of a single (non-trivial) slice degenerates to Clone.
if len(sliceArgs) == 1 {
s := sliceArgs[0]
@@ -92,7 +95,7 @@
}
// append(zerocap, s...) -> slices.Clone(s)
- slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+ _, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Clone", call.Pos())
pass.Report(analysis.Diagnostic{
Pos: call.Pos(),
End: call.End(),
@@ -103,7 +106,7 @@
TextEdits: append(importEdits, []analysis.TextEdit{{
Pos: call.Pos(),
End: call.End(),
- NewText: fmt.Appendf(nil, "%s.Clone(%s)", slicesName, analysisinternal.Format(pass.Fset, s)),
+ NewText: fmt.Appendf(nil, "%sClone(%s)", prefix, analysisinternal.Format(pass.Fset, s)),
}}...),
}},
})
@@ -111,12 +114,7 @@
}
// append(append(append(base, a...), b..., c...) -> slices.Concat(base, a, b, c)
- //
- // TODO(adonovan): simplify sliceArgs[0] further:
- // - slices.Clone(s) -> s
- // - s[:len(s):len(s)] -> s
- // - slices.Clip(s) -> s
- slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+ _, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Concat", call.Pos())
pass.Report(analysis.Diagnostic{
Pos: call.Pos(),
End: call.End(),
@@ -127,7 +125,7 @@
TextEdits: append(importEdits, []analysis.TextEdit{{
Pos: call.Pos(),
End: call.End(),
- NewText: fmt.Appendf(nil, "%s.Concat(%s)", slicesName, formatExprs(pass.Fset, sliceArgs)),
+ NewText: fmt.Appendf(nil, "%sConcat(%s)", prefix, formatExprs(pass.Fset, sliceArgs)),
}}...),
}},
})
@@ -172,25 +170,36 @@
}
}
-// isClippedSlice reports whether e denotes a slice that is definitely
-// clipped, that is, its len(s)==cap(s).
+// clippedSlice returns res != nil if e denotes a slice that is
+// definitely clipped, that is, its len(s)==cap(s).
//
-// In addition, it reports whether the slice is definitely empty.
+// The value of res is either the same as e or is a subexpression of e
+// that denotes the same slice but without the clipping operation.
+//
+// In addition, it reports whether the slice is definitely empty,
//
// Examples of clipped slices:
//
// x[:0:0] (empty)
// []T(nil) (empty)
// Slice{} (empty)
-// x[:len(x):len(x)] (nonempty)
+// x[:len(x):len(x)] (nonempty) res=x
// x[:k:k] (nonempty)
-// slices.Clip(x) (nonempty)
-func isClippedSlice(info *types.Info, e ast.Expr) (clipped, empty bool) {
+// slices.Clip(x) (nonempty) res=x
+func clippedSlice(info *types.Info, e ast.Expr) (res ast.Expr, empty bool) {
switch e := e.(type) {
case *ast.SliceExpr:
- // x[:0:0], x[:len(x):len(x)], x[:k:k], x[:0]
- clipped = e.Slice3 && e.High != nil && e.Max != nil && equalSyntax(e.High, e.Max) // x[:k:k]
- empty = e.High != nil && isZeroLiteral(e.High) // x[:0:*]
+ // x[:0:0], x[:len(x):len(x)], x[:k:k]
+ if e.Slice3 && e.High != nil && e.Max != nil && equalSyntax(e.High, e.Max) { // x[:k:k]
+ res = e
+ empty = isZeroLiteral(e.High) // x[:0:0]
+ if call, ok := e.High.(*ast.CallExpr); ok &&
+ typeutil.Callee(info, call) == builtinLen &&
+ equalSyntax(call.Args[0], e.X) {
+ res = e.X // x[:len(x):len(x)] -> x
+ }
+ return
+ }
return
case *ast.CallExpr:
@@ -198,20 +207,20 @@
if info.Types[e.Fun].IsType() &&
is[*ast.Ident](e.Args[0]) &&
info.Uses[e.Args[0].(*ast.Ident)] == builtinNil {
- return true, true
+ return e, true
}
// slices.Clip(x)?
obj := typeutil.Callee(info, e)
if analysisinternal.IsFunctionNamed(obj, "slices", "Clip") {
- return true, false
+ return e.Args[0], false // slices.Clip(x) -> x
}
case *ast.CompositeLit:
// Slice{}?
if len(e.Elts) == 0 {
- return true, true
+ return e, true
}
}
- return false, false
+ return nil, false
}
diff --git a/gopls/internal/analysis/modernize/slicescontains.go b/gopls/internal/analysis/modernize/slicescontains.go
index d860d64..0964244 100644
--- a/gopls/internal/analysis/modernize/slicescontains.go
+++ b/gopls/internal/analysis/modernize/slicescontains.go
@@ -158,9 +158,9 @@
}
// Prepare slices.Contains{,Func} call.
- slicesName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "slices", "slices")
- contains := fmt.Sprintf("%s.%s(%s, %s)",
- slicesName,
+ _, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", funcName, rng.Pos())
+ contains := fmt.Sprintf("%s%s(%s, %s)",
+ prefix,
funcName,
analysisinternal.Format(pass.Fset, rng.X),
analysisinternal.Format(pass.Fset, arg2))
diff --git a/gopls/internal/analysis/modernize/slicesdelete.go b/gopls/internal/analysis/modernize/slicesdelete.go
index f1f96c7..24b2182 100644
--- a/gopls/internal/analysis/modernize/slicesdelete.go
+++ b/gopls/internal/analysis/modernize/slicesdelete.go
@@ -13,6 +13,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/internal/analysisinternal"
)
// The slicesdelete pass attempts to replace instances of append(s[:i], s[i+k:]...)
@@ -22,7 +23,8 @@
func slicesdelete(pass *analysis.Pass) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
info := pass.TypesInfo
- report := func(call *ast.CallExpr, slice1, slice2 *ast.SliceExpr) {
+ report := func(file *ast.File, call *ast.CallExpr, slice1, slice2 *ast.SliceExpr) {
+ _, prefix, edits := analysisinternal.AddImport(info, file, "slices", "slices", "Delete", call.Pos())
pass.Report(analysis.Diagnostic{
Pos: call.Pos(),
End: call.End(),
@@ -30,12 +32,12 @@
Message: "Replace append with slices.Delete",
SuggestedFixes: []analysis.SuggestedFix{{
Message: "Replace append with slices.Delete",
- TextEdits: []analysis.TextEdit{
+ TextEdits: append(edits, []analysis.TextEdit{
// Change name of called function.
{
Pos: call.Fun.Pos(),
End: call.Fun.End(),
- NewText: []byte("slices.Delete"),
+ NewText: []byte(prefix + "Delete"),
},
// Delete ellipsis.
{
@@ -69,11 +71,12 @@
Pos: slice2.Low.End(),
End: slice2.Rbrack + 1,
},
- },
+ }...),
}},
})
}
for curFile := range filesUsing(inspect, info, "go1.21") {
+ file := curFile.Node().(*ast.File)
for curCall := range curFile.Preorder((*ast.CallExpr)(nil)) {
call := curCall.Node().(*ast.CallExpr)
if id, ok := call.Fun.(*ast.Ident); ok && len(call.Args) == 2 {
@@ -88,7 +91,7 @@
equalSyntax(slice1.X, slice2.X) &&
increasingSliceIndices(info, slice1.High, slice2.Low) {
// Have append(s[:a], s[b:]...) where we can verify a < b.
- report(call, slice1, slice2)
+ report(file, call, slice1, slice2)
}
}
}
diff --git a/gopls/internal/analysis/modernize/sortslice.go b/gopls/internal/analysis/modernize/sortslice.go
index 4f856d3..7f695d7 100644
--- a/gopls/internal/analysis/modernize/sortslice.go
+++ b/gopls/internal/analysis/modernize/sortslice.go
@@ -70,7 +70,8 @@
if isIndex(compare.X, i) && isIndex(compare.Y, j) {
// Have: sort.Slice(s, func(i, j int) bool { return s[i] < s[j] })
- slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+ _, prefix, importEdits := analysisinternal.AddImport(
+ info, file, "slices", "slices", "Sort", call.Pos())
pass.Report(analysis.Diagnostic{
// Highlight "sort.Slice".
@@ -85,7 +86,7 @@
// Replace sort.Slice with slices.Sort.
Pos: call.Fun.Pos(),
End: call.Fun.End(),
- NewText: []byte(slicesName + ".Sort"),
+ NewText: []byte(prefix + "Sort"),
},
{
// Eliminate FuncLit.
diff --git a/gopls/internal/analysis/modernize/splitseq.go b/gopls/internal/analysis/modernize/splitseq.go
new file mode 100644
index 0000000..1f3da85
--- /dev/null
+++ b/gopls/internal/analysis/modernize/splitseq.go
@@ -0,0 +1,112 @@
+// 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/token"
+ "go/types"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/go/types/typeutil"
+ "golang.org/x/tools/internal/analysisinternal"
+ "golang.org/x/tools/internal/astutil/edge"
+)
+
+// splitseq offers a fix to replace a call to strings.Split with
+// SplitSeq when it is the operand of a range loop, either directly:
+//
+// for _, line := range strings.Split() {...}
+//
+// or indirectly, if the variable's sole use is the range statement:
+//
+// lines := strings.Split()
+// for _, line := range lines {...}
+//
+// Variants:
+// - bytes.SplitSeq
+func splitseq(pass *analysis.Pass) {
+ if !analysisinternal.Imports(pass.Pkg, "strings") &&
+ !analysisinternal.Imports(pass.Pkg, "bytes") {
+ return
+ }
+ info := pass.TypesInfo
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+ for curFile := range filesUsing(inspect, info, "go1.24") {
+ for curRange := range curFile.Preorder((*ast.RangeStmt)(nil)) {
+ rng := curRange.Node().(*ast.RangeStmt)
+
+ // Reject "for i, line := ..." since SplitSeq is not an iter.Seq2.
+ // (We require that i is blank.)
+ if id, ok := rng.Key.(*ast.Ident); ok && id.Name != "_" {
+ continue
+ }
+
+ // Find the call operand of the range statement,
+ // whether direct or indirect.
+ call, ok := rng.X.(*ast.CallExpr)
+ if !ok {
+ if id, ok := rng.X.(*ast.Ident); ok {
+ if v, ok := info.Uses[id].(*types.Var); ok {
+ if ek, idx := curRange.Edge(); ek == edge.BlockStmt_List && idx > 0 {
+ curPrev, _ := curRange.PrevSibling()
+ if assign, ok := curPrev.Node().(*ast.AssignStmt); ok &&
+ assign.Tok == token.DEFINE &&
+ len(assign.Lhs) == 1 &&
+ len(assign.Rhs) == 1 &&
+ info.Defs[assign.Lhs[0].(*ast.Ident)] == v &&
+ soleUse(info, v) == id {
+ // Have:
+ // lines := ...
+ // for _, line := range lines {...}
+ // and no other uses of lines.
+ call, _ = assign.Rhs[0].(*ast.CallExpr)
+ }
+ }
+ }
+ }
+ }
+
+ if call != nil {
+ var edits []analysis.TextEdit
+ if rng.Key != nil {
+ // Delete (blank) RangeStmt.Key:
+ // for _, line := -> for line :=
+ // for _, _ := -> for
+ // for _ := -> for
+ end := rng.Range
+ if rng.Value != nil {
+ end = rng.Value.Pos()
+ }
+ edits = append(edits, analysis.TextEdit{
+ Pos: rng.Key.Pos(),
+ End: end,
+ })
+ }
+
+ if sel, ok := call.Fun.(*ast.SelectorExpr); ok &&
+ (analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "strings", "Split") ||
+ analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "bytes", "Split")) {
+ pass.Report(analysis.Diagnostic{
+ Pos: sel.Pos(),
+ End: sel.End(),
+ Category: "splitseq",
+ Message: "Ranging over SplitSeq is more efficient",
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: "Replace Split with SplitSeq",
+ TextEdits: append(edits, analysis.TextEdit{
+ // Split -> SplitSeq
+ Pos: sel.Sel.Pos(),
+ End: sel.Sel.End(),
+ NewText: []byte("SplitSeq")}),
+ }},
+ })
+ }
+ }
+ }
+ }
+}
diff --git a/gopls/internal/analysis/modernize/testdata/src/appendclipped/appendclipped.go.golden b/gopls/internal/analysis/modernize/testdata/src/appendclipped/appendclipped.go.golden
index 5d6761b..6352d52 100644
--- a/gopls/internal/analysis/modernize/testdata/src/appendclipped/appendclipped.go.golden
+++ b/gopls/internal/analysis/modernize/testdata/src/appendclipped/appendclipped.go.golden
@@ -20,7 +20,7 @@
print(slices.Concat(Bytes{1, 2, 3}, Bytes{4, 5, 6})) // want "Replace append with slices.Concat"
print(slices.Concat(s, other, other)) // want "Replace append with slices.Concat"
print(slices.Concat(os.Environ(), other, other)) // want "Replace append with slices.Concat"
- print(slices.Concat(other[:len(other):len(other)], s, other)) // want "Replace append with slices.Concat"
- print(slices.Concat(slices.Clip(other), s, other)) // want "Replace append with slices.Concat"
+ print(slices.Concat(other, s, other)) // want "Replace append with slices.Concat"
+ print(slices.Concat(other, s, other)) // want "Replace append with slices.Concat"
print(append(append(append(other[:0], s...), other...), other...)) // nope: intent may be to mutate other
}
diff --git a/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go b/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go
new file mode 100644
index 0000000..c33d43e
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go
@@ -0,0 +1,23 @@
+//go:build go1.23
+
+package mapsloop
+
+import . "maps"
+
+var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
+
+func useCopyDot(dst, src map[int]string) {
+ // Replace loop by maps.Copy.
+ for key, value := range src {
+ dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Copy"
+ }
+}
+
+func useCloneDot(src map[int]string) {
+ // Replace make(...) by maps.Clone.
+ dst := make(map[int]string, len(src))
+ for key, value := range src {
+ dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Clone"
+ }
+ println(dst)
+}
diff --git a/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go.golden b/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go.golden
new file mode 100644
index 0000000..d6a3053
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go.golden
@@ -0,0 +1,19 @@
+//go:build go1.23
+
+package mapsloop
+
+import . "maps"
+
+var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
+
+func useCopyDot(dst, src map[int]string) {
+ // Replace loop by maps.Copy.
+ Copy(dst, src)
+}
+
+func useCloneDot(src map[int]string) {
+ // Replace make(...) by maps.Clone.
+ dst := Clone(src)
+ println(dst)
+}
+
diff --git a/gopls/internal/analysis/modernize/testdata/src/slicesdelete/slicesdelete.go.golden b/gopls/internal/analysis/modernize/testdata/src/slicesdelete/slicesdelete.go.golden
index 8c2f21a..2d9447a 100644
--- a/gopls/internal/analysis/modernize/testdata/src/slicesdelete/slicesdelete.go.golden
+++ b/gopls/internal/analysis/modernize/testdata/src/slicesdelete/slicesdelete.go.golden
@@ -1,5 +1,7 @@
package slicesdelete
+import "slices"
+
var g struct{ f []int }
func slicesdelete(test, other []byte, i int) {
diff --git a/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice.go.golden b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice.go.golden
index d97636f..34af5aa 100644
--- a/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice.go.golden
+++ b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice.go.golden
@@ -2,8 +2,6 @@
import "slices"
-import "slices"
-
import "sort"
type myint int
diff --git a/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go
new file mode 100644
index 0000000..8502718
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go
@@ -0,0 +1,26 @@
+package sortslice
+
+import . "slices"
+import "sort"
+
+func _(s []myint) {
+ sort.Slice(s, func(i, j int) bool { return s[i] < s[j] }) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(x *struct{ s []int }) {
+ sort.Slice(x.s, func(first, second int) bool { return x.s[first] < x.s[second] }) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(s []int) {
+ sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
+}
+
+func _(s []int) {
+ sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
+}
+
+func _(s2 []struct{ x int }) {
+ sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+}
+
+func _() { Clip([]int{}) }
diff --git a/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go.golden b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go.golden
new file mode 100644
index 0000000..45c056d
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go.golden
@@ -0,0 +1,26 @@
+package sortslice
+
+import . "slices"
+import "sort"
+
+func _(s []myint) {
+ Sort(s) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(x *struct{ s []int }) {
+ Sort(x.s) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(s []int) {
+ sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
+}
+
+func _(s []int) {
+ sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
+}
+
+func _(s2 []struct{ x int }) {
+ sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+}
+
+func _() { Clip([]int{}) }
diff --git a/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go
new file mode 100644
index 0000000..4f533ed
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go
@@ -0,0 +1,42 @@
+//go:build go1.24
+
+package splitseq
+
+import (
+ "bytes"
+ "strings"
+)
+
+func _() {
+ for _, line := range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
+ println(line)
+ }
+ for i, line := range strings.Split("", "") { // nope: uses index var
+ println(i, line)
+ }
+ for i, _ := range strings.Split("", "") { // nope: uses index var
+ println(i)
+ }
+ for i := range strings.Split("", "") { // nope: uses index var
+ println(i)
+ }
+ for _ = range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
+ }
+ for range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
+ }
+ for range bytes.Split(nil, nil) { // want "Ranging over SplitSeq is more efficient"
+ }
+ {
+ lines := strings.Split("", "") // want "Ranging over SplitSeq is more efficient"
+ for _, line := range lines {
+ println(line)
+ }
+ }
+ {
+ lines := strings.Split("", "") // nope: lines is used not just by range
+ for _, line := range lines {
+ println(line)
+ }
+ println(lines)
+ }
+}
diff --git a/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go.golden b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go.golden
new file mode 100644
index 0000000..d10e0e8
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go.golden
@@ -0,0 +1,42 @@
+//go:build go1.24
+
+package splitseq
+
+import (
+ "bytes"
+ "strings"
+)
+
+func _() {
+ for line := range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
+ println(line)
+ }
+ for i, line := range strings.Split("", "") { // nope: uses index var
+ println(i, line)
+ }
+ for i, _ := range strings.Split("", "") { // nope: uses index var
+ println(i)
+ }
+ for i := range strings.Split("", "") { // nope: uses index var
+ println(i)
+ }
+ for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
+ }
+ for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
+ }
+ for range bytes.SplitSeq(nil, nil) { // want "Ranging over SplitSeq is more efficient"
+ }
+ {
+ lines := strings.SplitSeq("", "") // want "Ranging over SplitSeq is more efficient"
+ for line := range lines {
+ println(line)
+ }
+ }
+ {
+ lines := strings.Split("", "") // nope: lines is used not just by range
+ for _, line := range lines {
+ println(line)
+ }
+ println(lines)
+ }
+}
diff --git a/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq_go123.go b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq_go123.go
new file mode 100644
index 0000000..c3e86bb
--- /dev/null
+++ b/gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq_go123.go
@@ -0,0 +1 @@
+package splitseq
diff --git a/gopls/internal/cache/session_test.go b/gopls/internal/cache/session_test.go
index 5f9a59a..1b7472a 100644
--- a/gopls/internal/cache/session_test.go
+++ b/gopls/internal/cache/session_test.go
@@ -337,7 +337,8 @@
for _, f := range test.folders {
opts := settings.DefaultOptions()
if f.options != nil {
- for _, err := range opts.Set(f.options(dir)) {
+ _, errs := opts.Set(f.options(dir))
+ for _, err := range errs {
t.Fatal(err)
}
}
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index 5079edc..8f10107 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -475,7 +475,7 @@
},
{
"Name": "\"gofix\"",
- "Doc": "apply fixes based on go:fix comment directives\n\nThe gofix analyzer inlines functions that are marked for inlining\nand forwards constants that are marked for forwarding.",
+ "Doc": "apply fixes based on go:fix comment directives\n\nThe gofix analyzer inlines functions and constants that are marked for inlining.",
"Default": "true"
},
{
@@ -510,7 +510,7 @@
},
{
"Name": "\"modernize\"",
- "Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;",
+ "Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq;",
"Default": "true"
},
{
@@ -1147,7 +1147,7 @@
},
{
"Name": "gofix",
- "Doc": "apply fixes based on go:fix comment directives\n\nThe gofix analyzer inlines functions that are marked for inlining\nand forwards constants that are marked for forwarding.",
+ "Doc": "apply fixes based on go:fix comment directives\n\nThe gofix analyzer inlines functions and constants that are marked for inlining.",
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/gofix",
"Default": true
},
@@ -1189,7 +1189,7 @@
},
{
"Name": "modernize",
- "Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;",
+ "Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq;",
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
"Default": true
},
diff --git a/gopls/internal/golang/folding_range.go b/gopls/internal/golang/folding_range.go
index 9d80cc8..4352da2 100644
--- a/gopls/internal/golang/folding_range.go
+++ b/gopls/internal/golang/folding_range.go
@@ -6,6 +6,7 @@
import (
"bytes"
+ "cmp"
"context"
"go/ast"
"go/token"
@@ -20,14 +21,8 @@
"golang.org/x/tools/gopls/internal/util/safetoken"
)
-// FoldingRangeInfo holds range and kind info of folding for an ast.Node
-type FoldingRangeInfo struct {
- Range protocol.Range
- Kind protocol.FoldingRangeKind
-}
-
// FoldingRange gets all of the folding range for f.
-func FoldingRange(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) {
+func FoldingRange(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, lineFoldingOnly bool) ([]protocol.FoldingRange, error) {
// TODO(suzmue): consider limiting the number of folding ranges returned, and
// implement a way to prioritize folding ranges in that case.
pgf, err := snapshot.ParseGo(ctx, fh, parsego.Full)
@@ -48,27 +43,29 @@
}
// Get folding ranges for comments separately as they are not walked by ast.Inspect.
- ranges = append(ranges, commentsFoldingRange(pgf)...)
+ ranges := commentsFoldingRange(pgf)
- visit := func(n ast.Node) bool {
- rng := foldingRangeFunc(pgf, n, lineFoldingOnly)
- if rng != nil {
+ // Walk the ast and collect folding ranges.
+ ast.Inspect(pgf.File, func(n ast.Node) bool {
+ if rng, ok := foldingRangeFunc(pgf, n, lineFoldingOnly); ok {
ranges = append(ranges, rng)
}
return true
- }
- // Walk the ast and collect folding ranges.
- ast.Inspect(pgf.File, visit)
+ })
- slices.SortFunc(ranges, func(x, y *FoldingRangeInfo) int {
- return protocol.CompareRange(x.Range, y.Range)
+ // Sort by start position.
+ slices.SortFunc(ranges, func(x, y protocol.FoldingRange) int {
+ if d := cmp.Compare(x.StartLine, y.StartLine); d != 0 {
+ return d
+ }
+ return cmp.Compare(x.StartCharacter, y.StartCharacter)
})
return ranges, nil
}
// foldingRangeFunc calculates the line folding range for ast.Node n
-func foldingRangeFunc(pgf *parsego.File, n ast.Node, lineFoldingOnly bool) *FoldingRangeInfo {
+func foldingRangeFunc(pgf *parsego.File, n ast.Node, lineFoldingOnly bool) (protocol.FoldingRange, bool) {
// TODO(suzmue): include trailing empty lines before the closing
// parenthesis/brace.
var kind protocol.FoldingRangeKind
@@ -109,25 +106,22 @@
// Check that folding positions are valid.
if !start.IsValid() || !end.IsValid() {
- return nil
+ return protocol.FoldingRange{}, false
}
if start == end {
// Nothing to fold.
- return nil
+ return protocol.FoldingRange{}, false
}
// in line folding mode, do not fold if the start and end lines are the same.
if lineFoldingOnly && safetoken.Line(pgf.Tok, start) == safetoken.Line(pgf.Tok, end) {
- return nil
+ return protocol.FoldingRange{}, false
}
rng, err := pgf.PosRange(start, end)
if err != nil {
bug.Reportf("failed to create range: %s", err) // can't happen
- return nil
+ return protocol.FoldingRange{}, false
}
- return &FoldingRangeInfo{
- Range: rng,
- Kind: kind,
- }
+ return foldingRange(kind, rng), true
}
// getLineFoldingRange returns the folding range for nodes with parentheses/braces/brackets
@@ -196,7 +190,7 @@
// commentsFoldingRange returns the folding ranges for all comment blocks in file.
// The folding range starts at the end of the first line of the comment block, and ends at the end of the
// comment block and has kind protocol.Comment.
-func commentsFoldingRange(pgf *parsego.File) (comments []*FoldingRangeInfo) {
+func commentsFoldingRange(pgf *parsego.File) (comments []protocol.FoldingRange) {
tokFile := pgf.Tok
for _, commentGrp := range pgf.File.Comments {
startGrpLine, endGrpLine := safetoken.Line(tokFile, commentGrp.Pos()), safetoken.Line(tokFile, commentGrp.End())
@@ -218,11 +212,19 @@
bug.Reportf("failed to create mapped range: %s", err) // can't happen
continue
}
- comments = append(comments, &FoldingRangeInfo{
- // Fold from the end of the first line comment to the end of the comment block.
- Range: rng,
- Kind: protocol.Comment,
- })
+ // Fold from the end of the first line comment to the end of the comment block.
+ comments = append(comments, foldingRange(protocol.Comment, rng))
}
return comments
}
+
+func foldingRange(kind protocol.FoldingRangeKind, rng protocol.Range) protocol.FoldingRange {
+ return protocol.FoldingRange{
+ // I have no idea why LSP doesn't use a protocol.Range here.
+ StartLine: rng.Start.Line,
+ StartCharacter: rng.Start.Character,
+ EndLine: rng.End.Line,
+ EndCharacter: rng.End.Character,
+ Kind: string(kind),
+ }
+}
diff --git a/gopls/internal/protocol/command/interface.go b/gopls/internal/protocol/command/interface.go
index 32e03dd..34adf59 100644
--- a/gopls/internal/protocol/command/interface.go
+++ b/gopls/internal/protocol/command/interface.go
@@ -814,6 +814,9 @@
Detail string `json:"detail,omitempty"`
+ // protocol.SymbolKind maps an integer to an enum:
+ // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind
+ // i.e. File = 1
Kind protocol.SymbolKind `json:"kind"`
Tags []protocol.SymbolTag `json:"tags,omitempty"`
diff --git a/gopls/internal/server/folding_range.go b/gopls/internal/server/folding_range.go
index 95b2ffc..b05d530 100644
--- a/gopls/internal/server/folding_range.go
+++ b/gopls/internal/server/folding_range.go
@@ -26,24 +26,5 @@
if snapshot.FileKind(fh) != file.Go {
return nil, nil // empty result
}
- ranges, err := golang.FoldingRange(ctx, snapshot, fh, snapshot.Options().LineFoldingOnly)
- if err != nil {
- return nil, err
- }
- return toProtocolFoldingRanges(ranges)
-}
-
-func toProtocolFoldingRanges(ranges []*golang.FoldingRangeInfo) ([]protocol.FoldingRange, error) {
- result := make([]protocol.FoldingRange, 0, len(ranges))
- for _, info := range ranges {
- rng := info.Range
- result = append(result, protocol.FoldingRange{
- StartLine: rng.Start.Line,
- StartCharacter: rng.Start.Character,
- EndLine: rng.End.Line,
- EndCharacter: rng.End.Character,
- Kind: string(info.Kind),
- })
- }
- return result, nil
+ return golang.FoldingRange(ctx, snapshot, fh, snapshot.Options().LineFoldingOnly)
}
diff --git a/gopls/internal/server/general.go b/gopls/internal/server/general.go
index 3561494..de6b764 100644
--- a/gopls/internal/server/general.go
+++ b/gopls/internal/server/general.go
@@ -28,6 +28,7 @@
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/semtok"
"golang.org/x/tools/gopls/internal/settings"
+ "golang.org/x/tools/gopls/internal/telemetry"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/goversion"
"golang.org/x/tools/gopls/internal/util/moremaps"
@@ -74,7 +75,11 @@
// TODO(rfindley): eliminate this defer.
defer func() { s.SetOptions(options) }()
- s.handleOptionErrors(ctx, options.Set(params.InitializationOptions))
+ // Process initialization options.
+ {
+ res, errs := options.Set(params.InitializationOptions)
+ s.handleOptionResult(ctx, res, errs)
+ }
options.ForClientCapabilities(params.ClientInfo, params.Capabilities)
if options.ShowBugReports {
@@ -541,7 +546,8 @@
opts = opts.Clone()
for _, config := range configs {
- s.handleOptionErrors(ctx, opts.Set(config))
+ res, errs := opts.Set(config)
+ s.handleOptionResult(ctx, res, errs)
}
return opts, nil
}
@@ -555,7 +561,12 @@
s.notifications = append(s.notifications, msg)
}
-func (s *server) handleOptionErrors(ctx context.Context, optionErrors []error) {
+func (s *server) handleOptionResult(ctx context.Context, applied []telemetry.CounterPath, optionErrors []error) {
+ for _, path := range applied {
+ path = append(settings.CounterPath{"gopls", "setting"}, path...)
+ counter.Inc(path.FullName())
+ }
+
var warnings, errs []string
for _, err := range optionErrors {
if err == nil {
diff --git a/gopls/internal/settings/settings.go b/gopls/internal/settings/settings.go
index 8f33bda..7d64cbe 100644
--- a/gopls/internal/settings/settings.go
+++ b/gopls/internal/settings/settings.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/semtok"
+ "golang.org/x/tools/gopls/internal/telemetry"
"golang.org/x/tools/gopls/internal/util/frob"
)
@@ -822,10 +823,18 @@
// TODO: support "Manual"?
)
-// Set updates *options based on the provided JSON value:
+type CounterPath = telemetry.CounterPath
+
+// Set updates *Options based on the provided JSON value:
// null, bool, string, number, array, or object.
+//
+// The applied result describes settings that were applied. Each CounterPath
+// contains at least the name of the setting, but may also include sub-setting
+// names for settings that are themselves maps, and/or a non-empty bucket name
+// when bucketing is desirable.
+//
// On failure, it returns one or more non-nil errors.
-func (o *Options) Set(value any) (errors []error) {
+func (o *Options) Set(value any) (applied []CounterPath, errs []error) {
switch value := value.(type) {
case nil:
case map[string]any:
@@ -840,19 +849,32 @@
name = split[len(split)-1]
if _, ok := seen[name]; ok {
- errors = append(errors, fmt.Errorf("duplicate value for %s", name))
+ errs = append(errs, fmt.Errorf("duplicate value for %s", name))
}
seen[name] = struct{}{}
- if err := o.setOne(name, value); err != nil {
+ paths, err := o.setOne(name, value)
+ if err != nil {
err := fmt.Errorf("setting option %q: %w", name, err)
- errors = append(errors, err)
+ errs = append(errs, err)
+ }
+ _, soft := err.(*SoftError)
+ if err == nil || soft {
+ if len(paths) == 0 {
+ path := CounterPath{name, ""}
+ applied = append(applied, path)
+ } else {
+ for _, subpath := range paths {
+ path := append(CounterPath{name}, subpath...)
+ applied = append(applied, path)
+ }
+ }
}
}
default:
- errors = append(errors, fmt.Errorf("invalid options type %T (want JSON null or object)", value))
+ errs = append(errs, fmt.Errorf("invalid options type %T (want JSON null or object)", value))
}
- return errors
+ return applied, errs
}
func (o *Options) ForClientCapabilities(clientInfo *protocol.ClientInfo, caps protocol.ClientCapabilities) {
@@ -955,14 +977,26 @@
}
// setOne updates a field of o based on the name and value.
+//
+// The applied result describes the counter values to be updated as a result of
+// the applied setting. If the result is nil, the default counter for this
+// setting should be updated.
+//
+// For example, if the setting name is "foo",
+// - If applied is nil, update the count for "foo".
+// - If applied is []CounterPath{{"bucket"}}, update the count for
+// foo:bucket.
+// - If applied is []CounterPath{{"a","b"}, {"c","d"}}, update foo/a:b and
+// foo/c:d.
+//
// It returns an error if the value was invalid or duplicate.
// It is the caller's responsibility to augment the error with 'name'.
-func (o *Options) setOne(name string, value any) error {
+func (o *Options) setOne(name string, value any) (applied []CounterPath, _ error) {
switch name {
case "env":
env, ok := value.(map[string]any)
if !ok {
- return fmt.Errorf("invalid type %T (want JSON object)", value)
+ return nil, fmt.Errorf("invalid type %T (want JSON object)", value)
}
if o.Env == nil {
o.Env = make(map[string]string)
@@ -973,30 +1007,32 @@
case string, int:
o.Env[k] = fmt.Sprint(v)
default:
- return fmt.Errorf("invalid map value %T (want string)", v)
+ return nil, fmt.Errorf("invalid map value %T (want string)", v)
}
}
+ return nil, nil
case "buildFlags":
- return setStringSlice(&o.BuildFlags, value)
+ return nil, setStringSlice(&o.BuildFlags, value)
case "directoryFilters":
filterStrings, err := asStringSlice(value)
if err != nil {
- return err
+ return nil, err
}
var filters []string
for _, filterStr := range filterStrings {
filter, err := validateDirectoryFilter(filterStr)
if err != nil {
- return err
+ return nil, err
}
filters = append(filters, strings.TrimRight(filepath.FromSlash(filter), "/"))
}
o.DirectoryFilters = filters
+ return nil, nil
case "workspaceFiles":
- return setStringSlice(&o.WorkspaceFiles, value)
+ return nil, setStringSlice(&o.WorkspaceFiles, value)
case "completionDocumentation":
return setBool(&o.CompletionDocumentation, value)
case "usePlaceholders":
@@ -1006,7 +1042,7 @@
case "completeUnimported":
return setBool(&o.CompleteUnimported, value)
case "completionBudget":
- return setDuration(&o.CompletionBudget, value)
+ return nil, setDuration(&o.CompletionBudget, value)
case "importsSource":
return setEnum(&o.ImportsSource, value,
ImportsSourceOff,
@@ -1038,7 +1074,7 @@
case "hoverKind":
if s, ok := value.(string); ok && strings.EqualFold(s, "structured") {
- return deprecatedError("the experimental hoverKind='structured' setting was removed in gopls/v0.18.0 (https://go.dev/issue/70233)")
+ return nil, deprecatedError("the experimental hoverKind='structured' setting was removed in gopls/v0.18.0 (https://go.dev/issue/70233)")
}
return setEnum(&o.HoverKind, value,
NoDocumentation,
@@ -1047,7 +1083,7 @@
FullDocumentation)
case "linkTarget":
- return setString(&o.LinkTarget, value)
+ return nil, setString(&o.LinkTarget, value)
case "linksInHover":
switch value {
@@ -1058,9 +1094,9 @@
case "gopls":
o.LinksInHover = LinksInHover_Gopls
default:
- return fmt.Errorf(`invalid value %s; expect false, true, or "gopls"`,
- value)
+ return nil, fmt.Errorf(`invalid value %s; expect false, true, or "gopls"`, value)
}
+ return nil, nil
case "importShortcut":
return setEnum(&o.ImportShortcut, value,
@@ -1069,18 +1105,20 @@
DefinitionShortcut)
case "analyses":
- if err := setBoolMap(&o.Analyses, value); err != nil {
- return err
+ counts, err := setBoolMap(&o.Analyses, value)
+ if err != nil {
+ return nil, err
}
if o.Analyses["fieldalignment"] {
- return deprecatedError("the 'fieldalignment' analyzer was removed in gopls/v0.17.0; instead, hover over struct fields to see size/offset information (https://go.dev/issue/66861)")
+ return counts, deprecatedError("the 'fieldalignment' analyzer was removed in gopls/v0.17.0; instead, hover over struct fields to see size/offset information (https://go.dev/issue/66861)")
}
+ return counts, nil
case "hints":
return setBoolMap(&o.Hints, value)
case "annotations":
- return deprecatedError("the 'annotations' setting was removed in gopls/v0.18.0; all compiler optimization details are now shown")
+ return nil, deprecatedError("the 'annotations' setting was removed in gopls/v0.18.0; all compiler optimization details are now shown")
case "vulncheck":
return setEnum(&o.Vulncheck, value,
@@ -1090,7 +1128,7 @@
case "codelenses", "codelens":
lensOverrides, err := asBoolMap[CodeLensSource](value)
if err != nil {
- return err
+ return nil, err
}
if o.Codelenses == nil {
o.Codelenses = make(map[CodeLensSource]bool)
@@ -1098,15 +1136,21 @@
o.Codelenses = maps.Clone(o.Codelenses)
maps.Copy(o.Codelenses, lensOverrides)
- if name == "codelens" {
- return deprecatedError("codelenses")
+ var counts []CounterPath
+ for k, v := range lensOverrides {
+ counts = append(counts, CounterPath{string(k), fmt.Sprint(v)})
}
+ if name == "codelens" {
+ return counts, deprecatedError("codelenses")
+ }
+ return counts, nil
+
case "staticcheck":
return setBool(&o.Staticcheck, value)
case "local":
- return setString(&o.Local, value)
+ return nil, setString(&o.Local, value)
case "verboseOutput":
return setBool(&o.VerboseOutput, value)
@@ -1128,16 +1172,18 @@
// TODO(hxjiang): deprecate noSemanticString and noSemanticNumber.
case "noSemanticString":
- if err := setBool(&o.NoSemanticString, value); err != nil {
- return err
+ counts, err := setBool(&o.NoSemanticString, value)
+ if err != nil {
+ return nil, err
}
- return &SoftError{fmt.Sprintf("noSemanticString setting is deprecated, use semanticTokenTypes instead (though you can continue to apply them for the time being).")}
+ return counts, &SoftError{"noSemanticString setting is deprecated, use semanticTokenTypes instead (though you can continue to apply them for the time being)."}
case "noSemanticNumber":
- if err := setBool(&o.NoSemanticNumber, value); err != nil {
- return nil
+ counts, err := setBool(&o.NoSemanticNumber, value)
+ if err != nil {
+ return nil, err
}
- return &SoftError{fmt.Sprintf("noSemanticNumber setting is deprecated, use semanticTokenTypes instead (though you can continue to apply them for the time being).")}
+ return counts, &SoftError{"noSemanticNumber setting is deprecated, use semanticTokenTypes instead (though you can continue to apply them for the time being)."}
case "semanticTokenTypes":
return setBoolMap(&o.SemanticTokenTypes, value)
@@ -1157,15 +1203,16 @@
case "templateExtensions":
switch value := value.(type) {
case []any:
- return setStringSlice(&o.TemplateExtensions, value)
+ return nil, setStringSlice(&o.TemplateExtensions, value)
case nil:
o.TemplateExtensions = nil
default:
- return fmt.Errorf("unexpected type %T (want JSON array of string)", value)
+ return nil, fmt.Errorf("unexpected type %T (want JSON array of string)", value)
}
+ return nil, nil
case "diagnosticsDelay":
- return setDuration(&o.DiagnosticsDelay, value)
+ return nil, setDuration(&o.DiagnosticsDelay, value)
case "diagnosticsTrigger":
return setEnum(&o.DiagnosticsTrigger, value,
@@ -1175,11 +1222,8 @@
case "analysisProgressReporting":
return setBool(&o.AnalysisProgressReporting, value)
- case "allowImplicitNetworkAccess":
- return deprecatedError("")
-
case "standaloneTags":
- return setStringSlice(&o.StandaloneTags, value)
+ return nil, setStringSlice(&o.StandaloneTags, value)
case "subdirWatchPatterns":
return setEnum(&o.SubdirWatchPatterns, value,
@@ -1188,7 +1232,7 @@
SubdirWatchPatternsAuto)
case "reportAnalysisProgressAfter":
- return setDuration(&o.ReportAnalysisProgressAfter, value)
+ return nil, setDuration(&o.ReportAnalysisProgressAfter, value)
case "telemetryPrompt":
return setBool(&o.TelemetryPrompt, value)
@@ -1213,50 +1257,54 @@
// renamed
case "experimentalDisabledAnalyses":
- return deprecatedError("analyses")
+ return nil, deprecatedError("analyses")
case "disableDeepCompletion":
- return deprecatedError("deepCompletion")
+ return nil, deprecatedError("deepCompletion")
case "disableFuzzyMatching":
- return deprecatedError("fuzzyMatching")
+ return nil, deprecatedError("fuzzyMatching")
case "wantCompletionDocumentation":
- return deprecatedError("completionDocumentation")
+ return nil, deprecatedError("completionDocumentation")
case "wantUnimportedCompletions":
- return deprecatedError("completeUnimported")
+ return nil, deprecatedError("completeUnimported")
case "fuzzyMatching":
- return deprecatedError("matcher")
+ return nil, deprecatedError("matcher")
case "caseSensitiveCompletion":
- return deprecatedError("matcher")
+ return nil, deprecatedError("matcher")
case "experimentalDiagnosticsDelay":
- return deprecatedError("diagnosticsDelay")
+ return nil, deprecatedError("diagnosticsDelay")
// deprecated
+
+ case "allowImplicitNetworkAccess":
+ return nil, deprecatedError("")
+
case "memoryMode":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "tempModFile":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "experimentalWorkspaceModule":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "experimentalTemplateSupport":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "experimentalWatchedFileDelay":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "experimentalPackageCacheKey":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "allowModfileModifications":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "allExperiments":
// golang/go#65548: this setting is a no-op, but we fail don't report it as
@@ -1265,29 +1313,29 @@
// If, in the future, VS Code stops injecting this, we could theoretically
// report an error here, but it also seems harmless to keep ignoring this
// setting forever.
+ return nil, nil
case "experimentalUseInvalidMetadata":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "newDiff":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "wantSuggestedFixes":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "noIncrementalSync":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "watchFileChanges":
- return deprecatedError("")
+ return nil, deprecatedError("")
case "go-diff":
- return deprecatedError("")
+ return nil, deprecatedError("")
default:
- return fmt.Errorf("unexpected setting")
+ return nil, fmt.Errorf("unexpected setting")
}
- return nil
}
// EnabledSemanticTokenModifiers returns a map of modifiers to boolean.
@@ -1323,11 +1371,6 @@
return e.msg
}
-// softErrorf reports a soft error related to the current option.
-func softErrorf(format string, args ...any) error {
- return &SoftError{fmt.Sprintf(format, args...)}
-}
-
// deprecatedError reports the current setting as deprecated.
// The optional replacement is suggested to the user.
func deprecatedError(replacement string) error {
@@ -1341,13 +1384,13 @@
// setT() and asT() helpers: the setT forms write to the 'dest *T'
// variable only on success, to reduce boilerplate in Option.set.
-func setBool(dest *bool, value any) error {
+func setBool(dest *bool, value any) ([]CounterPath, error) {
b, err := asBool(value)
if err != nil {
- return err
+ return nil, err
}
*dest = b
- return nil
+ return []CounterPath{{fmt.Sprint(b)}}, nil
}
func asBool(value any) (bool, error) {
@@ -1371,13 +1414,17 @@
return nil
}
-func setBoolMap[K ~string](dest *map[K]bool, value any) error {
+func setBoolMap[K ~string](dest *map[K]bool, value any) ([]CounterPath, error) {
m, err := asBoolMap[K](value)
if err != nil {
- return err
+ return nil, err
}
*dest = m
- return nil
+ var counts []CounterPath
+ for k, v := range m {
+ counts = append(counts, CounterPath{string(k), fmt.Sprint(v)})
+ }
+ return counts, nil
}
func asBoolMap[K ~string](value any) (map[K]bool, error) {
@@ -1438,13 +1485,13 @@
return slice, nil
}
-func setEnum[S ~string](dest *S, value any, options ...S) error {
+func setEnum[S ~string](dest *S, value any, options ...S) ([]CounterPath, error) {
enum, err := asEnum(value, options...)
if err != nil {
- return err
+ return nil, err
}
*dest = enum
- return nil
+ return []CounterPath{{string(enum)}}, nil
}
func asEnum[S ~string](value any, options ...S) (S, error) {
diff --git a/gopls/internal/settings/settings_test.go b/gopls/internal/settings/settings_test.go
index 63b4ade..05afa8e 100644
--- a/gopls/internal/settings/settings_test.go
+++ b/gopls/internal/settings/settings_test.go
@@ -206,7 +206,7 @@
for _, test := range tests {
var opts Options
- err := opts.Set(map[string]any{test.name: test.value})
+ _, err := opts.Set(map[string]any{test.name: test.value})
if err != nil {
if !test.wantError {
t.Errorf("Options.set(%q, %v) failed: %v",
diff --git a/gopls/internal/telemetry/cmd/stacks/stacks.go b/gopls/internal/telemetry/cmd/stacks/stacks.go
index 7cb2001..36a675d 100644
--- a/gopls/internal/telemetry/cmd/stacks/stacks.go
+++ b/gopls/internal/telemetry/cmd/stacks/stacks.go
@@ -479,11 +479,20 @@
if err != nil {
return err
}
- // The literal should match complete words. It may match multiple words,
- // if it contains non-word runes like whitespace; but it must match word
- // boundaries at each end.
+ // The end of the literal (usually "symbol",
+ // "pkg.symbol", or "pkg.symbol:+1") must
+ // match a word boundary. However, the start
+ // of the literal need not: an input line such
+ // as "domain.name/dir/pkg.symbol:+1" should
+ // match literal "pkg.symbol", but the slash
+ // is not a word boundary (witness:
+ // https://go.dev/play/p/w-8ev_VUBSq).
+ //
+ // It may match multiple words if it contains
+ // non-word runes like whitespace.
+ //
// The constructed regular expression is always valid.
- literalRegexps[e] = regexp.MustCompile(`\b` + regexp.QuoteMeta(lit) + `\b`)
+ literalRegexps[e] = regexp.MustCompile(regexp.QuoteMeta(lit) + `\b`)
default:
return fmt.Errorf("syntax error (%T)", e)
@@ -1084,6 +1093,8 @@
newStacks []string // new stacks to add to existing issue (comments and IDs)
}
+func (issue *Issue) String() string { return fmt.Sprintf("#%d", issue.Number) }
+
type User struct {
Login string
HTMLURL string `json:"html_url"`
diff --git a/gopls/internal/telemetry/cmd/stacks/stacks_test.go b/gopls/internal/telemetry/cmd/stacks/stacks_test.go
index 452113a..9f798aa 100644
--- a/gopls/internal/telemetry/cmd/stacks/stacks_test.go
+++ b/gopls/internal/telemetry/cmd/stacks/stacks_test.go
@@ -85,13 +85,15 @@
want bool
}{
{`"x"`, `"x"`, true},
- {`"x"`, `"axe"`, false}, // literals match whole words
+ {`"x"`, `"axe"`, false}, // literals must match word ends
+ {`"xe"`, `"axe"`, true},
{`"x"`, "val:x+5", true},
{`"fu+12"`, "x:fu+12,", true},
- {`"fu+12"`, "snafu+12,", false},
+ {`"fu+12"`, "snafu+12,", true}, // literals needn't match word start
{`"fu+12"`, "x:fu+123,", false},
- {`"a.*b"`, "a.*b", true}, // regexp metachars are escaped
- {`"a.*b"`, "axxb", false}, // ditto
+ {`"foo:+12"`, "dir/foo:+12,", true}, // literals needn't match word start
+ {`"a.*b"`, "a.*b", true}, // regexp metachars are escaped
+ {`"a.*b"`, "axxb", false}, // ditto
{`"x"`, `"y"`, false},
{`!"x"`, "x", false},
{`!"x"`, "y", true},
diff --git a/gopls/internal/telemetry/counterpath.go b/gopls/internal/telemetry/counterpath.go
new file mode 100644
index 0000000..e6d9d84
--- /dev/null
+++ b/gopls/internal/telemetry/counterpath.go
@@ -0,0 +1,30 @@
+// 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 telemetry
+
+import "strings"
+
+// A CounterPath represents the components of a telemetry counter name.
+//
+// By convention, counter names follow the format path/to/counter:bucket. The
+// CounterPath holds the '/'-separated components of this path, along with a
+// final element representing the bucket.
+//
+// CounterPaths may be used to build up counters incrementally, such as when a
+// set of observed counters shared a common prefix, to be controlled by the
+// caller.
+type CounterPath []string
+
+// FullName returns the counter name for the receiver.
+func (p CounterPath) FullName() string {
+ if len(p) == 0 {
+ return ""
+ }
+ name := strings.Join([]string(p[:len(p)-1]), "/")
+ if bucket := p[len(p)-1]; bucket != "" {
+ name += ":" + bucket
+ }
+ return name
+}
diff --git a/gopls/internal/telemetry/counterpath_test.go b/gopls/internal/telemetry/counterpath_test.go
new file mode 100644
index 0000000..b6ac747
--- /dev/null
+++ b/gopls/internal/telemetry/counterpath_test.go
@@ -0,0 +1,47 @@
+// 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 telemetry
+
+import (
+ "testing"
+)
+
+// TestCounterPath tests the formatting of various counter paths.
+func TestCounterPath(t *testing.T) {
+ tests := []struct {
+ path CounterPath
+ want string
+ }{
+ {
+ path: CounterPath{},
+ want: "",
+ },
+ {
+ path: CounterPath{"counter"},
+ want: ":counter",
+ },
+ {
+ path: CounterPath{"counter", "bucket"},
+ want: "counter:bucket",
+ },
+ {
+ path: CounterPath{"path", "to", "counter"},
+ want: "path/to:counter",
+ },
+ {
+ path: CounterPath{"multi", "component", "path", "bucket"},
+ want: "multi/component/path:bucket",
+ },
+ {
+ path: CounterPath{"path", ""},
+ want: "path",
+ },
+ }
+ for _, tt := range tests {
+ if got := tt.path.FullName(); got != tt.want {
+ t.Errorf("CounterPath(%v).FullName() = %v, want %v", tt.path, got, tt.want)
+ }
+ }
+}
diff --git a/gopls/internal/telemetry/telemetry_test.go b/gopls/internal/telemetry/telemetry_test.go
index 7aaca41..4c41cc4 100644
--- a/gopls/internal/telemetry/telemetry_test.go
+++ b/gopls/internal/telemetry/telemetry_test.go
@@ -119,6 +119,50 @@
}
}
+func TestSettingTelemetry(t *testing.T) {
+ // counters that should be incremented by each session
+ sessionCounters := []*counter.Counter{
+ counter.New("gopls/setting/diagnosticsDelay"),
+ counter.New("gopls/setting/staticcheck:true"),
+ counter.New("gopls/setting/noSemanticString:true"),
+ counter.New("gopls/setting/analyses/deprecated:false"),
+ }
+
+ initialCounts := make([]uint64, len(sessionCounters))
+ for i, c := range sessionCounters {
+ count, err := countertest.ReadCounter(c)
+ if err != nil {
+ continue // counter db not open, or counter not found
+ }
+ initialCounts[i] = count
+ }
+
+ // Run gopls.
+ WithOptions(
+ Modes(Default),
+ Settings{
+ "staticcheck": true,
+ "analyses": map[string]bool{
+ "deprecated": false,
+ },
+ "diagnosticsDelay": "0s",
+ "noSemanticString": true,
+ },
+ ).Run(t, "", func(_ *testing.T, env *Env) {
+ })
+
+ for i, c := range sessionCounters {
+ count, err := countertest.ReadCounter(c)
+ if err != nil {
+ t.Errorf("ReadCounter(%q) failed: %v", c.Name(), err)
+ continue
+ }
+ if count <= initialCounts[i] {
+ t.Errorf("ReadCounter(%q) = %d, want > %d", c.Name(), count, initialCounts[i])
+ }
+ }
+}
+
func addForwardedCounters(env *Env, names []string, values []int64) {
args, err := command.MarshalArgs(command.AddTelemetryCountersArgs{
Names: names, Values: values,
diff --git a/internal/analysisinternal/addimport_test.go b/internal/analysisinternal/addimport_test.go
index f361bde..da7c7f9 100644
--- a/internal/analysisinternal/addimport_test.go
+++ b/internal/analysisinternal/addimport_test.go
@@ -18,9 +18,12 @@
"github.com/google/go-cmp/cmp"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/internal/analysisinternal"
+ "golang.org/x/tools/internal/testenv"
)
func TestAddImport(t *testing.T) {
+ testenv.NeedsDefaultImporter(t)
+
descr := func(s string) string {
if _, _, line, ok := runtime.Caller(1); ok {
return fmt.Sprintf("L%d %s", line, s)
@@ -185,6 +188,64 @@
foo
}`,
},
+ {
+ descr: descr("dot import unshadowed"),
+ src: `package a
+
+import . "fmt"
+
+func _() {
+ «. fmt»
+}`,
+ want: `package a
+
+import . "fmt"
+
+func _() {
+ .
+}`,
+ },
+ {
+ descr: descr("dot import shadowed"),
+ src: `package a
+
+import . "fmt"
+
+func _(Print fmt.Stringer) {
+ «fmt fmt»
+}`,
+ want: `package a
+
+import "fmt"
+
+import . "fmt"
+
+func _(Print fmt.Stringer) {
+ fmt
+}`,
+ },
+ {
+ descr: descr("add import to group"),
+ src: `package a
+
+import (
+ "io"
+)
+
+func _(io.Reader) {
+ «fmt fmt»
+}`,
+ want: `package a
+
+import (
+ "io"
+ "fmt"
+)
+
+func _(io.Reader) {
+ fmt
+}`,
+ },
} {
t.Run(test.descr, func(t *testing.T) {
// splice marker
@@ -212,13 +273,17 @@
Implicits: make(map[ast.Node]types.Object),
}
conf := &types.Config{
+ // We don't want to fail if there is an error during type checking:
+ // the error may be because we're missing an import, and adding imports
+ // is the whole point of AddImport.
Error: func(err error) { t.Log(err) },
Importer: importer.Default(),
}
conf.Check(f.Name.Name, fset, []*ast.File{f}, info)
// add import
- name, edits := analysisinternal.AddImport(info, f, pos, path, name)
+ // The "Print" argument is only relevant for dot-import tests.
+ name, prefix, edits := analysisinternal.AddImport(info, f, name, path, "Print", pos)
var edit analysis.TextEdit
switch len(edits) {
@@ -229,6 +294,15 @@
t.Fatalf("expected at most one edit, got %d", len(edits))
}
+ // prefix is a simple function of name.
+ wantPrefix := name + "."
+ if name == "." {
+ wantPrefix = ""
+ }
+ if prefix != wantPrefix {
+ t.Errorf("got prefix %q, want %q", prefix, wantPrefix)
+ }
+
// apply patch
start := fset.Position(edit.Pos)
end := fset.Position(edit.End)
diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go
index 7608692..d96d229 100644
--- a/internal/analysisinternal/analysis.go
+++ b/internal/analysisinternal/analysis.go
@@ -211,10 +211,17 @@
// that import is in scope at pos. If so, it returns the name under
// which it was imported and a zero edit. Otherwise, it adds a new
// import of pkgpath, using a name derived from the preferred name,
-// and returns the chosen name along with the edit for the new import.
+// and returns the chosen name, a prefix to be concatenated with member
+// to form a qualified name, and the edit for the new import.
+//
+// In the special case that pkgpath is dot-imported then member, the
+// identifer for which the import is being added, is consulted. If
+// member is not shadowed at pos, AddImport returns (".", "", nil).
+// (AddImport accepts the caller's implicit claim that the imported
+// package declares member.)
//
// It does not mutate its arguments.
-func AddImport(info *types.Info, file *ast.File, pos token.Pos, pkgpath, preferredName string) (name string, newImport []analysis.TextEdit) {
+func AddImport(info *types.Info, file *ast.File, preferredName, pkgpath, member string, pos token.Pos) (name, prefix string, newImport []analysis.TextEdit) {
// Find innermost enclosing lexical block.
scope := info.Scopes[file].Innermost(pos)
if scope == nil {
@@ -226,8 +233,14 @@
for _, spec := range file.Imports {
pkgname := info.PkgNameOf(spec)
if pkgname != nil && pkgname.Imported().Path() == pkgpath {
- if _, obj := scope.LookupParent(pkgname.Name(), pos); obj == pkgname {
- return pkgname.Name(), nil
+ name = pkgname.Name()
+ if name == "." {
+ // The scope of ident must be the file scope.
+ if s, _ := scope.LookupParent(member, pos); s == info.Scopes[file] {
+ return name, "", nil
+ }
+ } else if _, obj := scope.LookupParent(name, pos); obj == pkgname {
+ return name, name + ".", nil
}
}
}
@@ -242,16 +255,16 @@
newName = fmt.Sprintf("%s%d", preferredName, i)
}
- // For now, keep it real simple: create a new import
- // declaration before the first existing declaration (which
- // must exist), including its comments, and let goimports tidy it up.
+ // Create a new import declaration either before the first existing
+ // declaration (which must exist), including its comments; or
+ // inside the declaration, if it is an import group.
//
// Use a renaming import whenever the preferred name is not
// available, or the chosen name does not match the last
// segment of its path.
- newText := fmt.Sprintf("import %q\n\n", pkgpath)
+ newText := fmt.Sprintf("%q", pkgpath)
if newName != preferredName || newName != pathpkg.Base(pkgpath) {
- newText = fmt.Sprintf("import %s %q\n\n", newName, pkgpath)
+ newText = fmt.Sprintf("%s %q", newName, pkgpath)
}
decl0 := file.Decls[0]
var before ast.Node = decl0
@@ -265,9 +278,17 @@
before = decl0.Doc
}
}
- return newName, []analysis.TextEdit{{
- Pos: before.Pos(),
- End: before.Pos(),
+ // If the first decl is an import group, add this new import at the end.
+ if gd, ok := before.(*ast.GenDecl); ok && gd.Tok == token.IMPORT && gd.Rparen.IsValid() {
+ pos = gd.Rparen
+ newText = "\t" + newText + "\n"
+ } else {
+ pos = before.Pos()
+ newText = "import " + newText + "\n\n"
+ }
+ return newName, newName + ".", []analysis.TextEdit{{
+ Pos: pos,
+ End: pos,
NewText: []byte(newText),
}}
}
@@ -436,3 +457,30 @@
return nil
}
+
+// CanImport reports whether one package is allowed to import another.
+//
+// TODO(adonovan): allow customization of the accessibility relation
+// (e.g. for Bazel).
+func CanImport(from, to string) bool {
+ // TODO(adonovan): better segment hygiene.
+ if to == "internal" || strings.HasPrefix(to, "internal/") {
+ // Special case: only std packages may import internal/...
+ // We can't reliably know whether we're in std, so we
+ // use a heuristic on the first segment.
+ first, _, _ := strings.Cut(from, "/")
+ if strings.Contains(first, ".") {
+ return false // example.com/foo ∉ std
+ }
+ if first == "testdata" {
+ return false // testdata/foo ∉ std
+ }
+ }
+ if strings.HasSuffix(to, "/internal") {
+ return strings.HasPrefix(from, to[:len(to)-len("/internal")])
+ }
+ if i := strings.LastIndex(to, "/internal/"); i >= 0 {
+ return strings.HasPrefix(from, to[:i])
+ }
+ return true
+}
diff --git a/internal/analysisinternal/analysis_test.go b/internal/analysisinternal/analysis_test.go
new file mode 100644
index 0000000..0b21876
--- /dev/null
+++ b/internal/analysisinternal/analysis_test.go
@@ -0,0 +1,34 @@
+// 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 analysisinternal
+
+import "testing"
+
+func TestCanImport(t *testing.T) {
+ for _, tt := range []struct {
+ from string
+ to string
+ want bool
+ }{
+ {"fmt", "internal", true},
+ {"fmt", "internal/foo", true},
+ {"a.com/b", "internal", false},
+ {"a.com/b", "xinternal", true},
+ {"a.com/b", "internal/foo", false},
+ {"a.com/b", "xinternal/foo", true},
+ {"a.com/b", "a.com/internal", true},
+ {"a.com/b", "a.com/b/internal", true},
+ {"a.com/b", "a.com/b/internal/foo", true},
+ {"a.com/b", "a.com/c/internal", false},
+ {"a.com/b", "a.com/c/xinternal", true},
+ {"a.com/b", "a.com/c/internal/foo", false},
+ {"a.com/b", "a.com/c/xinternal/foo", true},
+ } {
+ got := CanImport(tt.from, tt.to)
+ if got != tt.want {
+ t.Errorf("CanImport(%q, %q) = %v, want %v", tt.from, tt.to, got, tt.want)
+ }
+ }
+}
diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go
index 2c897c2..96fbb8f 100644
--- a/internal/refactor/inline/inline.go
+++ b/internal/refactor/inline/inline.go
@@ -23,6 +23,7 @@
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/types/typeutil"
"golang.org/x/tools/imports"
+ "golang.org/x/tools/internal/analysisinternal"
internalastutil "golang.org/x/tools/internal/astutil"
"golang.org/x/tools/internal/typeparams"
"golang.org/x/tools/internal/typesinternal"
@@ -331,7 +332,7 @@
for _, imp := range res.newImports {
// Check that the new imports are accessible.
path, _ := strconv.Unquote(imp.spec.Path.Value)
- if !canImport(caller.Types.Path(), path) {
+ if !analysisinternal.CanImport(caller.Types.Path(), path) {
return nil, fmt.Errorf("can't inline function %v as its body refers to inaccessible package %q", callee, path)
}
importDecl.Specs = append(importDecl.Specs, imp.spec)
@@ -3196,30 +3197,6 @@
return *new(T)
}
-// canImport reports whether one package is allowed to import another.
-//
-// TODO(adonovan): allow customization of the accessibility relation
-// (e.g. for Bazel).
-func canImport(from, to string) bool {
- // TODO(adonovan): better segment hygiene.
- if strings.HasPrefix(to, "internal/") {
- // Special case: only std packages may import internal/...
- // We can't reliably know whether we're in std, so we
- // use a heuristic on the first segment.
- first, _, _ := strings.Cut(from, "/")
- if strings.Contains(first, ".") {
- return false // example.com/foo ∉ std
- }
- if first == "testdata" {
- return false // testdata/foo ∉ std
- }
- }
- if i := strings.LastIndex(to, "/internal/"); i >= 0 {
- return strings.HasPrefix(from, to[:i])
- }
- return true
-}
-
// consistentOffsets reports whether the portion of caller.Content
// that corresponds to caller.Call can be parsed as a call expression.
// If not, the client has provided inconsistent information, possibly
diff --git a/internal/stdlib/manifest.go b/internal/stdlib/manifest.go
index 9f0b871..e7d0aee 100644
--- a/internal/stdlib/manifest.go
+++ b/internal/stdlib/manifest.go
@@ -2151,6 +2151,8 @@
{"(Type).String", Method, 0},
{"(Version).GoString", Method, 0},
{"(Version).String", Method, 0},
+ {"(VersionIndex).Index", Method, 24},
+ {"(VersionIndex).IsHidden", Method, 24},
{"ARM_MAGIC_TRAMP_NUMBER", Const, 0},
{"COMPRESS_HIOS", Const, 6},
{"COMPRESS_HIPROC", Const, 6},
@@ -3834,6 +3836,7 @@
{"SymType", Type, 0},
{"SymVis", Type, 0},
{"Symbol", Type, 0},
+ {"Symbol.HasVersion", Field, 24},
{"Symbol.Info", Field, 0},
{"Symbol.Library", Field, 13},
{"Symbol.Name", Field, 0},
@@ -3843,18 +3846,12 @@
{"Symbol.Value", Field, 0},
{"Symbol.Version", Field, 13},
{"Symbol.VersionIndex", Field, 24},
- {"Symbol.VersionScope", Field, 24},
- {"SymbolVersionScope", Type, 24},
{"Type", Type, 0},
{"VER_FLG_BASE", Const, 24},
{"VER_FLG_INFO", Const, 24},
{"VER_FLG_WEAK", Const, 24},
{"Version", Type, 0},
- {"VersionScopeGlobal", Const, 24},
- {"VersionScopeHidden", Const, 24},
- {"VersionScopeLocal", Const, 24},
- {"VersionScopeNone", Const, 24},
- {"VersionScopeSpecific", Const, 24},
+ {"VersionIndex", Type, 24},
},
"debug/gosym": {
{"(*DecodingError).Error", Method, 0},
diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go
index 144f4f8..5c541b7 100644
--- a/internal/testenv/testenv.go
+++ b/internal/testenv/testenv.go
@@ -278,6 +278,16 @@
NeedsTool(t, "go")
}
+// NeedsDefaultImporter skips t if the test uses the default importer,
+// returned by [go/importer.Default].
+func NeedsDefaultImporter(t testing.TB) {
+ t.Helper()
+ // The default importer may call `go list`
+ // (in src/internal/exportdata/exportdata.go:lookupGorootExport),
+ // so check for the go tool.
+ NeedsTool(t, "go")
+}
+
// ExitIfSmallMachine emits a helpful diagnostic and calls os.Exit(0) if the
// current machine is a builder known to have scarce resources.
//
