Skip to content

Commit

Permalink
Remove flag --incompatible_string_is_not_iterable
Browse files Browse the repository at this point in the history
    bazelbuild/bazel#5830

    RELNOTES: None.
    PiperOrigin-RevId: 234784611
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent bebb27f commit a807d27
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl

@Option(
name = "incompatible_disallow_dict_plus",
defaultValue = "true",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
Expand Down Expand Up @@ -330,6 +330,18 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "instead return a list of provider instances.")
public boolean incompatibleDisallowStructProviderSyntax;

@Option(
name = "incompatible_generate_javacommon_source_jar",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "If set to true, java_common.compile will always generate an output source jar.")
public boolean incompatibleGenerateJavaCommonSourceJar;

/** Controls legacy arguments to ctx.actions.Args#add. */
@Option(
name = "incompatible_disallow_old_style_args_add",
Expand Down Expand Up @@ -535,6 +547,7 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
.incompatibleExpandDirectories(incompatibleExpandDirectories)
.incompatibleGenerateJavaCommonSourceJar(incompatibleGenerateJavaCommonSourceJar)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
.incompatibleNoAttrLicense(incompatibleNoAttrLicense)
.incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ public int compare(Object o1, Object o2) {
return compareLists((SkylarkList) o1, (SkylarkList) o2);
}

if (!(o1.getClass().isAssignableFrom(o2.getClass())
|| o2.getClass().isAssignableFrom(o1.getClass()))) {
throw new ComparisonException(
"Cannot compare " + getDataTypeName(o1) + " with " + getDataTypeName(o2));
}

if (o1 instanceof ClassObject) {
throw new ComparisonException("Cannot compare structs");
}
Expand Down Expand Up @@ -389,14 +395,14 @@ public static Iterable<?> toIterableStrict(Object o, Location loc, @Nullable Env
}

public static void lock(Object object, Location loc) {
if (object instanceof StarlarkMutable) {
((StarlarkMutable) object).lock(loc);
if (object instanceof SkylarkMutable) {
((SkylarkMutable) object).lock(loc);
}
}

public static void unlock(Object object, Location loc) {
if (object instanceof StarlarkMutable) {
((StarlarkMutable) object).unlock(loc);
if (object instanceof SkylarkMutable) {
((SkylarkMutable) object).unlock(loc);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean incompatibleExpandDirectories();

public abstract boolean incompatibleGenerateJavaCommonSourceJar();

public abstract boolean incompatibleNewActionsApi();

public abstract boolean incompatibleNoAttrLicense();
Expand Down Expand Up @@ -216,14 +218,15 @@ public static Builder builderWithDefaults() {
.incompatibleDisableDeprecatedAttrParams(false)
.incompatibleDisableObjcProviderResources(false)
.incompatibleDisallowDataTransition(true)
.incompatibleDisallowDictPlus(true)
.incompatibleDisallowDictPlus(false)
.incompatibleDisallowFileType(true)
.incompatibleDisallowLegacyJavaProvider(false)
.incompatibleDisallowLegacyJavaInfo(false)
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(false)
.incompatibleDisallowOldStyleArgsAdd(false)
.incompatibleDisallowStructProviderSyntax(false)
.incompatibleExpandDirectories(true)
.incompatibleGenerateJavaCommonSourceJar(true)
.incompatibleNewActionsApi(false)
.incompatibleNoAttrLicense(false)
.incompatibleNoOutputAttrDefault(false)
Expand Down Expand Up @@ -290,6 +293,8 @@ public abstract static class Builder {

public abstract Builder incompatibleExpandDirectories(boolean value);

public abstract Builder incompatibleGenerateJavaCommonSourceJar(boolean value);

public abstract Builder incompatibleNewActionsApi(boolean value);

public abstract Builder incompatibleNoAttrLicense(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_generate_javacommon_source_jar=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
"--incompatible_no_attr_license=" + rand.nextBoolean(),
"--incompatible_no_output_attr_default=" + rand.nextBoolean(),
Expand Down Expand Up @@ -193,6 +194,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleGenerateJavaCommonSourceJar(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
.incompatibleNoAttrLicense(rand.nextBoolean())
.incompatibleNoOutputAttrDefault(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -111,20 +112,25 @@ public void testComparatorWithDifferentTypes() throws Exception {
for (int i = 0; i < objects.length; ++i) {
for (int j = 0; j < objects.length; ++j) {
if (i != j) {
Object first = objects[i];
Object second = objects[j];
assertThrows(
ComparisonException.class, () -> EvalUtils.SKYLARK_COMPARATOR.compare(first, second));
try {
EvalUtils.SKYLARK_COMPARATOR.compare(objects[i], objects[j]);
fail("Shouldn't have compared different types");
} catch (ComparisonException e) {
// expected
}
}
}
}
}

@Test
public void testComparatorWithNones() throws Exception {
assertThrows(
ComparisonException.class,
() -> EvalUtils.SKYLARK_COMPARATOR.compare(Runtime.NONE, Runtime.NONE));
try {
EvalUtils.SKYLARK_COMPARATOR.compare(Runtime.NONE, Runtime.NONE);
fail("Shouldn't have compared nones");
} catch (ComparisonException e) {
// expected
}
}

@SkylarkModule(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,34 +678,4 @@ public void testArgBothPosKey() throws Exception {
+ "replace(old, new, maxsplit = None) of 'string'",
"'banana'.replace('a', 'o', 3, old='a')");
}

@Test
public void testDefInBuild() throws Exception {
new BuildTest()
.testIfErrorContains(
"function definitions are not allowed in BUILD files", "def func(): pass");
}

@Test
public void testForStatementForbiddenInBuild() throws Exception {
new BuildTest().testIfErrorContains("for loops are not allowed", "for _ in []: pass");
}

@Test
public void testIfStatementForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("if statements are not allowed in BUILD files", "if False: pass");
}

@Test
public void testKwargsForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("**kwargs arguments are not allowed in BUILD files", "func(**dict)");
}

@Test
public void testArgsForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("*args arguments are not allowed in BUILD files", "func(*array)");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2215,14 +2215,14 @@ public void testListComprehensionsShadowGlobalVariable() throws Exception {
public void testLoadStatementWithAbsolutePath() throws Exception {
checkEvalErrorContains(
"First argument of 'load' must be a label and start with either '//', ':', or '@'",
"load('/tmp/foo.bzl', 'arg')");
"load('/tmp/foo', 'arg')");
}

@Test
public void testLoadStatementWithRelativePath() throws Exception {
checkEvalErrorContains(
"First argument of 'load' must be a label and start with either '//', ':', or '@'",
"load('foo.bzl', 'arg')");
"load('foo', 'arg')");
}

@Test
Expand Down

0 comments on commit a807d27

Please sign in to comment.