Skip to content

Commit

Permalink
Fix uninitialized variables warnings (#542)
Browse files Browse the repository at this point in the history
  • Loading branch information
vladmos authored Feb 11, 2019
1 parent aa1408f commit a17901e
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 9 deletions.
13 changes: 11 additions & 2 deletions warn/warn_control_flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,21 @@ func findUninitializedVariables(stmts []build.Expr, previouslyInitialized map[st
return true, locallyInitialized
case *build.ForStmt:
// Although loop variables are defined as local variables, buildifier doesn't know whether
// the loop will be empty or not
// the collection will be empty or not.

// Traverse but ignore the result. Even if something is defined inside a for-loop, the loop
// may be empty and the variable initialization may not happen.
findUninitializedIdents(stmt.X, callback)
findUninitializedVariables(stmt.Body, initialized, callback)

// The loop can access the variables defined above, and also the for-loop variables.
initializedInLoop := make(map[string]bool)
for name := range initialized {
initializedInLoop[name] = true
}
for _, ident := range bzlenv.CollectLValues(stmt.Vars) {
initializedInLoop[ident.Name] = true
}
findUninitializedVariables(stmt.Body, initializedInLoop, callback)
continue
case *build.IfStmt:
findUninitializedIdents(stmt.Cond, callback)
Expand Down
28 changes: 28 additions & 0 deletions warn/warn_control_flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,4 +641,32 @@ def foo(y):
":9: Variable \"x\" may not have been initialized.",
},
scopeEverywhere)

checkFindings(t, "uninitialized", `
def foo():
for x in y:
print(x)
print(x.attr)
print(x)
print(x.attr)
`,
[]string{
":6: Variable \"x\" may not have been initialized.",
":7: Variable \"x\" may not have been initialized.",
},
scopeEverywhere)

checkFindings(t, "uninitialized", `
def foo():
for x in y:
a = x
print(a)
print(a)
`,
[]string{
":6: Variable \"a\" may not have been initialized.",
},
scopeEverywhere)
}
14 changes: 7 additions & 7 deletions warn/warn_docstring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def _f(x):
[]string{},
scopeEverywhere)

checkFindings(t, "function-docstring", `
checkFindings(t, "function-docstring", `
def _f(x):
"""Long private function
with a docstring"""
Expand All @@ -112,12 +112,12 @@ def _f(x):
x %= 5
return x
`,
[]string{
`:2: The docstring for the function "_f" should start with a one-line summary.`,
`:2: Argument "x" is not documented.`,
`:2: Return value of "_f" is not documented.`,
},
scopeEverywhere)
[]string{
`:2: The docstring for the function "_f" should start with a one-line summary.`,
`:2: Argument "x" is not documented.`,
`:2: Return value of "_f" is not documented.`,
},
scopeEverywhere)
}

func TestFunctionDocstringFormat(t *testing.T) {
Expand Down

0 comments on commit a17901e

Please sign in to comment.