gopls/internal/regtest: add a simpler API for diagnostic expectations

The number of different regtest expectations related to diagnostics has
grown significantly over time. Start to consolidate to just two
expectations: Diagnostics, which asserts on the existence of
diagnostics, and NoMatchingDiagnostics, which asserts on the
nonexistence of diagnostics. Both accept an argument list to filter the
set of diagnostics under consideration.

In a subsequent CL, NoMatchingDiagnostics will be renamed to
NoDiagnostics, once the existing expectation with that name has been
replaced.

Use this to eliminate the following expectations:
- DiagnosticAtRegexpFromSource -> Diagnostics(AtRegexp, FromSource)
- NoDiagnosticAtRegexp -> NoMatchingDiagnostics(AtRegexp)
- NoOutstandingDiagnostics -> NoMatchingDiagnostics

Updates golang/go#39384

Change-Id: I518b14eb00c9fcf62388a03efb668899939a8f82
Reviewed-on: https://go-review.googlesource.com/c/tools/+/461915
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

gopls/internal/lsp/regtest/expectation.go

@@ -700,23 +700,144 @@ func (e DiagnosticExpectation) Description() string {
 	return desc
 }
 
-// NoOutstandingDiagnostics asserts that the workspace has no outstanding
-// diagnostic messages.
-func NoOutstandingDiagnostics() Expectation {
+// Diagnostics asserts that there is at least one diagnostic matching the given
+// filters.
+func Diagnostics(filters ...DiagnosticFilter) Expectation {
 	check := func(s State) Verdict {
-		for _, diags := range s.diagnostics {
-			if len(diags.Diagnostics) > 0 {
+		diags := flattenDiagnostics(s)
+		for _, filter := range filters {
+			var filtered []flatDiagnostic
+			for _, d := range diags {
+				if filter.check(d.name, d.diag) {
+					filtered = append(filtered, d)
+				}
+			}
+			if len(filtered) == 0 {
+				// TODO(rfindley): if/when expectations describe their own failure, we
+				// can provide more useful information here as to which filter caused
+				// the failure.
 				return Unmet
 			}
+			diags = filtered
+		}
+		return Met
+	}
+	var descs []string
+	for _, filter := range filters {
+		descs = append(descs, filter.desc)
+	}
+	return SimpleExpectation{
+		check:       check,
+		description: "any diagnostics " + strings.Join(descs, ", "),
+	}
+}
+
+// NoMatchingDiagnostics asserts that there are no diagnostics matching the
+// given filters. Notably, if no filters are supplied this assertion checks
+// that there are no diagnostics at all, for any file.
+//
+// TODO(rfindley): replace NoDiagnostics with this, and rename.
+func NoMatchingDiagnostics(filters ...DiagnosticFilter) Expectation {
+	check := func(s State) Verdict {
+		diags := flattenDiagnostics(s)
+		for _, filter := range filters {
+			var filtered []flatDiagnostic
+			for _, d := range diags {
+				if filter.check(d.name, d.diag) {
+					filtered = append(filtered, d)
+				}
+			}
+			diags = filtered
+		}
+		if len(diags) > 0 {
+			return Unmet
 		}
 		return Met
 	}
+	var descs []string
+	for _, filter := range filters {
+		descs = append(descs, filter.desc)
+	}
 	return SimpleExpectation{
 		check:       check,
-		description: "no outstanding diagnostics",
+		description: "no diagnostics " + strings.Join(descs, ", "),
+	}
+}
+
+type flatDiagnostic struct {
+	name string
+	diag protocol.Diagnostic
+}
+
+func flattenDiagnostics(state State) []flatDiagnostic {
+	var result []flatDiagnostic
+	for name, diags := range state.diagnostics {
+		for _, diag := range diags.Diagnostics {
+			result = append(result, flatDiagnostic{name, diag})
+		}
 	}
+	return result
 }
 
+// -- Diagnostic filters --
+
+// A DiagnosticFilter filters the set of diagnostics, for assertion with
+// Diagnostics or NoMatchingDiagnostics.
+type DiagnosticFilter struct {
+	desc  string
+	check func(name string, _ protocol.Diagnostic) bool
+}
+
+// ForFile filters to diagnostics matching the sandbox-relative file name.
+func ForFile(name string) DiagnosticFilter {
+	return DiagnosticFilter{
+		desc: fmt.Sprintf("for file %q", name),
+		check: func(diagName string, _ protocol.Diagnostic) bool {
+			return diagName == name
+		},
+	}
+}
+
+// FromSource filters to diagnostics matching the given diagnostics source.
+func FromSource(source string) DiagnosticFilter {
+	return DiagnosticFilter{
+		desc: fmt.Sprintf("with source %q", source),
+		check: func(_ string, d protocol.Diagnostic) bool {
+			return d.Source == source
+		},
+	}
+}
+
+// AtRegexp filters to diagnostics in the file with sandbox-relative path name,
+// at the first position matching the given regexp pattern.
+//
+// TODO(rfindley): pass in the editor to expectations, so that they may depend
+// on editor state and AtRegexp can be a function rather than a method.
+func (e *Env) AtRegexp(name, pattern string) DiagnosticFilter {
+	pos := e.RegexpSearch(name, pattern)
+	return DiagnosticFilter{
+		desc: fmt.Sprintf("at the first position matching %q in %q", pattern, name),
+		check: func(diagName string, d protocol.Diagnostic) bool {
+			// TODO(rfindley): just use protocol.Position for Pos, rather than
+			// duplicating.
+			return diagName == name && d.Range.Start.Line == uint32(pos.Line) && d.Range.Start.Character == uint32(pos.Column)
+		},
+	}
+}
+
+// WithMessageContaining filters to diagnostics whose message contains the
+// given substring.
+func WithMessageContaining(substring string) DiagnosticFilter {
+	return DiagnosticFilter{
+		desc: fmt.Sprintf("with message containing %q", substring),
+		check: func(_ string, d protocol.Diagnostic) bool {
+			return strings.Contains(d.Message, substring)
+		},
+	}
+}
+
+// TODO(rfindley): eliminate all expectations below this point.
+
 // NoDiagnostics asserts that either no diagnostics are sent for the
 // workspace-relative path name, or empty diagnostics are sent.
 func NoDiagnostics(name string) Expectation {
@@ -749,43 +870,17 @@ func (e *Env) DiagnosticAtRegexpWithMessage(name, re, msg string) DiagnosticExpe
 	return DiagnosticExpectation{path: name, pos: &pos, re: re, present: true, message: msg}
 }
 
-// DiagnosticAtRegexpFromSource expects a diagnostic at the first position
-// matching re, from the given source.
-func (e *Env) DiagnosticAtRegexpFromSource(name, re, source string) DiagnosticExpectation {
-	e.T.Helper()
-	pos := e.RegexpSearch(name, re)
-	return DiagnosticExpectation{path: name, pos: &pos, re: re, present: true, source: source}
-}
-
 // DiagnosticAt asserts that there is a diagnostic entry at the position
 // specified by line and col, for the workdir-relative path name.
 func DiagnosticAt(name string, line, col int) DiagnosticExpectation {
 	return DiagnosticExpectation{path: name, pos: &fake.Pos{Line: line, Column: col}, present: true}
 }
 
-// NoDiagnosticAtRegexp expects that there is no diagnostic entry at the start
-// position matching the regexp search string re in the buffer specified by
-// name. Note that this currently ignores the end position.
-// This should only be used in combination with OnceMet for a given condition,
-// otherwise it may always succeed.
-func (e *Env) NoDiagnosticAtRegexp(name, re string) DiagnosticExpectation {
-	e.T.Helper()
-	pos := e.RegexpSearch(name, re)
-	return DiagnosticExpectation{path: name, pos: &pos, re: re, present: false}
-}
-
-// NoDiagnosticWithMessage asserts that there is no diagnostic entry with the
-// given message.
-//
-// This should only be used in combination with OnceMet for a given condition,
-// otherwise it may always succeed.
-func NoDiagnosticWithMessage(name, msg string) DiagnosticExpectation {
-	return DiagnosticExpectation{path: name, message: msg, present: false}
-}
-
 // GoSumDiagnostic asserts that a "go.sum is out of sync" diagnostic for the
 // given module (as formatted in a go.mod file, e.g. "example.com v1.0.0") is
 // present.
+//
+// TODO(rfindley): remove this.
 func (e *Env) GoSumDiagnostic(name, module string) Expectation {
 	e.T.Helper()
 	// In 1.16, go.sum diagnostics should appear on the relevant module. Earlier

gopls/internal/regtest/codelens/codelens_test.go

@@ -216,7 +216,7 @@ require golang.org/x/hello v1.2.3
 						// but there may be some subtlety in timing here, where this
 						// should always succeed, but may not actually test the correct
 						// behavior.
-						env.NoDiagnosticAtRegexp("b/go.mod", `require`),
+						NoMatchingDiagnostics(env.AtRegexp("b/go.mod", `require`)),
 					),
 				)
 				// Check for upgrades in b/go.mod and then clear them.

gopls/internal/regtest/diagnostics/diagnostics_test.go

@@ -1404,7 +1404,7 @@ func main() {
 		env.Await(
 			OnceMet(
 				InitialWorkspaceLoad,
-				NoDiagnosticWithMessage("", "illegal character U+0023 '#'"),
+				NoMatchingDiagnostics(WithMessageContaining("illegal character U+0023 '#'")),
 			),
 		)
 	})

gopls/internal/regtest/misc/staticcheck_test.go

@@ -64,13 +64,13 @@ var FooErr error = errors.New("foo")
 		Settings{"staticcheck": true},
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("a/a.go")
-		env.Await(
-			env.DiagnosticAtRegexpFromSource("a/a.go", "sort.Slice", "sortslice"),
-			env.DiagnosticAtRegexpFromSource("a/a.go", "sort.Slice.(slice)", "SA1028"),
-			env.DiagnosticAtRegexpFromSource("a/a.go", "var (FooErr)", "ST1012"),
-			env.DiagnosticAtRegexpFromSource("a/a.go", `"12234"`, "SA1024"),
-			env.DiagnosticAtRegexpFromSource("a/a.go", "testGenerics.*(p P)", "SA4009"),
-			env.DiagnosticAtRegexpFromSource("a/a.go", "q = (&\\*p)", "SA4001"),
+		env.AfterChange(
+			Diagnostics(env.AtRegexp("a/a.go", "sort.Slice"), FromSource("sortslice")),
+			Diagnostics(env.AtRegexp("a/a.go", "sort.Slice.(slice)"), FromSource("SA1028")),
+			Diagnostics(env.AtRegexp("a/a.go", "var (FooErr)"), FromSource("ST1012")),
+			Diagnostics(env.AtRegexp("a/a.go", `"12234"`), FromSource("SA1024")),
+			Diagnostics(env.AtRegexp("a/a.go", "testGenerics.*(p P)"), FromSource("SA4009")),
+			Diagnostics(env.AtRegexp("a/a.go", "q = (&\\*p)"), FromSource("SA4001")),
 		)
 	})
 }
@@ -103,11 +103,8 @@ func Foo(enabled interface{}) {
 		Settings{"staticcheck": true},
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("p.go")
-		env.Await(
-			OnceMet(
-				env.DoneWithOpen(),
-				env.DiagnosticAtRegexpFromSource("p.go", ", (enabled)", "SA9008"),
-			),
+		env.AfterChange(
+			Diagnostics(env.AtRegexp("p.go", ", (enabled)"), FromSource("SA9008")),
 		)
 	})
 }

gopls/internal/regtest/workspace/broken_test.go

@@ -160,16 +160,13 @@ const F = named.D - 3
 
 	Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("p/internal/bar/bar.go")
-		env.Await(
-			OnceMet(
-				env.DoneWithOpen(),
-				env.DiagnosticAtRegexp("p/internal/bar/bar.go", "\"mod.test/p/internal/foo\""),
-			),
+		env.AfterChange(
+			env.DiagnosticAtRegexp("p/internal/bar/bar.go", "\"mod.test/p/internal/foo\""),
 		)
 		env.OpenFile("go.mod")
 		env.RegexpReplace("go.mod", "mod.testx", "mod.test")
 		env.SaveBuffer("go.mod") // saving triggers a reload
-		env.Await(NoOutstandingDiagnostics())
+		env.AfterChange(NoMatchingDiagnostics())
 	})
 }
 

gopls/internal/regtest/workspace/standalone_test.go

@@ -74,29 +74,14 @@ func main() {
 		}
 
 		env.OpenFile("lib/lib.go")
-		env.Await(
-			OnceMet(
-				env.DoneWithOpen(),
-				NoOutstandingDiagnostics(),
-			),
-		)
+		env.AfterChange(NoMatchingDiagnostics())
 
 		// Replacing C with D should not cause any workspace diagnostics, since we
 		// haven't yet opened the standalone file.
 		env.RegexpReplace("lib/lib.go", "C", "D")
-		env.Await(
-			OnceMet(
-				env.DoneWithChange(),
-				NoOutstandingDiagnostics(),
-			),
-		)
+		env.AfterChange(NoMatchingDiagnostics())
 		env.RegexpReplace("lib/lib.go", "D", "C")
-		env.Await(
-			OnceMet(
-				env.DoneWithChange(),
-				NoOutstandingDiagnostics(),
-			),
-		)
+		env.AfterChange(NoMatchingDiagnostics())
 
 		refs := env.References("lib/lib.go", env.RegexpSearch("lib/lib.go", "C"))
 		checkLocations("References", refs, "lib/lib.go")
@@ -106,12 +91,7 @@ func main() {
 
 		// Opening the standalone file should not result in any diagnostics.
 		env.OpenFile("lib/ignore.go")
-		env.Await(
-			OnceMet(
-				env.DoneWithOpen(),
-				NoOutstandingDiagnostics(),
-			),
-		)
+		env.AfterChange(NoMatchingDiagnostics())
 
 		// Having opened the standalone file, we should find its symbols in the
 		// workspace.
@@ -151,21 +131,11 @@ func main() {
 		// Renaming "lib.C" to "lib.D" should cause a diagnostic in the standalone
 		// file.
 		env.RegexpReplace("lib/lib.go", "C", "D")
-		env.Await(
-			OnceMet(
-				env.DoneWithChange(),
-				env.DiagnosticAtRegexp("lib/ignore.go", "lib.(C)"),
-			),
-		)
+		env.AfterChange(env.DiagnosticAtRegexp("lib/ignore.go", "lib.(C)"))
 
 		// Undoing the replacement should fix diagnostics
 		env.RegexpReplace("lib/lib.go", "D", "C")
-		env.Await(
-			OnceMet(
-				env.DoneWithChange(),
-				NoOutstandingDiagnostics(),
-			),
-		)
+		env.AfterChange(NoMatchingDiagnostics())
 
 		// Now that our workspace has no errors, we should be able to find
 		// references and rename.

gopls/internal/regtest/workspace/workspace_test.go

@@ -272,7 +272,7 @@ func Hello() int {
 		env.AfterChange(
 			env.DiagnosticAtRegexp("moda/a/a.go", "x"),
 			env.DiagnosticAtRegexp("modb/b/b.go", "x"),
-			env.NoDiagnosticAtRegexp("moda/a/a.go", `"b.com/b"`),
+			NoMatchingDiagnostics(env.AtRegexp("moda/a/a.go", `"b.com/b"`)),
 		)
 	})
 }
@@ -702,7 +702,7 @@ module example.com/bar
 		// the diagnostic still shows up.
 		env.SetBufferContent("go.work", "go 1.18 \n\n use ./bar\n")
 		env.AfterChange(
-			env.NoDiagnosticAtRegexp("go.work", "use"),
+			NoMatchingDiagnostics(env.AtRegexp("go.work", "use")),
 		)
 		env.SetBufferContent("go.work", "go 1.18 \n\n use ./foo\n")
 		env.AfterChange(
@@ -1069,7 +1069,7 @@ use (
 	b
 )
 `)
-		env.AfterChange(NoOutstandingDiagnostics())
+		env.AfterChange(NoMatchingDiagnostics())
 		// Removing the go.work file should put us back where we started.
 		env.RemoveWorkspaceFile("go.work")