Introduce flag --incompatible_disallow_old_octal_notation
https://github.com/bazelbuild/bazel/issues/8059
When the flag is enabled, forbid octal numbers like `0123`; they should be written `0o123` instead.
RELNOTES: Flag `--incompatible_disallow_old_octal_notation` is added. See //github.com/bazelbuild/bazel/issues/8059
PiperOrigin-RevId: 243830436
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 916fb69..3bb2b55 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -332,6 +332,21 @@
+ "instead return a list of provider instances.")
public boolean incompatibleDisallowStructProviderSyntax;
+ @Option(
+ name = "incompatible_disallow_old_octal_notation",
+ defaultValue = "false",
+ category = "incompatible changes",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If set to true, octal numbers like `0123` are forbidden, they should be written "
+ + "`0o123` instead. See https://github.com/bazelbuild/bazel/issues/8059")
+ public boolean incompatibleDisallowOldOctalNotation;
+
/** Controls legacy arguments to ctx.actions.Args#add. */
@Option(
name = "incompatible_disallow_old_style_args_add",
@@ -564,6 +579,7 @@
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(
incompatibleDisallowLoadLabelsToCrossPackageBoundaries)
.incompatibleDisallowNativeInBuildFile(incompatibleDisallowNativeInBuildFile)
+ .incompatibleDisallowOldOctalNotation(incompatibleDisallowOldOctalNotation)
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
.incompatibleExpandDirectories(incompatibleExpandDirectories)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index da2ee67..fe334f1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -541,6 +541,7 @@
public static void execAndExport(BuildFileAST ast, Label extensionLabel,
EventHandler eventHandler,
com.google.devtools.build.lib.syntax.Environment extensionEnv) throws InterruptedException {
+ ast.replayLexerEvents(extensionEnv, eventHandler);
ImmutableList<Statement> statements = ast.getStatements();
for (Statement statement : statements) {
ast.execTopLevelStatement(statement, extensionEnv, eventHandler);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index 13c81ae..47beaf1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -49,6 +49,8 @@
*/
private final boolean containsErrors;
+ private final List<Event> octalEvents;
+
@Nullable private final String contentHashCode;
private BuildFileAST(
@@ -57,13 +59,15 @@
String contentHashCode,
Location location,
ImmutableList<Comment> comments,
- @Nullable ImmutableList<SkylarkImport> imports) {
+ @Nullable ImmutableList<SkylarkImport> imports,
+ List<Event> octalEvents) {
this.statements = statements;
this.containsErrors = containsErrors;
this.contentHashCode = contentHashCode;
this.comments = comments;
this.setLocation(location);
this.imports = imports;
+ this.octalEvents = octalEvents;
}
private static BuildFileAST create(
@@ -98,7 +102,8 @@
contentHashCode,
result.location,
ImmutableList.copyOf(result.comments),
- skylarkImports.second);
+ skylarkImports.second,
+ result.octalEvents);
}
private static BuildFileAST create(
@@ -135,7 +140,8 @@
null,
this.statements.get(firstStatement).getLocation(),
ImmutableList.of(),
- imports.build());
+ imports.build(),
+ octalEvents);
}
/**
@@ -201,6 +207,16 @@
}
return imports.build();
}
+
+ /** Returns true if there was no error event. */
+ public boolean replayLexerEvents(Environment env, EventHandler eventHandler) {
+ if (env.getSemantics().incompatibleDisallowOldOctalNotation() && !octalEvents.isEmpty()) {
+ Event.replayEventsOn(eventHandler, octalEvents);
+ return false;
+ }
+ return true;
+ }
+
/**
* Executes this build file in a given Environment.
*
@@ -219,6 +235,9 @@
*/
public boolean exec(Environment env, EventHandler eventHandler) throws InterruptedException {
boolean ok = true;
+ if (!replayLexerEvents(env, eventHandler)) {
+ return false;
+ }
for (Statement stmt : statements) {
if (!execTopLevelStatement(stmt, env, eventHandler)) {
ok = false;
@@ -367,10 +386,11 @@
.addAll(result.statements)
.build(),
result.containsErrors,
- /* contentHashCode= */null,
+ /* contentHashCode= */ null,
result.location,
ImmutableList.copyOf(result.comments),
- /* imports= */null);
+ /* imports= */ null,
+ result.octalEvents);
}
/**
@@ -383,7 +403,8 @@
if (valid || containsErrors) {
return this;
}
- return new BuildFileAST(statements, true, contentHashCode, getLocation(), comments, imports);
+ return new BuildFileAST(
+ statements, true, contentHashCode, getLocation(), comments, imports, octalEvents);
}
public static BuildFileAST parseString(EventHandler eventHandler, String... content) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index 5507053..ea93e00 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -104,6 +104,14 @@
private int dents; // number of saved INDENT (>0) or OUTDENT (<0) tokens to return
/**
+ * OctalEvents contains the errors related to the old octal notation (0123 instead of 0o123). This
+ * is not handled by the normal eventHandler. Instead, it is passed to the parser and then the
+ * AST. During the evaluation, we can decide to show the events based on a flag in
+ * StarlarkSemantics. This code is temporary, during the migration.
+ */
+ private List<Event> octalEvents = new ArrayList<>();
+
+ /**
* Constructs a lexer which tokenizes the contents of the specified InputBuffer. Any errors during
* lexing are reported on "handler".
*/
@@ -129,6 +137,10 @@
return comments;
}
+ List<Event> getOctalEvents() {
+ return octalEvents;
+ }
+
/**
* Returns the filename from which the lexer's input came. Returns an empty value if the input
* came from a string.
@@ -689,6 +701,10 @@
} else if (literal.startsWith("0") && literal.length() > 1) {
radix = 8;
substring = literal.substring(1);
+ octalEvents.add(
+ Event.error(
+ createLocation(oldPos, pos),
+ "Invalid octal value `" + literal + "`, should be: `0o" + substring + "`."));
} else {
radix = 10;
substring = literal;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 81f34c3..cec994e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -62,14 +62,21 @@
/** Whether the file contained any errors. */
public final boolean containsErrors;
- public ParseResult(List<Statement> statements, List<Comment> comments, Location location,
- boolean containsErrors) {
+ public final List<Event> octalEvents;
+
+ public ParseResult(
+ List<Statement> statements,
+ List<Comment> comments,
+ Location location,
+ boolean containsErrors,
+ List<Event> octalEvents) {
// No need to copy here; when the object is created, the parser instance is just about to go
// out of scope and be garbage collected.
this.statements = Preconditions.checkNotNull(statements);
this.comments = Preconditions.checkNotNull(comments);
this.location = location;
this.containsErrors = containsErrors;
+ this.octalEvents = octalEvents;
}
}
@@ -202,7 +209,11 @@
}
boolean errors = parser.errorsCount > 0 || lexer.containsErrors();
return new ParseResult(
- statements, lexer.getComments(), locationFromStatements(lexer, statements), errors);
+ statements,
+ lexer.getComments(),
+ locationFromStatements(lexer, statements),
+ errors,
+ lexer.getOctalEvents());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index e07179e..aaddb98 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -151,6 +151,8 @@
public abstract boolean incompatibleDisallowNativeInBuildFile();
+ public abstract boolean incompatibleDisallowOldOctalNotation();
+
public abstract boolean incompatibleDisallowOldStyleArgsAdd();
public abstract boolean incompatibleDisallowStructProviderSyntax();
@@ -219,6 +221,7 @@
.incompatibleDisallowLegacyJavaInfo(false)
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(true)
.incompatibleDisallowNativeInBuildFile(false)
+ .incompatibleDisallowOldOctalNotation(false)
.incompatibleDisallowOldStyleArgsAdd(true)
.incompatibleDisallowStructProviderSyntax(false)
.incompatibleExpandDirectories(true)
@@ -279,6 +282,8 @@
public abstract Builder incompatibleDisallowLoadLabelsToCrossPackageBoundaries(boolean value);
+ public abstract Builder incompatibleDisallowOldOctalNotation(boolean value);
+
public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
public abstract Builder incompatibleDisallowNativeInBuildFile(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 5b08481..294d329 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -143,6 +143,7 @@
"--incompatible_disallow_legacy_java_provider=" + rand.nextBoolean(),
"--incompatible_disallow_load_labels_to_cross_package_boundaries=" + rand.nextBoolean(),
"--incompatible_disallow_native_in_build_file=" + rand.nextBoolean(),
+ "--incompatible_disallow_old_octal_notation=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_expand_directories=" + rand.nextBoolean(),
@@ -190,6 +191,7 @@
.incompatibleDisallowLegacyJavaProvider(rand.nextBoolean())
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(rand.nextBoolean())
.incompatibleDisallowNativeInBuildFile(rand.nextBoolean())
+ .incompatibleDisallowOldOctalNotation(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleExpandDirectories(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index ab46118..43b5c4b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -2964,4 +2964,28 @@
}
}
}
+
+ @Test
+ public void testOldOctalNotationIsForbidden() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_old_octal_notation=true");
+
+ scratch.file("test/extension.bzl", "y = 0246");
+
+ scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent("Invalid octal value `0246`, should be: `0o246`");
+ }
+
+ @Test
+ public void testOldOctalNotation() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_old_octal_notation=false");
+
+ scratch.file("test/extension.bzl", "y = 0246");
+
+ scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
+
+ getConfiguredTarget("//test:r");
+ }
}
