Skip to content

Commit 5d86d38

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent e428a8e commit 5d86d38

File tree

6 files changed

+272
-0
lines changed

6 files changed

+272
-0
lines changed

gopls/doc/analyzers.md

+17
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,23 @@ This is one of the simplifications that "gofmt -s" applies.
364364

365365
Default value: `true`.
366366

367+
### **simplifyslice**
368+
369+
check for slice simplifications
370+
371+
A slice expression of the form:
372+
```go
373+
s[a:len(s)]
374+
```
375+
will be simplified to:
376+
```go
377+
s[a:]
378+
```
379+
380+
This is one of the simplifications that "gofmt -s" applies.
381+
382+
Default value: `true`.
383+
367384
### **sortslice**
368385

369386
check the argument type of sort.Slice
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package simplifyslice defines an Analyzer that simplifies slice statements.
6+
// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go
7+
// https://golang.org/cmd/gofmt/#hdr-The_simplify_command
8+
package simplifyslice
9+
10+
import (
11+
"bytes"
12+
"fmt"
13+
"go/ast"
14+
"go/printer"
15+
16+
"golang.org/x/tools/go/analysis"
17+
"golang.org/x/tools/go/analysis/passes/inspect"
18+
"golang.org/x/tools/go/ast/inspector"
19+
)
20+
21+
const Doc = `check for slice simplifications
22+
23+
A slice expression of the form:
24+
s[a:len(s)]
25+
will be simplified to:
26+
s[a:]
27+
28+
This is one of the simplifications that "gofmt -s" applies.`
29+
30+
var Analyzer = &analysis.Analyzer{
31+
Name: "simplifyslice",
32+
Doc: Doc,
33+
Requires: []*analysis.Analyzer{inspect.Analyzer},
34+
Run: run,
35+
}
36+
37+
// Note: We could also simplify slice expressions of the form s[0:b] to s[:b]
38+
// but we leave them as is since sometimes we want to be very explicit
39+
// about the lower bound.
40+
// An example where the 0 helps:
41+
// x, y, z := b[0:2], b[2:4], b[4:6]
42+
// An example where it does not:
43+
// x, y := b[:n], b[n:]
44+
45+
func run(pass *analysis.Pass) (interface{}, error) {
46+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
47+
nodeFilter := []ast.Node{
48+
(*ast.SliceExpr)(nil),
49+
}
50+
inspect.Preorder(nodeFilter, func(n ast.Node) {
51+
expr := n.(*ast.SliceExpr)
52+
// - 3-index slices always require the 2nd and 3rd index
53+
if expr.Max != nil {
54+
return
55+
}
56+
s, ok := expr.X.(*ast.Ident)
57+
// the array/slice object is a single, resolved identifier
58+
if !ok || s.Obj == nil {
59+
return
60+
}
61+
call, ok := expr.High.(*ast.CallExpr)
62+
// the high expression is a function call with a single argument
63+
if !ok || len(call.Args) != 1 || call.Ellipsis.IsValid() {
64+
return
65+
}
66+
fun, ok := call.Fun.(*ast.Ident)
67+
// the function called is "len" and it is not locally defined; and
68+
// because we don't have dot imports, it must be the predefined len()
69+
if !ok || fun.Name != "len" || fun.Obj != nil {
70+
return
71+
}
72+
arg, ok := call.Args[0].(*ast.Ident)
73+
// the len argument is the array/slice object
74+
if !ok || arg.Obj != s.Obj {
75+
return
76+
}
77+
old := expr.High
78+
var b bytes.Buffer
79+
printer.Fprint(&b, pass.Fset, old)
80+
pass.Report(analysis.Diagnostic{
81+
Pos: old.Pos(),
82+
End: old.End(),
83+
Message: fmt.Sprintf("unneeded: %s", b.String()),
84+
SuggestedFixes: []analysis.SuggestedFix{{
85+
Message: fmt.Sprintf("Remove '%s'", b.String()),
86+
TextEdits: []analysis.TextEdit{{
87+
Pos: old.Pos(),
88+
End: old.End(),
89+
NewText: []byte{},
90+
}},
91+
}},
92+
})
93+
expr.High = old
94+
})
95+
return nil, nil
96+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package simplifyslice_test
6+
7+
import (
8+
"testing"
9+
10+
"golang.org/x/tools/go/analysis/analysistest"
11+
"golang.org/x/tools/internal/lsp/analysis/simplifyslice"
12+
)
13+
14+
func Test(t *testing.T) {
15+
testdata := analysistest.TestData()
16+
analysistest.RunWithSuggestedFixes(t, testdata, simplifyslice.Analyzer, "a")
17+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package testdata
6+
7+
var (
8+
a [10]byte
9+
b [20]float32
10+
s []int
11+
t struct {
12+
s []byte
13+
}
14+
15+
_ = a[0:]
16+
_ = a[1:10]
17+
_ = a[2:len(a)] // want "unneeded: len\\(a\\)"
18+
_ = a[3:(len(a))]
19+
_ = a[len(a)-1 : len(a)] // want "unneeded: len\\(a\\)"
20+
_ = a[2:len(a):len(a)]
21+
22+
_ = a[:]
23+
_ = a[:10]
24+
_ = a[:len(a)] // want "unneeded: len\\(a\\)"
25+
_ = a[:(len(a))]
26+
_ = a[:len(a)-1]
27+
_ = a[:len(a):len(a)]
28+
29+
_ = s[0:]
30+
_ = s[1:10]
31+
_ = s[2:len(s)] // want "unneeded: len\\(s\\)"
32+
_ = s[3:(len(s))]
33+
_ = s[len(a) : len(s)-1]
34+
_ = s[0:len(b)]
35+
_ = s[2:len(s):len(s)]
36+
37+
_ = s[:]
38+
_ = s[:10]
39+
_ = s[:len(s)] // want "unneeded: len\\(s\\)"
40+
_ = s[:(len(s))]
41+
_ = s[:len(s)-1]
42+
_ = s[:len(b)]
43+
_ = s[:len(s):len(s)]
44+
45+
_ = t.s[0:]
46+
_ = t.s[1:10]
47+
_ = t.s[2:len(t.s)]
48+
_ = t.s[3:(len(t.s))]
49+
_ = t.s[len(a) : len(t.s)-1]
50+
_ = t.s[0:len(b)]
51+
_ = t.s[2:len(t.s):len(t.s)]
52+
53+
_ = t.s[:]
54+
_ = t.s[:10]
55+
_ = t.s[:len(t.s)]
56+
_ = t.s[:(len(t.s))]
57+
_ = t.s[:len(t.s)-1]
58+
_ = t.s[:len(b)]
59+
_ = t.s[:len(t.s):len(t.s)]
60+
)
61+
62+
func _() {
63+
s := s[0:len(s)] // want "unneeded: len\\(s\\)"
64+
_ = s
65+
}
66+
67+
func m() {
68+
maps := []int{}
69+
_ = maps[1:len(maps)] // want "unneeded: len\\(maps\\)"
70+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package testdata
6+
7+
var (
8+
a [10]byte
9+
b [20]float32
10+
s []int
11+
t struct {
12+
s []byte
13+
}
14+
15+
_ = a[0:]
16+
_ = a[1:10]
17+
_ = a[2:] // want "unneeded: len\\(a\\)"
18+
_ = a[3:(len(a))]
19+
_ = a[len(a)-1:] // want "unneeded: len\\(a\\)"
20+
_ = a[2:len(a):len(a)]
21+
22+
_ = a[:]
23+
_ = a[:10]
24+
_ = a[:] // want "unneeded: len\\(a\\)"
25+
_ = a[:(len(a))]
26+
_ = a[:len(a)-1]
27+
_ = a[:len(a):len(a)]
28+
29+
_ = s[0:]
30+
_ = s[1:10]
31+
_ = s[2:] // want "unneeded: len\\(s\\)"
32+
_ = s[3:(len(s))]
33+
_ = s[len(a) : len(s)-1]
34+
_ = s[0:len(b)]
35+
_ = s[2:len(s):len(s)]
36+
37+
_ = s[:]
38+
_ = s[:10]
39+
_ = s[:] // want "unneeded: len\\(s\\)"
40+
_ = s[:(len(s))]
41+
_ = s[:len(s)-1]
42+
_ = s[:len(b)]
43+
_ = s[:len(s):len(s)]
44+
45+
_ = t.s[0:]
46+
_ = t.s[1:10]
47+
_ = t.s[2:len(t.s)]
48+
_ = t.s[3:(len(t.s))]
49+
_ = t.s[len(a) : len(t.s)-1]
50+
_ = t.s[0:len(b)]
51+
_ = t.s[2:len(t.s):len(t.s)]
52+
53+
_ = t.s[:]
54+
_ = t.s[:10]
55+
_ = t.s[:len(t.s)]
56+
_ = t.s[:(len(t.s))]
57+
_ = t.s[:len(t.s)-1]
58+
_ = t.s[:len(b)]
59+
_ = t.s[:len(t.s):len(t.s)]
60+
)
61+
62+
func _() {
63+
s := s[0:] // want "unneeded: len\\(s\\)"
64+
_ = s
65+
}
66+
67+
func m() {
68+
maps := []int{}
69+
_ = maps[1:] // want "unneeded: len\\(maps\\)"
70+
}

internal/lsp/source/options.go

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"golang.org/x/tools/go/analysis/passes/unusedresult"
3939
"golang.org/x/tools/internal/lsp/analysis/simplifycompositelit"
4040
"golang.org/x/tools/internal/lsp/analysis/simplifyrange"
41+
"golang.org/x/tools/internal/lsp/analysis/simplifyslice"
4142
"golang.org/x/tools/internal/lsp/analysis/unusedparams"
4243
"golang.org/x/tools/internal/lsp/debug/tag"
4344
"golang.org/x/tools/internal/lsp/diff"
@@ -517,5 +518,6 @@ func defaultAnalyzers() map[string]Analyzer {
517518
// gofmt -s suite:
518519
simplifycompositelit.Analyzer.Name: {Analyzer: simplifycompositelit.Analyzer, Enabled: true, HighConfidence: true},
519520
simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, HighConfidence: true},
521+
simplifyslice.Analyzer.Name: {Analyzer: simplifyslice.Analyzer, Enabled: true, HighConfidence: true},
520522
}
521523
}

0 commit comments

Comments
 (0)