From 92398ad77b896e2a8f4800b877da8974ff8d978b Mon Sep 17 00:00:00 2001 From: Dominik Honnef Date: Tue, 14 Apr 2020 02:30:34 +0200 Subject: [PATCH] go/analysis/analysistest: print unified diff for test failures Instead of printing the golden file and actual output using %#v, print a unified diff. That way, instead of a possibly long, hard to decipher error like this analysistest.go:134: suggested fixes failed for /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go, expected: "package pkg\n\nfunc fn() {\n\tvar x int = 1\n\t_ = x\n\n\tvar y interface{} = 1\n\t_ = y\n\n\tif true {\n\t\tvar x string = \"a\"\n\t\t_ = x\n\t}\n\n\tvar z []string\n\tz = append(z, \"\")\n\t_ = z\n\n\tvar f func()\n\tf = func() { f() }\n\t_ = f\n\n\tvar a int\n\ta = 1\n\ta = 2\n\t_ = a\n\n\tvar b int\n\tb = 1\n\t// do stuff\n\tb = 2\n\t_ = b\n}\n" got: "package pkg\n\nfunc fn() {\n\tvar x int = 1\n\t_ = x\n\n\tvar y interface{} = 1\n\t_ = y\n\n\tif true {\n\t\tvar x string = \"\"\n\t\t_ = x\n\t}\n\n\tvar z []string\n\tz = append(z, \"\")\n\t_ = z\n\n\tvar f func()\n\tf = func() { f() }\n\t_ = f\n\n\tvar a int\n\ta = 1\n\ta = 2\n\t_ = a\n\n\tvar b int\n\tb = 1\n\t// do stuff\n\tb = 2\n\t_ = b\n}\n" we get a much more concise and readable diff: analysistest.go:133: suggested fixes failed for /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go: --- /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go.golden +++ actual @@ -8,7 +8,7 @@ _ = y if true { - var x string = "a" + var x string = "" _ = x } One downside of this approach is that unprintable characters won't be visible in the diff. However, the vast majority of Go code does not contain unprintable characters, and an even smaller amount of suggested fixes affect unprintable characters. It is worth optimizing readability for the common case. Change-Id: I857aa6b6ee719f0fb018d5007eb162882e79cc25 Reviewed-on: https://go-review.googlesource.com/c/tools/+/228118 Run-TryBot: Dominik Honnef TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler Reviewed-by: Michael Matloob --- go/analysis/analysistest/analysistest.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 169e7ace7e..506b77fbfb 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -20,6 +20,7 @@ import ( "golang.org/x/tools/go/analysis/internal/checker" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/lsp/diff/myers" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" ) @@ -128,7 +129,8 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns continue } if string(want) != string(formatted) { - t.Errorf("suggested fixes failed for %s, expected:\n%#v\ngot:\n%#v", file.Name(), string(want), string(formatted)) + d := myers.ComputeEdits("", string(want), string(formatted)) + t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), diff.ToUnified(file.Name()+".golden", "actual", string(want), d)) } } return r