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>
diff --git a/gopls/internal/lsp/regtest/expectation.go b/gopls/internal/lsp/regtest/expectation.go
index 3711386..a86dfe1 100644
--- a/gopls/internal/lsp/regtest/expectation.go
+++ b/gopls/internal/lsp/regtest/expectation.go
@@ -700,23 +700,144 @@
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: "no outstanding diagnostics",
+ 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 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 @@
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
diff --git a/gopls/internal/regtest/codelens/codelens_test.go b/gopls/internal/regtest/codelens/codelens_test.go
index 8ea5e9c..13ba4e1 100644
--- a/gopls/internal/regtest/codelens/codelens_test.go
+++ b/gopls/internal/regtest/codelens/codelens_test.go
@@ -216,7 +216,7 @@
// 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.
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index 6631a92..b7ccb5b 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -1404,7 +1404,7 @@
env.Await(
OnceMet(
InitialWorkspaceLoad,
- NoDiagnosticWithMessage("", "illegal character U+0023 '#'"),
+ NoMatchingDiagnostics(WithMessageContaining("illegal character U+0023 '#'")),
),
)
})
diff --git a/gopls/internal/regtest/misc/staticcheck_test.go b/gopls/internal/regtest/misc/staticcheck_test.go
index f2ba3cc..9242e19 100644
--- a/gopls/internal/regtest/misc/staticcheck_test.go
+++ b/gopls/internal/regtest/misc/staticcheck_test.go
@@ -64,13 +64,13 @@
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 @@
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")),
)
})
}
diff --git a/gopls/internal/regtest/workspace/broken_test.go b/gopls/internal/regtest/workspace/broken_test.go
index 711d596..9a65030 100644
--- a/gopls/internal/regtest/workspace/broken_test.go
+++ b/gopls/internal/regtest/workspace/broken_test.go
@@ -160,16 +160,13 @@
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())
})
}
diff --git a/gopls/internal/regtest/workspace/standalone_test.go b/gopls/internal/regtest/workspace/standalone_test.go
index 698c8aa..dffbbea 100644
--- a/gopls/internal/regtest/workspace/standalone_test.go
+++ b/gopls/internal/regtest/workspace/standalone_test.go
@@ -74,29 +74,14 @@
}
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 @@
// 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 @@
// 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.
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
index a279b8e..49e9e27 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -272,7 +272,7 @@
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 @@
// 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 @@
b
)
`)
- env.AfterChange(NoOutstandingDiagnostics())
+ env.AfterChange(NoMatchingDiagnostics())
// Removing the go.work file should put us back where we started.
env.RemoveWorkspaceFile("go.work")
