Introduce --incompatible_static_name_resolution
This implements a part of the name resolution document: https://github.com/bazelbuild/proposals/blob/master/docs/2018-06-18-name-resolution.md
When the flag is set:
- Builtins may be shadowed (e.g. `len = 2`) on top-level
- Symbols may be used before their definition point, e.g.
def fct(): return x
x = 2
In a followup change, we'll need to update the dynamic Environment too (to have truly lexical binding).
https://github.com/bazelbuild/bazel/issues/5637
RELNOTES: None.
PiperOrigin-RevId: 210412609
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 eff7ba3..90588f9 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
@@ -144,6 +144,7 @@
"--incompatible_range_type=" + rand.nextBoolean(),
"--incompatible_remove_native_git_repository=" + rand.nextBoolean(),
"--incompatible_remove_native_http_archive=" + rand.nextBoolean(),
+ "--incompatible_static_name_resolution=" + rand.nextBoolean(),
"--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
"--internal_skylark_flag_test_canary=" + rand.nextBoolean());
}
@@ -179,6 +180,7 @@
.incompatibleRangeType(rand.nextBoolean())
.incompatibleRemoveNativeGitRepository(rand.nextBoolean())
.incompatibleRemoveNativeHttpArchive(rand.nextBoolean())
+ .incompatibleStaticNameResolution(rand.nextBoolean())
.incompatibleStringIsNotIterable(rand.nextBoolean())
.internalSkylarkFlagTestCanary(rand.nextBoolean())
.build();
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index 73f66fb..f11cfe7 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -245,19 +245,17 @@
}
@Test
- public void testReadOnly() throws Exception {
- Environment env = newSkylarkEnvironment()
- .setup("special_var", 42)
- .update("global_var", 666);
+ public void testBuiltinsCanBeShadowed() throws Exception {
+ Environment env =
+ newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=true")
+ .setup("special_var", 42);
+ BuildFileAST.eval(env, "special_var = 41");
+ assertThat(env.lookup("special_var")).isEqualTo(41);
+ }
- // We don't even get a runtime exception trying to modify these,
- // because we get compile-time exceptions even before we reach runtime!
- try {
- BuildFileAST.eval(env, "special_var = 41");
- throw new AssertionError("failed to fail");
- } catch (EvalException e) {
- assertThat(e).hasMessageThat().contains("Variable special_var is read only");
- }
+ @Test
+ public void testVariableIsReferencedBeforeAssignment() throws Exception {
+ Environment env = newSkylarkEnvironment().update("global_var", 666);
try {
BuildFileAST.eval(env, "def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 3a003b1..403bfa3 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -93,6 +93,18 @@
@Test
public void testDefinitionByItself() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=true");
+ // Variables are assumed to be statically visible in the block (even if they might not be
+ // initialized).
+ parse("a = a");
+ parse("a += a");
+ parse("[[] for a in a]");
+ parse("def f():", " for a in a: pass");
+ }
+
+ @Test
+ public void testDefinitionByItselfLegacy() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=false");
checkError("name 'a' is not defined", "a = a");
checkError("name 'a' is not defined", "a += a");
checkError("name 'a' is not defined", "[[] for a in a]");
@@ -105,11 +117,18 @@
}
@Test
- public void testBuiltinSymbolsAreReadOnly() throws Exception {
+ public void testBuiltinSymbolsAreReadOnlyLegacy() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=false");
checkError("Variable repr is read only", "repr = 1");
}
@Test
+ public void testBuiltinsCanBeShadowed() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=true");
+ parse("repr = 1");
+ }
+
+ @Test
public void testSkylarkGlobalVariablesAreReadonly() throws Exception {
checkError("Variable a is read only", "a = 1", "a = 2");
}
@@ -130,6 +149,23 @@
}
@Test
+ public void testGlobalDefinedBelow() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=true");
+ parse("def bar(): return x", "x = 5\n");
+ }
+
+ @Test
+ public void testLocalVariableDefinedBelow() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_static_name_resolution=true");
+ parse(
+ "def bar():",
+ " for i in range(5):",
+ " if i > 2: return x",
+ " x = i" // x is visible in the entire function block
+ );
+ }
+
+ @Test
public void testFunctionDoesNotExist() {
checkError("name 'foo' is not defined", "def bar(): a = foo() + 'a'");
}
