Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9095e9b

Browse files
adonovangopherbot
authored andcommitted
internal/analysisinternal: extract DeleteVar
This CL extracts the "delete variable" operation from theunusedvariable analyzer to a new library function.The implementation is completely rewritten to handlethe surprising number of edge cases more systematically,and to make greater use of the library, notably:- Cursor instead of PathEnclosingInterval;- analysisinternal.DeleteStmt instead of its own deleteStmtFromBlock algorithm; and- typesinternal.NoEffects (formerly modernize.noeffects) instead of its own weaker algorithm.It also avoids cloning/mutating/formatting syntax trees,which we have learned is not a robust way to compute edits.The change also includes a comprehensive test suite.The existing analysisinternal.DeleteStmt algorithm preservestrailing line comments in places where the DeleteVar logicwants to remove them. I think DeleteStmt needs to change,but to avoid making yet more subtle behavior changes inthis CL, I've commented out 3 test cases until I candiscuss it with pjw.Change-Id: Ie53383361d598eaa12e68f20b4a6188b6f218bdbReviewed-on:https://go-review.googlesource.com/c/tools/+/709055LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>Auto-Submit: Alan Donovan <adonovan@google.com>Reviewed-by: Robert Findley <rfindley@google.com>
1 parent62a1b26 commit9095e9b

File tree

11 files changed

+791
-352
lines changed

11 files changed

+791
-352
lines changed

‎go/analysis/passes/modernize/modernize.go‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -151,44 +151,6 @@ var (
151151
omitemptyRegex=regexp.MustCompile(`(?:^json| json):"[^"]*(,omitempty)(?:"|,[^"]*")\s?`)
152152
)
153153

154-
// noEffects reports whether the expression has no side effects, i.e., it
155-
// does not modify the memory state. This function is conservative: it may
156-
// return false even when the expression has no effect.
157-
funcnoEffects(info*types.Info,expr ast.Expr)bool {
158-
noEffects:=true
159-
ast.Inspect(expr,func(n ast.Node)bool {
160-
switchv:=n.(type) {
161-
casenil,*ast.Ident,*ast.BasicLit,*ast.BinaryExpr,*ast.ParenExpr,
162-
*ast.SelectorExpr,*ast.IndexExpr,*ast.SliceExpr,*ast.TypeAssertExpr,
163-
*ast.StarExpr,*ast.CompositeLit,*ast.ArrayType,*ast.StructType,
164-
*ast.MapType,*ast.InterfaceType,*ast.KeyValueExpr:
165-
// No effect
166-
case*ast.UnaryExpr:
167-
// Channel send <-ch has effects
168-
ifv.Op==token.ARROW {
169-
noEffects=false
170-
}
171-
case*ast.CallExpr:
172-
// Type conversion has no effects
173-
if!info.Types[v.Fun].IsType() {
174-
// TODO(adonovan): Add a case for built-in functions without side
175-
// effects (by using callsPureBuiltin from tools/internal/refactor/inline)
176-
177-
noEffects=false
178-
}
179-
case*ast.FuncLit:
180-
// A FuncLit has no effects, but do not descend into it.
181-
returnfalse
182-
default:
183-
// All other expressions have effects
184-
noEffects=false
185-
}
186-
187-
returnnoEffects
188-
})
189-
returnnoEffects
190-
}
191-
192154
// lookup returns the symbol denoted by name at the position of the cursor.
193155
funclookup(info*types.Info,cur inspector.Cursor,namestring) types.Object {
194156
scope:=analysisinternal.EnclosingScope(info,cur)

‎go/analysis/passes/modernize/reflect.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func reflecttypefor(pass *analysis.Pass) (any, error) {
4949
// Have: reflect.TypeOf(expr)
5050

5151
expr:=call.Args[0]
52-
if!noEffects(info,expr) {
52+
if!typesinternal.NoEffects(info,expr) {
5353
continue// don't eliminate operand: may have effects
5454
}
5555

‎go/analysis/passes/modernize/slicesdelete.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"golang.org/x/tools/internal/analysisinternal"
1717
"golang.org/x/tools/internal/analysisinternal/generated"
1818
"golang.org/x/tools/internal/astutil"
19+
"golang.org/x/tools/internal/typesinternal"
1920
)
2021

2122
// Warning: this analyzer is not safe to enable by default (not nil-preserving).
@@ -141,7 +142,8 @@ func slicesdelete(pass *analysis.Pass) (any, error) {
141142
slice2,ok2:=call.Args[1].(*ast.SliceExpr)
142143
ifok1&&slice1.Low==nil&&!slice1.Slice3&&
143144
ok2&&slice2.High==nil&&!slice2.Slice3&&
144-
astutil.EqualSyntax(slice1.X,slice2.X)&&noEffects(info,slice1.X)&&
145+
astutil.EqualSyntax(slice1.X,slice2.X)&&
146+
typesinternal.NoEffects(info,slice1.X)&&
145147
increasingSliceIndices(info,slice1.High,slice2.Low) {
146148
// Have append(s[:a], s[b:]...) where we can verify a < b.
147149
report(file,call,slice1,slice2)

‎gopls/internal/analysis/unusedvariable/testdata/src/assign/a.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,10 @@ func sideEffects(cBool chan bool, cInt chan int) {
5555
f:=map[int]int{// want `declared (and|but) not used`
5656
fInt():<-cInt,
5757
}
58-
g:= []int{<-cInt}// want `declared (and|but) not used`
59-
h:=func(sstring) {}// want `declared (and|but) not used`
58+
g:= []int{<-cInt}// want `declared (and|but) not used`
59+
h:=func(sstring) {}// want `declared (and|but) not used`
60+
61+
// (ill-typed)
6062
i:=func(sstring) {}()// want `declared (and|but) not used`
6163
}
6264

‎gopls/internal/analysis/unusedvariable/testdata/src/assign/a.go.golden‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@ type A struct {
1414
}
1515

1616
func singleAssignment() {
17+
// want `declared (and|but) not used`
18+
1719
if 1 == 1 {
20+
// want `declared (and|but) not used`
1821
}
1922

2023
panic("I should survive")
2124
}
2225

2326
func noOtherStmtsInBlock() {
27+
// want `declared (and|but) not used`
2428
}
2529

2630
func partOfMultiAssignment() {
@@ -29,32 +33,39 @@ func partOfMultiAssignment() {
2933
}
3034

3135
func sideEffects(cBool chan bool, cInt chan int) {
32-
<-c // want `declared (and|but) not used`
33-
fmt.Sprint("") // want `declared (and|but) not used`
34-
A{ // want `declared (and|but) not used`
36+
_ =<-c // want `declared (and|but) not used`
37+
_ =fmt.Sprint("") // want `declared (and|but) not used`
38+
_ =A{ // want `declared (and|but) not used`
3539
b: func() int {
3640
return 1
3741
}(),
3842
}
39-
A{<-cInt} // want `declared (and|but) not used`
40-
fInt() + <-cInt // want `declared (and|but) not used`
41-
fBool() && <-cBool // want `declared (and|but) not used`
42-
map[int]int{ // want `declared (and|but) not used`
43+
_ =A{<-cInt} // want `declared (and|but) not used`
44+
_ =fInt() + <-cInt // want `declared (and|but) not used`
45+
_ =fBool() && <-cBool // want `declared (and|but) not used`
46+
_ =map[int]int{ // want `declared (and|but) not used`
4347
fInt(): <-cInt,
4448
}
45-
[]int{<-cInt} // want `declared (and|but) not used`
46-
func(s string) {}() // want `declared (and|but) not used`
49+
_ = []int{<-cInt} // want `declared (and|but) not used`
50+
// want `declared (and|but) not used`
51+
52+
// (ill-typed)
53+
_ = func(s string) {}() // want `declared (and|but) not used`
4754
}
4855

4956
func commentAbove() {
5057
// v is a variable
58+
// want `declared (and|but) not used`
5159
}
5260

5361
func commentBelow() {
62+
// want `declared (and|but) not used`
5463
// v is a variable
5564
}
5665

5766
func commentSpaceBelow() {
67+
// want `declared (and|but) not used`
68+
5869
// v is a variable
5970
}
6071

‎gopls/internal/analysis/unusedvariable/testdata/src/decl/a.go.golden‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,19 @@ func a() {
99
panic(c)
1010

1111
if 1 == 1 {
12+
// want `declared (and|but) not used`
1213
}
1314
}
1415

1516
func b() {
1617
// b is a variable
18+
// want `declared (and|but) not used`
1719
}
1820

1921
func c() {
2022
var (
2123
d string
2224
)
25+
2326
panic(d)
2427
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp