Merge branch 'golang:master' into master

cmd/file2fuzz/main.go

@@ -13,7 +13,7 @@
 // output to stdout. If any position arguments are provided stdin is ignored
 // and the arguments are assumed to be input files to convert.
 //
-// The -o flag provides an path to write output files to. If only one positional
+// The -o flag provides a path to write output files to. If only one positional
 // argument is specified it may be a file path or an existing directory, if there are
 // multiple inputs specified it must be a directory. If a directory is provided
 // the name of the file will be the SHA-256 hash of its contents.

go/analysis/analysistest/analysistest.go

@@ -167,6 +167,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 		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)
 
@@ -179,6 +180,8 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 					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.
+
 				for _, edit := range fix.TextEdits {
 					start, end := edit.Pos, edit.End
 					if !end.IsValid() {
@@ -275,7 +278,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 				}
 			} else {
 				// all suggested fixes are represented by a single file
-
+				// TODO(adonovan): fix: this makes no sense if len(fixes) > 1.
 				var catchallEdits []diff.Edit
 				for _, edits := range fixes {
 					catchallEdits = append(catchallEdits, edits...)

go/analysis/checker/checker.go

@@ -35,6 +35,7 @@ import (
 	"go/types"
 	"io"
 	"log"
+	"os"
 	"reflect"
 	"sort"
 	"strings"
@@ -55,9 +56,10 @@ type Options struct {
 	SanityCheck bool      // check fact encoding is ok and deterministic
 	FactLog     io.Writer // if non-nil, log each exported fact to it
 
-	// TODO(adonovan): add ReadFile so that an Overlay specified
+	// TODO(adonovan): expose ReadFile so that an Overlay specified
 	// in the [packages.Config] can be communicated via
 	// Pass.ReadFile to each Analyzer.
+	readFile analysisinternal.ReadFileFunc
 }
 
 // Graph holds the results of a round of analysis, including the graph
@@ -335,16 +337,26 @@ func (act *Action) execOnce() {
 		TypeErrors:   act.Package.TypeErrors,
 		Module:       module,
 
-		ResultOf:          inputs,
-		Report:            func(d analysis.Diagnostic) { act.Diagnostics = append(act.Diagnostics, d) },
+		ResultOf: inputs,
+		Report: func(d analysis.Diagnostic) {
+			// Assert that SuggestedFixes are well formed.
+			if err := analysisinternal.ValidateFixes(act.Package.Fset, act.Analyzer, d.SuggestedFixes); err != nil {
+				panic(err)
+			}
+			act.Diagnostics = append(act.Diagnostics, d)
+		},
 		ImportObjectFact:  act.ObjectFact,
 		ExportObjectFact:  act.exportObjectFact,
 		ImportPackageFact: act.PackageFact,
 		ExportPackageFact: act.exportPackageFact,
 		AllObjectFacts:    act.AllObjectFacts,
 		AllPackageFacts:   act.AllPackageFacts,
 	}
-	pass.ReadFile = analysisinternal.MakeReadFile(pass)
+	readFile := os.ReadFile
+	if act.opts.readFile != nil {
+		readFile = act.opts.readFile
+	}
+	pass.ReadFile = analysisinternal.CheckedReadFile(pass, readFile)
 	act.pass = pass
 
 	act.Result, act.Err = func() (any, error) {

go/analysis/diagnostic.go

@@ -65,7 +65,9 @@ type RelatedInformation struct {
 // user can choose to apply to their code. Usually the SuggestedFix is
 // meant to fix the issue flagged by the diagnostic.
 //
-// The TextEdits must not overlap, nor contain edits for other packages.
+// The TextEdits must not overlap, nor contain edits for other
+// packages. Edits need not be totally ordered, but the order
+// determines how insertions at the same point will be applied.
 type SuggestedFix struct {
 	// A verb phrase describing the fix, to be shown to
 	// a user trying to decide whether to accept it.

go/analysis/internal/checker/checker.go

@@ -20,6 +20,7 @@ import (
 	"go/token"
 	"io"
 	"io/ioutil"
+
 	"log"
 	"os"
 	"runtime"
@@ -139,6 +140,9 @@ func Run(args []string, analyzers []*analysis.Analyzer) int {
 		return 1
 	}
 
+	// 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.

go/analysis/internal/checker/checker_test.go

@@ -25,6 +25,7 @@ import (
 
 func TestApplyFixes(t *testing.T) {
 	testenv.NeedsGoPackages(t)
+	testenv.RedirectStderr(t) // associated checker.Run output with this test
 
 	files := map[string]string{
 		"rename/test.go": `package rename
@@ -83,6 +84,7 @@ var otherAnalyzer = &analysis.Analyzer{ // like analyzer but with a different Na
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
+	// TODO(adonovan): replace this entangled test with something completely data-driven.
 	const (
 		from      = "bar"
 		to        = "baz"
@@ -108,16 +110,46 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			}
 			switch pass.Pkg.Name() {
 			case conflict:
-				edits = append(edits, []analysis.TextEdit{
-					{Pos: ident.Pos() - 1, End: ident.End(), NewText: []byte(to)},
-					{Pos: ident.Pos(), End: ident.End() - 1, NewText: []byte(to)},
-					{Pos: ident.Pos(), End: ident.End(), NewText: []byte("lorem ipsum")},
-				}...)
+				// Conflicting edits are legal, so long as they appear in different fixes.
+				pass.Report(analysis.Diagnostic{
+					Pos:     ident.Pos(),
+					End:     ident.End(),
+					Message: msg,
+					SuggestedFixes: []analysis.SuggestedFix{{
+						Message: msg, TextEdits: []analysis.TextEdit{
+							{Pos: ident.Pos() - 1, End: ident.End(), NewText: []byte(to)},
+						},
+					}},
+				})
+				pass.Report(analysis.Diagnostic{
+					Pos:     ident.Pos(),
+					End:     ident.End(),
+					Message: msg,
+					SuggestedFixes: []analysis.SuggestedFix{{
+						Message: msg, TextEdits: []analysis.TextEdit{
+							{Pos: ident.Pos(), End: ident.End() - 1, NewText: []byte(to)},
+						},
+					}},
+				})
+				pass.Report(analysis.Diagnostic{
+					Pos:     ident.Pos(),
+					End:     ident.End(),
+					Message: msg,
+					SuggestedFixes: []analysis.SuggestedFix{{
+						Message: msg, TextEdits: []analysis.TextEdit{
+							{Pos: ident.Pos(), End: ident.End(), NewText: []byte("lorem ipsum")},
+						},
+					}},
+				})
+				return
+
 			case duplicate:
+				// Duplicate (non-insertion) edits are disallowed,
+				// so this is a buggy analyzer, and validatedFixes should reject it.
 				edits = append(edits, edits...)
 			case other:
 				if pass.Analyzer.Name == other {
-					edits[0].Pos = edits[0].Pos + 1 // shift by one to mismatch analyzer and other
+					edits[0].Pos++ // shift by one to mismatch analyzer and other
 				}
 			}
 			pass.Report(analysis.Diagnostic{
@@ -133,6 +165,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 func TestRunDespiteErrors(t *testing.T) {
 	testenv.NeedsGoPackages(t)
+	testenv.RedirectStderr(t) // associate checker.Run output with this test
 
 	files := map[string]string{
 		"rderr/test.go": `package rderr
@@ -360,4 +393,7 @@ hello from other
 	if !ran {
 		t.Error("analyzer did not run")
 	}
+
+	// TODO(adonovan): test that fixes are applied to the
+	// pass.ReadFile virtual file tree.
 }