From b5e55d198461206bca9558e65cdd518f8e4f2735 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 1 Sep 2023 17:18:31 -0400 Subject: [PATCH] go/analysis/analysistest: give better hint in SuggestedFix assertion Also, better documentation on the underlying cause. Change-Id: I0ee93b6e9f2ada52d9a32a322b77fda783ddf076 Reviewed-on: https://go-review.googlesource.com/c/tools/+/525215 TryBot-Result: Gopher Robot Run-TryBot: Alan Donovan Reviewed-by: Robert Findley --- go/analysis/analysistest/analysistest.go | 34 +++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 6a27edb1064..63ca6e9eb2e 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -98,6 +98,34 @@ type Testing interface { // println() // } // } +// +// # Conflicts +// +// A single analysis pass may offer two or more suggested fixes that +// (1) conflict but are nonetheless logically composable, (e.g. +// because both update the import declaration), or (2) are +// fundamentally incompatible (e.g. alternative fixes to the same +// statement). +// +// It is up to the driver to decide how to apply such fixes. A +// sophisticated driver could attempt to resolve conflicts of the +// first kind, but this test driver simply reports the fact of the +// conflict with the expectation that the user will split their tests +// into nonconflicting parts. +// +// Conflicts of the second kind can be avoided by giving the +// alternative fixes different names (SuggestedFix.Message) and using +// a multi-section .txtar file with a named section for each +// alternative fix. +// +// Analyzers that compute fixes from a textual diff of the +// before/after file contents (instead of directly from syntax tree +// positions) may produce fixes that, although logically +// non-conflicting, nonetheless conflict due to the particulars of the +// diff algorithm. In such cases it may suffice to introduce +// 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. func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result { r := Run(t, dir, a, patterns...) @@ -135,7 +163,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns continue } if _, ok := fileContents[file]; !ok { - contents, err := ioutil.ReadFile(file.Name()) + contents, err := os.ReadFile(file.Name()) if err != nil { t.Errorf("error reading %s: %v", file.Name(), err) } @@ -186,7 +214,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns found = true out, err := diff.ApplyBytes(orig, edits) if err != nil { - t.Errorf("%s: error applying fixes: %v", file.Name(), err) + t.Errorf("%s: error applying fixes: %v (see possible explanations at RunWithSuggestedFixes)", file.Name(), err) continue } // the file may contain multiple trailing @@ -220,7 +248,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns out, err := diff.ApplyBytes(orig, catchallEdits) if err != nil { - t.Errorf("%s: error applying fixes: %v", file.Name(), err) + t.Errorf("%s: error applying fixes: %v (see possible explanations at RunWithSuggestedFixes)", file.Name(), err) continue } want := string(ar.Comment)