diff --git a/warn/warn_control_flow.go b/warn/warn_control_flow.go index fbd768cf5..1215aed14 100644 --- a/warn/warn_control_flow.go +++ b/warn/warn_control_flow.go @@ -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) diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go index d02f7da74..dc11b003a 100644 --- a/warn/warn_control_flow_test.go +++ b/warn/warn_control_flow_test.go @@ -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) } diff --git a/warn/warn_docstring_test.go b/warn/warn_docstring_test.go index 9a5da30cd..22e4dd6e4 100644 --- a/warn/warn_docstring_test.go +++ b/warn/warn_docstring_test.go @@ -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""" @@ -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) {