diff --git a/pants.toml b/pants.toml index 7a439e83bd1..12f80ccbe3d 100644 --- a/pants.toml +++ b/pants.toml @@ -91,9 +91,8 @@ enabled = true repo_id = "7775F8D5-FC58-4DBC-9302-D00AE4A1505F" [cli.alias] -all-changed = "--changed-since=HEAD --changed-dependents=transitive" run-pyupgrade = "--backend-packages=pants.backend.experimental.python.lint.pyupgrade fmt" - +--all-changed = "--changed-since=HEAD --changed-dependents=transitive" [source] root_patterns = [ diff --git a/src/python/pants/option/alias.py b/src/python/pants/option/alias.py index 1f2a7ef8c27..122f7cc0043 100644 --- a/src/python/pants/option/alias.py +++ b/src/python/pants/option/alias.py @@ -64,15 +64,15 @@ class CliAlias: definitions: FrozenDict[str, tuple[str, ...]] = field(default_factory=FrozenDict) def __post_init__(self): - valid_alias_re = re.compile(r"\w(\w|-)*\w$", re.IGNORECASE) + valid_alias_re = re.compile(r"(--)?\w(\w|-)*\w$", re.IGNORECASE) for alias in self.definitions.keys(): if not re.match(valid_alias_re, alias): raise CliAliasInvalidError( softwrap( f""" Invalid alias in `[cli].alias` option: {alias!r}. May only contain alpha - numerical letters and the separators `-` and `_`, and may not begin/end - with a `-`. + numerical letters and the separators `-` and `_`. Flags can be defined using + `--`. A single dash is not allowed. """ ) ) @@ -104,9 +104,12 @@ def expand( ) ) - def check_name_conflicts(self, known_scopes: dict[str, ScopeInfo]) -> None: + def check_name_conflicts( + self, known_scopes: dict[str, ScopeInfo], known_flags: dict[str, frozenset[str]] + ) -> None: for alias in self.definitions.keys(): scope = known_scopes.get(alias) + if scope: raise CliAliasInvalidError( softwrap( @@ -117,6 +120,19 @@ def check_name_conflicts(self, known_scopes: dict[str, ScopeInfo]) -> None: ) ) + for scope_name, args in known_flags.items(): + for alias in self.definitions.keys(): + if alias in args: + scope_name = scope_name or "global" + raise CliAliasInvalidError( + softwrap( + f""" + Invalid flag-like alias in `[cli].alias` option: {alias!r}. This is + already a registered flag in the {scope_name!r} scope. + """ + ) + ) + def expand_args(self, args: tuple[str, ...]) -> tuple[str, ...]: if not self.definitions: return args diff --git a/src/python/pants/option/alias_test.py b/src/python/pants/option/alias_test.py index a2b9bb6de9c..f7e65b5d9a0 100644 --- a/src/python/pants/option/alias_test.py +++ b/src/python/pants/option/alias_test.py @@ -36,6 +36,13 @@ def test_maybe_expand_alias(alias: str, expanded: tuple[str, ...] | None) -> Non ) assert cli_alias.maybe_expand("alias") == expanded + cli_alias = CliAlias.from_dict( + { + "--alias": alias, + } + ) + assert cli_alias.maybe_expand("--alias") == expanded + @pytest.mark.parametrize( "args, expanded", @@ -60,6 +67,29 @@ def test_expand_args(args: tuple[str, ...], expanded: tuple[str, ...]) -> None: assert cli_alias.expand_args(args) == expanded +@pytest.mark.parametrize( + "args, expanded", + [ + ( + ("some", "--alias", "target"), + ("some", "--flag", "goal", "target"), + ), + ( + # Don't touch pass through args. + ("some", "--", "--alias", "target"), + ("some", "--", "--alias", "target"), + ), + ], +) +def test_expand_args_flag(args: tuple[str, ...], expanded: tuple[str, ...]) -> None: + cli_alias = CliAlias.from_dict( + { + "--alias": "--flag goal", + } + ) + assert cli_alias.expand_args(args) == expanded + + def test_no_expand_when_no_aliases() -> None: args = ("./pants",) cli_alias = CliAlias() @@ -110,6 +140,32 @@ def test_no_expand_when_no_aliases() -> None: ), ), ), + ( + { + "cycle": "--other-alias", + "--other-alias": "cycle", + }, + pytest.raises( + CliAliasCycleError, + match=( + r"CLI alias cycle detected in `\[cli\]\.alias` option:\n" + + r"--other-alias -> cycle -> --other-alias" + ), + ), + ), + ( + { + "--cycle": "--other-alias", + "--other-alias": "--cycle", + }, + pytest.raises( + CliAliasCycleError, + match=( + r"CLI alias cycle detected in `\[cli\]\.alias` option:\n" + + r"--other-alias -> --cycle -> --other-alias" + ), + ), + ), ], ) def test_nested_alias(alias, definitions: dict | ContextManager) -> None: @@ -128,9 +184,7 @@ def test_nested_alias(alias, definitions: dict | ContextManager) -> None: "file.name", "target:name", "-o", - "--o", "-option", - "--option", ], ) def test_invalid_alias_name(alias: str) -> None: @@ -148,4 +202,35 @@ def test_banned_alias_names() -> None: r"Invalid alias in `\[cli\]\.alias` option: 'fmt'\. This is already a registered goal\." ), ): - cli_alias.check_name_conflicts({"fmt": ScopeInfo("fmt", is_goal=True)}) + cli_alias.check_name_conflicts({"fmt": ScopeInfo("fmt", is_goal=True)}, {}) + + +@pytest.mark.parametrize( + "alias, info, expected", + [ + ( + {"--keep-sandboxes": "--foobar"}, + {"": "--keep-sandboxes"}, + pytest.raises( + CliAliasInvalidError, + match=( + r"Invalid flag-like alias in `\[cli\]\.alias` option: '--keep-sandboxes'\. This is already a registered flag in the 'global' scope\." + ), + ), + ), + ( + {"--changed-since": "--foobar"}, + {"changed": "--changed-since"}, + pytest.raises( + CliAliasInvalidError, + match=( + r"Invalid flag-like alias in `\[cli\]\.alias` option: '--changed-since'\. This is already a registered flag in the 'changed' scope\." + ), + ), + ), + ], +) +def test_banned_alias_flag_names(alias, info, expected) -> None: + cli_alias = CliAlias.from_dict(alias) + with expected: + cli_alias.check_name_conflicts({}, info) diff --git a/src/python/pants/option/options.py b/src/python/pants/option/options.py index b9320dc6970..8be9d9864dd 100644 --- a/src/python/pants/option/options.py +++ b/src/python/pants/option/options.py @@ -226,6 +226,10 @@ def unknown_goals(self) -> list[str]: def known_scope_to_info(self) -> dict[str, ScopeInfo]: return self._known_scope_to_info + @property + def known_scope_to_scoped_args(self) -> dict[str, frozenset[str]]: + return {scope: parser.known_scoped_args for scope, parser in self._parser_by_scope.items()} + @property def scope_to_flags(self) -> dict[str, list[str]]: return self._scope_to_flags diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index 8ec410386e1..7c1fb66db6f 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -305,7 +305,9 @@ def full_options( allow_unknown_options=build_configuration.allow_unknown_options, ) GlobalOptions.validate_instance(options.for_global_scope()) - self.alias.check_name_conflicts(options.known_scope_to_info) + self.alias.check_name_conflicts( + options.known_scope_to_info, options.known_scope_to_scoped_args + ) return options diff --git a/src/python/pants/option/parser.py b/src/python/pants/option/parser.py index fabfdc7fdfe..643a42a4a31 100644 --- a/src/python/pants/option/parser.py +++ b/src/python/pants/option/parser.py @@ -138,6 +138,11 @@ def scope_info(self) -> ScopeInfo: def scope(self) -> str: return self._scope + @property + def known_scoped_args(self) -> frozenset[str]: + prefix = f"{self.scope}-" if self.scope != GLOBAL_SCOPE else "" + return frozenset(f"--{prefix}{arg.lstrip('--')}" for arg in self._known_args) + def history(self, dest: str) -> OptionValueHistory | None: return self._history.get(dest)