diff options
9 files changed, 65 insertions, 35 deletions
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md index 61eabd1098..4a67bf4d2b 100644 --- a/site/docs/skylark/backward-compatibility.md +++ b/site/docs/skylark/backward-compatibility.md @@ -30,6 +30,7 @@ To check if your code will be compatible with future releases you can: The following are the backward incompatible changes that are implemented and guarded behind flags in the current release: +* [Set constructor](#set-constructor) * [Keyword-only arguments](#keyword-only-arguments) * [Mutating `+=`](#mutating) * [Dictionary concatenation](#dictionary-concatenation) @@ -43,6 +44,22 @@ guarded behind flags in the current release: * [New actions API](#new-actions-api) * [Checked arithmetic](#checked-arithmetic) +### Set constructor + +To maintain a clear distinction between the specialized [`depset`](depsets.md) +data structure and Python's native `set` datatype (which does not currently +exist in Skylark), the `set` constructor has been superseded by `depset`. It is +no longer allowed to run code that calls the old `set` constructor. + +However, for a limited time, it will not be an error to reference the `set` +constructor from code that is not executed (e.g. a function that is never +called). Enable this flag to confirm that your code does not still refer to the +old `set` constructor from unexecuted code. + +* Flag: `--incompatible_disallow_uncalled_set_constructor` +* Default: `false` + + ### Keyword-only arguments Keyword-only parameters are parameters that can be called only using their name. diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java index 52db1ce009..1ffb7e2cee 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java @@ -48,6 +48,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus()); codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs()); codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement()); + codedOut.writeBoolNoTag(semantics.incompatibleDisallowUncalledSetConstructor()); codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace()); codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel()); codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi()); @@ -70,6 +71,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics builder.incompatibleDisallowDictPlus(codedIn.readBool()); builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool()); builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool()); + builder.incompatibleDisallowUncalledSetConstructor(codedIn.readBool()); builder.incompatibleListPlusEqualsInplace(codedIn.readBool()); builder.incompatibleLoadArgumentIsLabel(codedIn.readBool()); builder.incompatibleNewActionsApi(codedIn.readBool()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java index 0f8e0c4ad2..6545d064b5 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java @@ -159,6 +159,17 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable public boolean incompatibleDisallowToplevelIfStatement; @Option( + name = "incompatible_disallow_uncalled_set_constructor", + defaultValue = "false", + category = "incompatible changes", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = "If set to true, it's not allowed to use `set()` even if that code is never executed." + ) + public boolean incompatibleDisallowUncalledSetConstructor; + + @Option( name = "incompatible_list_plus_equals_inplace", defaultValue = "true", category = "incompatible changes", @@ -244,6 +255,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus) .incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs) .incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement) + .incompatibleDisallowUncalledSetConstructor(incompatibleDisallowUncalledSetConstructor) .incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace) .incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel) .incompatibleNewActionsApi(incompatibleNewActionsApi) diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java index a60ec53337..110c9e1f8c 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java @@ -175,36 +175,6 @@ public class BazelLibrary { } @SkylarkSignature( - name = "set", - returnType = SkylarkNestedSet.class, - doc = - "A temporary alias for <a href=\"#depset\">depset</a>. " - + "Deprecated in favor of <code>depset</code>.", - parameters = { - @Param( - name = "items", - type = Object.class, - defaultValue = "[]", - doc = "Same as for <a href=\"#depset\">depset</a>." - ), - @Param( - name = "order", - type = String.class, - defaultValue = "\"default\"", - doc = "Same as for <a href=\"#depset\">depset</a>." - ) - }, - useLocation = true - ) - private static final BuiltinFunction set = - new BuiltinFunction("set") { - public SkylarkNestedSet invoke(Object items, String order, Location loc) - throws EvalException { - throw new EvalException(loc, "The function 'set' has been removed in favor of 'depset'."); - } - }; - - @SkylarkSignature( name = "union", objectType = SkylarkNestedSet.class, returnType = SkylarkNestedSet.class, @@ -285,7 +255,7 @@ public class BazelLibrary { }; private static Environment.Frame createGlobals() { - List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, set, type); + List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type); try (Mutability mutability = Mutability.create("BUILD")) { Environment env = Environment.builder(mutability) diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java index a0dc773469..1bbf78c225 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java @@ -94,6 +94,13 @@ public final class Identifier extends Expression { if (name.equals("$error$")) { return new EvalException(getLocation(), "contains syntax error(s)", true); } + if (name.equals("set")) { + //TODO(vladmos): Remove as soon as the flag is removed + return new EvalException(getLocation(), + "The function 'set' has been removed in favor of 'depset', please use the latter. " + + "You can temporarily refer to the old 'set' constructor from unexecuted code " + + "by using --incompatible_disallow_uncalled_set_constructor=false"); + } String suggestion = SpellChecker.didYouMean(name, symbols); return new EvalException(getLocation(), "name '" + name + "' is not defined" + suggestion); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java index 105768fd3d..39a7673998 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java @@ -47,6 +47,7 @@ public abstract class SkylarkSemantics { public abstract boolean incompatibleDisallowDictPlus(); public abstract boolean incompatibleDisallowKeywordOnlyArgs(); public abstract boolean incompatibleDisallowToplevelIfStatement(); + public abstract boolean incompatibleDisallowUncalledSetConstructor(); public abstract boolean incompatibleListPlusEqualsInplace(); public abstract boolean incompatibleLoadArgumentIsLabel(); public abstract boolean incompatibleNewActionsApi(); @@ -68,6 +69,7 @@ public abstract class SkylarkSemantics { .incompatibleDisallowDictPlus(false) .incompatibleDisallowKeywordOnlyArgs(true) .incompatibleDisallowToplevelIfStatement(true) + .incompatibleDisallowUncalledSetConstructor(false) .incompatibleListPlusEqualsInplace(true) .incompatibleLoadArgumentIsLabel(false) .incompatibleNewActionsApi(false) @@ -89,6 +91,7 @@ public abstract class SkylarkSemantics { public abstract Builder incompatibleDisallowDictPlus(boolean value); public abstract Builder incompatibleDisallowKeywordOnlyArgs(boolean value); public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value); + public abstract Builder incompatibleDisallowUncalledSetConstructor(boolean value); public abstract Builder incompatibleListPlusEqualsInplace(boolean value); public abstract Builder incompatibleLoadArgumentIsLabel(boolean value); public abstract Builder incompatibleNewActionsApi(boolean value); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java index 451b0d5297..cbe6094d14 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java @@ -68,6 +68,12 @@ public final class ValidationEnvironment extends SyntaxTreeVisitor { block.variables.addAll(builtinVariables); block.readOnlyVariables.addAll(builtinVariables); semantics = env.getSemantics(); + + // If the flag is set to false, it should be allowed to have `set` + // in non-executable parts of the code. + if (!env.getSemantics().incompatibleDisallowUncalledSetConstructor()) { + block.variables.add("set"); + } } @Override 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 f0f203e142..2d388ca348 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 @@ -113,13 +113,13 @@ public class SkylarkSemanticsConsistencyTest { "--incompatible_disallow_dict_plus=" + rand.nextBoolean(), "--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(), "--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(), + "--incompatible_disallow_uncalled_set_constructor=" + rand.nextBoolean(), "--incompatible_list_plus_equals_inplace=" + rand.nextBoolean(), "--incompatible_load_argument_is_label=" + rand.nextBoolean(), "--incompatible_new_actions_api=" + rand.nextBoolean(), "--incompatible_string_is_not_iterable=" + rand.nextBoolean(), "--internal_do_not_export_builtins=" + rand.nextBoolean(), - "--internal_skylark_flag_test_canary=" + rand.nextBoolean() - ); + "--internal_skylark_flag_test_canary=" + rand.nextBoolean()); } /** @@ -137,6 +137,7 @@ public class SkylarkSemanticsConsistencyTest { .incompatibleDisallowDictPlus(rand.nextBoolean()) .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean()) .incompatibleDisallowToplevelIfStatement(rand.nextBoolean()) + .incompatibleDisallowUncalledSetConstructor(rand.nextBoolean()) .incompatibleListPlusEqualsInplace(rand.nextBoolean()) .incompatibleLoadArgumentIsLabel(rand.nextBoolean()) .incompatibleNewActionsApi(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java index c8fdba5ab6..3ba85635d2 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java @@ -37,15 +37,27 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testLegacyConstructorNotCalled() throws Exception { - env = newEnvironmentWithSkylarkOptions(); + env = + newEnvironmentWithSkylarkOptions("--incompatible_disallow_uncalled_set_constructor=false"); eval("s = set([1, 2]) if False else depset([3, 4])"); SkylarkNestedSet s = get("s"); assertThat(s.getSet(Object.class)).containsExactly(3, 4); + + // Static check are only enabled in .bzl files + new SkylarkTest("--incompatible_disallow_uncalled_set_constructor=true") + .testIfErrorContains("The function 'set' has been removed in favor of 'depset'", + "s = set([1, 2]) if False else depset([3, 4])"); + new BuildTest("--incompatible_disallow_uncalled_set_constructor=true") + .testEval("s = set([1, 2]) if False else depset([3, 4]); s.to_list()", "[3, 4]"); } @Test public void testLegacyConstructorCalled() throws Exception { - new BothModesTest() + new BothModesTest("--incompatible_disallow_uncalled_set_constructor=false") + .testIfErrorContains("The function 'set' has been removed in favor of 'depset'", + "s = set([1, 2])"); + + new BothModesTest("--incompatible_disallow_uncalled_set_constructor=true") .testIfErrorContains("The function 'set' has been removed in favor of 'depset'", "s = set([1, 2])"); } |