From a8576e2603b2bc62c97e830174b002dba60d6a0c Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Wed, 6 Feb 2019 11:24:10 -0500 Subject: [PATCH] internal/lsp: connect basic analysis functionality This starts hooking up the analysis framework into the LSP. It runs the Tests analysis (which I think might be the only one that doesn't need facts or results) and reports its diagnostics if there are no parse or typecheck failures. Next step: figure out how to pass through results. Change-Id: I21702d1cf5d54da399df54437f556b9351caa864 Reviewed-on: https://go-review.googlesource.com/c/161358 Run-TryBot: Rebecca Stambler Reviewed-by: Rebecca Stambler --- go/packages/packagestest/expect.go | 18 ++++++ internal/lsp/diagnostics.go | 6 +- internal/lsp/lsp_test.go | 6 +- internal/lsp/source/diagnostics.go | 57 +++++++++++++++++++ internal/lsp/testdata/analyzer/bad_test.go | 7 +++ internal/lsp/testdata/bad/bad0.go | 6 +- internal/lsp/testdata/bad/bad1.go | 10 ++-- internal/lsp/testdata/format/bad_format.go.in | 2 +- internal/lsp/testdata/good/good0.go | 2 +- internal/lsp/testdata/good/good1.go | 2 +- internal/lsp/testdata/noparse/noparse.go.in | 2 +- .../noparse_format/noparse_format.go.in | 2 +- internal/lsp/testdata/self/self.go.in | 2 +- internal/lsp/testdata/testy/testy_test.go | 2 +- 14 files changed, 105 insertions(+), 19 deletions(-) create mode 100644 internal/lsp/testdata/analyzer/bad_test.go diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index 077c30512e..d431cea7c9 100644 --- a/go/packages/packagestest/expect.go +++ b/go/packages/packagestest/expect.go @@ -236,6 +236,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { }, nil case pt == identifierType: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } arg := args[0] args = args[1:] switch arg := arg.(type) { @@ -248,6 +251,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { case pt == regexType: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } arg := args[0] args = args[1:] if _, ok := arg.(*regexp.Regexp); !ok { @@ -258,6 +264,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { case pt.Kind() == reflect.String: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } arg := args[0] args = args[1:] switch arg := arg.(type) { @@ -271,6 +280,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { }, nil case pt.Kind() == reflect.Int64: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } arg := args[0] args = args[1:] switch arg := arg.(type) { @@ -282,6 +294,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { }, nil case pt.Kind() == reflect.Bool: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } arg := args[0] args = args[1:] b, ok := arg.(bool) @@ -310,6 +325,9 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { default: if pt.Kind() == reflect.Interface && pt.NumMethod() == 0 { return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + if len(args) < 1 { + return reflect.Value{}, nil, fmt.Errorf("missing argument") + } return reflect.ValueOf(args[0]), args[1:], nil }, nil } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index c714918552..e928613469 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -51,11 +51,15 @@ func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []sou reports := []protocol.Diagnostic{} for _, diag := range diagnostics { tok := v.FileSet().File(diag.Start) + source := diag.Source + if source == "" { + source = "LSP" + } reports = append(reports, protocol.Diagnostic{ Message: diag.Message, Range: toProtocolRange(tok, diag.Range), Severity: protocol.SeverityError, // all diagnostics have error severity for now - Source: "LSP", + Source: source, }) } return reports diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 94bad72ac3..0edfac176d 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -36,7 +36,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { // We hardcode the expected number of test cases to ensure that all tests // are being executed. If a test is added, this number must be changed. const expectedCompletionsCount = 63 - const expectedDiagnosticsCount = 14 + const expectedDiagnosticsCount = 15 const expectedFormatCount = 3 const expectedDefinitionsCount = 16 const expectedTypeDefinitionsCount = 2 @@ -168,7 +168,7 @@ func (d diagnostics) test(t *testing.T, v source.View) int { return count } -func (d diagnostics) collect(fset *token.FileSet, rng packagestest.Range, msg string) { +func (d diagnostics) collect(fset *token.FileSet, rng packagestest.Range, msgSource, msg string) { f := fset.File(rng.Start) if _, ok := d[f.Name()]; !ok { d[f.Name()] = []protocol.Diagnostic{} @@ -181,7 +181,7 @@ func (d diagnostics) collect(fset *token.FileSet, rng packagestest.Range, msg st want := protocol.Diagnostic{ Range: toProtocolRange(f, source.Range(rng)), Severity: protocol.SeverityError, - Source: "LSP", + Source: msgSource, Message: msg, } d[f.Name()] = append(d[f.Name()], want) diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 7c393aece3..53bcb0a04b 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -12,12 +12,16 @@ import ( "strconv" "strings" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/packages" ) type Diagnostic struct { Range Message string + Source string } func Diagnostics(ctx context.Context, v View, uri URI) (map[string][]Diagnostic, error) { @@ -89,6 +93,24 @@ func Diagnostics(ctx context.Context, v View, uri URI) (map[string][]Diagnostic, reports[pos.Filename] = append(reports[pos.Filename], diagnostic) } } + if len(diags) > 0 { + return reports, nil + } + // Type checking and parsing succeeded. Run analyses. + runAnalyses(pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) { + pos := pkg.Fset.Position(diag.Pos) + category := a.Name + if diag.Category != "" { + category += "." + category + } + + reports[pos.Filename] = append(reports[pos.Filename], Diagnostic{ + Source: category, + Range: Range{Start: diag.Pos, End: diag.Pos}, + Message: fmt.Sprintf(diag.Message), + }) + }) + return reports, nil } @@ -142,3 +164,38 @@ func identifierEnd(content []byte, l, c int) (int, error) { } return bytes.IndexAny(line[c-1:], " \n,():;[]"), nil } + +func runAnalyses(pkg *packages.Package, report func(a *analysis.Analyzer, diag analysis.Diagnostic)) error { + analyzers := []*analysis.Analyzer{ + tests.Analyzer, // an analyzer that doesn't have facts or requires + } + for _, a := range analyzers { + if len(a.FactTypes) > 0 { + panic("for analyzer " + a.Name + " modular analyses needing facts are not yet supported") + } + if len(a.Requires) > 0 { + panic("for analyzer " + a.Name + " analyses requiring results are not yet supported") + } + pass := &analysis.Pass{ + Analyzer: a, + + Fset: pkg.Fset, + Files: pkg.Syntax, + OtherFiles: pkg.OtherFiles, + Pkg: pkg.Types, + TypesInfo: pkg.TypesInfo, + TypesSizes: pkg.TypesSizes, + + Report: func(diagnostic analysis.Diagnostic) { report(a, diagnostic) }, + + // TODO(matloob): Fill in the fields ResultOf, ImportObjectFact, ImportPackageFact, + // ExportObjectFact, ExportPackageFact once modular facts and results are supported. + } + _, err := a.Run(pass) + if err != nil { + return err + } + } + + return nil +} diff --git a/internal/lsp/testdata/analyzer/bad_test.go b/internal/lsp/testdata/analyzer/bad_test.go new file mode 100644 index 0000000000..cd8be6552c --- /dev/null +++ b/internal/lsp/testdata/analyzer/bad_test.go @@ -0,0 +1,7 @@ +package analyzer + +import "testing" + +func Testbad(t *testing.T) { //@diag("", "tests", "Testbad has malformed name: first letter after 'Test' must not be lowercase") + +} diff --git a/internal/lsp/testdata/bad/bad0.go b/internal/lsp/testdata/bad/bad0.go index 72cf120708..e6df4c1c5d 100644 --- a/internal/lsp/testdata/bad/bad0.go +++ b/internal/lsp/testdata/bad/bad0.go @@ -4,9 +4,9 @@ package bad func stuff() { //@item(stuff, "stuff()", "", "func") x := "heeeeyyyy" - random2(x) //@diag("x", "cannot use x (variable of type string) as int value in argument to random2") + random2(x) //@diag("x", "LSP", "cannot use x (variable of type string) as int value in argument to random2") random2(1) //@complete("dom", random, random2, random3) - y := 3 //@diag("y", "y declared but not used") + y := 3 //@diag("y", "LSP", "y declared but not used") } type bob struct { //@item(bob, "bob", "struct{...}", "struct") @@ -16,6 +16,6 @@ type bob struct { //@item(bob, "bob", "struct{...}", "struct") func _() { var q int _ = &bob{ - f: q, //@diag("f", "unknown field f in struct literal") + f: q, //@diag("f", "LSP", "unknown field f in struct literal") } } diff --git a/internal/lsp/testdata/bad/bad1.go b/internal/lsp/testdata/bad/bad1.go index c7456f5c3a..0949b66808 100644 --- a/internal/lsp/testdata/bad/bad1.go +++ b/internal/lsp/testdata/bad/bad1.go @@ -3,10 +3,10 @@ package bad // import ( -// "github.com/bob/pkg" //@diag("\"github.com/bob/pkg\"", "unable to import "\"github.com/bob/pkg\"") +// "github.com/bob/pkg" //@diag("\"github.com/bob/pkg\"", "LSP", "unable to import "\"github.com/bob/pkg\"") // ) -var a unknown //@item(global_a, "a", "unknown", "var"),diag("unknown", "undeclared name: unknown") +var a unknown //@item(global_a, "a", "unknown", "var"),diag("unknown", "LSP", "undeclared name: unknown") func random() int { //@item(random, "random()", "int", "func") //@complete("", global_a, bob, random, random2, random3, stuff) @@ -14,9 +14,9 @@ func random() int { //@item(random, "random()", "int", "func") } func random2(y int) int { //@item(random2, "random2(y int)", "int", "func"),item(bad_y_param, "y", "int", "var") - x := 6 //@item(x, "x", "int", "var"),diag("x", "x declared but not used") - var q blah //@item(q, "q", "blah", "var"),diag("q", "q declared but not used"),diag("blah", "undeclared name: blah") - var t blob //@item(t, "t", "blob", "var"),diag("t", "t declared but not used"),diag("blob", "undeclared name: blob") + x := 6 //@item(x, "x", "int", "var"),diag("x", "LSP", "x declared but not used") + var q blah //@item(q, "q", "blah", "var"),diag("q", "LSP", "q declared but not used"),diag("blah", "LSP", "undeclared name: blah") + var t blob //@item(t, "t", "blob", "var"),diag("t", "LSP", "t declared but not used"),diag("blob", "LSP", "undeclared name: blob") //@complete("", q, t, x, bad_y_param, global_a, bob, random, random2, random3, stuff) return y diff --git a/internal/lsp/testdata/format/bad_format.go.in b/internal/lsp/testdata/format/bad_format.go.in index 94187d2025..9809a7c6c7 100644 --- a/internal/lsp/testdata/format/bad_format.go.in +++ b/internal/lsp/testdata/format/bad_format.go.in @@ -11,7 +11,7 @@ func hello() { - var x int //@diag("x", "x declared but not used") + var x int //@diag("x", "LSP", "x declared but not used") } func hi() { diff --git a/internal/lsp/testdata/good/good0.go b/internal/lsp/testdata/good/good0.go index 5bcb77ded6..4e204e3fdb 100644 --- a/internal/lsp/testdata/good/good0.go +++ b/internal/lsp/testdata/good/good0.go @@ -1,4 +1,4 @@ -package good //@diag("package", "") +package good //@diag("package", "", "") func stuff() { //@item(good_stuff, "stuff()", "", "func") x := 5 diff --git a/internal/lsp/testdata/good/good1.go b/internal/lsp/testdata/good/good1.go index 8647f80c41..477a277d62 100644 --- a/internal/lsp/testdata/good/good1.go +++ b/internal/lsp/testdata/good/good1.go @@ -1,4 +1,4 @@ -package good //@diag("package", "") +package good //@diag("package", "", "") import ( "golang.org/x/tools/internal/lsp/types" //@item(types_import, "types", "\"golang.org/x/tools/internal/lsp/types\"", "package") diff --git a/internal/lsp/testdata/noparse/noparse.go.in b/internal/lsp/testdata/noparse/noparse.go.in index 9dede9c743..52bf306e74 100644 --- a/internal/lsp/testdata/noparse/noparse.go.in +++ b/internal/lsp/testdata/noparse/noparse.go.in @@ -8,4 +8,4 @@ func stuff() { x := 5 } -func .() {} //@diag(".", "expected 'IDENT', found '.'") +func .() {} //@diag(".", "LSP", "expected 'IDENT', found '.'") diff --git a/internal/lsp/testdata/noparse_format/noparse_format.go.in b/internal/lsp/testdata/noparse_format/noparse_format.go.in index eb2ad2e6e4..111bb4f4e9 100644 --- a/internal/lsp/testdata/noparse_format/noparse_format.go.in +++ b/internal/lsp/testdata/noparse_format/noparse_format.go.in @@ -4,6 +4,6 @@ package noparse_format //@format("package") func what() { var b int - if { hi() //@diag("{", "missing condition in if statement") + if { hi() //@diag("{", "LSP", "missing condition in if statement") } } \ No newline at end of file diff --git a/internal/lsp/testdata/self/self.go.in b/internal/lsp/testdata/self/self.go.in index 3da79aa509..ad0d41764f 100644 --- a/internal/lsp/testdata/self/self.go.in +++ b/internal/lsp/testdata/self/self.go.in @@ -3,7 +3,7 @@ package self import ( - "golang.org/x/tools/internal/lsp/self" //@diag("\"golang.org/x/tools/internal/lsp/self\"", "could not import golang.org/x/tools/internal/lsp/self (import cycle: [golang.org/x/tools/internal/lsp/self])") + "golang.org/x/tools/internal/lsp/self" //@diag("\"golang.org/x/tools/internal/lsp/self\"", "LSP", "could not import golang.org/x/tools/internal/lsp/self (import cycle: [golang.org/x/tools/internal/lsp/self])") ) func Hello() {} diff --git a/internal/lsp/testdata/testy/testy_test.go b/internal/lsp/testdata/testy/testy_test.go index b1e0c82dc6..f4a35970c8 100644 --- a/internal/lsp/testdata/testy/testy_test.go +++ b/internal/lsp/testdata/testy/testy_test.go @@ -3,6 +3,6 @@ package testy import "testing" func TestSomething(t *testing.T) { //@item(TestSomething, "TestSomething(t *testing.T)", "", "func") - var x int //@diag("x", "x declared but not used") + var x int //@diag("x", "LSP", "x declared but not used") a() }