Skip to content

Commit

Permalink
Rebrand Parser to OptionRegistrar
Browse files Browse the repository at this point in the history
  • Loading branch information
benjyw committed Nov 16, 2024
1 parent 889eed4 commit 0cb6221
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 76 deletions.
26 changes: 13 additions & 13 deletions src/python/pants/help/help_formatter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
14 changes: 7 additions & 7 deletions src/python/pants/help/help_info_extracter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 = "",
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
18 changes: 9 additions & 9 deletions src/python/pants/help/help_info_extracter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 11 additions & 9 deletions src/python/pants/help/help_tools_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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


Expand Down
51 changes: 25 additions & 26 deletions src/python/pants/option/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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())
Expand Down Expand Up @@ -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()
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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]]:
Expand Down Expand Up @@ -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."""
Expand All @@ -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}")

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
6 changes: 1 addition & 5 deletions src/python/pants/option/options_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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,
)

Expand Down
Loading

0 comments on commit 0cb6221

Please sign in to comment.