all: merge master (68e4702) into gopls-release-branch.0.17

Also add back the x/tools replace directive.

For golang/go#70301

Merge List:

+ 2024-11-26 68e4702 go/analysis/passes/printf: add missing call to Func.Origin
+ 2024-11-26 30a3bd9 gopls/internal/golang: refactor.extract.variable: allow all exprs
+ 2024-11-26 0edd1ab gopls/internal/cache/methodsets: refine crash for missing object path
+ 2024-11-25 07a58bc gopls/internal/golang: refine crash golang/go#70553
+ 2024-11-25 c622026 completions/inference: infer polymorphic types in completions
+ 2024-11-25 dcfb0b6 gopls/internal/golang: change signature via renaming 'func'
+ 2024-11-25 bfcbc1b internal/refactor/inline: avoid unnecessary desugaring in selectors
+ 2024-11-25 b93a72a internal/refactor/inline: fix spurious caller mutation
+ 2024-11-25 41f04a0 internal/refactor/inline: fix comment movement due to added imports
+ 2024-11-25 0841661 internal/refactor/inline: avoid unnecessary interface conversions
+ 2024-11-25 68b67b4 gopls/internal/golang: support parameter movement refactorings
+ 2024-11-22 51e54e8 gopls/doc/features: enable and document source.addTest code action
+ 2024-11-22 458067f gopls/internal/golang: improve test package name selection for new file
+ 2024-11-22 68caf84 gopls/internal/golang: don't lose ... when split/joining variadics
+ 2024-11-21 1e0d4ee go/analysis/checker: disable Example on wasm
+ 2024-11-21 8b6e84b gopls/internal/crash: don't crash in xrefs on out of bound nodes
+ 2024-11-21 936a401 gopls/internal/golang: preserve copyright and build constraint
+ 2024-11-21 1ffc3a1 gopls/internal/test/marker: add defloc, to bind positions by definition
+ 2024-11-21 442d6be gopls/internal/test/marker: document named parameters
+ 2024-11-21 ae39b13 go/analysis/checker: a go/packages-based driver library
+ 2024-11-21 c3a6283 go/packages: undeprecate Load* style flags
+ 2024-11-20 9dff42e gopls/internal/golang/extract: preserve comments in extracted block
+ 2024-11-20 8c3ba8c internal/refactor: undo variadic elimination during substitution
+ 2024-11-20 3b0b264 internal/refactor: handle qualified names in inlined assignments
+ 2024-11-20 9311800 gopls/internal/test/marker: ignore diags in fixedbugs/issue59944.txt
+ 2024-11-20 c1aa79d gopls/internal/golang: fix gopls hover doc link
+ 2024-11-20 e751756 internal/analysisinternal: unify zero value function to typesinternal
+ 2024-11-20 a287481 internal/imports: test Source for go mod cache
+ 2024-11-19 9387a39 gopls/doc/contributing.md: update expectations
+ 2024-11-19 e08fcf7 gopls/internal/analysis/undeclaredname: merge into CodeAction
+ 2024-11-18 0c01408 internal/refactor/inline: avoid binding decl for name used by other args
+ 2024-11-18 63e03c3 gopls/internal/test/marker: generalize codeaction with named args
+ 2024-11-18 acc2a74 go/analysis/passes/copylock: enable unfortunate tests
+ 2024-11-18 39cb6f0 internal/facts: use alias type parameters and arguments during imports
+ 2024-11-18 9b9871d go/analysis/passes/copylock: test for noCopy for sync Map, Mutex, Once
+ 2024-11-18 a54bd37 gopls/internal/golang: don't try to inline dynamic calls
+ 2024-11-18 52eb446 internal/imports: adjust TestStdlibSelfImports pkg
+ 2024-11-18 b8ff201 gopls/internal/cache: refine bug reports for inconsistent dep view
+ 2024-11-18 e59fd36 go/ssa: use ZeroString unconditionally
+ 2024-11-18 60bc93d gopls/internal/cache: fix handling of cgo standalone files
+ 2024-11-18 ed19fc7 gopls/internal/test: synchronize notifications during commands
+ 2024-11-15 254baba gopls/internal/cache: failure to extract diagnostic fixes is an error
+ 2024-11-15 56ec111 gopls/internal/server: remove spurious Async in legacy RunGoVulncheck
+ 2024-11-14 b1c39aa gopls/internal/cache: use a named bool type for allowNetwork
+ 2024-11-14 c043599 gopls/internal/protocol: add DocumentURI.DirPath
+ 2024-11-14 29f4edb gopls/internal/cache: simplify usage of snapshot.GoCommandInvocation
+ 2024-11-14 3c20e3f gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes
+ 2024-11-14 221e94d gopls/internal/cache: id command-line-arguments packages using GoFiles
+ 2024-11-14 84e9c33 gopls/internal/golang: more idiomatic result naming in extract
+ 2024-11-14 8bb5da3 gopls/internal/golang: special handling for input context.Context
+ 2024-11-14 b4332e0 gopls/internal/golang, go/ssa: remove unnamed input parameter
+ 2024-11-14 a8d0fa5 go/packages: call testenv.NeedsGoPackages for TestDirAndForTest
+ 2024-11-14 47a5f7d gopls/internal/golang: fix bad slice append in function extraction

Change-Id: I2abfc8fd925da3b4ee5964e1cf32bb7fada0e688

go/analysis/analysis.go

@@ -50,7 +50,7 @@ type Analyzer struct {
 	// RunDespiteErrors allows the driver to invoke
 	// the Run method of this analyzer even on a
 	// package that contains parse or type errors.
-	// The Pass.TypeErrors field may consequently be non-empty.
+	// The [Pass.TypeErrors] field may consequently be non-empty.
 	RunDespiteErrors bool
 
 	// Requires is a set of analyzers that must run successfully

go/analysis/analysistest/analysistest.go

@@ -23,7 +23,8 @@ import (
 	"text/scanner"
 
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/internal/checker"
+	"golang.org/x/tools/go/analysis/checker"
+	"golang.org/x/tools/go/analysis/internal"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/diff"
 	"golang.org/x/tools/internal/testenv"
@@ -137,7 +138,7 @@ type Testing interface {
 // analyzers that offer alternative fixes are advised to put each fix
 // in a separate .go file in the testdata.
 func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
-	r := Run(t, dir, a, patterns...)
+	results := Run(t, dir, a, patterns...)
 
 	// If the immediate caller of RunWithSuggestedFixes is in
 	// x/tools, we apply stricter checks as required by gopls.
@@ -162,7 +163,9 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 	// Validating the results separately means as long as the two analyses
 	// don't produce conflicting suggestions for a single file, everything
 	// should match up.
-	for _, act := range r {
+	for _, result := range results {
+		act := result.Action
+
 		// file -> message -> edits
 		fileEdits := make(map[*token.File]map[string][]diff.Edit)
 		fileContents := make(map[*token.File][]byte)
@@ -185,14 +188,14 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 					if start > end {
 						t.Errorf(
 							"diagnostic for analysis %v contains Suggested Fix with malformed edit: pos (%v) > end (%v)",
-							act.Pass.Analyzer.Name, start, end)
+							act.Analyzer.Name, start, end)
 						continue
 					}
-					file, endfile := act.Pass.Fset.File(start), act.Pass.Fset.File(end)
+					file, endfile := act.Package.Fset.File(start), act.Package.Fset.File(end)
 					if file == nil || endfile == nil || file != endfile {
 						t.Errorf(
 							"diagnostic for analysis %v contains Suggested Fix with malformed spanning files %v and %v",
-							act.Pass.Analyzer.Name, file.Name(), endfile.Name())
+							act.Analyzer.Name, file.Name(), endfile.Name())
 						continue
 					}
 					if _, ok := fileContents[file]; !ok {
@@ -275,7 +278,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 			}
 		}
 	}
-	return r
+	return results
 }
 
 // applyDiffsAndCompare applies edits to src and compares the results against
@@ -355,24 +358,76 @@ func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Res
 		return nil
 	}
 
-	if err := analysis.Validate([]*analysis.Analyzer{a}); err != nil {
-		t.Errorf("Validate: %v", err)
+	// Print parse and type errors to the test log.
+	// (Do not print them to stderr, which would pollute
+	// the log in cases where the tests pass.)
+	if t, ok := t.(testing.TB); ok && !a.RunDespiteErrors {
+		packages.Visit(pkgs, nil, func(pkg *packages.Package) {
+			for _, err := range pkg.Errors {
+				t.Log(err)
+			}
+		})
+	}
+
+	res, err := checker.Analyze([]*analysis.Analyzer{a}, pkgs, nil)
+	if err != nil {
+		t.Errorf("Analyze: %v", err)
 		return nil
 	}
 
-	results := checker.TestAnalyzer(a, pkgs)
-	for _, result := range results {
-		if result.Err != nil {
-			t.Errorf("error analyzing %s: %v", result.Pass, result.Err)
+	var results []*Result
+	for _, act := range res.Roots {
+		if act.Err != nil {
+			t.Errorf("error analyzing %s: %v", act, act.Err)
 		} else {
-			check(t, dir, result.Pass, result.Diagnostics, result.Facts)
+			check(t, dir, act)
 		}
+
+		// Compute legacy map of facts relating to this package.
+		facts := make(map[types.Object][]analysis.Fact)
+		for _, objFact := range act.AllObjectFacts() {
+			if obj := objFact.Object; obj.Pkg() == act.Package.Types {
+				facts[obj] = append(facts[obj], objFact.Fact)
+			}
+		}
+		for _, pkgFact := range act.AllPackageFacts() {
+			if pkgFact.Package == act.Package.Types {
+				facts[nil] = append(facts[nil], pkgFact.Fact)
+			}
+		}
+
+		// Construct the legacy result.
+		results = append(results, &Result{
+			Pass:        internal.Pass(act),
+			Diagnostics: act.Diagnostics,
+			Facts:       facts,
+			Result:      act.Result,
+			Err:         act.Err,
+			Action:      act,
+		})
 	}
 	return results
 }
 
 // A Result holds the result of applying an analyzer to a package.
-type Result = checker.TestAnalyzerResult
+//
+// Facts contains only facts associated with the package and its objects.
+//
+// This internal type was inadvertently and regrettably exposed
+// through a public type alias. It is essentially redundant with
+// [checker.Action], but must be retained for compatibility. Clients may
+// access the public fields of the Pass but must not invoke any of
+// its "verbs", since the pass is already complete.
+type Result struct {
+	Action *checker.Action
+
+	// legacy fields
+	Facts       map[types.Object][]analysis.Fact // nil key => package fact
+	Pass        *analysis.Pass
+	Diagnostics []analysis.Diagnostic // see Action.Diagnostics
+	Result      any                   // see Action.Result
+	Err         error                 // see Action.Err
+}
 
 // loadPackages uses go/packages to load a specified packages (from source, with
 // dependencies) from dir, which is the root of a GOPATH-style project tree.
@@ -421,16 +476,6 @@ func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*pack
 		}
 	}
 
-	// Do NOT print errors if the analyzer will continue running.
-	// It is incredibly confusing for tests to be printing to stderr
-	// willy-nilly instead of their test logs, especially when the
-	// errors are expected and are going to be fixed.
-	if !a.RunDespiteErrors {
-		if packages.PrintErrors(pkgs) > 0 {
-			return nil, fmt.Errorf("there were package loading errors (and RunDespiteErrors is false)")
-		}
-	}
-
 	if len(pkgs) == 0 {
 		return nil, fmt.Errorf("no packages matched %s", patterns)
 	}
@@ -441,7 +486,7 @@ func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*pack
 // been run, and verifies that all reported diagnostics and facts match
 // specified by the contents of "// want ..." comments in the package's
 // source files, which must have been parsed with comments enabled.
-func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis.Diagnostic, facts map[types.Object][]analysis.Fact) {
+func check(t Testing, gopath string, act *checker.Action) {
 	type key struct {
 		file string
 		line int
@@ -468,7 +513,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis
 	}
 
 	// Extract 'want' comments from parsed Go files.
-	for _, f := range pass.Files {
+	for _, f := range act.Package.Syntax {
 		for _, cgroup := range f.Comments {
 			for _, c := range cgroup.List {
 
@@ -491,7 +536,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis
 				// once outside the loop, but it's
 				// incorrect because it can change due
 				// to //line directives.
-				posn := pass.Fset.Position(c.Pos())
+				posn := act.Package.Fset.Position(c.Pos())
 				filename := sanitize(gopath, posn.Filename)
 				processComment(filename, posn.Line, text)
 			}
@@ -500,7 +545,17 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis
 
 	// Extract 'want' comments from non-Go files.
 	// TODO(adonovan): we may need to handle //line directives.
-	for _, filename := range pass.OtherFiles {
+	files := act.Package.OtherFiles
+
+	// Hack: these two analyzers need to extract expectations from
+	// all configurations, so include the files are are usually
+	// ignored. (This was previously a hack in the respective
+	// analyzers' tests.)
+	if act.Analyzer.Name == "buildtag" || act.Analyzer.Name == "directive" {
+		files = append(files[:len(files):len(files)], act.Package.IgnoredFiles...)
+	}
+
+	for _, filename := range files {
 		data, err := os.ReadFile(filename)
 		if err != nil {
 			t.Errorf("can't read '// want' comments from %s: %v", filename, err)
@@ -553,45 +608,38 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis
 	}
 
 	// Check the diagnostics match expectations.
-	for _, f := range diagnostics {
+	for _, f := range act.Diagnostics {
 		// TODO(matloob): Support ranges in analysistest.
-		posn := pass.Fset.Position(f.Pos)
+		posn := act.Package.Fset.Position(f.Pos)
 		checkMessage(posn, "diagnostic", "", f.Message)
 	}
 
 	// Check the facts match expectations.
-	// Report errors in lexical order for determinism.
+	// We check only facts relating to the current package.
+	//
+	// We report errors in lexical order for determinism.
 	// (It's only deterministic within each file, not across files,
 	// because go/packages does not guarantee file.Pos is ascending
 	// across the files of a single compilation unit.)
-	var objects []types.Object
-	for obj := range facts {
-		objects = append(objects, obj)
-	}
-	sort.Slice(objects, func(i, j int) bool {
-		// Package facts compare less than object facts.
-		ip, jp := objects[i] == nil, objects[j] == nil // whether i, j is a package fact
-		if ip != jp {
-			return ip && !jp
-		}
-		return objects[i].Pos() < objects[j].Pos()
-	})
-	for _, obj := range objects {
-		var posn token.Position
-		var name string
-		if obj != nil {
-			// Object facts are reported on the declaring line.
-			name = obj.Name()
-			posn = pass.Fset.Position(obj.Pos())
-		} else {
-			// Package facts are reported at the start of the file.
-			name = "package"
-			posn = pass.Fset.Position(pass.Files[0].Pos())
-			posn.Line = 1
+
+	// package facts: reported at start of first file
+	for _, pkgFact := range act.AllPackageFacts() {
+		if pkgFact.Package == act.Package.Types {
+			posn := act.Package.Fset.Position(act.Package.Syntax[0].Pos())
+			posn.Line, posn.Column = 1, 1
+			checkMessage(posn, "fact", "package", fmt.Sprint(pkgFact))
 		}
+	}
 
-		for _, fact := range facts[obj] {
-			checkMessage(posn, "fact", name, fmt.Sprint(fact))
+	// object facts: reported at line of object declaration
+	objFacts := act.AllObjectFacts()
+	sort.Slice(objFacts, func(i, j int) bool {
+		return objFacts[i].Object.Pos() < objFacts[j].Object.Pos()
+	})
+	for _, objFact := range objFacts {
+		if obj := objFact.Object; obj.Pkg() == act.Package.Types {
+			posn := act.Package.Fset.Position(obj.Pos())
+			checkMessage(posn, "fact", obj.Name(), fmt.Sprint(objFact.Fact))
 		}
 	}