diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index e5e149c138..3ef28f12dc 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -85,7 +85,7 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa 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 6f270eef87..a1588d46fb 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -84,6 +84,13 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { 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 121d6c11f0..51cc41dbcc 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -108,9 +108,10 @@ type diagnosticSet struct { 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 @@ func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI] 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 b74ec42a5c..fee66ee65d 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -62,6 +62,13 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { 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 38ca24e7b8..ad25225e21 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 0000000000..abd2beef06 --- /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 0000000000..036998787c --- /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 ceb26c5df6..98f03b090c 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 95a9250fe8..184fbd26dd 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 d649864ba7..73063c833f 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -28,7 +28,7 @@ import ( const ( ExpectedCompletionsCount = 144 ExpectedCompletionSnippetCount = 15 - ExpectedDiagnosticsCount = 17 + ExpectedDiagnosticsCount = 21 ExpectedFormatCount = 6 ExpectedImportCount = 2 ExpectedDefinitionsCount = 38 @@ -420,11 +420,6 @@ func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg string) { 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 48979d670f..68abec0eb9 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -7,6 +7,7 @@ package lsp import ( "bytes" "context" + "fmt" "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" @@ -28,8 +29,17 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume // 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")