Repository: bazelbuild/buildtools
Pull request #542 (2019-02-11 16:37:39)

Fix uninitialized variables warnings
For-loop variables are not guaranteed to be initialized after the loop, but are initialized inside it.


Patch:
diff --git a/warn/warn_control_flow.go b/warn/warn_control_flow.go
index fbd768c..1215aed 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)


Test patch:
diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go
index d02f7da..dc11b00 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 9a5da30..22e4dd6 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) {