Remove the --incompatible_disallow_old_octal_notation flag

It is not useful to keep the flag around.

bazelbuild#8059

RELNOTES: None.
PiperOrigin-RevId: 249537925

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -359,21 +359,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "instead return a list of provider instances.")
   public boolean incompatibleDisallowStructProviderSyntax;
 
-  @Option(
-      name = "incompatible_disallow_old_octal_notation",
-      defaultValue = "true",
-      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",
@@ -634,7 +619,6 @@ public StarlarkSemantics toSkylarkSemantics() {
         .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(
             incompatibleDisallowLoadLabelsToCrossPackageBoundaries)
         .incompatibleDisallowNativeInBuildFile(incompatibleDisallowNativeInBuildFile)
-        .incompatibleDisallowOldOctalNotation(incompatibleDisallowOldOctalNotation)
         .incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
         .incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
         .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(

src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java

@@ -541,7 +541,6 @@ private Extension createExtension(
   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);

src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java

@@ -49,8 +49,6 @@ public class BuildFileAST extends ASTNode {
    */
   private final boolean containsErrors;
 
-  private final List<Event> octalEvents;
-
   @Nullable private final String contentHashCode;
 
   private BuildFileAST(
@@ -59,15 +57,13 @@ private BuildFileAST(
       String contentHashCode,
       Location location,
       ImmutableList<Comment> comments,
-      @Nullable ImmutableList<SkylarkImport> imports,
-      List<Event> octalEvents) {
+      @Nullable ImmutableList<SkylarkImport> imports) {
     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(
@@ -102,8 +98,7 @@ private static BuildFileAST create(
         contentHashCode,
         result.location,
         ImmutableList.copyOf(result.comments),
-        skylarkImports.second,
-        result.octalEvents);
+        skylarkImports.second);
   }
 
   private static BuildFileAST create(
@@ -140,8 +135,7 @@ public BuildFileAST subTree(int firstStatement, int lastStatement) {
         null,
         this.statements.get(firstStatement).getLocation(),
         ImmutableList.of(),
-        imports.build(),
-        octalEvents);
+        imports.build());
   }
 
   /**
@@ -208,15 +202,6 @@ public ImmutableList<StringLiteral> getRawImports() {
     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.
    *
@@ -235,9 +220,6 @@ public boolean replayLexerEvents(Environment env, EventHandler eventHandler) {
    */
   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;
@@ -389,8 +371,7 @@ public static BuildFileAST parseSkylarkFileWithoutImports(
         /* contentHashCode= */ null,
         result.location,
         ImmutableList.copyOf(result.comments),
-        /* imports= */ null,
-        result.octalEvents);
+        /* imports= */ null);
   }
 
   /**
@@ -403,8 +384,7 @@ public BuildFileAST validate(Environment env, EventHandler eventHandler) {
     if (valid || containsErrors) {
       return this;
     }
-    return new BuildFileAST(
-        statements, true, contentHashCode, getLocation(), comments, imports, octalEvents);
+    return new BuildFileAST(statements, true, contentHashCode, getLocation(), comments, imports);
   }
 
   public static BuildFileAST parseString(EventHandler eventHandler, String... content) {

src/main/java/com/google/devtools/build/lib/syntax/Lexer.java

@@ -103,14 +103,6 @@ private static class LocationInfo {
 
   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".
@@ -137,10 +129,6 @@ List<Comment> getComments() {
     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.
@@ -701,10 +689,7 @@ private void integer() {
     } 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 + "`."));
+      error("invalid octal value `" + literal + "`, should be: `0o" + substring + "`");
     } else {
       radix = 10;
       substring = literal;

src/main/java/com/google/devtools/build/lib/syntax/Parser.java

@@ -62,21 +62,17 @@ public static final class ParseResult {
     /** Whether the file contained any errors. */
     public final boolean containsErrors;
 
-    public final List<Event> octalEvents;
-
     public ParseResult(
         List<Statement> statements,
         List<Comment> comments,
         Location location,
-        boolean containsErrors,
-        List<Event> octalEvents) {
+        boolean containsErrors) {
       // 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;
     }
   }
 
@@ -209,11 +205,7 @@ public static ParseResult parseFile(ParserInputSource input, EventHandler eventH
     }
     boolean errors = parser.errorsCount > 0 || lexer.containsErrors();
     return new ParseResult(
-        statements,
-        lexer.getComments(),
-        locationFromStatements(lexer, statements),
-        errors,
-        lexer.getOctalEvents());
+        statements, lexer.getComments(), locationFromStatements(lexer, statements), errors);
   }
 
   /**

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -157,8 +157,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleDisallowNativeInBuildFile();
 
-  public abstract boolean incompatibleDisallowOldOctalNotation();
-
   public abstract boolean incompatibleDisallowOldStyleArgsAdd();
 
   public abstract boolean incompatibleDisallowRuleExecutionPlatformConstraintsAllowed();
@@ -234,7 +232,6 @@ public static Builder builderWithDefaults() {
           .incompatibleDisallowLegacyJavaInfo(false)
           .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(true)
           .incompatibleDisallowNativeInBuildFile(true)
-          .incompatibleDisallowOldOctalNotation(true)
           .incompatibleDisallowOldStyleArgsAdd(true)
           .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
           .incompatibleDisallowStructProviderSyntax(false)
@@ -300,8 +297,6 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleDisallowLoadLabelsToCrossPackageBoundaries(boolean value);
 
-    public abstract Builder incompatibleDisallowOldOctalNotation(boolean value);
-
     public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
 
     public abstract Builder incompatibleDisallowNativeInBuildFile(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -145,7 +145,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--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_disallow_rule_execution_platform_constraints_allowed=" + rand.nextBoolean(),
@@ -197,7 +196,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisallowLegacyJavaProvider(rand.nextBoolean())
         .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(rand.nextBoolean())
         .incompatibleDisallowNativeInBuildFile(rand.nextBoolean())
-        .incompatibleDisallowOldOctalNotation(rand.nextBoolean())
         .incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
         .incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
         .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java

@@ -2977,28 +2977,4 @@ public void testRecursiveImport2() throws Exception {
                   + "//test/skylark:ext3.bzl, //test/skylark:ext4.bzl]");
     }
   }
-
-  @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");
-  }
 }

src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java

@@ -205,7 +205,6 @@ public void testIntegers() throws Exception {
     assertThat(values(tokens("12345-"))).isEqualTo("INT(12345) MINUS NEWLINE EOF");
 
     // octal
-    assertThat(values(tokens("012345-"))).isEqualTo("INT(5349) MINUS NEWLINE EOF");
     assertThat(values(tokens("0o12345-"))).isEqualTo("INT(5349) MINUS NEWLINE EOF");
     assertThat(values(tokens("0O77"))).isEqualTo("INT(63) NEWLINE EOF");
 
@@ -218,6 +217,10 @@ public void testIntegers() throws Exception {
     assertThat(lastError.toString())
         .isEqualTo("/some/path.txt:1: invalid base-8 integer constant: 0o");
 
+    assertThat(values(tokens("012345"))).isEqualTo("INT(5349) NEWLINE EOF");
+    assertThat(lastError.toString())
+        .isEqualTo("/some/path.txt:1: invalid octal value `012345`, should be: `0o12345`");
+
     // hexadecimal (uppercase)
     assertThat(values(tokens("0X12345F-"))).isEqualTo("INT(1193055) MINUS NEWLINE EOF");