From 94fe02cb5c65d0ebe9da5374c16138457ea8184e Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Sun, 15 Mar 2020 12:51:33 -0400 Subject: [PATCH] internal/lsp/analysis: add simplify-range pass from "gofmt -s" This is one of the simplifications that "gofmt -s" applies. https://golang.org/cmd/gofmt/#hdr-The_simplify_command A range of the form: for x, _ = range v {...} will be simplified to: for x = range v {...} A range of the form: for _ = range v {...} will be simplified to: for range v {...} Updates golang/go#37221 Change-Id: Ic6babbd0b8ab961ebb4d0d6df72df52d9acde6e7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/223661 Run-TryBot: Rohan Challa TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick Reviewed-by: Michael Matloob Reviewed-by: Rebecca Stambler --- gopls/doc/analyzers.md | 26 ++++ .../analysis/simplifyrange/simplifyrange.go | 114 ++++++++++++++++++ .../simplifyrange/simplifyrange_test.go | 17 +++ .../simplifyrange/testdata/src/a/a.go | 16 +++ .../simplifyrange/testdata/src/a/a.go.golden | 16 +++ internal/lsp/source/options.go | 2 + 6 files changed, 191 insertions(+) create mode 100644 internal/lsp/analysis/simplifyrange/simplifyrange.go create mode 100644 internal/lsp/analysis/simplifyrange/simplifyrange_test.go create mode 100644 internal/lsp/analysis/simplifyrange/testdata/src/a/a.go create mode 100644 internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index f5dd8468f8..77bc4a80c1 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -321,6 +321,32 @@ errors is discouraged. Default value: `true`. +### **simplifyrange** + +check for range statement simplifications + +A range of the form: +```go +for x, _ = range v {...} +``` +will be simplified to: +```go +for x = range v {...} +``` + +A range of the form: +```go +for _ = range v {...} +``` +will be simplified to: +```go +for range v {...} +``` + +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/simplifyrange/simplifyrange.go b/internal/lsp/analysis/simplifyrange/simplifyrange.go new file mode 100644 index 0000000000..e53f50d2f1 --- /dev/null +++ b/internal/lsp/analysis/simplifyrange/simplifyrange.go @@ -0,0 +1,114 @@ +// 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 simplifyrange defines an Analyzer that simplifies range statements. +// https://golang.org/cmd/gofmt/#hdr-The_simplify_command +// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go +package simplifyrange + +import ( + "bytes" + "go/ast" + "go/printer" + "go/token" + + "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 range statement simplifications + +A range of the form: + for x, _ = range v {...} +will be simplified to: + for x = range v {...} + +A range of the form: + for _ = range v {...} +will be simplified to: + for range v {...} + +This is one of the simplifications that "gofmt -s" applies.` + +var Analyzer = &analysis.Analyzer{ + Name: "simplifyrange", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.RangeStmt)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + stmt := n.(*ast.RangeStmt) + end := newlineIndex(pass.Fset, stmt) + var old ast.Expr + // Range statements of the form: for i, _ := range x {} + if isBlank(stmt.Value) { + old = stmt.Value + defer func() { + stmt.Value = old + }() + stmt.Value = nil + } + // Range statements of the form: for _ := range x {} + if isBlank(stmt.Key) && stmt.Value == nil { + old = stmt.Key + defer func() { + stmt.Key = old + }() + stmt.Key = nil + } + // Return early if neither if condition is met. + if old == nil { + return + } + pass.Report(analysis.Diagnostic{ + Pos: old.Pos(), + End: old.End(), + Message: "simplify range expression", + SuggestedFixes: suggestedFixes(pass.Fset, stmt, end), + }) + }) + return nil, nil +} + +func suggestedFixes(fset *token.FileSet, rng *ast.RangeStmt, end token.Pos) []analysis.SuggestedFix { + var b bytes.Buffer + printer.Fprint(&b, fset, rng) + stmt := b.Bytes() + index := bytes.Index(stmt, []byte("\n")) + // If there is a new line character, then don't replace the body. + if index != -1 { + stmt = stmt[:index] + } + return []analysis.SuggestedFix{{ + Message: "Remove empty value", + TextEdits: []analysis.TextEdit{{ + Pos: rng.Pos(), + End: end, + NewText: stmt[:index], + }}, + }} +} + +func newlineIndex(fset *token.FileSet, rng *ast.RangeStmt) token.Pos { + var b bytes.Buffer + printer.Fprint(&b, fset, rng) + contents := b.Bytes() + index := bytes.Index(contents, []byte("\n")) + if index == -1 { + return rng.End() + } + return rng.Pos() + token.Pos(index) +} + +func isBlank(x ast.Expr) bool { + ident, ok := x.(*ast.Ident) + return ok && ident.Name == "_" +} diff --git a/internal/lsp/analysis/simplifyrange/simplifyrange_test.go b/internal/lsp/analysis/simplifyrange/simplifyrange_test.go new file mode 100644 index 0000000000..ecc7a96925 --- /dev/null +++ b/internal/lsp/analysis/simplifyrange/simplifyrange_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 simplifyrange_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/simplifyrange" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, simplifyrange.Analyzer, "a") +} diff --git a/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go b/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go new file mode 100644 index 0000000000..49face1e96 --- /dev/null +++ b/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go @@ -0,0 +1,16 @@ +// 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 + +import "log" + +func m() { + maps := make(map[string]string) + for k, _ := range maps { // want "simplify range expression" + log.Println(k) + } + for _ = range maps { // want "simplify range expression" + } +} diff --git a/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden b/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..ec8490ab33 --- /dev/null +++ b/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden @@ -0,0 +1,16 @@ +// 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 + +import "log" + +func m() { + maps := make(map[string]string) + for k := range maps { // want "simplify range expression" + log.Println(k) + } + for range maps { // want "simplify range expression" + } +} diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index b70b3ba1f1..363f42636f 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -36,6 +36,7 @@ import ( "golang.org/x/tools/go/analysis/passes/unreachable" "golang.org/x/tools/go/analysis/passes/unsafeptr" "golang.org/x/tools/go/analysis/passes/unusedresult" + "golang.org/x/tools/internal/lsp/analysis/simplifyrange" "golang.org/x/tools/internal/lsp/debug/tag" "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/diff/myers" @@ -511,5 +512,6 @@ func defaultAnalyzers() map[string]Analyzer { testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true}, // gofmt -s suite: + simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, HighConfidence: true}, } }