Fix forbidding of If statements in BUILD files

The bug also permitted For statements in BUILD files so long as they were contained within an If statement (i.e. not at the top level).

Also add minor guidance to error messages.

RELNOTES: None
PiperOrigin-RevId: 182236172

1 files changed, 40 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index cbe6094d14..38a478120e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -327,32 +327,54 @@ public final class ValidationEnvironment extends SyntaxTreeVisitor {
     // TODO(laurentlb): Merge with the visitor above when possible (i.e. when BUILD files use it).
     SyntaxTreeVisitor checker =
         new SyntaxTreeVisitor() {
+
+          private void error(ASTNode node, String message) {
+            eventHandler.handle(Event.error(node.getLocation(), message));
+            success[0] = false;
+          }
+
+          @Override
+          public void visit(FunctionDefStatement node) {
+            error(
+                node,
+                "function definitions are not allowed in BUILD files. You may move the function to "
+                    + "a .bzl file and load it.");
+          }
+
+          @Override
+          public void visit(ForStatement node) {
+            error(
+                node,
+                "for statements are not allowed in BUILD files. You may inline the loop, move it "
+                    + "to a function definition (in a .bzl file), or as a last resort use a list "
+                    + "comprehension.");
+          }
+
+          @Override
+          public void visit(IfStatement node) {
+            error(
+                node,
+                "if statements are not allowed in BUILD files. You may move conditional logic to a "
+                    + "function definition (in a .bzl file), or for simple cases use an if "
+                    + "expression.");
+          }
+
           @Override
           public void visit(FuncallExpression node) {
             for (Argument.Passed arg : node.getArguments()) {
               if (arg.isStarStar()) {
-                eventHandler.handle(
-                    Event.error(
-                        node.getLocation(), "**kwargs arguments are not allowed in BUILD files"));
-                success[0] = false;
+                error(
+                    node,
+                    "**kwargs arguments are not allowed in BUILD files. Pass the arguments in "
+                        + "explicitly.");
               } else if (arg.isStar()) {
-                eventHandler.handle(
-                    Event.error(
-                        node.getLocation(), "*args arguments are not allowed in BUILD files"));
-                success[0] = false;
+                error(
+                    node,
+                    "*args arguments are not allowed in BUILD files. Pass the arguments in "
+                        + "explicitly.");
               }
             }
           }
-
-          @Override
-          public void visit(FunctionDefStatement node) {
-            eventHandler.handle(
-                Event.error(
-                    node.getLocation(),
-                    "syntax error at 'def': This is not supported in BUILD files. "
-                        + "Move the block to a .bzl file and load it"));
-            success[0] = false;
-          }
         };
     checker.visitAll(statements);
     return success[0];