From 5d86d385bf88f37d685892cf7b0bcdbe84b10c1e Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Sun, 15 Mar 2020 13:01:52 -0400 Subject: [PATCH] internal/lsp/analysis: add simplify-slice pass from "gofmt -s" This is one of the simplifications that "gofmt -s" applies. https://golang.org/cmd/gofmt/#hdr-The_simplify_command A slice expression of the form: s[a:len(s)] will be simplified to: s[a:] Updates golang/go#37221 Change-Id: Ibd4dedaadc9b129d5d39524f0c1ccc8a95bf7e0d Reviewed-on: https://go-review.googlesource.com/c/tools/+/223659 Run-TryBot: Rohan Challa TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler Reviewed-by: Michael Matloob --- gopls/doc/analyzers.md | 17 ++++ .../analysis/simplifyslice/simplifyslice.go | 96 +++++++++++++++++++ .../simplifyslice/simplifyslice_test.go | 17 ++++ .../simplifyslice/testdata/src/a/a.go | 70 ++++++++++++++ .../simplifyslice/testdata/src/a/a.go.golden | 70 ++++++++++++++ internal/lsp/source/options.go | 2 + 6 files changed, 272 insertions(+) create mode 100644 internal/lsp/analysis/simplifyslice/simplifyslice.go create mode 100644 internal/lsp/analysis/simplifyslice/simplifyslice_test.go create mode 100644 internal/lsp/analysis/simplifyslice/testdata/src/a/a.go create mode 100644 internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 9ebbaec12d..5ddd52587e 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -364,6 +364,23 @@ This is one of the simplifications that "gofmt -s" applies. Default value: `true`. +### **simplifyslice** + +check for slice simplifications + +A slice expression of the form: +```go +s[a:len(s)] +``` +will be simplified to: +```go +s[a:] +``` + +This is one of the simplifications that "gofmt -s" applies. + +Default value: `true`. + ### **sortslice** check the argument type of sort.Slice diff --git a/internal/lsp/analysis/simplifyslice/simplifyslice.go b/internal/lsp/analysis/simplifyslice/simplifyslice.go new file mode 100644 index 0000000000..01a6bc3d8c --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/simplifyslice.go @@ -0,0 +1,96 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package simplifyslice defines an Analyzer that simplifies slice statements. +// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go +// https://golang.org/cmd/gofmt/#hdr-The_simplify_command +package simplifyslice + +import ( + "bytes" + "fmt" + "go/ast" + "go/printer" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `check for slice simplifications + +A slice expression of the form: + s[a:len(s)] +will be simplified to: + s[a:] + +This is one of the simplifications that "gofmt -s" applies.` + +var Analyzer = &analysis.Analyzer{ + Name: "simplifyslice", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +// Note: We could also simplify slice expressions of the form s[0:b] to s[:b] +// but we leave them as is since sometimes we want to be very explicit +// about the lower bound. +// An example where the 0 helps: +// x, y, z := b[0:2], b[2:4], b[4:6] +// An example where it does not: +// x, y := b[:n], b[n:] + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.SliceExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + expr := n.(*ast.SliceExpr) + // - 3-index slices always require the 2nd and 3rd index + if expr.Max != nil { + return + } + s, ok := expr.X.(*ast.Ident) + // the array/slice object is a single, resolved identifier + if !ok || s.Obj == nil { + return + } + call, ok := expr.High.(*ast.CallExpr) + // the high expression is a function call with a single argument + if !ok || len(call.Args) != 1 || call.Ellipsis.IsValid() { + return + } + fun, ok := call.Fun.(*ast.Ident) + // the function called is "len" and it is not locally defined; and + // because we don't have dot imports, it must be the predefined len() + if !ok || fun.Name != "len" || fun.Obj != nil { + return + } + arg, ok := call.Args[0].(*ast.Ident) + // the len argument is the array/slice object + if !ok || arg.Obj != s.Obj { + return + } + old := expr.High + var b bytes.Buffer + printer.Fprint(&b, pass.Fset, old) + pass.Report(analysis.Diagnostic{ + Pos: old.Pos(), + End: old.End(), + Message: fmt.Sprintf("unneeded: %s", b.String()), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Remove '%s'", b.String()), + TextEdits: []analysis.TextEdit{{ + Pos: old.Pos(), + End: old.End(), + NewText: []byte{}, + }}, + }}, + }) + expr.High = old + }) + return nil, nil +} diff --git a/internal/lsp/analysis/simplifyslice/simplifyslice_test.go b/internal/lsp/analysis/simplifyslice/simplifyslice_test.go new file mode 100644 index 0000000000..91db76ae02 --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/simplifyslice_test.go @@ -0,0 +1,17 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package simplifyslice_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/simplifyslice" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, simplifyslice.Analyzer, "a") +} diff --git a/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go b/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go new file mode 100644 index 0000000000..20792105dd --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go @@ -0,0 +1,70 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package testdata + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:len(a)] // want "unneeded: len\\(a\\)" + _ = a[3:(len(a))] + _ = a[len(a)-1 : len(a)] // want "unneeded: len\\(a\\)" + _ = a[2:len(a):len(a)] + + _ = a[:] + _ = a[:10] + _ = a[:len(a)] // want "unneeded: len\\(a\\)" + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(a):len(a)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:len(s)] // want "unneeded: len\\(s\\)" + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + _ = s[2:len(s):len(s)] + + _ = s[:] + _ = s[:10] + _ = s[:len(s)] // want "unneeded: len\\(s\\)" + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + _ = s[:len(s):len(s)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + _ = t.s[2:len(t.s):len(t.s)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] + _ = t.s[:len(t.s):len(t.s)] +) + +func _() { + s := s[0:len(s)] // want "unneeded: len\\(s\\)" + _ = s +} + +func m() { + maps := []int{} + _ = maps[1:len(maps)] // want "unneeded: len\\(maps\\)" +} diff --git a/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden b/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..45c791421c --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden @@ -0,0 +1,70 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package testdata + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:] // want "unneeded: len\\(a\\)" + _ = a[3:(len(a))] + _ = a[len(a)-1:] // want "unneeded: len\\(a\\)" + _ = a[2:len(a):len(a)] + + _ = a[:] + _ = a[:10] + _ = a[:] // want "unneeded: len\\(a\\)" + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(a):len(a)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:] // want "unneeded: len\\(s\\)" + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + _ = s[2:len(s):len(s)] + + _ = s[:] + _ = s[:10] + _ = s[:] // want "unneeded: len\\(s\\)" + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + _ = s[:len(s):len(s)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + _ = t.s[2:len(t.s):len(t.s)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] + _ = t.s[:len(t.s):len(t.s)] +) + +func _() { + s := s[0:] // want "unneeded: len\\(s\\)" + _ = s +} + +func m() { + maps := []int{} + _ = maps[1:] // want "unneeded: len\\(maps\\)" +} diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index d96785761d..37fe965521 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -38,6 +38,7 @@ import ( "golang.org/x/tools/go/analysis/passes/unusedresult" "golang.org/x/tools/internal/lsp/analysis/simplifycompositelit" "golang.org/x/tools/internal/lsp/analysis/simplifyrange" + "golang.org/x/tools/internal/lsp/analysis/simplifyslice" "golang.org/x/tools/internal/lsp/analysis/unusedparams" "golang.org/x/tools/internal/lsp/debug/tag" "golang.org/x/tools/internal/lsp/diff" @@ -517,5 +518,6 @@ func defaultAnalyzers() map[string]Analyzer { // gofmt -s suite: simplifycompositelit.Analyzer.Name: {Analyzer: simplifycompositelit.Analyzer, Enabled: true, HighConfidence: true}, simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, HighConfidence: true}, + simplifyslice.Analyzer.Name: {Analyzer: simplifyslice.Analyzer, Enabled: true, HighConfidence: true}, } }