Automated rollback of commit 4b6da43db972d335d182d9bde708b075710c2f6b.
*** Reason for rollback ***
roll forward, after fix and new test
*** Original change description ***
Automated rollback of commit 395fbbdb7f17b97ed140fc0e4a1e061c7bf73843.
*** Reason for rollback ***
Breaking blaze guitar tests
*** Original change description ***
Allow shadowing of builtins in bzl files
This was previously allowed only with --incompatible_static_name_resolution (although it was a backward-compatible change). Removing the code path simplifies the code and makes easier to implement the static name resolution proposal.
The downside is that it will temporarily allow this corner-case:
```
x = len(.....
***
#5827
RELNOTES: None.
PiperOrigin-RevId: 212436910
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 8333245..810aaa2 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -1789,15 +1789,17 @@
@Test
public void testFunctionCallBadOrdering() throws Exception {
- new SkylarkTest().testIfErrorContains("name 'foo' is not defined",
- "def func(): return foo() * 2",
- "x = func()",
- "def foo(): return 2");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "name 'foo' is not defined",
+ "def func(): return foo() * 2",
+ "x = func()",
+ "def foo(): return 2");
}
@Test
public void testLocalVariableDefinedBelow() throws Exception {
- new SkylarkTest("--incompatible_static_name_resolution=true")
+ new SkylarkTest()
.setUp(
"def beforeEven(li):", // returns the value before the first even number
" for i in li:",
@@ -1815,17 +1817,32 @@
.testIfErrorContains(
/* error message */ "local variable 'gl' is referenced before assignment",
"gl = 5",
- "def foo():", // returns the value before the first even number
+ "def foo():",
" if False: gl = 2",
" return gl",
"res = foo()");
}
@Test
- public void testLegacyGlobalIsNotInitialized() throws Exception {
+ public void testLegacyGlobalVariableNotShadowed() throws Exception {
new SkylarkTest("--incompatible_static_name_resolution=false")
- .setUp("a = len")
- .testIfErrorContains("Variable len is read only", "len = 2");
+ .setUp(
+ "gl = 5",
+ "def foo():",
+ " if False: gl = 2",
+ // The legacy behavior is that the global variable is returned.
+ // With --incompatible_static_name_resolution set to true, this becomes an error.
+ " return gl",
+ "res = foo()")
+ .testLookup("res", 5);
+ }
+
+ @Test
+ public void testShadowBuiltin() throws Exception {
+ // TODO(laurentlb): Forbid this.
+ new SkylarkTest("--incompatible_static_name_resolution=false")
+ .setUp("x = len('abc')", "len = 2", "y = x + len")
+ .testLookup("y", 5);
}
@Test
