Skip to content

Commit b59dd37

Browse files
committed
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: golang/go#67049 (comment) 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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Tim King <[email protected]> Auto-Submit: Alan Donovan <[email protected]>
1 parent 4419f4f commit b59dd37

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed

go/analysis/analysistest/analysistest.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ type Testing interface {
126126
// sufficient separation of the statements in the test input so that
127127
// the computed diffs do not overlap. If that fails, break the test
128128
// into smaller parts.
129+
//
130+
// TODO(adonovan): the behavior of RunWithSuggestedFixes as documented
131+
// above is impractical for tests that report multiple diagnostics and
132+
// offer multiple alternative fixes for the same diagnostic, and it is
133+
// inconsistent with the interpretation of multiple diagnostics
134+
// described at Diagnostic.SuggestedFixes.
135+
// We need to rethink the analyzer testing API to better support such
136+
// cases. In the meantime, users of RunWithSuggestedFixes testing
137+
// analyzers that offer alternative fixes are advised to put each fix
138+
// in a separate .go file in the testdata.
129139
func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
130140
r := Run(t, dir, a, patterns...)
131141

go/analysis/diagnostic.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,17 @@ type Diagnostic struct {
3333
URL string
3434

3535
// SuggestedFixes is an optional list of fixes to address the
36-
// problem described by the diagnostic, each one representing
36+
// problem described by the diagnostic. Each one represents
3737
// an alternative strategy; at most one may be applied.
38+
//
39+
// Fixes for different diagnostics should be treated as
40+
// independent changes to the same baseline file state,
41+
// analogous to a set of git commits all with the same parent.
42+
// Combining fixes requires resolving any conflicts that
43+
// arise, analogous to a git merge.
44+
// Any conflicts that remain may be dealt with, depending on
45+
// the tool, by discarding fixes, consulting the user, or
46+
// aborting the operation.
3847
SuggestedFixes []SuggestedFix
3948

4049
// Related contains optional secondary positions and messages

0 commit comments

Comments
 (0)