Do not leak subsystems as goals to the command line interface (#12337)

Closes #12333.

Signed-off-by: Andreas Stenius <[email protected]>

[ci skip-rust]

src/python/pants/engine/goal.py

@@ -8,6 +8,7 @@
 
 from typing_extensions import final
 
+from pants.option.scope import ScopeInfo
 from pants.option.subsystem import Subsystem
 from pants.util.meta import classproperty
 
@@ -37,6 +38,10 @@ def list(console: Console, list_subsystem: ListSubsystem) -> List:
     # it should declare the union types that must have members.
     required_union_implementations: Tuple[Type, ...] = ()
 
+    @classmethod
+    def create_scope_info(cls, **scope_info_kwargs) -> ScopeInfo:
+        return super().create_scope_info(is_goal=True, **scope_info_kwargs)
+
     @classproperty
     @abstractmethod
     def name(cls):

src/python/pants/engine/goal_test.py

@@ -4,6 +4,7 @@
 from pants.engine.console import Console
 from pants.engine.goal import Goal, GoalSubsystem, LineOriented
 from pants.engine.rules import goal_rule
+from pants.option.scope import ScopeInfo
 from pants.testutil.option_util import create_goal_subsystem, create_options_bootstrapper
 from pants.testutil.rule_runner import mock_console, run_rule_with_mocks
 
@@ -33,3 +34,14 @@ def output_rule(console: Console, options: OutputtingGoalOptions) -> OutputtingG
         )
         assert result.exit_code == 0
         assert stdio_reader.get_stdout() == "output...line oriented\n"
+
+
+def test_goal_scope_flag() -> None:
+    class DummyGoal(GoalSubsystem):
+        name = "dummy"
+
+    dummy = create_goal_subsystem(DummyGoal)
+    assert dummy.get_scope_info().is_goal is True
+    assert dummy.known_scope_infos() == {
+        ScopeInfo(scope="dummy", optionable_cls=DummyGoal, is_goal=True)
+    }

src/python/pants/option/arg_splitter.py

@@ -90,6 +90,7 @@ def __init__(self, known_scope_infos: Iterable[ScopeInfo], buildroot: str) -> No
             "help-advanced",
             "help-all",
         }
+        self._known_goal_scopes = {si.scope for si in known_scope_infos if si.is_goal}
         self._unconsumed_args: List[
             str
         ] = []  # In reverse order, for efficient popping off the end.
@@ -171,7 +172,10 @@ def assign_flag_to_scope(flg: str, default_scope: str) -> None:
         while scope:
             if not self._check_for_help_request(scope.lower()):
                 add_scope(scope)
-                goals.add(scope.partition(".")[0])
+                if scope in self._known_goal_scopes:
+                    goals.add(scope.partition(".")[0])
+                else:
+                    unknown_scopes.append(scope)
                 for flag in flags:
                     assign_flag_to_scope(flag, scope)
             scope, flags = self._consume_scope()

src/python/pants/option/arg_splitter_test.py

@@ -22,15 +22,18 @@
 
 class ArgSplitterTest(unittest.TestCase):
     _known_scope_infos = [
-        ScopeInfo("compile"),
-        ScopeInfo("compile.java"),
-        ScopeInfo("compile.scala"),
-        ScopeInfo("jvm"),
-        ScopeInfo("jvm.test.junit"),
-        ScopeInfo("reporting"),
-        ScopeInfo("test"),
-        ScopeInfo("test.junit"),
-    ]
+        ScopeInfo(scope, is_goal=True)
+        for scope in [
+            "compile",
+            "compile.java",
+            "compile.scala",
+            "jvm",
+            "jvm.test.junit",
+            "reporting",
+            "test",
+            "test.junit",
+        ]
+    ] + [ScopeInfo("hidden", is_goal=False)]
 
     @staticmethod
     def assert_valid_split(
@@ -384,3 +387,6 @@ def test_no_goal_detection(self) -> None:
         splitter = ArgSplitter(ArgSplitterTest._known_scope_infos, buildroot=os.getcwd())
         splitter.split_args(shlex.split("./pants foo/bar:baz"))
         self.assertTrue(isinstance(splitter.help_request, NoGoalHelp))
+
+    def test_hidden_scope_is_unknown_goal(self) -> None:
+        self.assert_unknown_goal("./pants hidden", ["hidden"])

src/python/pants/option/optionable.py

@@ -94,12 +94,17 @@ def validate_scope_name_component(cls, s: str) -> None:
                 f"dash-separated-words, with words consisting only of lower-case letters and digits."
             )
 
+    @classmethod
+    def create_scope_info(cls, **scope_info_kwargs) -> ScopeInfo:
+        """One place to create scope info, to allow subclasses to inject custom scope args."""
+        return ScopeInfo(**scope_info_kwargs)
+
     @classmethod
     def get_scope_info(cls) -> ScopeInfo:
         """Returns a ScopeInfo instance representing this Optionable's options scope."""
         if cls.options_scope is None:
             raise OptionsError(f"{cls.__name__} must set options_scope.")
-        return ScopeInfo(scope=cast(str, cls.options_scope), optionable_cls=cls)
+        return cls.create_scope_info(scope=cast(str, cls.options_scope), optionable_cls=cls)
 
     @classmethod
     def subscope(cls, scope) -> str:

src/python/pants/option/options_test.py

@@ -59,7 +59,7 @@ def global_scope() -> ScopeInfo:
 
 
 def task(scope: str) -> ScopeInfo:
-    return ScopeInfo(scope)
+    return ScopeInfo(scope, is_goal=True)
 
 
 def intermediate(scope: str) -> ScopeInfo:

src/python/pants/option/scope.py

@@ -34,6 +34,9 @@ class ScopeInfo:
     removal_version: Optional[str] = None
     removal_hint: Optional[str] = None
 
+    # command line goal scope flag
+    is_goal: bool = False
+
     @property
     def description(self) -> str:
         return cast(str, getattr(self.optionable_cls, "help"))

src/python/pants/option/subsystem.py

@@ -96,7 +96,7 @@ def get_scope_info(cls, subscope=None) -> ScopeInfo:
         if subscope is None:
             return super().get_scope_info()
         else:
-            return ScopeInfo(cls.subscope(subscope), cls)
+            return cls.create_scope_info(scope=cls.subscope(subscope), optionable_cls=cls)
 
     def __init__(self, scope: str, options: OptionValueContainer) -> None:
         super().__init__()
@@ -191,9 +191,9 @@ def collect_scope_infos(optionable_cls, scoped_to, removal_version=None, removal
                 if scoped_to == GLOBAL_SCOPE
                 else optionable_cls.subscope(scoped_to)
             )
-            scope_info = ScopeInfo(
-                scope,
-                optionable_cls,
+            scope_info = cls.create_scope_info(
+                scope=scope,
+                optionable_cls=optionable_cls,
                 removal_version=removal_version,
                 removal_hint=removal_hint,
             )