Skip to content

Commit

Permalink
Flip --incompatible_restrict_named_params to true.
Browse files Browse the repository at this point in the history 
    RELNOTES: Make a number of parameters of Starlark builtin functions positional-only (as opposed to specifiable by keyword). See bazelbuild/bazel#8147 for details.
    PiperOrigin-RevId: 265995993
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent b8c2cd5 commit f2a728e
Show file tree
Hide file tree
Showing 6 changed files with 205 additions and 257 deletions.
2 changes: 1 addition & 1 deletion ....bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl

@Option(
name = "incompatible_disallow_legacy_java_provider",
defaultValue = "true",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
Expand Down
2 changes: 1 addition & 1 deletion ...azelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public static Builder builderWithDefaults() {
.incompatibleDisableDepsetItems(false)
.incompatibleDisallowDictPlus(true)
.incompatibleDisallowEmptyGlob(false)
.incompatibleDisallowLegacyJavaProvider(true)
.incompatibleDisallowLegacyJavaProvider(false)
.incompatibleDisallowOldStyleArgsAdd(true)
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(true)
.incompatibleDisallowStructProviderSyntax(false)
Expand Down
43 changes: 23 additions & 20 deletions ...el/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,19 @@
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.packages.StructProvider;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction;
import com.google.devtools.build.lib.skylark.util.SkylarkTestCase;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.ParserInput;
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.syntax.StarlarkFile;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.Collection;
Expand Down Expand Up @@ -704,13 +702,10 @@ public void testRuleAddAttribute() throws Exception {
}

protected void evalAndExport(String... lines) throws Exception {
ParserInput input = ParserInput.fromLines(lines);
StarlarkFile file = StarlarkFile.parseAndValidateSkylark(input, ev.getStarlarkThread());
if (!file.errors().isEmpty()) {
throw new SyntaxError(file.errors());
}
BuildFileAST buildFileAST = BuildFileAST.parseAndValidateSkylarkString(
ev.getEnvironment(), lines);
SkylarkImportLookupFunction.execAndExport(
file, FAKE_LABEL, ev.getEventHandler(), ev.getStarlarkThread());
buildFileAST, FAKE_LABEL, ev.getEventHandler(), ev.getEnvironment());
}

@Test
Expand Down Expand Up @@ -1258,6 +1253,14 @@ public void testStructsInDicts() throws Exception {
"{struct(a = []): 'foo'}");
}

@Test
public void testStructMembersAreImmutable() throws Exception {
checkErrorContains(
"cannot assign to 's.x'",
"s = struct(x = 'a')",
"s.x = 'b'\n");
}

@Test
public void testStructDictMembersAreMutable() throws Exception {
eval(
Expand All @@ -1284,20 +1287,20 @@ private static StructImpl makeStruct(String field, Object value) {
return StructProvider.STRUCT.create(ImmutableMap.of(field, value), "no field '%'");
}

private static StructImpl makeBigStruct(StarlarkThread thread) {
private static StructImpl makeBigStruct(Environment env) {
// struct(a=[struct(x={1:1}), ()], b=(), c={2:2})
return StructProvider.STRUCT.create(
ImmutableMap.<String, Object>of(
"a",
MutableList.<Object>of(
thread,
env,
StructProvider.STRUCT.create(
ImmutableMap.<String, Object>of(
"x", SkylarkDict.<Object, Object>of(thread, 1, 1)),
"x", SkylarkDict.<Object, Object>of(env, 1, 1)),
"no field '%s'"),
Tuple.of()),
"b", Tuple.of(),
"c", SkylarkDict.<Object, Object>of(thread, 2, 2)),
"c", SkylarkDict.<Object, Object>of(env, 2, 2)),
"no field '%s'");
}

Expand All @@ -1306,8 +1309,8 @@ public void testStructMutabilityShallow() throws Exception {
assertThat(EvalUtils.isImmutable(makeStruct("a", 1))).isTrue();
}

private static MutableList<Object> makeList(StarlarkThread thread) {
return MutableList.<Object>of(thread, 1, 2, 3);
private static MutableList<Object> makeList(Environment env) {
return MutableList.<Object>of(env, 1, 2, 3);
}

@Test
Expand All @@ -1316,9 +1319,9 @@ public void testStructMutabilityDeep() throws Exception {
assertThat(EvalUtils.isImmutable(makeStruct("a", makeList(null)))).isTrue();
assertThat(EvalUtils.isImmutable(makeBigStruct(null))).isTrue();

assertThat(EvalUtils.isImmutable(Tuple.<Object>of(makeList(ev.getStarlarkThread())))).isFalse();
assertThat(EvalUtils.isImmutable(makeStruct("a", makeList(ev.getStarlarkThread())))).isFalse();
assertThat(EvalUtils.isImmutable(makeBigStruct(ev.getStarlarkThread()))).isFalse();
assertThat(EvalUtils.isImmutable(Tuple.<Object>of(makeList(ev.getEnvironment())))).isFalse();
assertThat(EvalUtils.isImmutable(makeStruct("a", makeList(ev.getEnvironment())))).isFalse();
assertThat(EvalUtils.isImmutable(makeBigStruct(ev.getEnvironment()))).isFalse();
}

@Test
Expand Down
98 changes: 33 additions & 65 deletions ...a/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import static com.google.common.truth.Truth.assertThat;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.events.EventCollector;
import com.google.devtools.build.lib.packages.BazelLibrary;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
Expand Down Expand Up @@ -53,30 +51,6 @@ protected ModalTestCase newTest(String... skylarkOptions) {
return new BuildTest(skylarkOptions);
}

@Test
public void testExecutionStopsAtFirstError() throws Exception {
EventCollector printEvents = new EventCollector(); // for print events
StarlarkThread thread =
StarlarkThread.builder(mutability)
.useDefaultSemantics()
.setGlobals(BazelLibrary.GLOBALS) // for print... this should not be necessary
.setEventHandler(printEvents)
.build();
ParserInput input = ParserInput.fromLines("print('hello'); x = 1//0; print('goodbye')");
try {
StarlarkFile.eval(input, thread);
throw new AssertionError("execution succeeded unexpectedly");
} catch (EvalException ex) {
// ok, division by zero
}
if (!printEvents.toString().contains("hello")) {
throw new AssertionError("first print statement not executed: " + printEvents);
}
if (printEvents.toString().contains("goodbye")) {
throw new AssertionError("first print statement unexpected executed: " + printEvents);
}
}

@Test
public void testExprs() throws Exception {
newTest()
Expand Down Expand Up @@ -144,21 +118,17 @@ public void testSetComparison() throws Exception {

@Test
public void testSumFunction() throws Exception {
BaseFunction sum =
new BaseFunction("sum") {
@Override
public Object call(
List<Object> args,
Map<String, Object> kwargs,
FuncallExpression ast,
StarlarkThread thread) {
int sum = 0;
for (Object arg : args) {
sum += (Integer) arg;
}
return sum;
}
};
BaseFunction sum = new BaseFunction("sum") {
@Override
public Object call(List<Object> args, Map<String, Object> kwargs,
FuncallExpression ast, Environment env) {
int sum = 0;
for (Object arg : args) {
sum += (Integer) arg;
}
return sum;
}
};

newTest().update(sum.getName(), sum).testStatement("sum(1, 2, 3, 4, 5, 6)", 21)
.testStatement("sum", sum).testStatement("sum(a=1, b=2)", 0);
Expand All @@ -181,17 +151,15 @@ public void testComplexFunctionCall() throws Exception {
public void testKeywordArgs() throws Exception {

// This function returns the map of keyword arguments passed to it.
BaseFunction kwargs =
new BaseFunction("kwargs") {
@Override
public Object call(
List<Object> args,
final Map<String, Object> kwargs,
FuncallExpression ast,
StarlarkThread thread) {
return SkylarkDict.copyOf(thread, kwargs);
}
};
BaseFunction kwargs = new BaseFunction("kwargs") {
@Override
public Object call(List<Object> args,
final Map<String, Object> kwargs,
FuncallExpression ast,
Environment env) {
return SkylarkDict.copyOf(env, kwargs);
}
};

newTest()
.update(kwargs.getName(), kwargs)
Expand Down Expand Up @@ -277,7 +245,7 @@ public void testConcatLists() throws Exception {
// list
Object x = eval("[1,2] + [3,4]");
assertThat((Iterable<Object>) x).containsExactly(1, 2, 3, 4).inOrder();
assertThat(x).isEqualTo(MutableList.of(thread, 1, 2, 3, 4));
assertThat(x).isEqualTo(MutableList.of(env, 1, 2, 3, 4));
assertThat(EvalUtils.isImmutable(x)).isFalse();

// tuple
Expand Down Expand Up @@ -464,26 +432,26 @@ public void testDictComprehensions_MultipleKey() throws Exception {
@Test
public void testListConcatenation() throws Exception {
newTest()
.testStatement("[1, 2] + [3, 4]", MutableList.of(thread, 1, 2, 3, 4))
.testStatement("[1, 2] + [3, 4]", MutableList.of(env, 1, 2, 3, 4))
.testStatement("(1, 2) + (3, 4)", Tuple.of(1, 2, 3, 4))
.testIfExactError(
"unsupported operand type(s) for +: 'list' and 'tuple'", "[1, 2] + (3, 4)")
.testIfExactError(
"unsupported operand type(s) for +: 'tuple' and 'list'", "(1, 2) + [3, 4]");
.testIfExactError("unsupported operand type(s) for +: 'list' and 'tuple'",
"[1, 2] + (3, 4)")
.testIfExactError("unsupported operand type(s) for +: 'tuple' and 'list'",
"(1, 2) + [3, 4]");
}

@Test
public void testListMultiply() throws Exception {
newTest()
.testStatement("[1, 2, 3] * 1", MutableList.of(thread, 1, 2, 3))
.testStatement("[1, 2] * 2", MutableList.of(thread, 1, 2, 1, 2))
.testStatement("[1, 2] * 3", MutableList.of(thread, 1, 2, 1, 2, 1, 2))
.testStatement("[1, 2] * 4", MutableList.of(thread, 1, 2, 1, 2, 1, 2, 1, 2))
.testStatement("[8] * 5", MutableList.of(thread, 8, 8, 8, 8, 8))
.testStatement("[1, 2, 3] * 1", MutableList.of(env, 1, 2, 3))
.testStatement("[1, 2] * 2", MutableList.of(env, 1, 2, 1, 2))
.testStatement("[1, 2] * 3", MutableList.of(env, 1, 2, 1, 2, 1, 2))
.testStatement("[1, 2] * 4", MutableList.of(env, 1, 2, 1, 2, 1, 2, 1, 2))
.testStatement("[8] * 5", MutableList.of(env, 8, 8, 8, 8, 8))
.testStatement("[ ] * 10", MutableList.empty())
.testStatement("[1, 2] * 0", MutableList.empty())
.testStatement("[1, 2] * -4", MutableList.empty())
.testStatement("2 * [1, 2]", MutableList.of(thread, 1, 2, 1, 2))
.testStatement("2 * [1, 2]", MutableList.of(env, 1, 2, 1, 2))
.testStatement("10 * []", MutableList.empty())
.testStatement("0 * [1, 2]", MutableList.empty())
.testStatement("-4 * [1, 2]", MutableList.empty());
Expand Down Expand Up @@ -558,7 +526,7 @@ public void testListComprehensionOnDictionary() throws Exception {
public void testListComprehensionOnDictionaryCompositeExpression() throws Exception {
new BuildTest()
.setUp("d = {1:'a',2:'b'}", "l = [d[x] for x in d]")
.testLookup("l", MutableList.of(thread, "a", "b"));
.testLookup("l", MutableList.of(env, "a", "b"));
}

@Test
Expand Down
Loading

0 comments on commit f2a728e

Please sign in to comment.