gopls/internal/regtest: simplify expectation return values

Returning 'metBy' from expectations was not an ideal API, as extracting
the result value was cumbersome and error prone. It also forced quite a
bit of plumbing.

Using OnceMet, we can improve this by instead using a 'ReadDiagnostics'
expectation that reads diagnostics into a variable.

For golang/go#39384

Change-Id: Ia73e5b496089240df3200626e5696305cb507c9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/255126
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

gopls/internal/regtest/diagnostics_test.go

@@ -259,13 +259,13 @@ func Hello() {
 			env.Await(
 				EmptyDiagnostics("main.go"),
 			)
-			metBy := env.Await(
-				env.DiagnosticAtRegexp("bob/bob.go", "x"),
+			var d protocol.PublishDiagnosticsParams
+			env.Await(
+				OnceMet(
+					env.DiagnosticAtRegexp("bob/bob.go", "x"),
+					ReadDiagnostics("bob/bob.go", &d),
+				),
 			)
-			d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
-			if !ok {
-				t.Fatalf("unexpected met by result %v (%T)", metBy, metBy)
-			}
 			if len(d.Diagnostics) != 1 {
 				t.Fatalf("expected 1 diagnostic, got %v", len(d.Diagnostics))
 			}
@@ -490,13 +490,13 @@ func _() {
 	runner.Run(t, generated, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		original := env.ReadWorkspaceFile("main.go")
-		metBy := env.Await(
-			DiagnosticAt("main.go", 5, 8),
+		var d protocol.PublishDiagnosticsParams
+		env.Await(
+			OnceMet(
+				DiagnosticAt("main.go", 5, 8),
+				ReadDiagnostics("main.go", &d),
+			),
 		)
-		d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
-		if !ok {
-			t.Fatalf("unexpected met by result %v (%T)", metBy, metBy)
-		}
 		// Apply fixes and save the buffer.
 		env.ApplyQuickFixes("main.go", d.Diagnostics)
 		env.SaveBuffer("main.go")
@@ -649,13 +649,13 @@ func main() {
 		// "github.com/ardanlabs/conf" to the go.mod file.
 		env.OpenFile("go.mod")
 		env.OpenFile("main.go")
-		metBy := env.Await(
-			env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+		var d protocol.PublishDiagnosticsParams
+		env.Await(
+			OnceMet(
+				env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+				ReadDiagnostics("main.go", &d),
+			),
 		)
-		d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
-		if !ok {
-			t.Fatalf("unexpected type for metBy (%T)", metBy)
-		}
 		env.ApplyQuickFixes("main.go", d.Diagnostics)
 		env.SaveBuffer("go.mod")
 		env.Await(
@@ -669,14 +669,13 @@ func main() {
 		)
 		env.SaveBuffer("main.go")
 		// Expect a diagnostic and fix to remove the dependency in the go.mod.
-		metBy = env.Await(
-			EmptyDiagnostics("main.go"),
-			env.DiagnosticAtRegexp("go.mod", "require github.com/ardanlabs/conf"),
+		env.Await(EmptyDiagnostics("main.go"))
+		env.Await(
+			OnceMet(
+				env.DiagnosticAtRegexp("go.mod", "require github.com/ardanlabs/conf"),
+				ReadDiagnostics("go.mod", &d),
+			),
 		)
-		d, ok = metBy[1].(*protocol.PublishDiagnosticsParams)
-		if !ok {
-			t.Fatalf("unexpected type for metBy (%T)", metBy)
-		}
 		env.ApplyQuickFixes("go.mod", d.Diagnostics)
 		env.SaveBuffer("go.mod")
 		env.Await(
@@ -715,13 +714,13 @@ func main() {
 }
 `))
 		env.SaveBuffer("main.go")
-		metBy := env.Await(
-			env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+		var d protocol.PublishDiagnosticsParams
+		env.Await(
+			OnceMet(
+				env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+				ReadDiagnostics("main.go", &d),
+			),
 		)
-		d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
-		if !ok {
-			t.Fatalf("unexpected type for diagnostics (%T)", d)
-		}
 		env.ApplyQuickFixes("main.go", d.Diagnostics)
 		env.Await(
 			EmptyDiagnostics("main.go"),

gopls/internal/regtest/env.go

@@ -233,29 +233,25 @@ func (e *Env) onUnregistration(_ context.Context, m *protocol.UnregistrationPara
 
 func (e *Env) checkConditionsLocked() {
 	for id, condition := range e.waiters {
-		if v, _, _ := checkExpectations(e.state, condition.expectations); v != Unmet {
+		if v, _ := checkExpectations(e.state, condition.expectations); v != Unmet {
 			delete(e.waiters, id)
 			condition.verdict <- v
 		}
 	}
 }
 
 // checkExpectations reports whether s meets all expectations.
-func checkExpectations(s State, expectations []Expectation) (Verdict, string, []interface{}) {
+func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
 	finalVerdict := Met
-	var metBy []interface{}
 	var summary strings.Builder
 	for _, e := range expectations {
-		v, mb := e.Check(s)
-		if v == Met {
-			metBy = append(metBy, mb)
-		}
+		v := e.Check(s)
 		if v > finalVerdict {
 			finalVerdict = v
 		}
 		summary.WriteString(fmt.Sprintf("\t%v: %s\n", v, e.Description()))
 	}
-	return finalVerdict, summary.String(), metBy
+	return finalVerdict, summary.String()
 }
 
 // DiagnosticsFor returns the current diagnostics for the file. It is useful
@@ -269,16 +265,16 @@ func (e *Env) DiagnosticsFor(name string) *protocol.PublishDiagnosticsParams {
 
 // Await waits for all expectations to simultaneously be met. It should only be
 // called from the main test goroutine.
-func (e *Env) Await(expectations ...Expectation) []interface{} {
+func (e *Env) Await(expectations ...Expectation) {
 	e.T.Helper()
 	e.mu.Lock()
 	// Before adding the waiter, we check if the condition is currently met or
 	// failed to avoid a race where the condition was realized before Await was
 	// called.
-	switch verdict, summary, metBy := checkExpectations(e.state, expectations); verdict {
+	switch verdict, summary := checkExpectations(e.state, expectations); verdict {
 	case Met:
 		e.mu.Unlock()
-		return metBy
+		return
 	case Unmeetable:
 		e.mu.Unlock()
 		e.T.Fatalf("unmeetable expectations:\n%s\nstate:\n%v", summary, e.state)
@@ -302,12 +298,11 @@ func (e *Env) Await(expectations ...Expectation) []interface{} {
 	}
 	e.mu.Lock()
 	defer e.mu.Unlock()
-	_, summary, metBy := checkExpectations(e.state, expectations)
+	_, summary := checkExpectations(e.state, expectations)
 
 	// Debugging an unmet expectation can be tricky, so we put some effort into
 	// nicely formatting the failure.
 	if err != nil {
 		e.T.Fatalf("waiting on:\n%s\nerr:%v\n\nstate:\n%v", summary, err, e.state)
 	}
-	return metBy
 }