diff --git a/src/python/pants/help/help_formatter_test.py b/src/python/pants/help/help_formatter_test.py index 2af34178f9b..9c9f1519d85 100644 --- a/src/python/pants/help/help_formatter_test.py +++ b/src/python/pants/help/help_formatter_test.py @@ -9,8 +9,8 @@ from pants.help.help_info_extracter import HelpInfoExtracter, OptionHelpInfo from pants.option.global_options import GlobalOptions from pants.option.native_options import NativeOptionParser -from pants.option.parser import OptionValueHistory, Parser from pants.option.ranked_value import Rank, RankedValue +from pants.option.registrar import OptionRegistrar, OptionValueHistory class TestOptionHelpFormatter: @@ -58,8 +58,8 @@ def test_format_help_choices(self) -> None: assert default_line.lstrip() == "default: kiwi" @classmethod - def _get_parser(cls) -> Tuple[Parser, NativeOptionParser]: - return Parser( + def _get_registrar_and_parser(cls) -> Tuple[OptionRegistrar, NativeOptionParser]: + return OptionRegistrar( scope=GlobalOptions.options_scope, ), NativeOptionParser( args=[], env={}, config_sources=[], allow_pantsrc=False, include_derivation=True @@ -69,36 +69,36 @@ def _get_parser(cls) -> Tuple[Parser, NativeOptionParser]: def _format_for_global_scope( cls, show_advanced: bool, show_deprecated: bool, args: list[str], kwargs ) -> list[str]: - parser, native_parser = cls._get_parser() - parser.register(*args, **kwargs) + registrar, native_parser = cls._get_registrar_and_parser() + registrar.register(*args, **kwargs) return cls._format_for_global_scope_with_parser( - parser, native_parser, show_advanced=show_advanced, show_deprecated=show_deprecated + registrar, native_parser, show_advanced=show_advanced, show_deprecated=show_deprecated ) @classmethod def _format_for_global_scope_with_parser( cls, - parser: Parser, + registrar: OptionRegistrar, native_parser: NativeOptionParser, show_advanced: bool, show_deprecated: bool, ) -> list[str]: oshi = HelpInfoExtracter("").get_option_scope_help_info( - "", parser, native_parser, False, "help.test" + "", registrar, native_parser, False, "help.test" ) return HelpFormatter( show_advanced=show_advanced, show_deprecated=show_deprecated, color=False ).format_options(oshi) def test_suppress_advanced(self) -> None: - parser, native_parser = self._get_parser() - parser.register("--foo", advanced=True) + registrar, native_parser = self._get_registrar_and_parser() + registrar.register("--foo", advanced=True) # must have a non advanced option to be able to supress showing advanced options. - parser.register("--jerry", advanced=False) - lines = self._format_for_global_scope_with_parser(parser, native_parser, False, False) + registrar.register("--jerry", advanced=False) + lines = self._format_for_global_scope_with_parser(registrar, native_parser, False, False) assert len(lines) == 15 assert not any("--foo" in line for line in lines) - lines = self._format_for_global_scope_with_parser(parser, native_parser, True, False) + lines = self._format_for_global_scope_with_parser(registrar, native_parser, True, False) assert len(lines) == 24 def test_suppress_deprecated(self) -> None: diff --git a/src/python/pants/help/help_info_extracter.py b/src/python/pants/help/help_info_extracter.py index 0697c952efc..d4c2b93e7ec 100644 --- a/src/python/pants/help/help_info_extracter.py +++ b/src/python/pants/help/help_info_extracter.py @@ -45,8 +45,8 @@ from pants.option.native_options import NativeOptionParser, parse_dest from pants.option.option_util import is_dict_option, is_list_option from pants.option.options import Options -from pants.option.parser import OptionValueHistory, Parser from pants.option.ranked_value import Rank, RankedValue +from pants.option.registrar import OptionRegistrar, OptionValueHistory from pants.option.scope import GLOBAL_SCOPE, ScopeInfo from pants.util.frozendict import LazyFrozenDict from pants.util.strutil import first_paragraph, strval @@ -508,7 +508,7 @@ def load() -> OptionScopeHelpInfo: ) return HelpInfoExtracter(scope_info.scope).get_option_scope_help_info( scope_info.description, - options.get_parser(scope_info.scope), + options.get_registrar(scope_info.scope), options.native_parser.with_derivation(), # `filter` should be treated as a subsystem for `help`, even though it still # works as a goal for backwards compatibility. @@ -1003,7 +1003,7 @@ def __init__(self, scope: str): def get_option_scope_help_info( self, description: str, - parser: Parser, + registrar: OptionRegistrar, native_parser: NativeOptionParser, is_goal: bool, provider: str = "", @@ -1014,11 +1014,11 @@ def get_option_scope_help_info( basic_options = [] advanced_options = [] deprecated_options = [] - for args, kwargs in parser.option_registrations_iter(): + for args, kwargs in registrar.option_registrations_iter(): derivation = native_parser.get_derivation( - scope=parser.scope, registration_args=args, registration_kwargs=kwargs + scope=registrar.scope, registration_args=args, registration_kwargs=kwargs ) - # Massage the derivation structure returned by the NativeParser into an + # Massage the derivation structure returned by the NativeOptionParser into an # OptionValueHistory as returned by the legacy parser. # TODO: Once we get rid of the legacy parser we can probably simplify by # using the native structure directly. @@ -1071,7 +1071,7 @@ def get_option_help_info(self, args, kwargs): scoped_arg = arg scoped_cmd_line_args.append(scoped_arg) - if Parser.is_bool(kwargs): + if OptionRegistrar.is_bool(kwargs): if is_short_arg: display_args.append(scoped_arg) else: diff --git a/src/python/pants/help/help_info_extracter_test.py b/src/python/pants/help/help_info_extracter_test.py index 4fec7d68a1a..f5606a8c583 100644 --- a/src/python/pants/help/help_info_extracter_test.py +++ b/src/python/pants/help/help_info_extracter_test.py @@ -18,8 +18,8 @@ from pants.option.native_options import NativeOptionParser from pants.option.option_types import BoolOption, IntListOption, StrListOption from pants.option.options import Options -from pants.option.parser import Parser from pants.option.ranked_value import Rank +from pants.option.registrar import OptionRegistrar from pants.option.scope import GLOBAL_SCOPE from pants.option.subsystem import Subsystem from pants.util.logging import LogLevel @@ -98,15 +98,15 @@ def do_test( def test_default() -> None: def do_test(args, kwargs, expected_default_str): - # Defaults are computed in the parser and added into the kwargs, so we + # Defaults are computed in the registrar and added into the kwargs, so we # must jump through this hoop in this test. - parser = Parser( + registrar = OptionRegistrar( scope=GlobalOptions.options_scope, ) native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True) - parser.register(*args, **kwargs) - oshi = HelpInfoExtracter(parser.scope).get_option_scope_help_info( - "description", parser, native_parser, False, "provider" + registrar.register(*args, **kwargs) + oshi = HelpInfoExtracter(registrar.scope).get_option_scope_help_info( + "description", registrar, native_parser, False, "provider" ) assert oshi.description == "description" assert oshi.provider == "provider" @@ -198,11 +198,11 @@ 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=GlobalOptions.options_scope) + registrar = OptionRegistrar(scope=GlobalOptions.options_scope) native_parser = NativeOptionParser([], {}, [], allow_pantsrc=False, include_derivation=True) - parser.register("--foo", **kwargs) + registrar.register("--foo", **kwargs) oshi = HelpInfoExtracter("").get_option_scope_help_info( - "", parser, native_parser, False, "" + "", registrar, native_parser, False, "" ) assert exp_to_len(expected_basic) == len(oshi.basic) assert exp_to_len(expected_advanced) == len(oshi.advanced) diff --git a/src/python/pants/help/help_tools_test.py b/src/python/pants/help/help_tools_test.py index 7aa1490bb4c..65558fdd589 100644 --- a/src/python/pants/help/help_tools_test.py +++ b/src/python/pants/help/help_tools_test.py @@ -11,12 +11,12 @@ from pants.help.maybe_color import MaybeColor from pants.option.global_options import GlobalOptions from pants.option.native_options import NativeOptionParser -from pants.option.parser import Parser +from pants.option.registrar import OptionRegistrar @pytest.fixture -def parser() -> Parser: - return Parser(scope=GlobalOptions.options_scope) +def registrar() -> OptionRegistrar: + return OptionRegistrar(scope=GlobalOptions.options_scope) @pytest.fixture @@ -30,17 +30,19 @@ def extracter() -> HelpInfoExtracter: @pytest.fixture -def tool_info(extracter, parser, native_parser) -> ToolHelpInfo: - 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) +def tool_info(extracter, registrar, native_parser) -> ToolHelpInfo: + registrar.register("--version", type=str, default="1.0") + registrar.register("--url-template", type=str, default="https://download/{version}") + oshi = extracter.get_option_scope_help_info( + "Test description.", registrar, native_parser, False + ) tool_info = ToolHelpInfo.from_option_scope_help_info(oshi) assert tool_info is not None return tool_info -def test_no_tool_help_info(extracter, parser, native_parser) -> None: - oshi = extracter.get_option_scope_help_info("", parser, native_parser, False) +def test_no_tool_help_info(extracter, registrar, native_parser) -> None: + oshi = extracter.get_option_scope_help_info("", registrar, native_parser, False) assert ToolHelpInfo.from_option_scope_help_info(oshi) is None diff --git a/src/python/pants/option/options.py b/src/python/pants/option/options.py index 0eae8cc5260..c7aff939ca6 100644 --- a/src/python/pants/option/options.py +++ b/src/python/pants/option/options.py @@ -21,8 +21,8 @@ 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.parser import Parser from pants.option.ranked_value import Rank, RankedValue +from pants.option.registrar import OptionRegistrar from pants.option.scope import GLOBAL_SCOPE, GLOBAL_SCOPE_CONFIG_SECTION, ScopeInfo from pants.util.memo import memoized_method from pants.util.ordered_set import FrozenOrderedSet, OrderedSet @@ -122,9 +122,6 @@ def create( args: Sequence[str], bootstrap_option_values: OptionValueContainer | None = None, allow_unknown_options: bool = False, - # We default to error to be strict in tests, but set explicitly in OptionsBootstrapper - # for user-facing behavior. - native_options_validation: NativeOptionsValidation = NativeOptionsValidation.error, native_options_config_discovery: bool = True, include_derivation: bool = False, ) -> Options: @@ -137,12 +134,11 @@ def create( :param bootstrap_option_values: An optional namespace containing the values of bootstrap options. We can use these values when registering other options. :param allow_unknown_options: Whether to ignore or error on unknown cmd-line flags. - :param native_options_validation: How to validate the native options parser against the - legacy Python parser. - :param native_options_config_discovery: Whether to discover config files in the native parser or use the ones supplied + :param native_options_config_discovery: Whether to discover config files in the native + parser or use the ones supplied. :param include_derivation: Whether to gather option value derivation information. """ - # We need parsers for all the intermediate scopes, so inherited option values + # We need registrars for all the intermediate scopes, so inherited option values # can propagate through them. complete_known_scope_infos = cls.complete_scopes(known_scope_infos) splitter = ArgSplitter(complete_known_scope_infos, get_buildroot()) @@ -170,7 +166,9 @@ def create( [line for line in [line.strip() for line in f] if line] ) - parser_by_scope = {si.scope: Parser(si.scope) for si in complete_known_scope_infos} + registrar_by_scope = { + si.scope: OptionRegistrar(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() @@ -190,9 +188,8 @@ def create( scope_to_flags=split_args.scope_to_flags, specs=split_args.specs, passthru=split_args.passthru, - parser_by_scope=parser_by_scope, + registrar_by_scope=registrar_by_scope, native_parser=native_parser, - native_options_validation=native_options_validation, bootstrap_option_values=bootstrap_option_values, known_scope_to_info=known_scope_to_info, allow_unknown_options=allow_unknown_options, @@ -206,9 +203,8 @@ def __init__( scope_to_flags: dict[str, list[str]], specs: list[str], passthru: list[str], - parser_by_scope: dict[str, Parser], + registrar_by_scope: dict[str, OptionRegistrar], native_parser: NativeOptionParser, - native_options_validation: NativeOptionsValidation, bootstrap_option_values: OptionValueContainer | None, known_scope_to_info: dict[str, ScopeInfo], allow_unknown_options: bool = False, @@ -223,7 +219,7 @@ def __init__( self._scope_to_flags = scope_to_flags self._specs = specs self._passthru = passthru - self._parser_by_scope = parser_by_scope + self._registrar_by_scope = registrar_by_scope self._native_parser = native_parser self._bootstrap_option_values = bootstrap_option_values self._known_scope_to_info = known_scope_to_info @@ -271,7 +267,10 @@ def known_scope_to_info(self) -> dict[str, ScopeInfo]: @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()} + return { + scope: registrar.known_scoped_args + for scope, registrar in self._registrar_by_scope.items() + } @property def scope_to_flags(self) -> dict[str, list[str]]: @@ -318,10 +317,10 @@ def is_known_scope(self, scope: str) -> bool: def register(self, scope: str, *args, **kwargs) -> None: """Register an option in the given scope.""" - self.get_parser(scope).register(*args, **kwargs) + self.get_registrar(scope).register(*args, **kwargs) deprecated_scope = self.known_scope_to_info[scope].deprecated_scope if deprecated_scope: - self.get_parser(deprecated_scope).register(*args, **kwargs) + self.get_registrar(deprecated_scope).register(*args, **kwargs) def registration_function_for_subsystem(self, subsystem_cls): """Returns a function for registering options on the given scope.""" @@ -337,15 +336,15 @@ def register(*args, **kwargs): register.scope = subsystem_cls.options_scope return register - def get_parser(self, scope: str) -> Parser: - """Returns the parser for the given scope, so code can register on it directly. + def get_registrar(self, scope: str) -> OptionRegistrar: + """Returns the registrar for the given scope, so code can register on it directly. - :param scope: The scope to retrieve the parser for. - :return: The parser for the given scope. + :param scope: The scope to retrieve the registrar for. + :return: The registrar for the given scope. :raises pants.option.errors.ConfigValidationError: if the scope is not known. """ try: - return self._parser_by_scope[scope] + return self._registrar_by_scope[scope] except KeyError: raise ConfigValidationError(f"No such options scope: {scope}") @@ -418,10 +417,10 @@ def for_scope( """ builder = OptionValueContainerBuilder() mutex_map = defaultdict(list) - parser = self.get_parser(scope) + registrar = self.get_registrar(scope) scope_str = "global scope" if scope == GLOBAL_SCOPE else f"scope '{scope}'" - for args, kwargs in parser.option_registrations_iter(): + for args, kwargs in registrar.option_registrations_iter(): dest = kwargs["dest"] val, rank = self._native_parser.get_value( scope=scope, registration_args=args, registration_kwargs=kwargs @@ -483,9 +482,9 @@ def get_fingerprintable_for_scope( """ pairs = [] - parser = self.get_parser(scope) + registrar = self.get_registrar(scope) # Sort the arguments, so that the fingerprint is consistent. - for _, kwargs in sorted(parser.option_registrations_iter()): + for _, kwargs in sorted(registrar.option_registrations_iter()): if not kwargs.get("fingerprint", True): continue if daemon_only and not kwargs.get("daemon", False): diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index e4257d1547f..5d4eeabf73b 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -18,7 +18,7 @@ from pants.option.custom_types import DictValueComponent, ListValueComponent from pants.option.global_options import BootstrapOptions, GlobalOptions from pants.option.option_types import collect_options_info -from pants.option.options import NativeOptionsValidation, Options +from pants.option.options import Options from pants.option.scope import GLOBAL_SCOPE, ScopeInfo from pants.option.subsystem import Subsystem from pants.util.dirutil import read_file @@ -97,9 +97,6 @@ def parse_bootstrap_options( config=config, known_scope_infos=[GlobalOptions.get_scope_info()], args=args, - # We ignore validation to ensure bootstrapping succeeds. - # The bootstrap options will be validated anyway when we parse the full options. - native_options_validation=NativeOptionsValidation.ignore, native_options_config_discovery=False, ) @@ -259,7 +256,6 @@ def _full_options( args=self.args, bootstrap_option_values=bootstrap_option_values, allow_unknown_options=allow_unknown_options, - native_options_validation=bootstrap_option_values.native_options_validation, native_options_config_discovery=False, ) diff --git a/src/python/pants/option/parser.py b/src/python/pants/option/registrar.py similarity index 96% rename from src/python/pants/option/parser.py rename to src/python/pants/option/registrar.py index 4800d2c51e5..56bb2600e09 100644 --- a/src/python/pants/option/parser.py +++ b/src/python/pants/option/registrar.py @@ -53,8 +53,8 @@ def final_value(self) -> RankedValue: return self.ranked_values[-1] -class Parser: - """An argument parser.""" +class OptionRegistrar: + """Holds information about registered options.""" @staticmethod def is_bool(kwargs: Mapping[str, Any]) -> bool: @@ -89,13 +89,13 @@ def _invert(cls, s: bool | str | None) -> bool | None: return not b def __init__(self, scope: str) -> None: - """Create a Parser instance. + """Create an OptionRegistrar instance. - :param scope: the scope this parser acts for. + :param scope: the scope this registrar acts for. """ self._scope = scope - # All option args registered with this parser. Used to prevent conflicts. + # All option args registered with this registrar. Used to prevent conflicts. self._known_args: set[str] = set() # List of (args, kwargs) registration pairs, exactly as captured at registration time. @@ -115,7 +115,7 @@ def known_scoped_args(self) -> frozenset[str]: def option_registrations_iter(self): """Returns an iterator over the normalized registration arguments of each option in this - parser. + registrar. Useful for generating help and other documentation. @@ -308,4 +308,4 @@ def to_value_type(cls, val_str, type_arg, member_type): ) def __str__(self) -> str: - return f"Parser({self.scope})" + return f"OptionRegistrar({self.scope})"