internal/lsp: don't offer suggested fixes for generated files

As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.

Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.

Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

internal/lsp/code_action.go

@@ -67,6 +67,11 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 			})
 		}
 	case source.Go:
+		// Don't suggest fixes for generated files, since they are generally
+		// not useful and some editors may apply them automatically on save.
+		if source.IsGenerated(ctx, snapshot, uri) {
+			return nil, nil
+		}
 		diagnostics := params.Context.Diagnostics
 
 		// First, process any missing imports and pair them with the

internal/lsp/fake/editor.go

@@ -502,16 +502,16 @@ func (e *Editor) OrganizeImports(ctx context.Context, path string) error {
 
 // ApplyQuickFixes requests and performs the quickfix codeAction.
 func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, diagnostics []protocol.Diagnostic) error {
-	return e.codeAction(ctx, path, diagnostics, protocol.QuickFix)
+	return e.codeAction(ctx, path, diagnostics, protocol.QuickFix, protocol.SourceFixAll)
 }
 
-func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []protocol.Diagnostic, only protocol.CodeActionKind) error {
+func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) error {
 	if e.server == nil {
 		return nil
 	}
 	params := &protocol.CodeActionParams{}
 	params.TextDocument.URI = e.ws.URI(path)
-	params.Context.Only = []protocol.CodeActionKind{only}
+	params.Context.Only = only
 	if diagnostics != nil {
 		params.Context.Diagnostics = diagnostics
 	}
@@ -522,17 +522,25 @@ func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []prot
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	for _, action := range actions {
-		if action.Kind == only {
-			for _, change := range action.Edit.DocumentChanges {
-				path := e.ws.URIToPath(change.TextDocument.URI)
-				if float64(e.buffers[path].version) != change.TextDocument.Version {
-					// Skip edits for old versions.
-					continue
-				}
-				edits := convertEdits(change.Edits)
-				if err := e.editBufferLocked(ctx, path, edits); err != nil {
-					return fmt.Errorf("editing buffer %q: %v", path, err)
-				}
+		var match bool
+		for _, o := range only {
+			if action.Kind == o {
+				match = true
+				break
+			}
+		}
+		if !match {
+			continue
+		}
+		for _, change := range action.Edit.DocumentChanges {
+			path := e.ws.URIToPath(change.TextDocument.URI)
+			if float64(e.buffers[path].version) != change.TextDocument.Version {
+				// Skip edits for old versions.
+				continue
+			}
+			edits := convertEdits(change.Edits)
+			if err := e.editBufferLocked(ctx, path, edits); err != nil {
+				return fmt.Errorf("editing buffer %q: %v", path, err)
 			}
 		}
 	}

internal/lsp/mod/diagnostics.go

@@ -44,10 +44,9 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File
 	}
 	for _, e := range parseErrors {
 		diag := &source.Diagnostic{
-			Message:        e.Message,
-			Range:          e.Range,
-			SuggestedFixes: e.SuggestedFixes,
-			Source:         e.Category,
+			Message: e.Message,
+			Range:   e.Range,
+			Source:  e.Category,
 		}
 		if e.Category == "syntax" {
 			diag.Severity = protocol.SeverityError

internal/lsp/regtest/diagnostics_test.go

@@ -10,6 +10,7 @@ import (
 	"golang.org/x/tools/internal/lsp"
 	"golang.org/x/tools/internal/lsp/fake"
 	"golang.org/x/tools/internal/lsp/protocol"
+	"golang.org/x/tools/internal/lsp/tests"
 )
 
 // Use mod.com for all go.mod files due to golang/go#35230.
@@ -403,3 +404,39 @@ var X = 0
 		env.Await(EmptyDiagnostics("main.go"))
 	}, WithEnv("GOFLAGS=-tags=foo"))
 }
+
+func TestNoSuggestedFixesForGeneratedFiles_Issue38467(t *testing.T) {
+	const generated = `
+-- go.mod --
+module mod.com
+
+-- main.go --
+package main
+
+// Code generated by generator.go. DO NOT EDIT.
+
+func _() {
+	for i, _ := range []string{} {
+		_ = i
+	}
+}
+`
+	runner.Run(t, generated, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+		original := env.ReadWorkspaceFile("main.go")
+		metBy := env.Await(
+			DiagnosticAt("main.go", 5, 8),
+		)
+		d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
+		if !ok {
+			t.Fatalf("unexpected met by result %v (%T)", metBy, metBy)
+		}
+		// Apply fixes and save the buffer.
+		env.ApplyQuickFixes("main.go", d.Diagnostics)
+		env.SaveBuffer("main.go")
+		fixed := env.ReadWorkspaceFile("main.go")
+		if original != fixed {
+			t.Fatalf("generated file was changed by quick fixes:\n%s", tests.Diff(original, fixed))
+		}
+	})
+}

internal/lsp/source/diagnostics.go

@@ -27,8 +27,7 @@ type Diagnostic struct {
 	Severity protocol.DiagnosticSeverity
 	Tags     []protocol.DiagnosticTag
 
-	SuggestedFixes []SuggestedFix
-	Related        []RelatedInformation
+	Related []RelatedInformation
 }
 
 type SuggestedFix struct {
@@ -295,13 +294,12 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][
 			tags = append(tags, protocol.Unnecessary)
 		}
 		if err := addReports(snapshot, reports, e.URI, &Diagnostic{
-			Range:          e.Range,
-			Message:        e.Message,
-			Source:         e.Category,
-			Severity:       protocol.SeverityWarning,
-			Tags:           tags,
-			SuggestedFixes: e.SuggestedFixes,
-			Related:        e.Related,
+			Range:    e.Range,
+			Message:  e.Message,
+			Source:   e.Category,
+			Severity: protocol.SeverityWarning,
+			Tags:     tags,
+			Related:  e.Related,
 		}); err != nil {
 			return err
 		}
@@ -344,7 +342,6 @@ func addReports(snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, uri s
 				if d1.Message != d2.Message {
 					continue
 				}
-				reports[identity][i].SuggestedFixes = append(reports[identity][i].SuggestedFixes, d1.SuggestedFixes...)
 				reports[identity][i].Tags = append(reports[identity][i].Tags, d1.Tags...)
 			}
 			return nil