internal/lsp: show "do not edit" message when user opens generated file
This is probably a better approach than showing an extra diagnostic,
since a user cannot dismiss a diagnostic.
Fixes golang/go#33397
Change-Id: I92b9a00f51a463673993793abfd4cfb99ce69a91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/188766
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index e5e149c..3ef28f1 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -85,7 +85,7 @@
defer func() { <-parseLimit }()
parserMode := parser.AllErrors | parser.ParseComments
if mode == source.ParseHeader {
- parserMode = parser.ImportsOnly
+ parserMode = parser.ImportsOnly | parser.ParseComments
}
ast, err := parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode)
if ast != nil {
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 6f270ee..a1588d4 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -84,6 +84,13 @@
t.Fatal(err)
}
got := results[uri]
+ // A special case to test that there are no diagnostics for a file.
+ if len(want) == 1 && want[0].Source == "no_diagnostics" {
+ if len(got) != 0 {
+ t.Errorf("expected no diagnostics for %s, got %v", uri, got)
+ }
+ continue
+ }
if diff := diffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 121d6c1..51cc41d 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -108,9 +108,10 @@
listErrors, parseErrors, typeErrors []Diagnostic
}
-func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI][]Diagnostic) bool {
+func diagnostics(ctx context.Context, view View, pkg Package, reports map[span.URI][]Diagnostic) bool {
ctx, done := trace.StartSpan(ctx, "source.diagnostics", telemetry.Package.Of(pkg.ID()))
defer done()
+
diagSets := make(map[span.URI]*diagnosticSet)
for _, err := range pkg.GetErrors() {
diag := Diagnostic{
@@ -129,7 +130,7 @@
set.parseErrors = append(set.parseErrors, diag)
case packages.TypeError:
if diag.Span.IsPoint() {
- diag.Span = pointToSpan(ctx, v, diag.Span)
+ diag.Span = pointToSpan(ctx, view, diag.Span)
}
set.typeErrors = append(set.typeErrors, diag)
default:
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index b74ec42..fee66ee 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -62,6 +62,13 @@
t.Fatal(err)
}
got := results[uri]
+ // A special case to test that there are no diagnostics for a file.
+ if len(want) == 1 && want[0].Source == "no_diagnostics" {
+ if len(got) != 0 {
+ t.Errorf("expected no diagnostics for %s, got %v", uri, got)
+ }
+ continue
+ }
if diff := diffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 38ca24e..ad25225 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -5,14 +5,49 @@
package source
import (
+ "context"
"fmt"
"go/ast"
"go/token"
"go/types"
"path/filepath"
+ "regexp"
"strings"
+
+ "golang.org/x/tools/internal/span"
)
+func IsGenerated(ctx context.Context, view View, uri span.URI) bool {
+ f, err := view.GetFile(ctx, uri)
+ if err != nil {
+ return false
+ }
+ ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseHeader)
+ parsed, err := ph.Parse(ctx)
+ if parsed == nil {
+ return false
+ }
+ tok := view.Session().Cache().FileSet().File(parsed.Pos())
+ if tok == nil {
+ return false
+ }
+ for _, commentGroup := range parsed.Comments {
+ for _, comment := range commentGroup.List {
+ if matched := generatedRx.MatchString(comment.Text); matched {
+ // Check if comment is at the beginning of the line in source.
+ if pos := tok.Position(comment.Slash); pos.Column == 1 {
+ return true
+ }
+ }
+ }
+ }
+ return false
+}
+
+// Matches cgo generated comment as well as the proposed standard:
+// https://golang.org/s/generatedcode
+var generatedRx = regexp.MustCompile(`// .*DO NOT EDIT\.?`)
+
func DetectLanguage(langID, filename string) FileKind {
switch langID {
case "go":
diff --git a/internal/lsp/testdata/generated/generated.go b/internal/lsp/testdata/generated/generated.go
new file mode 100644
index 0000000..abd2bee
--- /dev/null
+++ b/internal/lsp/testdata/generated/generated.go
@@ -0,0 +1,7 @@
+package generated
+
+// Code generated by generator.go. DO NOT EDIT.
+
+func _() {
+ var y int //@diag("y", "LSP", "y declared but not used")
+}
diff --git a/internal/lsp/testdata/generated/generator.go b/internal/lsp/testdata/generated/generator.go
new file mode 100644
index 0000000..0369987
--- /dev/null
+++ b/internal/lsp/testdata/generated/generator.go
@@ -0,0 +1,5 @@
+package generated
+
+func _() {
+ var x int //@diag("x", "LSP", "x declared but not used")
+}
diff --git a/internal/lsp/testdata/good/good0.go b/internal/lsp/testdata/good/good0.go
index ceb26c5..98f03b0 100644
--- a/internal/lsp/testdata/good/good0.go
+++ b/internal/lsp/testdata/good/good0.go
@@ -1,4 +1,4 @@
-package good //@diag("package", "", "")
+package good //@diag("package", "no_diagnostics", "")
func stuff() { //@item(good_stuff, "stuff", "func()", "func")
x := 5
diff --git a/internal/lsp/testdata/good/good1.go b/internal/lsp/testdata/good/good1.go
index 95a9250..184fbd2 100644
--- a/internal/lsp/testdata/good/good1.go
+++ b/internal/lsp/testdata/good/good1.go
@@ -1,4 +1,4 @@
-package good //@diag("package", "", "")
+package good //@diag("package", "no_diagnostics", "")
import (
"golang.org/x/tools/internal/lsp/types" //@item(types_import, "types", "\"golang.org/x/tools/internal/lsp/types\"", "package")
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index d649864..73063c8 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -28,7 +28,7 @@
const (
ExpectedCompletionsCount = 144
ExpectedCompletionSnippetCount = 15
- ExpectedDiagnosticsCount = 17
+ ExpectedDiagnosticsCount = 21
ExpectedFormatCount = 6
ExpectedImportCount = 2
ExpectedDefinitionsCount = 38
@@ -420,11 +420,6 @@
if _, ok := data.Diagnostics[spn.URI()]; !ok {
data.Diagnostics[spn.URI()] = []source.Diagnostic{}
}
- // If a file has an empty diagnostic message, return. This allows us to
- // avoid testing diagnostics in files that may have a lot of them.
- if msg == "" {
- return
- }
severity := source.SeverityError
if strings.Contains(string(spn.URI()), "analyzer") {
severity = source.SeverityWarning
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 48979d6..68abec0 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -7,6 +7,7 @@
import (
"bytes"
"context"
+ "fmt"
"golang.org/x/tools/internal/jsonrpc2"
"golang.org/x/tools/internal/lsp/protocol"
@@ -28,8 +29,17 @@
// Open the file.
s.session.DidOpen(ctx, uri, fileKind, text)
- // Run diagnostics on the newly-changed file.
view := s.session.ViewOf(uri)
+
+ // TODO: Ideally, we should be able to specify that a generated file should be opened as read-only.
+ if source.IsGenerated(ctx, view, uri) {
+ s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+ Message: fmt.Sprintf("Do not edit this file! %s is a generated file.", uri.Filename()),
+ Type: protocol.Warning,
+ })
+ }
+
+ // Run diagnostics on the newly-changed file.
go func() {
ctx := view.BackgroundContext()
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
