gopls/internal/analysis/embeddirective: call AddImport directly
This CL causes the embeddirective analyzer to compute the necessary
edits itself, using AddImport, rather than using gopls' hacky
lazy-edits mechanism, which requires gopls's ApplyFix command
to compute the edits later.
Also, support blank imports in AddImport.
Change-Id: Ie51a316503933fbea05ed418ed251f7af9cc624f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/705475
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/gopls/internal/analysis/embeddirective/embeddirective.go b/gopls/internal/analysis/embeddirective/embeddirective.go
index 7590cba..58b4a36 100644
--- a/gopls/internal/analysis/embeddirective/embeddirective.go
+++ b/gopls/internal/analysis/embeddirective/embeddirective.go
@@ -26,8 +26,6 @@
URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/embeddirective",
}
-const FixCategory = "addembedimport" // recognized by gopls ApplyFix
-
func run(pass *analysis.Pass) (any, error) {
for _, f := range pass.Files {
comments := embedDirectiveComments(f)
@@ -47,16 +45,19 @@
pos, end := c.Pos(), c.Pos()+token.Pos(len("//go:embed"))
if !hasEmbedImport {
- pass.Report(analysis.Diagnostic{
- Pos: pos,
- End: end,
- Message: `must import "embed" when using go:embed directives`,
- Category: FixCategory,
- SuggestedFixes: []analysis.SuggestedFix{{
- Message: `Add missing "embed" import`,
- // No TextEdits => computed by a gopls command.
- }},
- })
+ // Add blank import of "embed".
+ _, _, edits := analysisinternal.AddImport(pass.TypesInfo, f, "_", "embed", "", c.Pos())
+ if len(edits) > 0 {
+ pass.Report(analysis.Diagnostic{
+ Pos: pos,
+ End: end,
+ Message: `must import "embed" when using go:embed directives`,
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: `Add missing "embed" import`,
+ TextEdits: edits,
+ }},
+ })
+ }
}
var msg string
diff --git a/gopls/internal/analysis/embeddirective/testdata/src/a/import_missing.go.golden b/gopls/internal/analysis/embeddirective/testdata/src/a/import_missing.go.golden
new file mode 100644
index 0000000..c2609cb
--- /dev/null
+++ b/gopls/internal/analysis/embeddirective/testdata/src/a/import_missing.go.golden
@@ -0,0 +1,18 @@
+// Copyright 2022 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 a
+
+import (
+ _ "embed"
+ "fmt"
+)
+
+//go:embed embedtext // want "must import \"embed\" when using go:embed directives"
+var s string
+
+// This is main function
+func main() {
+ fmt.Println(s)
+}
diff --git a/gopls/internal/golang/fix.go b/gopls/internal/golang/fix.go
index 0308c38..84ff1a9 100644
--- a/gopls/internal/golang/fix.go
+++ b/gopls/internal/golang/fix.go
@@ -10,7 +10,6 @@
"go/token"
"golang.org/x/tools/go/analysis"
- "golang.org/x/tools/gopls/internal/analysis/embeddirective"
"golang.org/x/tools/gopls/internal/analysis/fillstruct"
"golang.org/x/tools/gopls/internal/analysis/unusedparams"
"golang.org/x/tools/gopls/internal/cache"
@@ -18,7 +17,6 @@
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
- "golang.org/x/tools/internal/imports"
)
// A fixer is a function that suggests a fix for a diagnostic produced
@@ -94,8 +92,7 @@
fixers := map[string]fixer{
// Fixes for analyzer-provided diagnostics.
// These match the Diagnostic.Category.
- embeddirective.FixCategory: addEmbedImport,
- fillstruct.FixCategory: singleFile(fillstruct.SuggestedFix),
+ fillstruct.FixCategory: singleFile(fillstruct.SuggestedFix),
// Ad-hoc fixers: these are used when the command is
// constructed directly by logic in server/code_action.
@@ -183,36 +180,3 @@
}
return changes, nil
}
-
-// addEmbedImport adds a missing embed "embed" import with blank name.
-func addEmbedImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, _, _ token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
- // Like golang.AddImport, but with _ as Name and using our pgf.
- protoEdits, err := ComputeImportFixEdits(snapshot.Options().Local, pgf.Src, &imports.ImportFix{
- StmtInfo: imports.ImportInfo{
- ImportPath: "embed",
- Name: "_",
- },
- FixType: imports.AddImport,
- })
- if err != nil {
- return nil, nil, fmt.Errorf("compute edits: %w", err)
- }
-
- var edits []analysis.TextEdit
- for _, e := range protoEdits {
- start, end, err := pgf.RangePos(e.Range)
- if err != nil {
- return nil, nil, err // e.g. invalid range
- }
- edits = append(edits, analysis.TextEdit{
- Pos: start,
- End: end,
- NewText: []byte(e.NewText),
- })
- }
-
- return pkg.FileSet(), &analysis.SuggestedFix{
- Message: "Add embed import",
- TextEdits: edits,
- }, nil
-}
diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go
index bc7f998..cea89d3 100644
--- a/internal/analysisinternal/analysis.go
+++ b/internal/analysisinternal/analysis.go
@@ -193,18 +193,23 @@
return fmt.Errorf("Pass.ReadFile: %s is not among OtherFiles, IgnoredFiles, or names of Files", filename)
}
-// AddImport checks whether this file already imports pkgpath and
-// 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, a prefix to be concatenated with member
-// to form a qualified name, and the edit for the new import.
+// AddImport checks whether this file already imports pkgpath and that
+// the import is in scope at pos. If so, it returns the name under
+// which it was imported and no edits. Otherwise, it adds a new import
+// of pkgpath, using a name derived from the preferred name, 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
-// identifier 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.)
+// The member argument indicates the name of the desired symbol within
+// the imported package. This is needed in the case when the existing
+// import is a dot import, because then it is possible that the
+// desired symbol is shadowed by other declarations in the current
+// package. If member is not shadowed at pos, AddImport returns (".",
+// "", nil). (AddImport accepts the caller's implicit claim that the
+// imported package declares member.)
+//
+// Use a preferredName of "_" to request a blank import;
+// member is ignored in this case.
//
// It does not mutate its arguments.
func AddImport(info *types.Info, file *ast.File, preferredName, pkgpath, member string, pos token.Pos) (name, prefix string, newImport []analysis.TextEdit) {
@@ -220,6 +225,10 @@
pkgname := info.PkgNameOf(spec)
if pkgname != nil && pkgname.Imported().Path() == pkgpath {
name = pkgname.Name()
+ if preferredName == "_" {
+ // Request for blank import; any existing import will do.
+ return name, "", nil
+ }
if name == "." {
// The scope of ident must be the file scope.
if s, _ := scope.LookupParent(member, pos); s == info.Scopes[file] {
@@ -232,8 +241,12 @@
}
// We must add a new import.
+
// Ensure we have a fresh name.
- newName := FreshName(scope, pos, preferredName)
+ newName := preferredName
+ if preferredName != "_" {
+ newName = FreshName(scope, pos, preferredName)
+ }
// Create a new import declaration either before the first existing
// declaration (which must exist), including its comments; or
@@ -246,6 +259,7 @@
if newName != preferredName || newName != pathpkg.Base(pkgpath) {
newText = fmt.Sprintf("%s %q", newName, pkgpath)
}
+
decl0 := file.Decls[0]
var before ast.Node = decl0
switch decl0 := decl0.(type) {
