diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index 38b955e3bdb7ab..3ed2eb3bae24f0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.DotBazelFileSyntaxChecker; import com.google.devtools.build.lib.packages.StarlarkExportable; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; @@ -277,10 +278,11 @@ private ModuleFileGlobals execModuleFile( ModuleFileGlobals moduleFileGlobals = new ModuleFileGlobals(builtinModules, moduleKey, registry, ignoreDevDeps); try (Mutability mu = Mutability.create("module file", moduleKey)) { + new DotBazelFileSyntaxChecker("MODULE.bazel files", /* canLoadBzl= */ false) + .check(starlarkFile); net.starlark.java.eval.Module predeclaredEnv = getPredeclaredEnv(moduleFileGlobals, starlarkSemantics); Program program = Program.compileFile(starlarkFile, predeclaredEnv); - // TODO(wyv): check that `program` has no `def`, `if`, etc StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics); if (printIsNoop) { thread.setPrintHandler((t, msg) -> {}); diff --git a/src/main/java/com/google/devtools/build/lib/packages/DotBazelFileSyntaxChecker.java b/src/main/java/com/google/devtools/build/lib/packages/DotBazelFileSyntaxChecker.java new file mode 100644 index 00000000000000..ac34c8e0508861 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/DotBazelFileSyntaxChecker.java @@ -0,0 +1,150 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package com.google.devtools.build.lib.packages; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile; +import net.starlark.java.syntax.Argument; +import net.starlark.java.syntax.CallExpression; +import net.starlark.java.syntax.DefStatement; +import net.starlark.java.syntax.ForStatement; +import net.starlark.java.syntax.IfStatement; +import net.starlark.java.syntax.LambdaExpression; +import net.starlark.java.syntax.LoadStatement; +import net.starlark.java.syntax.Location; +import net.starlark.java.syntax.NodeVisitor; +import net.starlark.java.syntax.StarlarkFile; +import net.starlark.java.syntax.SyntaxError; + +/** + * A {@link NodeVisitor} that can be used to check that a Starlark AST conforms to the restricted + * syntax that BUILD, WORKSPACE, REPO.bazel, and MODULE.bazel files use. This restricted syntax + * disallows: + * + * + */ +@ThreadHostile +public class DotBazelFileSyntaxChecker extends NodeVisitor { + private final String where; + private final boolean canLoadBzl; + private ImmutableList.Builder errors = ImmutableList.builder(); + + /** + * @param where describes the type of file being checked. + * @param canLoadBzl whether the file type being check supports load statements. This is used to + * generate more informative error messages. + */ + public DotBazelFileSyntaxChecker(String where, boolean canLoadBzl) { + this.where = where; + this.canLoadBzl = canLoadBzl; + } + + public final void check(StarlarkFile file) throws SyntaxError.Exception { + this.errors = ImmutableList.builder(); + visit(file); + ImmutableList errors = this.errors.build(); + if (!errors.isEmpty()) { + throw new SyntaxError.Exception(errors); + } + } + + protected void error(Location loc, String message) { + errors.add(new SyntaxError(loc, message)); + } + + // Reject f(*args) and f(**kwargs) calls. + private void rejectStarArgs(CallExpression call) { + for (Argument arg : call.getArguments()) { + if (arg instanceof Argument.StarStar) { + error( + arg.getStartLocation(), + "**kwargs arguments are not allowed in " + + where + + ". Pass the arguments in explicitly."); + } else if (arg instanceof Argument.Star) { + error( + arg.getStartLocation(), + "*args arguments are not allowed in " + where + ". Pass the arguments in explicitly."); + } + } + } + + @Override + public void visit(LoadStatement node) { + if (!canLoadBzl) { + error(node.getStartLocation(), "`load` statements may not be used in " + where); + } + } + + // We prune the traversal if we encounter disallowed keywords, as we have already reported the + // root error and there's no point reporting more. + + @Override + public void visit(DefStatement node) { + error( + node.getStartLocation(), + "functions may not be defined in " + + where + + (canLoadBzl ? ". You may move the function to a .bzl file and load it." : ".")); + } + + @Override + public void visit(LambdaExpression node) { + error( + node.getStartLocation(), + "functions may not be defined in " + + where + + (canLoadBzl ? ". You may move the function to a .bzl file and load it." : ".")); + } + + @Override + public void visit(ForStatement node) { + error( + node.getStartLocation(), + "`for` statements are not allowed in " + + where + + ". You may inline the loop" + + (canLoadBzl ? ", 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.getStartLocation(), + "`if` statements are not allowed in " + + where + + ". You may" + + (canLoadBzl + ? " move conditional logic to a function definition (in a .bzl file), or" + : "") + + " use an `if` expression for simple cases."); + } + + @Override + public void visit(CallExpression node) { + rejectStarArgs(node); + // Continue traversal so as not to miss nested calls + // like cc_binary(..., f(**kwargs), ...). + super.visit(node); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index daa73c462e3ca3..ab4f71523ab094 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -50,7 +50,6 @@ import java.util.Optional; import java.util.OptionalLong; import java.util.concurrent.ForkJoinPool; -import java.util.function.Consumer; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; import net.starlark.java.eval.Mutability; @@ -61,16 +60,11 @@ import net.starlark.java.eval.StarlarkThread; import net.starlark.java.syntax.Argument; import net.starlark.java.syntax.CallExpression; -import net.starlark.java.syntax.DefStatement; import net.starlark.java.syntax.Expression; -import net.starlark.java.syntax.ForStatement; import net.starlark.java.syntax.Identifier; -import net.starlark.java.syntax.IfStatement; import net.starlark.java.syntax.IntLiteral; -import net.starlark.java.syntax.LambdaExpression; import net.starlark.java.syntax.ListExpression; import net.starlark.java.syntax.Location; -import net.starlark.java.syntax.NodeVisitor; import net.starlark.java.syntax.Program; import net.starlark.java.syntax.StarlarkFile; import net.starlark.java.syntax.StringLiteral; @@ -484,8 +478,8 @@ private void executeBuildFileImpl( /** * checkBuildSyntax is a static pass over the syntax tree of a BUILD (not .bzl) file. * - *

It reports an error to the event handler if it discovers a {@code def}, {@code if}, or - * {@code for} statement, or a {@code f(*args)} or {@code f(**kwargs)} call. + *

It throws a {@link SyntaxError.Exception} if it discovers disallowed elements (see {@link + * DotBazelFileSyntaxChecker}). * *

It extracts literal {@code glob(include="pattern")} patterns and adds them to {@code globs}, * or to {@code globsWithDirs} if the call had a {@code exclude_directories=0} argument. @@ -493,159 +487,92 @@ private void executeBuildFileImpl( *

It records in {@code generatorNameByLocation} all calls of the form {@code f(name="foo", * ...)} so that any rules instantiated during the call to {@code f} can be ascribed a "generator * name" of {@code "foo"}. - * - *

It returns true if it reported no errors. */ // TODO(adonovan): restructure so that this is called from the sole place that executes BUILD // files. Also, make private; there's no reason for tests to call this directly. - public static boolean checkBuildSyntax( + public static void checkBuildSyntax( StarlarkFile file, Collection globs, Collection globsWithDirs, Collection subpackages, - Map generatorNameByLocation, - Consumer errors) { - final boolean[] success = {true}; - NodeVisitor checker = - new NodeVisitor() { - void error(Location loc, String message) { - errors.accept(new SyntaxError(loc, message)); - success[0] = false; + Map generatorNameByLocation) + throws SyntaxError.Exception { + new DotBazelFileSyntaxChecker("BUILD files", /* canLoadBzl= */ true) { + // Extract literal glob patterns from calls of the form: + // glob(include = ["pattern"]) + // glob(["pattern"]) + // subpackages(include = ["pattern"]) + // This may spuriously match user-defined functions named glob or + // subpackages; that's ok, it's only a heuristic. + void extractGlobPatterns(CallExpression call) { + if (call.getFunction() instanceof Identifier) { + String functionName = ((Identifier) call.getFunction()).getName(); + if (!functionName.equals("glob") && !functionName.equals("subpackages")) { + return; } - // Extract literal glob patterns from calls of the form: - // glob(include = ["pattern"]) - // glob(["pattern"]) - // subpackages(include = ["pattern"]) - // This may spuriously match user-defined functions named glob or - // subpackages; that's ok, it's only a heuristic. - void extractGlobPatterns(CallExpression call) { - if (call.getFunction() instanceof Identifier) { - String functionName = ((Identifier) call.getFunction()).getName(); - if (!functionName.equals("glob") && !functionName.equals("subpackages")) { - return; + Expression excludeDirectories = null; + Expression include = null; + ImmutableList arguments = call.getArguments(); + for (int i = 0; i < arguments.size(); i++) { + Argument arg = arguments.get(i); + String name = arg.getName(); + if (name == null) { + if (i == 0) { // first positional argument + include = arg.getValue(); } - - Expression excludeDirectories = null; - Expression include = null; - ImmutableList arguments = call.getArguments(); - for (int i = 0; i < arguments.size(); i++) { - Argument arg = arguments.get(i); - String name = arg.getName(); - if (name == null) { - if (i == 0) { // first positional argument - include = arg.getValue(); + } else if (name.equals("include")) { + include = arg.getValue(); + } else if (name.equals("exclude_directories")) { + excludeDirectories = arg.getValue(); + } + } + if (include instanceof ListExpression) { + for (Expression elem : ((ListExpression) include).getElements()) { + if (elem instanceof StringLiteral) { + String pattern = ((StringLiteral) elem).getValue(); + // exclude_directories is (oddly) an int with default 1. + boolean exclude = true; + if (excludeDirectories instanceof IntLiteral) { + Number v = ((IntLiteral) excludeDirectories).getValue(); + if (v instanceof Integer && (Integer) v == 0) { + exclude = false; } - } else if (name.equals("include")) { - include = arg.getValue(); - } else if (name.equals("exclude_directories")) { - excludeDirectories = arg.getValue(); } - } - if (include instanceof ListExpression) { - for (Expression elem : ((ListExpression) include).getElements()) { - if (elem instanceof StringLiteral) { - String pattern = ((StringLiteral) elem).getValue(); - // exclude_directories is (oddly) an int with default 1. - boolean exclude = true; - if (excludeDirectories instanceof IntLiteral) { - Number v = ((IntLiteral) excludeDirectories).getValue(); - if (v instanceof Integer && (Integer) v == 0) { - exclude = false; - } - } - if (functionName.equals("glob")) { - (exclude ? globs : globsWithDirs).add(pattern); - } else { - subpackages.add(pattern); - } - } + if (functionName.equals("glob")) { + (exclude ? globs : globsWithDirs).add(pattern); + } else { + subpackages.add(pattern); } } } } + } + } - // Reject f(*args) and f(**kwargs) calls in BUILD files. - void rejectStarArgs(CallExpression call) { - for (Argument arg : call.getArguments()) { - if (arg instanceof Argument.StarStar) { - error( - arg.getStartLocation(), - "**kwargs arguments are not allowed in BUILD files. Pass the arguments in " - + "explicitly."); - } else if (arg instanceof Argument.Star) { - error( - arg.getStartLocation(), - "*args arguments are not allowed in BUILD files. Pass the arguments in " - + "explicitly."); - } - } - } - - // Record calls of the form f(name="foo", ...) - // so that we can later ascribe "foo" as the "generator name" - // of any rules instantiated during the call of f. - void recordGeneratorName(CallExpression call) { - for (Argument arg : call.getArguments()) { - if (arg instanceof Argument.Keyword - && arg.getName().equals("name") - && arg.getValue() instanceof StringLiteral) { - generatorNameByLocation.put( - call.getLparenLocation(), ((StringLiteral) arg.getValue()).getValue()); - } - } - } - - // We prune the traversal if we encounter def/if/for, - // as we have already reported the root error and there's - // no point reporting more. - - @Override - public void visit(DefStatement node) { - error( - node.getStartLocation(), - "functions may not be defined in BUILD files. You may move the function to " - + "a .bzl file and load it."); - } - - @Override - public void visit(LambdaExpression node) { - error( - node.getStartLocation(), - "functions may not be defined in BUILD files. You may move the function to " - + "a .bzl file and load it."); - } - - @Override - public void visit(ForStatement node) { - error( - node.getStartLocation(), - "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.getStartLocation(), - "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."); + // Record calls of the form f(name="foo", ...) + // so that we can later ascribe "foo" as the "generator name" + // of any rules instantiated during the call of f. + void recordGeneratorName(CallExpression call) { + for (Argument arg : call.getArguments()) { + if (arg instanceof Argument.Keyword + && arg.getName().equals("name") + && arg.getValue() instanceof StringLiteral) { + generatorNameByLocation.put( + call.getLparenLocation(), ((StringLiteral) arg.getValue()).getValue()); } + } + } - @Override - public void visit(CallExpression node) { - extractGlobPatterns(node); - rejectStarArgs(node); - recordGeneratorName(node); - // Continue traversal so as not to miss nested calls - // like cc_binary(..., f(**kwargs), srcs=glob(...), ...). - super.visit(node); - } - }; - checker.visit(file); - return success[0]; + @Override + public void visit(CallExpression node) { + extractGlobPatterns(node); + recordGeneratorName(node); + // Continue traversal so as not to miss nested calls + // like cc_binary(..., f(**kwargs), srcs=glob(...), ...). + super.visit(node); + } + }.check(file); } // Install profiler hooks into Starlark interpreter. diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 5f9c3c68a55cdb..7ba40e29c9c7ba 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -24,9 +24,7 @@ import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading; import com.google.devtools.build.lib.vfs.Path; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; @@ -115,6 +113,7 @@ public void execute( StoredEventHandler localReporter = new StoredEventHandler(); try { // compile + new DotBazelFileSyntaxChecker("WORKSPACE files", /* canLoadBzl= */ true).check(file); Program prog = Program.compileFile(file, module); // create thread @@ -131,31 +130,18 @@ public void execute( // are, by definition, not in an external repository and so they don't need the mapping new BazelStarlarkContext( BazelStarlarkContext.Phase.WORKSPACE, - /*toolsRepository=*/ null, - /*fragmentNameToClass=*/ null, + /* toolsRepository= */ null, + /* fragmentNameToClass= */ null, new SymbolGenerator<>(workspaceFileKey), - /*analysisRuleLabel=*/ null, - /*networkAllowlistForTests=*/ null) + /* analysisRuleLabel= */ null, + /* networkAllowlistForTests= */ null) .storeInThread(thread); - List globs = new ArrayList<>(); // unused - if (PackageFactory.checkBuildSyntax( - file, - /*globs=*/ globs, - /*globsWithDirs=*/ globs, - /*subpackages=*/ globs, - new HashMap<>(), - error -> - localReporter.handle( - Package.error( - error.location(), error.message(), PackageLoading.Code.SYNTAX_ERROR)))) { - try { - Starlark.execFileProgram(prog, module, thread); - } catch (EvalException ex) { - localReporter.handle( - Package.error( - null, ex.getMessageWithStack(), PackageLoading.Code.STARLARK_EVAL_ERROR)); - } + try { + Starlark.execFileProgram(prog, module, thread); + } catch (EvalException ex) { + localReporter.handle( + Package.error(null, ex.getMessageWithStack(), PackageLoading.Code.STARLARK_EVAL_ERROR)); } // Accumulate the global bindings created by this chunk of the WORKSPACE file, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index f3ca4d71a1a832..67becce8625721 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -1520,10 +1520,10 @@ private CompiledBuildFile compileBuildFile( Set globsWithDirs = new HashSet<>(); Set subpackages = new HashSet<>(); Map generatorMap = new HashMap<>(); - ImmutableList.Builder errors = ImmutableList.builder(); - if (!PackageFactory.checkBuildSyntax( - file, globs, globsWithDirs, subpackages, generatorMap, errors::add)) { - return new CompiledBuildFile(errors.build()); + try { + PackageFactory.checkBuildSyntax(file, globs, globsWithDirs, subpackages, generatorMap); + } catch (SyntaxError.Exception ex) { + return new CompiledBuildFile(ex.errors()); } // Load (optional) prelude, which determines environment. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java index 4d77052f11c935..912dfc4b1975c9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; +import com.google.devtools.build.lib.packages.DotBazelFileSyntaxChecker; import com.google.devtools.build.lib.packages.LabelConverter; import com.google.devtools.build.lib.packages.PackageArgs; import com.google.devtools.build.lib.packages.RepoThreadContext; @@ -147,11 +148,12 @@ private PackageArgs evalRepoFile( ExtendedEventHandler handler) throws RepoFileFunctionException, InterruptedException { try (Mutability mu = Mutability.create("repo file", repoName)) { + new DotBazelFileSyntaxChecker("REPO.bazel files", /* canLoadBzl= */ false) + .check(starlarkFile); Module predeclared = Module.withPredeclared( starlarkSemantics, starlarkEnv.getStarlarkGlobals().getRepoToplevels()); Program program = Program.compileFile(starlarkFile, predeclared); - // TODO(wyv): check that `program` has no `def`, `if`, etc StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics); thread.setPrintHandler(Event.makeDebugPrintHandler(handler)); RepoThreadContext context = diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 782ce8b53bc724..aa7126deed6038 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -1047,6 +1047,22 @@ public void module_calledLate() throws Exception { assertContainsEvent("if module() is called, it must be called before any other functions"); } + @Test + public void restrictedSyntax() throws Exception { + scratch.file( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + "if 3+5>7: module(name='aaa',version='0.1',repo_name='bbb')"); + FakeRegistry registry = registryFactory.newFakeRegistry("/foo"); + ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); + + reporter.removeHandler(failFastHandler); // expect failures + evaluator.evaluate(ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + + assertContainsEvent( + "`if` statements are not allowed in MODULE.bazel files. You may use an `if` expression for" + + " simple cases"); + } + @Test public void isolatedUsageWithoutImports() throws Exception { PrecomputedValue.STARLARK_SEMANTICS.set( diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java index fe6f4e151cde40..db210f600deb72 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java @@ -1151,7 +1151,7 @@ public void testPackageDefaultRestrictionDuplicates() throws Exception { } @Test - public void testGlobPatternExtractor() { + public void testGlobPatternExtractor() throws Exception { StarlarkFile file = StarlarkFile.parse( ParserInput.fromLines( @@ -1166,8 +1166,7 @@ public void testGlobPatternExtractor() { List globs = new ArrayList<>(); List globsWithDirs = new ArrayList<>(); List subpackages = new ArrayList<>(); - PackageFactory.checkBuildSyntax( - file, globs, globsWithDirs, subpackages, new HashMap<>(), /* errors= */ null); + PackageFactory.checkBuildSyntax(file, globs, globsWithDirs, subpackages, new HashMap<>()); assertThat(globs).containsExactly("ab", "a", "**/*"); assertThat(globsWithDirs).containsExactly("c"); assertThat(subpackages).isEmpty(); @@ -1193,14 +1192,14 @@ public void testLambdaInBuild() throws Exception { public void testForStatementForbiddenInBuild() throws Exception { checkBuildDialectError( "for _ in []: pass", // - "for statements are not allowed in BUILD files"); + "`for` statements are not allowed in BUILD files"); } @Test public void testIfStatementForbiddenInBuild() throws Exception { checkBuildDialectError( "if False: pass", // - "if statements are not allowed in BUILD files"); + "`if` statements are not allowed in BUILD files"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java index a9165512eb5f1d..2094463e97da28 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java @@ -122,4 +122,16 @@ public void featureMerger() throws Exception { assertThat(ruleContext.getFeatures()).containsExactly("b", "c", "d"); assertThat(ruleContext.getDisabledFeatures()).containsExactly("a"); } + + @Test + public void restrictedSyntax() throws Exception { + scratch.overwriteFile( + "REPO.bazel", "if 3+5>7: repo(default_deprecation='EVERYTHING IS DEPRECATED')"); + scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')"); + reporter.removeHandler(failFastHandler); + assertTargetError( + "//abc/def:what", + "`if` statements are not allowed in REPO.bazel files. You may use an `if` expression for" + + " simple cases."); + } }