Skip to content

Commit 331a1c6

Browse files
committed
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 <[email protected]> Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent c9b82f2 commit 331a1c6

File tree

7 files changed

+152
-93
lines changed

7 files changed

+152
-93
lines changed

gopls/internal/lsp/regtest/expectation.go

Lines changed: 129 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -700,23 +700,144 @@ func (e DiagnosticExpectation) Description() string {
700700
return desc
701701
}
702702

703-
// NoOutstandingDiagnostics asserts that the workspace has no outstanding
704-
// diagnostic messages.
705-
func NoOutstandingDiagnostics() Expectation {
703+
// Diagnostics asserts that there is at least one diagnostic matching the given
704+
// filters.
705+
func Diagnostics(filters ...DiagnosticFilter) Expectation {
706706
check := func(s State) Verdict {
707-
for _, diags := range s.diagnostics {
708-
if len(diags.Diagnostics) > 0 {
707+
diags := flattenDiagnostics(s)
708+
for _, filter := range filters {
709+
var filtered []flatDiagnostic
710+
for _, d := range diags {
711+
if filter.check(d.name, d.diag) {
712+
filtered = append(filtered, d)
713+
}
714+
}
715+
if len(filtered) == 0 {
716+
// TODO(rfindley): if/when expectations describe their own failure, we
717+
// can provide more useful information here as to which filter caused
718+
// the failure.
709719
return Unmet
710720
}
721+
diags = filtered
722+
}
723+
return Met
724+
}
725+
var descs []string
726+
for _, filter := range filters {
727+
descs = append(descs, filter.desc)
728+
}
729+
return SimpleExpectation{
730+
check: check,
731+
description: "any diagnostics " + strings.Join(descs, ", "),
732+
}
733+
}
734+
735+
// NoMatchingDiagnostics asserts that there are no diagnostics matching the
736+
// given filters. Notably, if no filters are supplied this assertion checks
737+
// that there are no diagnostics at all, for any file.
738+
//
739+
// TODO(rfindley): replace NoDiagnostics with this, and rename.
740+
func NoMatchingDiagnostics(filters ...DiagnosticFilter) Expectation {
741+
check := func(s State) Verdict {
742+
diags := flattenDiagnostics(s)
743+
for _, filter := range filters {
744+
var filtered []flatDiagnostic
745+
for _, d := range diags {
746+
if filter.check(d.name, d.diag) {
747+
filtered = append(filtered, d)
748+
}
749+
}
750+
diags = filtered
751+
}
752+
if len(diags) > 0 {
753+
return Unmet
711754
}
712755
return Met
713756
}
757+
var descs []string
758+
for _, filter := range filters {
759+
descs = append(descs, filter.desc)
760+
}
714761
return SimpleExpectation{
715762
check: check,
716-
description: "no outstanding diagnostics",
763+
description: "no diagnostics " + strings.Join(descs, ", "),
764+
}
765+
}
766+
767+
type flatDiagnostic struct {
768+
name string
769+
diag protocol.Diagnostic
770+
}
771+
772+
func flattenDiagnostics(state State) []flatDiagnostic {
773+
var result []flatDiagnostic
774+
for name, diags := range state.diagnostics {
775+
for _, diag := range diags.Diagnostics {
776+
result = append(result, flatDiagnostic{name, diag})
777+
}
717778
}
779+
return result
718780
}
719781

782+
// -- Diagnostic filters --
783+
784+
// A DiagnosticFilter filters the set of diagnostics, for assertion with
785+
// Diagnostics or NoMatchingDiagnostics.
786+
type DiagnosticFilter struct {
787+
desc string
788+
check func(name string, _ protocol.Diagnostic) bool
789+
}
790+
791+
// ForFile filters to diagnostics matching the sandbox-relative file name.
792+
func ForFile(name string) DiagnosticFilter {
793+
return DiagnosticFilter{
794+
desc: fmt.Sprintf("for file %q", name),
795+
check: func(diagName string, _ protocol.Diagnostic) bool {
796+
return diagName == name
797+
},
798+
}
799+
}
800+
801+
// FromSource filters to diagnostics matching the given diagnostics source.
802+
func FromSource(source string) DiagnosticFilter {
803+
return DiagnosticFilter{
804+
desc: fmt.Sprintf("with source %q", source),
805+
check: func(_ string, d protocol.Diagnostic) bool {
806+
return d.Source == source
807+
},
808+
}
809+
}
810+
811+
// AtRegexp filters to diagnostics in the file with sandbox-relative path name,
812+
// at the first position matching the given regexp pattern.
813+
//
814+
// TODO(rfindley): pass in the editor to expectations, so that they may depend
815+
// on editor state and AtRegexp can be a function rather than a method.
816+
func (e *Env) AtRegexp(name, pattern string) DiagnosticFilter {
817+
pos := e.RegexpSearch(name, pattern)
818+
return DiagnosticFilter{
819+
desc: fmt.Sprintf("at the first position matching %q in %q", pattern, name),
820+
check: func(diagName string, d protocol.Diagnostic) bool {
821+
// TODO(rfindley): just use protocol.Position for Pos, rather than
822+
// duplicating.
823+
return diagName == name && d.Range.Start.Line == uint32(pos.Line) && d.Range.Start.Character == uint32(pos.Column)
824+
},
825+
}
826+
}
827+
828+
// WithMessageContaining filters to diagnostics whose message contains the
829+
// given substring.
830+
func WithMessageContaining(substring string) DiagnosticFilter {
831+
return DiagnosticFilter{
832+
desc: fmt.Sprintf("with message containing %q", substring),
833+
check: func(_ string, d protocol.Diagnostic) bool {
834+
return strings.Contains(d.Message, substring)
835+
},
836+
}
837+
}
838+
839+
// TODO(rfindley): eliminate all expectations below this point.
840+
720841
// NoDiagnostics asserts that either no diagnostics are sent for the
721842
// workspace-relative path name, or empty diagnostics are sent.
722843
func NoDiagnostics(name string) Expectation {
@@ -749,43 +870,17 @@ func (e *Env) DiagnosticAtRegexpWithMessage(name, re, msg string) DiagnosticExpe
749870
return DiagnosticExpectation{path: name, pos: &pos, re: re, present: true, message: msg}
750871
}
751872

752-
// DiagnosticAtRegexpFromSource expects a diagnostic at the first position
753-
// matching re, from the given source.
754-
func (e *Env) DiagnosticAtRegexpFromSource(name, re, source string) DiagnosticExpectation {
755-
e.T.Helper()
756-
pos := e.RegexpSearch(name, re)
757-
return DiagnosticExpectation{path: name, pos: &pos, re: re, present: true, source: source}
758-
}
759-
760873
// DiagnosticAt asserts that there is a diagnostic entry at the position
761874
// specified by line and col, for the workdir-relative path name.
762875
func DiagnosticAt(name string, line, col int) DiagnosticExpectation {
763876
return DiagnosticExpectation{path: name, pos: &fake.Pos{Line: line, Column: col}, present: true}
764877
}
765878

766-
// NoDiagnosticAtRegexp expects that there is no diagnostic entry at the start
767-
// position matching the regexp search string re in the buffer specified by
768-
// name. Note that this currently ignores the end position.
769-
// This should only be used in combination with OnceMet for a given condition,
770-
// otherwise it may always succeed.
771-
func (e *Env) NoDiagnosticAtRegexp(name, re string) DiagnosticExpectation {
772-
e.T.Helper()
773-
pos := e.RegexpSearch(name, re)
774-
return DiagnosticExpectation{path: name, pos: &pos, re: re, present: false}
775-
}
776-
777-
// NoDiagnosticWithMessage asserts that there is no diagnostic entry with the
778-
// given message.
779-
//
780-
// This should only be used in combination with OnceMet for a given condition,
781-
// otherwise it may always succeed.
782-
func NoDiagnosticWithMessage(name, msg string) DiagnosticExpectation {
783-
return DiagnosticExpectation{path: name, message: msg, present: false}
784-
}
785-
786879
// GoSumDiagnostic asserts that a "go.sum is out of sync" diagnostic for the
787880
// given module (as formatted in a go.mod file, e.g. "example.com v1.0.0") is
788881
// present.
882+
//
883+
// TODO(rfindley): remove this.
789884
func (e *Env) GoSumDiagnostic(name, module string) Expectation {
790885
e.T.Helper()
791886
// In 1.16, go.sum diagnostics should appear on the relevant module. Earlier

gopls/internal/regtest/codelens/codelens_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ require golang.org/x/hello v1.2.3
216216
// but there may be some subtlety in timing here, where this
217217
// should always succeed, but may not actually test the correct
218218
// behavior.
219-
env.NoDiagnosticAtRegexp("b/go.mod", `require`),
219+
NoMatchingDiagnostics(env.AtRegexp("b/go.mod", `require`)),
220220
),
221221
)
222222
// Check for upgrades in b/go.mod and then clear them.

gopls/internal/regtest/diagnostics/diagnostics_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ func main() {
14041404
env.Await(
14051405
OnceMet(
14061406
InitialWorkspaceLoad,
1407-
NoDiagnosticWithMessage("", "illegal character U+0023 '#'"),
1407+
NoMatchingDiagnostics(WithMessageContaining("illegal character U+0023 '#'")),
14081408
),
14091409
)
14101410
})

gopls/internal/regtest/misc/staticcheck_test.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ var FooErr error = errors.New("foo")
6464
Settings{"staticcheck": true},
6565
).Run(t, files, func(t *testing.T, env *Env) {
6666
env.OpenFile("a/a.go")
67-
env.Await(
68-
env.DiagnosticAtRegexpFromSource("a/a.go", "sort.Slice", "sortslice"),
69-
env.DiagnosticAtRegexpFromSource("a/a.go", "sort.Slice.(slice)", "SA1028"),
70-
env.DiagnosticAtRegexpFromSource("a/a.go", "var (FooErr)", "ST1012"),
71-
env.DiagnosticAtRegexpFromSource("a/a.go", `"12234"`, "SA1024"),
72-
env.DiagnosticAtRegexpFromSource("a/a.go", "testGenerics.*(p P)", "SA4009"),
73-
env.DiagnosticAtRegexpFromSource("a/a.go", "q = (&\\*p)", "SA4001"),
67+
env.AfterChange(
68+
Diagnostics(env.AtRegexp("a/a.go", "sort.Slice"), FromSource("sortslice")),
69+
Diagnostics(env.AtRegexp("a/a.go", "sort.Slice.(slice)"), FromSource("SA1028")),
70+
Diagnostics(env.AtRegexp("a/a.go", "var (FooErr)"), FromSource("ST1012")),
71+
Diagnostics(env.AtRegexp("a/a.go", `"12234"`), FromSource("SA1024")),
72+
Diagnostics(env.AtRegexp("a/a.go", "testGenerics.*(p P)"), FromSource("SA4009")),
73+
Diagnostics(env.AtRegexp("a/a.go", "q = (&\\*p)"), FromSource("SA4001")),
7474
)
7575
})
7676
}
@@ -103,11 +103,8 @@ func Foo(enabled interface{}) {
103103
Settings{"staticcheck": true},
104104
).Run(t, files, func(t *testing.T, env *Env) {
105105
env.OpenFile("p.go")
106-
env.Await(
107-
OnceMet(
108-
env.DoneWithOpen(),
109-
env.DiagnosticAtRegexpFromSource("p.go", ", (enabled)", "SA9008"),
110-
),
106+
env.AfterChange(
107+
Diagnostics(env.AtRegexp("p.go", ", (enabled)"), FromSource("SA9008")),
111108
)
112109
})
113110
}

gopls/internal/regtest/workspace/broken_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,13 @@ const F = named.D - 3
160160

161161
Run(t, files, func(t *testing.T, env *Env) {
162162
env.OpenFile("p/internal/bar/bar.go")
163-
env.Await(
164-
OnceMet(
165-
env.DoneWithOpen(),
166-
env.DiagnosticAtRegexp("p/internal/bar/bar.go", "\"mod.test/p/internal/foo\""),
167-
),
163+
env.AfterChange(
164+
env.DiagnosticAtRegexp("p/internal/bar/bar.go", "\"mod.test/p/internal/foo\""),
168165
)
169166
env.OpenFile("go.mod")
170167
env.RegexpReplace("go.mod", "mod.testx", "mod.test")
171168
env.SaveBuffer("go.mod") // saving triggers a reload
172-
env.Await(NoOutstandingDiagnostics())
169+
env.AfterChange(NoMatchingDiagnostics())
173170
})
174171
}
175172

gopls/internal/regtest/workspace/standalone_test.go

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,14 @@ func main() {
7474
}
7575

7676
env.OpenFile("lib/lib.go")
77-
env.Await(
78-
OnceMet(
79-
env.DoneWithOpen(),
80-
NoOutstandingDiagnostics(),
81-
),
82-
)
77+
env.AfterChange(NoMatchingDiagnostics())
8378

8479
// Replacing C with D should not cause any workspace diagnostics, since we
8580
// haven't yet opened the standalone file.
8681
env.RegexpReplace("lib/lib.go", "C", "D")
87-
env.Await(
88-
OnceMet(
89-
env.DoneWithChange(),
90-
NoOutstandingDiagnostics(),
91-
),
92-
)
82+
env.AfterChange(NoMatchingDiagnostics())
9383
env.RegexpReplace("lib/lib.go", "D", "C")
94-
env.Await(
95-
OnceMet(
96-
env.DoneWithChange(),
97-
NoOutstandingDiagnostics(),
98-
),
99-
)
84+
env.AfterChange(NoMatchingDiagnostics())
10085

10186
refs := env.References("lib/lib.go", env.RegexpSearch("lib/lib.go", "C"))
10287
checkLocations("References", refs, "lib/lib.go")
@@ -106,12 +91,7 @@ func main() {
10691

10792
// Opening the standalone file should not result in any diagnostics.
10893
env.OpenFile("lib/ignore.go")
109-
env.Await(
110-
OnceMet(
111-
env.DoneWithOpen(),
112-
NoOutstandingDiagnostics(),
113-
),
114-
)
94+
env.AfterChange(NoMatchingDiagnostics())
11595

11696
// Having opened the standalone file, we should find its symbols in the
11797
// workspace.
@@ -151,21 +131,11 @@ func main() {
151131
// Renaming "lib.C" to "lib.D" should cause a diagnostic in the standalone
152132
// file.
153133
env.RegexpReplace("lib/lib.go", "C", "D")
154-
env.Await(
155-
OnceMet(
156-
env.DoneWithChange(),
157-
env.DiagnosticAtRegexp("lib/ignore.go", "lib.(C)"),
158-
),
159-
)
134+
env.AfterChange(env.DiagnosticAtRegexp("lib/ignore.go", "lib.(C)"))
160135

161136
// Undoing the replacement should fix diagnostics
162137
env.RegexpReplace("lib/lib.go", "D", "C")
163-
env.Await(
164-
OnceMet(
165-
env.DoneWithChange(),
166-
NoOutstandingDiagnostics(),
167-
),
168-
)
138+
env.AfterChange(NoMatchingDiagnostics())
169139

170140
// Now that our workspace has no errors, we should be able to find
171141
// references and rename.

gopls/internal/regtest/workspace/workspace_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ func Hello() int {
272272
env.AfterChange(
273273
env.DiagnosticAtRegexp("moda/a/a.go", "x"),
274274
env.DiagnosticAtRegexp("modb/b/b.go", "x"),
275-
env.NoDiagnosticAtRegexp("moda/a/a.go", `"b.com/b"`),
275+
NoMatchingDiagnostics(env.AtRegexp("moda/a/a.go", `"b.com/b"`)),
276276
)
277277
})
278278
}
@@ -702,7 +702,7 @@ module example.com/bar
702702
// the diagnostic still shows up.
703703
env.SetBufferContent("go.work", "go 1.18 \n\n use ./bar\n")
704704
env.AfterChange(
705-
env.NoDiagnosticAtRegexp("go.work", "use"),
705+
NoMatchingDiagnostics(env.AtRegexp("go.work", "use")),
706706
)
707707
env.SetBufferContent("go.work", "go 1.18 \n\n use ./foo\n")
708708
env.AfterChange(
@@ -1069,7 +1069,7 @@ use (
10691069
b
10701070
)
10711071
`)
1072-
env.AfterChange(NoOutstandingDiagnostics())
1072+
env.AfterChange(NoMatchingDiagnostics())
10731073
// Removing the go.work file should put us back where we started.
10741074
env.RemoveWorkspaceFile("go.work")
10751075

0 commit comments

Comments
 (0)