Skip to content

Commit

Permalink
Delete the flag --incompatible_restrict_named_params.
Browse files Browse the repository at this point in the history
#8147

RELNOTES: The flag `--incompatible_restrict_named_params` is removed.
PiperOrigin-RevId: 303323249
  • Loading branch information
Googler authored and copybara-github committed Mar 27, 2020
1 parent fbba074 commit c0a11dc
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -583,20 +583,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "migration instructions.")
public boolean incompatibleUseCcConfigureFromRulesCc;

@Option(
name = "incompatible_restrict_named_params",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, restricts a number of Starlark built-in function parameters to be "
+ "only specifiable positionally (and not by keyword).")
public boolean incompatibleRestrictNamedParams;

@Option(
name = "incompatible_depset_for_libraries_to_link_getter",
defaultValue = "true",
Expand Down Expand Up @@ -705,7 +691,6 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)
.incompatibleRunShellCommandString(incompatibleRunShellCommandString)
.incompatibleVisibilityPrivateAttributesAtDefinition(
incompatibleVisibilityPrivateAttributesAtDefinition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ static ParamDescriptor of(Param param, StarlarkSemantics starlarkSemantics) {
type,
generic,
noneable,
param.named()
|| (param.legacyNamed() && !starlarkSemantics.incompatibleRestrictNamedParams()),
param.named(),
param.positional(),
getType(type, generic, param.allowedTypes(), noneable),
disabledByFlag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,6 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli

public abstract boolean incompatibleNoTargetOutputGroup();

public abstract boolean incompatibleRestrictNamedParams();

public abstract boolean incompatibleRunShellCommandString();

public abstract boolean incompatibleVisibilityPrivateAttributesAtDefinition();
Expand Down Expand Up @@ -362,7 +360,6 @@ public static Builder builderWithDefaults() {
.incompatibleNoSupportToolsInActionInputs(true)
.incompatibleNoTargetOutputGroup(true)
.incompatibleRunShellCommandString(false)
.incompatibleRestrictNamedParams(true)
.incompatibleVisibilityPrivateAttributesAtDefinition(false)
.internalSkylarkFlagTestCanary(false)
.incompatibleDoNotSplitLinkingCmdline(true)
Expand Down Expand Up @@ -446,8 +443,6 @@ public abstract static class Builder {

public abstract Builder incompatibleNoTargetOutputGroup(boolean value);

public abstract Builder incompatibleRestrictNamedParams(boolean value);

public abstract Builder incompatibleRunShellCommandString(boolean value);

public abstract Builder incompatibleVisibilityPrivateAttributesAtDefinition(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
"--incompatible_no_rule_outputs_param=" + rand.nextBoolean(),
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_restrict_named_params=" + rand.nextBoolean(),
"--incompatible_run_shell_command_string=" + rand.nextBoolean(),
"--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(),
"--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(),
Expand Down Expand Up @@ -213,7 +212,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.incompatibleNoRuleOutputsParam(rand.nextBoolean())
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
.incompatibleNoTargetOutputGroup(rand.nextBoolean())
.incompatibleRestrictNamedParams(rand.nextBoolean())
.incompatibleRunShellCommandString(rand.nextBoolean())
.incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean())
.incompatibleRequireLinkerInputCcApi(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,50 +690,9 @@ public void testTupleCoercion() throws Exception {
.testExpression("tuple({1: 'foo', 2: 'bar'}) == (1, 2)", true);
}

// Verifies some legacy functionality that should be deprecated and removed via
// an incompatible-change flag: parameters in MethodLibrary functions may be specified by
// keyword, or may be None, even in places where it does not quite make sense.
@Test
public void testLegacyNamed() throws Exception {
new Scenario("--incompatible_restrict_named_params=false")
// Parameters which may be specified by keyword but are not explicitly 'named'.
.testExpression("all(elements=[True, True])", Boolean.TRUE)
.testExpression("any(elements=[True, False])", Boolean.TRUE)
.testEval("sorted(iterable=[3, 0, 2], key=None, reverse=False)", "[0, 2, 3]")
.testEval("reversed(sequence=[3, 2, 0])", "[0, 2, 3]")
.testEval("tuple(x=[1, 2])", "(1, 2)")
.testEval("list(x=(1, 2))", "[1, 2]")
.testEval("len(x=(1, 2))", "2")
.testEval("str(x=(1, 2))", "'(1, 2)'")
.testEval("repr(x=(1, 2))", "'(1, 2)'")
.testExpression("bool(x=3)", Boolean.TRUE)
.testEval("int(x=3)", "3")
.testEval("dict(args=[(1, 2)])", "{1 : 2}")
.testExpression("bool(x=3)", Boolean.TRUE)
.testEval("enumerate(list=[40, 41])", "[(0, 40), (1, 41)]")
.testExpression("hash(value='hello')", "hello".hashCode())
.testEval("range(start_or_stop=3, stop_or_none=9, step=2)", "range(3, 9, 2)")
.testExpression("hasattr(x=depset(), name='to_list')", Boolean.TRUE)
.testExpression("bool(x=3)", Boolean.TRUE)
.testExpression("getattr(x='hello', name='cnt', default='default')", "default")
.testEval(
"dir(x={})",
"[\"clear\", \"get\", \"items\", \"keys\","
+ " \"pop\", \"popitem\", \"setdefault\", \"update\", \"values\"]")
.testEval("type(x=5)", "'int'")
.testEval("str(depset(items=[0,1]))", "'depset([0, 1])'")
.testIfErrorContains("hello", "fail(msg='hello', attr='someattr')")
// Parameters which may be None but are not explicitly 'noneable'
.testExpression("hasattr(x=None, name='to_list')", Boolean.FALSE)
.testEval("getattr(x=None, name='count', default=None)", "None")
.testEval("dir(None)", "[]")
.testIfErrorContains("None", "fail(msg=None)")
.testEval("type(None)", "'NoneType'");
}

@Test
public void testExperimentalStarlarkConfig() throws Exception {
new Scenario("--incompatible_restrict_named_params")
public void testPositionalOnlyArgument() throws Exception {
new Scenario()
.testIfErrorContains(
"join() got named argument for positional-only parameter 'elements'",
"','.join(elements=['foo', 'bar'])");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1093,32 +1093,6 @@ public void testProxyMethodsObject() throws Exception {
.testLookup("b", "with_params(1, true, false, true, false, a)");
}

@Test
public void testLegacyNamed() throws Exception {
new Scenario("--incompatible_restrict_named_params=false")
.update("mock", new Mock())
.setUp("b = mock.legacy_method(True, legacyNamed=True, named=True)")
.testLookup("b", "legacy_method(true, true, true)");

new Scenario("--incompatible_restrict_named_params=false")
.update("mock", new Mock())
.setUp("b = mock.legacy_method(True, True, named=True)")
.testLookup("b", "legacy_method(true, true, true)");

// Verify legacyNamed also works with proxy method objects.
new Scenario("--incompatible_restrict_named_params=false")
.update("mock", new Mock())
.setUp(
"m = mock.proxy_methods_object()",
"b = m.legacy_method(True, legacyNamed=True, named=True)")
.testLookup("b", "legacy_method(true, true, true)");

new Scenario("--incompatible_restrict_named_params=false")
.update("mock", new Mock())
.setUp("m = mock.proxy_methods_object()", "b = m.legacy_method(True, True, named=True)")
.testLookup("b", "legacy_method(true, true, true)");
}

/**
* This test verifies an error is raised when a method parameter is set both positionally and
* by name.
Expand Down

0 comments on commit c0a11dc

Please sign in to comment.