diff --git a/src/python/pants/help/help_formatter_test.py b/src/python/pants/help/help_formatter_test.py index bd2a20108b6..2af34178f9b 100644 --- a/src/python/pants/help/help_formatter_test.py +++ b/src/python/pants/help/help_formatter_test.py @@ -60,7 +60,7 @@ def test_format_help_choices(self) -> None: @classmethod def _get_parser(cls) -> Tuple[Parser, NativeOptionParser]: return Parser( - scope_info=GlobalOptions.get_scope_info(), + scope=GlobalOptions.options_scope, ), NativeOptionParser( args=[], env={}, config_sources=[], allow_pantsrc=False, include_derivation=True ) diff --git a/src/python/pants/help/help_info_extracter_test.py b/src/python/pants/help/help_info_extracter_test.py index 69e2a1b8f5d..4fec7d68a1a 100644 --- a/src/python/pants/help/help_info_extracter_test.py +++ b/src/python/pants/help/help_info_extracter_test.py @@ -101,7 +101,7 @@ def do_test(args, kwargs, expected_default_str): # Defaults are computed in the parser and added into the kwargs, so we # must jump through this hoop in this test. parser = Parser( - scope_info=GlobalOptions.get_scope_info(), + scope=GlobalOptions.options_scope, ) native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True) parser.register(*args, **kwargs) @@ -198,9 +198,7 @@ def do_test(kwargs, expected_basic=False, expected_advanced=False): def exp_to_len(exp): return int(exp) # True -> 1, False -> 0. - parser = Parser( - scope_info=GlobalOptions.get_scope_info(), - ) + parser = Parser(scope=GlobalOptions.options_scope) native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True) parser.register("--foo", **kwargs) oshi = HelpInfoExtracter("").get_option_scope_help_info( diff --git a/src/python/pants/help/help_tools_test.py b/src/python/pants/help/help_tools_test.py index 0209c2f4a8a..7aa1490bb4c 100644 --- a/src/python/pants/help/help_tools_test.py +++ b/src/python/pants/help/help_tools_test.py @@ -16,9 +16,7 @@ @pytest.fixture def parser() -> Parser: - return Parser( - scope_info=GlobalOptions.get_scope_info(), - ) + return Parser(scope=GlobalOptions.options_scope) @pytest.fixture @@ -33,8 +31,8 @@ def extracter() -> HelpInfoExtracter: @pytest.fixture def tool_info(extracter, parser, native_parser) -> ToolHelpInfo: - parser.register("version", typ=str, default="1.0") - parser.register("url-template", typ=str, default="https://download/{version}") + parser.register("--version", type=str, default="1.0") + parser.register("--url-template", type=str, default="https://download/{version}") oshi = extracter.get_option_scope_help_info("Test description.", parser, native_parser, False) tool_info = ToolHelpInfo.from_option_scope_help_info(oshi) assert tool_info is not None diff --git a/src/python/pants/option/native_options.py b/src/python/pants/option/native_options.py index 43af47d7fbf..b6c63154abd 100644 --- a/src/python/pants/option/native_options.py +++ b/src/python/pants/option/native_options.py @@ -23,7 +23,7 @@ logger = logging.getLogger() -def parse_dest(*args, **kwargs): +def parse_dest(*args: str, **kwargs) -> str: """Return the dest for an option registration. If an explicit `dest` is specified, returns that and otherwise derives a default from the @@ -36,7 +36,7 @@ def parse_dest(*args, **kwargs): """ dest = kwargs.get("dest") if dest: - return dest + return str(dest) # No explicit dest, so compute one based on the first long arg, or the short arg # if that's all there is. arg = next((a for a in args if a.startswith("--")), args[0]) diff --git a/src/python/pants/option/options.py b/src/python/pants/option/options.py index a22171ff06b..93e4a324443 100644 --- a/src/python/pants/option/options.py +++ b/src/python/pants/option/options.py @@ -15,7 +15,7 @@ from pants.option.errors import ConfigValidationError, UnknownFlagsError from pants.option.native_options import NativeOptionParser from pants.option.option_util import is_list_option -from pants.option.option_value_container import OptionValueContainer, OptionValueContainerBuilder +from pants.option.option_value_container import OptionValueContainer from pants.option.parser import Parser from pants.option.scope import GLOBAL_SCOPE, GLOBAL_SCOPE_CONFIG_SECTION, ScopeInfo from pants.util.memo import memoized_method @@ -164,7 +164,7 @@ def create( [line for line in [line.strip() for line in f] if line] ) - parser_by_scope = {si.scope: Parser(si) for si in complete_known_scope_infos} + parser_by_scope = {si.scope: Parser(si.scope) for si in complete_known_scope_infos} known_scope_to_info = {s.scope: s for s in complete_known_scope_infos} config_to_pass = None if native_options_config_discovery else config.sources() @@ -392,16 +392,6 @@ def _check_and_apply_deprecations(self, scope, values): hint=f"Use scope {scope} instead (options: {', '.join(explicit_keys)})", ) - def _make_parse_args_request( - self, flags_in_scope, namespace: OptionValueContainerBuilder - ) -> Parser.ParseArgsRequest: - return Parser.ParseArgsRequest( - flags_in_scope=flags_in_scope, - namespace=namespace, - passthrough_args=self._passthru, - allow_unknown_flags=self._allow_unknown_options, - ) - # TODO: Eagerly precompute backing data for this? @memoized_method def for_scope( diff --git a/src/python/pants/option/options_test.py b/src/python/pants/option/options_test.py index 1a9f6dab958..735fb7224d6 100644 --- a/src/python/pants/option/options_test.py +++ b/src/python/pants/option/options_test.py @@ -1014,7 +1014,7 @@ def test_parse_dest() -> None: def test_validation() -> None: - def assertError(expected_error, *args, **kwargs): + def assert_error(expected_error, *args, **kwargs): with pytest.raises(expected_error): options = Options.create( args=["./pants"], @@ -1025,17 +1025,17 @@ def assertError(expected_error, *args, **kwargs): options.register(GLOBAL_SCOPE, *args, **kwargs) options.for_global_scope() - assertError(NoOptionNames) - assertError(OptionNameDoubleDash, "badname") - assertError(OptionNameDoubleDash, "-badname") - assertError(InvalidKwarg, "--foo", badkwarg=42) - assertError(BooleanOptionNameWithNo, "--no-foo", type=bool) - assertError(MemberTypeNotAllowed, "--foo", member_type=int) - assertError(MemberTypeNotAllowed, "--foo", type=dict, member_type=int) - assertError(InvalidMemberType, "--foo", type=list, member_type=set) - assertError(InvalidMemberType, "--foo", type=list, member_type=list) - assertError(HelpType, "--foo", help=()) - assertError(HelpType, "--foo", help=("Help!",)) + assert_error(NoOptionNames) + assert_error(OptionNameDoubleDash, "badname") + assert_error(OptionNameDoubleDash, "-badname") + assert_error(InvalidKwarg, "--foo", badkwarg=42) + assert_error(BooleanOptionNameWithNo, "--no-foo", type=bool) + assert_error(MemberTypeNotAllowed, "--foo", member_type=int) + assert_error(MemberTypeNotAllowed, "--foo", type=dict, member_type=int) + assert_error(InvalidMemberType, "--foo", type=list, member_type=set) + assert_error(InvalidMemberType, "--foo", type=list, member_type=list) + assert_error(HelpType, "--foo", help=()) + assert_error(HelpType, "--foo", help=("Help!",)) def test_shadowing() -> None: @@ -1046,7 +1046,7 @@ def test_shadowing() -> None: args=["./pants"], ) options.register("", "--opt1") - options.register("foo", "-o", "--opt2") + options.register("foo", "--opt2") def test_is_known_scope() -> None: diff --git a/src/python/pants/option/parser.py b/src/python/pants/option/parser.py index e5ccf9bee76..7f8d54b8e37 100644 --- a/src/python/pants/option/parser.py +++ b/src/python/pants/option/parser.py @@ -10,7 +10,7 @@ from collections import defaultdict from dataclasses import dataclass from enum import Enum -from typing import Any, Iterable, Mapping +from typing import Any, Mapping from pants.base.deprecated import validate_deprecation_semver, warn_or_error from pants.option.custom_types import ( @@ -42,7 +42,7 @@ from pants.option.native_options import NativeOptionParser, parse_dest from pants.option.option_value_container import OptionValueContainer, OptionValueContainerBuilder from pants.option.ranked_value import Rank, RankedValue -from pants.option.scope import GLOBAL_SCOPE, ScopeInfo +from pants.option.scope import GLOBAL_SCOPE from pants.util.strutil import softwrap logger = logging.getLogger(__name__) @@ -92,16 +92,12 @@ def _invert(cls, s: bool | str | None) -> bool | None: b = cls.ensure_bool(s) return not b - def __init__( - self, - scope_info: ScopeInfo, - ) -> None: + def __init__(self, scope: str) -> None: """Create a Parser instance. :param scope_info: the scope this parser acts for. """ - self._scope_info = scope_info - self._scope = self._scope_info.scope + self._scope = scope # All option args registered with this parser. Used to prevent conflicts. self._known_args: set[str] = set() @@ -112,10 +108,6 @@ def __init__( # Map of dest -> history. self._history: dict[str, OptionValueHistory] = {} - @property - def scope_info(self) -> ScopeInfo: - return self._scope_info - @property def scope(self) -> str: return self._scope @@ -125,30 +117,6 @@ 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) - - @dataclass(frozen=True) - class ParseArgsRequest: - namespace: OptionValueContainerBuilder - passthrough_args: list[str] - allow_unknown_flags: bool - - def __init__( - self, - flags_in_scope: Iterable[str], - namespace: OptionValueContainerBuilder, - passthrough_args: list[str], - allow_unknown_flags: bool, - ) -> None: - """ - :param flags_in_scope: Iterable of arg strings to parse into flag values. - :param namespace: The object to register the flag values on - """ - object.__setattr__(self, "namespace", namespace) - object.__setattr__(self, "passthrough_args", passthrough_args) - object.__setattr__(self, "allow_unknown_flags", allow_unknown_flags) - def parse_args_native( self, native_parser: NativeOptionParser, @@ -156,7 +124,6 @@ def parse_args_native( namespace = OptionValueContainerBuilder() mutex_map = defaultdict(list) for args, kwargs in self._option_registrations: - self._validate(args, kwargs) dest = parse_dest(*args, **kwargs) val, rank = native_parser.get_value( scope=self.scope, registration_args=args, registration_kwargs=kwargs @@ -213,9 +180,9 @@ def normalize_kwargs(orig_args, orig_kwargs): def register(self, *args, **kwargs) -> None: """Register an option.""" - if args: - dest = parse_dest(*args, **kwargs) - self._check_deprecated(dest, kwargs, print_warning=False) + self._validate(args, kwargs) + dest = parse_dest(*args, **kwargs) + self._check_deprecated(dest, kwargs, print_warning=False) if self.is_bool(kwargs): default = kwargs.get("default") @@ -316,7 +283,7 @@ def error( default_value = kwargs.get("default") if default_value is not None: if isinstance(default_value, str) and type_arg != str: - # attempt to parse default value, for correctness.. + # attempt to parse default value, for correctness. # custom function types may implement their own validation default_value = self.to_value_type(default_value, type_arg, member_type) if hasattr(default_value, "val"): @@ -324,7 +291,11 @@ def error( # fall through to type check, to verify that custom types returned a value of correct type - if isinstance(type_arg, type) and not isinstance(default_value, type_arg): + if ( + isinstance(type_arg, type) + and not isinstance(default_value, type_arg) + and not (issubclass(type_arg, bool) and default_value == UnsetBool) + ): error( DefaultValueType, option_type=type_arg.__name__,