go/analysis: document interpretation of multiple diagnostics
RunWithSuggestedFixes was written without a clear understanding
of what the semantics of multiple fixes should be. We believe
the only sensible interpretation is the one described here:
https://github.com/golang/go/issues/67049#issuecomment-2163687538
This change documents the interpretation at Diagnostics.SuggestedFixes,
and adds a TODO at RunWithSuggestedFixes to note that its
conflict resolution is problematic, plus a suggested workaround.
A later change will fix RunWithSuggestedFixes, or provide a
replacement if that should prove infeasible.
Updates golang/go#67049
Change-Id: I59d93d77f05e13e2458daa2d9897057ed9f82d06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/592495
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index 1eead85..c1b2dd4 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -126,6 +126,16 @@
// sufficient separation of the statements in the test input so that
// the computed diffs do not overlap. If that fails, break the test
// into smaller parts.
+//
+// TODO(adonovan): the behavior of RunWithSuggestedFixes as documented
+// above is impractical for tests that report multiple diagnostics and
+// offer multiple alternative fixes for the same diagnostic, and it is
+// inconsistent with the interpretation of multiple diagnostics
+// described at Diagnostic.SuggestedFixes.
+// We need to rethink the analyzer testing API to better support such
+// cases. In the meantime, users of RunWithSuggestedFixes testing
+// 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...)
diff --git a/go/analysis/diagnostic.go b/go/analysis/diagnostic.go
index 9a850ec..ee083a2 100644
--- a/go/analysis/diagnostic.go
+++ b/go/analysis/diagnostic.go
@@ -33,8 +33,17 @@
URL string
// SuggestedFixes is an optional list of fixes to address the
- // problem described by the diagnostic, each one representing
+ // problem described by the diagnostic. Each one represents
// an alternative strategy; at most one may be applied.
+ //
+ // Fixes for different diagnostics should be treated as
+ // independent changes to the same baseline file state,
+ // analogous to a set of git commits all with the same parent.
+ // Combining fixes requires resolving any conflicts that
+ // arise, analogous to a git merge.
+ // Any conflicts that remain may be dealt with, depending on
+ // the tool, by discarding fixes, consulting the user, or
+ // aborting the operation.
SuggestedFixes []SuggestedFix
// Related contains optional secondary positions and messages
