Reject empty separator in string.split
Related: #7355 #8726
Closes #8836.
PiperOrigin-RevId: 257614252
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 6efc954..72de9de 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -17,6 +17,7 @@
* [Dictionary lookup of unhashable types](#dictionary-lookup-of-unhashable-types)
* [Dictionary concatenation](#dictionary-concatenation)
* [String escapes](#string-escapes)
+* [String.split empty separator](#string.split-empty-separator)
* [Load must appear at top of file](#load-must-appear-at-top-of-file)
* [Depset is no longer iterable](#depset-is-no-longer-iterable)
* [Depset union](#depset-union)
@@ -97,6 +98,15 @@
* Default: `false`
* Tracking issue: [#8380](https://github.com/bazelbuild/bazel/issues/8380)
+### String.split empty separator
+
+We are disallowing empty strings as separators to `string.split`. If `sep` is
+the empty string, `split` will fail.
+
+* Flag: `--incompatible_disallow_split_empty_separator`
+* Default: `false`
+* Tracking issue: [#7355](https://github.com/bazelbuild/bazel/issues/7355)
+
### Load must appear at top of file
Previously, the `load` statement could appear anywhere in a `.bzl` file so long
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index a266ff7..13d233a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -329,6 +329,18 @@
public boolean incompatibleDisallowRuleExecutionPlatformConstraintsAllowed;
@Option(
+ name = "incompatible_disallow_split_empty_separator",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "If set to true, `string.split` will fail if `sep` is the empty string.")
+ public boolean incompatibleDisallowSplitEmptySeparator;
+
+ @Option(
name = "incompatible_string_join_requires_strings",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -668,6 +680,7 @@
.incompatibleDoNotSplitLinkingCmdline(incompatibleDoNotSplitLinkingCmdline)
.incompatibleDepsetForLibrariesToLinkGetter(incompatibleDepsetForLibrariesToLinkGetter)
.incompatibleRestrictStringEscapes(incompatibleRestrictStringEscapes)
+ .incompatibleDisallowSplitEmptySeparator(incompatibleDisallowSplitEmptySeparator)
.incompatibleDisallowDictLookupUnhashableKeys(
incompatibleDisallowDictLookupUnhashableKeys)
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 67d7b7f..048262e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -204,6 +204,8 @@
public abstract boolean incompatibleRestrictStringEscapes();
+ public abstract boolean incompatibleDisallowSplitEmptySeparator();
+
public abstract boolean incompatibleDisallowDictLookupUnhashableKeys();
@Memoized
@@ -280,6 +282,7 @@
.incompatibleDoNotSplitLinkingCmdline(true)
.incompatibleDepsetForLibrariesToLinkGetter(true)
.incompatibleRestrictStringEscapes(false)
+ .incompatibleDisallowSplitEmptySeparator(false)
.incompatibleDisallowDictLookupUnhashableKeys(false)
.build();
@@ -371,6 +374,8 @@
public abstract Builder incompatibleRestrictStringEscapes(boolean value);
+ public abstract Builder incompatibleDisallowSplitEmptySeparator(boolean value);
+
public abstract Builder incompatibleDisallowDictLookupUnhashableKeys(boolean value);
public abstract StarlarkSemantics build();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java b/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
index f5f0728..4a6c6f9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
@@ -329,6 +329,9 @@
public MutableList<String> split(
String self, String sep, Object maxSplitO, Location loc, Environment env)
throws EvalException {
+ if (env.getSemantics().incompatibleDisallowSplitEmptySeparator() && sep.isEmpty()) {
+ throw new EvalException(loc, "Empty separator");
+ }
int maxSplit =
Type.INTEGER.convertOptional(maxSplitO, "'split' argument of 'split'", /*label*/ null, -2);
// + 1 because the last result is the remainder. The default is -2 so that after +1,
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 6aa2045..3d50b23 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
@@ -145,8 +145,9 @@
"--incompatible_disallow_legacy_javainfo=" + rand.nextBoolean(),
"--incompatible_disallow_legacy_java_provider=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
- "--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_disallow_rule_execution_platform_constraints_allowed=" + rand.nextBoolean(),
+ "--incompatible_disallow_split_empty_separator=" + rand.nextBoolean(),
+ "--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(),
"--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
@@ -198,8 +199,9 @@
.incompatibleDisallowLegacyJavaInfo(rand.nextBoolean())
.incompatibleDisallowLegacyJavaProvider(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
- .incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(rand.nextBoolean())
+ .incompatibleDisallowSplitEmptySeparator(rand.nextBoolean())
+ .incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleDoNotSplitLinkingCmdline(rand.nextBoolean())
.incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index f78d647..5a913d9 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -3100,6 +3100,30 @@
}
@Test
+ public void testSplitEmptySeparatorForbidden() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_split_empty_separator=true");
+
+ scratch.file("test/extension.bzl", "y = 'abc'.split('')");
+
+ scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent("Empty separator");
+ }
+
+ @Test
+ public void testSplitEmptySeparator() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_split_empty_separator=false");
+
+ scratch.file("test/extension.bzl", "y = 'abc'.split('')");
+
+ scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
+
+ getConfiguredTarget("//test:r");
+ }
+
+ @Test
public void testNoOutputsError() throws Exception {
scratch.file(
"test/skylark/test_rule.bzl",
diff --git a/src/test/starlark/testdata/string_split.sky b/src/test/starlark/testdata/string_split.sky
index e8c5ba1..4cefcd6 100644
--- a/src/test/starlark/testdata/string_split.sky
+++ b/src/test/starlark/testdata/string_split.sky
@@ -4,10 +4,15 @@
assert_eq('a,e,i,o,u'.split(',', 2), ['a', 'e', 'i,o,u'])
assert_eq(' 1 2 3 '.split(' '), ['', '', '1', '', '2', '', '3', '', ''])
+---
+
# rsplit
assert_eq('abcdabef'.rsplit('ab'), ['', 'cd', 'ef'])
assert_eq('google_or_gogol'.rsplit('go'), ['', 'ogle_or_', '', 'l'])
+'abc'.rsplit("") ### Empty separator
+---
+
# rsplit regex
assert_eq('foo/bar.lisp'.rsplit('.'), ['foo/bar', 'lisp'])
assert_eq('foo/bar.?lisp'.rsplit('.?'), ['foo/bar', 'lisp'])
