[internal] Require at least one flag name (#14762)

There's a pothole in the new options types. If someone forgets the flag name(s) they get a mysterious error pants.option.errors.NoOptionNames: No option names provided. ..... Now it's a type error with a clear message. tada [ci skip-build-wheels] [ci skip-rust]
pantsbuild · Mar 11, 2022 · 86d9183 · 86d9183
1 parent 732d944
commit 86d9183
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 17 deletions.
diff --git a/src/python/pants/option/option_types.py b/src/python/pants/option/option_types.py
@@ -78,7 +78,8 @@ class _OptionBase(Generic[_OptT, _DefaultT]):
     # `__new__` and mypy has issues if your class defines both.
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         default: _MaybeDynamicT[_DefaultT],
         help: _HelpT,
         register_if: _RegisterIfFuncT | None = None,
@@ -94,7 +95,7 @@ def __new__(
         removal_version: str | None = None,
     ):
         self = super().__new__(cls)
-        self._flag_names = flag_names
+        self._flag_names = (flag_name, *additional_flag_names)
         self._default = default
         self._help = help
         self._register_if = register_if or (lambda cls: True)
@@ -172,7 +173,8 @@ class _ListOptionBase(
 
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         default: _MaybeDynamicT[list[_ListMemberT]] = [],
         help: _HelpT,
         register_if: _RegisterIfFuncT | None = None,
@@ -190,7 +192,8 @@ def __new__(
         default = default or []
         instance = super().__new__(
             cls,  # type: ignore[arg-type]
-            *flag_names,
+            flag_name,
+            *additional_flag_names,
             default=default,  # type: ignore[arg-type]
             help=help,
             register_if=register_if,
@@ -385,7 +388,8 @@ class EnumOption(_OptionBase[_OptT, _DefaultT]):
     @overload
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         default: _EnumT,
         help: _HelpT,
         register_if: _RegisterIfFuncT | None = None,
@@ -406,7 +410,8 @@ def __new__(
     @overload  # Case: dynamic default
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         enum_type: type[_EnumT],
         default: _DynamicDefaultT,
         help: _HelpT,
@@ -428,7 +433,8 @@ def __new__(
     @overload  # Case: default is `None`
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         enum_type: type[_EnumT],
         default: None,
         help: _HelpT,
@@ -448,7 +454,8 @@ def __new__(
 
     def __new__(
         cls,
-        *flag_names,
+        flag_name,
+        *additional_flag_names,
         enum_type=None,
         default,
         help,
@@ -466,7 +473,8 @@ def __new__(
     ):
         instance = super().__new__(
             cls,
-            *flag_names,
+            flag_name,
+            *additional_flag_names,
             default=default,
             help=help,
             register_if=register_if,
@@ -515,7 +523,8 @@ class EnumListOption(_ListOptionBase[_OptT], Generic[_OptT]):
     @overload  # Case: static default
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         default: list[_EnumT],
         help: _HelpT,
         register_if: _RegisterIfFuncT | None = None,
@@ -536,7 +545,8 @@ def __new__(
     @overload  # Case: dynamic default
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         enum_type: type[_EnumT],
         default: _DynamicDefaultT,
         help: _HelpT,
@@ -558,7 +568,8 @@ def __new__(
     @overload  # Case: implicit default
     def __new__(
         cls,
-        *flag_names: str,
+        flag_name: str,
+        *additional_flag_names: str,
         enum_type: type[_EnumT],
         help: _HelpT,
         register_if: _RegisterIfFuncT | None = None,
@@ -577,7 +588,8 @@ def __new__(
 
     def __new__(
         cls,
-        *flag_names,
+        flag_name,
+        *additional_flag_names,
         enum_type=None,
         default=[],
         help,
@@ -595,7 +607,8 @@ def __new__(
     ):
         instance = super().__new__(
             cls,
-            *flag_names,
+            flag_name,
+            *additional_flag_names,
             default=default,
             help=help,
             register_if=register_if,
@@ -657,7 +670,8 @@ class DictOption(_OptionBase["dict[str, _ValueT]", "dict[str, _ValueT]"], Generi
 
     def __new__(
         cls,
-        *flag_names,
+        flag_name: str,
+        *additional_flag_names: str,
         default: _MaybeDynamicT[dict[str, _ValueT]] = {},
         help,
         register_if: _RegisterIfFuncT | None = None,
@@ -673,7 +687,8 @@ def __new__(
     ):
         return super().__new__(
             cls,  # type: ignore[arg-type]
-            *flag_names,
+            flag_name,
+            *additional_flag_names,
             default=default,  # type: ignore[arg-type]
             help=help,
             register_if=register_if,

diff --git a/src/python/pants/option/option_types_test.py b/src/python/pants/option/option_types_test.py
@@ -417,7 +417,7 @@ def __init__(self):
         dict_opt5 = DictOption("--opt", default=dict(key="val"), help="")
         dict_opt6 = DictOption("--opt", default=dict(key=1), help="")
         dict_opt7 = DictOption("--opt", default=dict(key1=1, key2="str"), help="")
-        dyn_dict_opt = DictOption[str]("--opt", lambda cls: cls.default, help="")
+        dyn_dict_opt = DictOption[str]("--opt", default=lambda cls: cls.default, help="")
 
         # Specialized Opts
         skip_opt = SkipOption("fmt")