enum cleanup (#7269)

### Problem

Resolves #7232, resolves #7248, and addresses #7249 (comment).

### Solution

- Remove enum defaults and the `.create()` method with complicated argument handling.
- Allow enums to be `==` compared as long as they're the same type.
- Allow accessing enum instances with class attributes.

### Result

enums are nicer!

src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py

@@ -118,9 +118,9 @@ def classify_target(cls, target):
     if target.has_sources('.java') or \
       isinstance(target, JUnitTests) or \
       (isinstance(target, ScalaLibrary) and tuple(target.java_sources)):
-      target_type = _JvmCompileWorkflowType.create('zinc-only')
+      target_type = _JvmCompileWorkflowType('zinc-only')
     elif target.has_sources('.scala'):
-      target_type = _JvmCompileWorkflowType.create('rsc-then-zinc')
+      target_type = _JvmCompileWorkflowType('rsc-then-zinc')
     else:
       target_type = None
     return target_type

src/python/pants/backend/jvm/tasks/nailgun_task.py

@@ -31,7 +31,7 @@ class ExecutionStrategy(enum([NAILGUN, SUBPROCESS, HERMETIC])): pass
   def register_options(cls, register):
     super(NailgunTaskBase, cls).register_options(register)
     register_enum_option(
-      register, cls.ExecutionStrategy, '--execution-strategy',
+      register, cls.ExecutionStrategy, '--execution-strategy', default=cls.NAILGUN,
       help='If set to nailgun, nailgun will be enabled and repeated invocations of this '
            'task will be quicker. If set to subprocess, then the task will be run without nailgun. '
            'Hermetic execution is an experimental subprocess execution framework.')
@@ -52,7 +52,7 @@ def execution_strategy_enum(self):
     # TODO: This .create() call can be removed when the enum interface is more stable as the option
     # is converted into an instance of self.ExecutionStrategy via the `type` argument through
     # register_enum_option().
-    return self.ExecutionStrategy.create(self.get_options().execution_strategy)
+    return self.ExecutionStrategy(self.get_options().execution_strategy)
 
   @classmethod
   def subsystem_dependencies(cls):

src/python/pants/backend/native/config/environment.py

@@ -15,9 +15,12 @@
 from pants.util.strutil import create_path_env_var
 
 
-class Platform(enum('normalized_os_name', all_normalized_os_names())):
+class Platform(enum(all_normalized_os_names())):
 
-  default_value = get_normalized_os_name()
+  # TODO: try to turn all of these accesses into v2 dependency injections!
+  @memoized_classproperty
+  def current(cls):
+    return cls(get_normalized_os_name())
 
 
 def _list_field(func):
@@ -155,7 +158,7 @@ def extra_args(self):
     :rtype: list of str
     """
 
-  _platform = Platform.create()
+  _platform = Platform.current
 
   @property
   def invocation_environment_dict(self):
@@ -299,5 +302,5 @@ class HostLibcDev(datatype(['crti_object', 'fingerprint'])): pass
 
 def create_native_environment_rules():
   return [
-    SingletonRule(Platform, Platform.create()),
+    SingletonRule(Platform, Platform.current),
   ]

src/python/pants/backend/native/subsystems/native_build_step.py

@@ -36,15 +36,21 @@ def register_options(cls, register):
                   'for targets of this language.')
 
     register_enum_option(
-      register, ToolchainVariant, '--toolchain-variant', advanced=True,
+      register, ToolchainVariant, '--toolchain-variant', advanced=True, default='gnu',
       help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
            "linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")
 
   def get_compiler_option_sets_for_target(self, target):
     return self.get_target_mirrored_option('compiler_option_sets', target)
 
   def get_toolchain_variant_for_target(self, target):
-    return ToolchainVariant.create(self.get_target_mirrored_option('toolchain_variant', target))
+    # TODO(#7233): convert this option into an enum instance using the `type` argument in option
+    # registration!
+    enum_or_value = self.get_target_mirrored_option('toolchain_variant', target)
+    if isinstance(enum_or_value, ToolchainVariant):
+      return enum_or_value
+    else:
+      return ToolchainVariant(enum_or_value)
 
   @classproperty
   def get_compiler_option_sets_enabled_default_value(cls):

src/python/pants/backend/native/subsystems/native_toolchain.py

@@ -110,7 +110,7 @@ def select_libc_objects(platform, native_toolchain):
 
 @rule(Assembler, [Select(Platform), Select(NativeToolchain)])
 def select_assembler(platform, native_toolchain):
-  if platform.normalized_os_name == 'darwin':
+  if platform == Platform.darwin:
     assembler = yield Get(Assembler, XCodeCLITools, native_toolchain._xcode_cli_tools)
   else:
     assembler = yield Get(Assembler, Binutils, native_toolchain._binutils)
@@ -127,7 +127,7 @@ class BaseLinker(datatype([('linker', Linker)])):
 # TODO: select the appropriate `Platform` in the `@rule` decl using variants!
 @rule(BaseLinker, [Select(Platform), Select(NativeToolchain)])
 def select_base_linker(platform, native_toolchain):
-  if platform.normalized_os_name == 'darwin':
+  if platform == Platform.darwin:
     # TODO(#5663): turn this into LLVM when lld works.
     linker = yield Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools)
   else:
@@ -172,7 +172,7 @@ def select_gcc_install_location(gcc):
 def select_llvm_c_toolchain(platform, native_toolchain):
   provided_clang = yield Get(CCompiler, LLVM, native_toolchain._llvm)
 
-  if platform.normalized_os_name == 'darwin':
+  if platform == Platform.darwin:
     xcode_clang = yield Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
     joined_c_compiler = provided_clang.sequence(xcode_clang)
   else:
@@ -200,9 +200,9 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
 
   # On OSX, we use the libc++ (LLVM) C++ standard library implementation. This is feature-complete
   # for OSX, but not for Linux (see https://libcxx.llvm.org/ for more info).
-  if platform.normalized_os_name == 'darwin':
-    xcode_clang = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
-    joined_cpp_compiler = provided_clangpp.sequence(xcode_clang)
+  if platform == Platform.darwin:
+    xcode_clangpp = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
+    joined_cpp_compiler = provided_clangpp.sequence(xcode_clangpp)
     extra_llvm_linking_library_dirs = []
     linker_extra_args = []
   else:
@@ -242,7 +242,7 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
 def select_gcc_c_toolchain(platform, native_toolchain):
   provided_gcc = yield Get(CCompiler, GCC, native_toolchain._gcc)
 
-  if platform.normalized_os_name == 'darwin':
+  if platform == Platform.darwin:
     # GCC needs access to some headers that are only provided by the XCode toolchain
     # currently (e.g. "_stdio.h"). These headers are unlikely to change across versions, so this is
     # probably safe.
@@ -267,7 +267,7 @@ def select_gcc_c_toolchain(platform, native_toolchain):
 def select_gcc_cpp_toolchain(platform, native_toolchain):
   provided_gpp = yield Get(CppCompiler, GCC, native_toolchain._gcc)
 
-  if platform.normalized_os_name == 'darwin':
+  if platform == Platform.darwin:
     # GCC needs access to some headers that are only provided by the XCode toolchain
     # currently (e.g. "_stdio.h"). These headers are unlikely to change across versions, so this is
     # probably safe.

src/python/pants/backend/native/targets/native_library.py

@@ -51,7 +51,7 @@ def toolchain_variant(self):
     if not self.payload.toolchain_variant:
       return None
 
-    return ToolchainVariant.create(self.payload.toolchain_variant)
+    return ToolchainVariant(self.payload.toolchain_variant)
 
   @property
   def strict_deps(self):

src/python/pants/backend/native/tasks/conan_fetch.py

@@ -124,7 +124,7 @@ def _conan_user_home(self, conan, in_workdir=False):
 
   @memoized_property
   def _conan_os_name(self):
-    return Platform.create().resolve_for_enum_variant({
+    return Platform.current.resolve_for_enum_variant({
       'darwin': 'Macos',
       'linux': 'Linux',
     })

src/python/pants/backend/native/tasks/link_shared_libraries.py

@@ -64,7 +64,7 @@ def linker(self, native_library_target):
   @memoized_property
   def platform(self):
     # TODO: convert this to a v2 engine dependency injection.
-    return Platform.create()
+    return Platform.current
 
   def execute(self):
     targets_providing_artifacts = self.context.targets(NativeLibrary.produces_ctypes_native_library)

src/python/pants/engine/fs.py

@@ -52,16 +52,18 @@ def __new__(cls, include, exclude=(), glob_match_error_behavior=None, conjunctio
 
     :param include: A list of filespecs to include.
     :param exclude: A list of filespecs to exclude.
-    :param glob_match_error_behavior: The value to pass to GlobMatchErrorBehavior.create()
+    :param GlobMatchErrorBehavior glob_match_error_behavior: How to respond to globs matching no
+                                                             files.
+    :param GlobExpansionConjunction conjunction: Whether all globs are expected to match at least
+                                                 one file, or if any glob matching is ok.
     :rtype: :class:`PathGlobs`
     """
     return super(PathGlobs, cls).__new__(
       cls,
       include=tuple(include),
       exclude=tuple(exclude),
-      glob_match_error_behavior=GlobMatchErrorBehavior.create(glob_match_error_behavior,
-                                                              none_is_default=True),
-      conjunction=GlobExpansionConjunction.create(conjunction, none_is_default=True))
+      glob_match_error_behavior=(glob_match_error_behavior or GlobMatchErrorBehavior.ignore),
+      conjunction=(conjunction or GlobExpansionConjunction.any_match))
 
 
 class Digest(datatype([('fingerprint', text_type), ('serialized_bytes_length', int)])):

src/python/pants/engine/legacy/structs.py

@@ -66,13 +66,13 @@ def get_sources(self):
         globs = Globs(*self.default_sources_globs,
                       spec_path=self.address.spec_path,
                       exclude=self.default_sources_exclude_globs or [])
-        conjunction_globs = GlobsWithConjunction(globs, GlobExpansionConjunction.create('any_match'))
+        conjunction_globs = GlobsWithConjunction(globs, GlobExpansionConjunction.any_match)
       else:
         globs = None
         conjunction_globs = None
     else:
       globs = BaseGlobs.from_sources_field(sources, self.address.spec_path)
-      conjunction_globs = GlobsWithConjunction(globs, GlobExpansionConjunction.create('all_match'))
+      conjunction_globs = GlobsWithConjunction(globs, GlobExpansionConjunction.all_match)
 
     return conjunction_globs
 
@@ -235,7 +235,7 @@ def _construct_bundles_field(self):
       # TODO: we want to have this field set from the global option --glob-expansion-failure, or
       # something set on the target. Should we move --glob-expansion-failure to be a bootstrap
       # option? See #5864.
-      path_globs = base_globs.to_path_globs(rel_root, GlobExpansionConjunction.create('all_match'))
+      path_globs = base_globs.to_path_globs(rel_root, GlobExpansionConjunction.all_match)
 
       filespecs_list.append(base_globs.filespecs)
       path_globs_list.append(path_globs)
@@ -263,7 +263,7 @@ def field_adaptors(self):
       if getattr(self, 'resources', None) is None:
         return field_adaptors
       base_globs = BaseGlobs.from_sources_field(self.resources, self.address.spec_path)
-      path_globs = base_globs.to_path_globs(self.address.spec_path, GlobExpansionConjunction.create('all_match'))
+      path_globs = base_globs.to_path_globs(self.address.spec_path, GlobExpansionConjunction.all_match)
       sources_field = SourcesField(self.address,
                                    'resources',
                                    base_globs.filespecs,
@@ -425,4 +425,4 @@ class GlobsWithConjunction(datatype([
 
   @classmethod
   def for_literal_files(cls, file_paths, spec_path):
-    return cls(Files(*file_paths, spec_path=spec_path), GlobExpansionConjunction.create('all_match'))
+    return cls(Files(*file_paths, spec_path=spec_path), GlobExpansionConjunction.all_match)

src/python/pants/init/engine_initializer.py

@@ -262,7 +262,9 @@ def setup_legacy_graph(native, options_bootstrapper, build_configuration):
       options_bootstrapper,
       build_configuration,
       native=native,
-      glob_match_error_behavior=bootstrap_options.glob_expansion_failure,
+      # TODO(#7233): convert this into an enum instance using the `type` argument in option
+      # registration!
+      glob_match_error_behavior=GlobMatchErrorBehavior(bootstrap_options.glob_expansion_failure),
       build_ignore_patterns=bootstrap_options.build_ignore,
       exclude_target_regexps=bootstrap_options.exclude_target_regexp,
       subproject_roots=bootstrap_options.subproject_roots,
@@ -346,8 +348,7 @@ def setup_legacy_graph_extended(
     rules = (
       [
         RootRule(Console),
-        SingletonRule.from_instance(GlobMatchErrorBehavior.create(glob_match_error_behavior,
-                                                                  none_is_default=True)),
+        SingletonRule.from_instance(glob_match_error_behavior or GlobMatchErrorBehavior.ignore),
         SingletonRule.from_instance(build_configuration),
         SingletonRule(SymbolTable, symbol_table),
       ] +

src/python/pants/option/custom_types.py

@@ -308,9 +308,9 @@ def __repr__(self):
     return '{} {}'.format(self.action, self.val)
 
 
-class GlobExpansionConjunction(enum('conjunction', ['any_match', 'all_match'])):
+class GlobExpansionConjunction(enum(['any_match', 'all_match'])):
+  """Describe whether to require that only some or all glob strings match in a target's sources.
 
-  # NB: The `default_value` is automatically the first element of the value list, but can be
-  # overridden or made more explicit in the body of the class. We want to be explicit here about
-  # which behavior we have when merging globs, as that can affect performance.
-  default_value = 'any_match'
+  NB: this object is interpreted from within Snapshot::lift_path_globs() -- that method will need to
+  be aware of any changes to this object's definition.
+  """

src/python/pants/option/global_options.py

@@ -19,7 +19,7 @@
 from pants.util.objects import datatype, enum, register_enum_option
 
 
-class GlobMatchErrorBehavior(enum('failure_behavior', ['ignore', 'warn', 'error'])):
+class GlobMatchErrorBehavior(enum(['ignore', 'warn', 'error'])):
   """Describe the action to perform when matching globs in BUILD files to source files.
 
   NB: this object is interpreted from within Snapshot::lift_path_globs() -- that method will need to

src/python/pants/util/BUILD

@@ -115,6 +115,7 @@ python_library(
   dependencies = [
     '3rdparty/python/twitter/commons:twitter.common.collections',
     '3rdparty/python:future',
+    '3rdparty/python:six',
     ':collections_abc_backport',
     ':memo',
     ':meta',