From 21c746085679145e82dcde4dc41f24701513ded1 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Thu, 17 Oct 2019 22:35:50 -0500
Subject: [PATCH 01/10] add some more ergonomics to sentinel decorators

---
 src/python/pants/engine/native.py    |  3 +-
 src/python/pants/engine/objects.py   | 36 +++++++++++++++++++++++
 src/python/pants/engine/rules.py     | 43 ++--------------------------
 src/python/pants/engine/scheduler.py |  4 +--
 src/python/pants/util/meta.py        | 31 ++++++++++++++++++--
 5 files changed, 71 insertions(+), 46 deletions(-)

diff --git a/src/python/pants/engine/native.py b/src/python/pants/engine/native.py
index 827fd02502b..2bc6949dff1 100644
--- a/src/python/pants/engine/native.py
+++ b/src/python/pants/engine/native.py
@@ -37,6 +37,7 @@
   FallibleExecuteProcessResult,
   MultiPlatformExecuteProcessRequest,
 )
+from pants.engine.objects import union
 from pants.engine.selectors import Get
 from pants.util.contextutil import temporary_dir
 from pants.util.dirutil import read_file, safe_mkdir, safe_mkdtemp
@@ -315,7 +316,7 @@ def extern_is_union(self, context_handle, type_id):
     """Return whether or not a type is a member of a union"""
     c = self._ffi.from_handle(context_handle)
     input_type = c.from_id(type_id.tup_0)
-    return bool(getattr(input_type, '_is_union', None))
+    return union.is_instance(input_type)
 
   _do_raise_keyboardinterrupt_on_identify = bool(os.environ.get('_RAISE_KEYBOARDINTERRUPT_IN_CFFI_IDENTIFY', False))
 
diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py
index 1cba87c6ee2..eef0e705748 100644
--- a/src/python/pants/engine/objects.py
+++ b/src/python/pants/engine/objects.py
@@ -167,3 +167,39 @@ def __iter__(self) -> Iterator[_C]:
 
   def __bool__(self) -> bool:
     return bool(self.dependencies)
+
+
+@sentinel_attribute('_is_union')
+def union(cls):
+  """A class decorator which other classes can specify that they can resolve to with `UnionRule`.
+
+  Annotating a class with @union allows other classes to use a UnionRule() instance to indicate that
+  they can be resolved to this base union class. This class will never be instantiated, and should
+  have no members -- it is used as a tag only, and will be replaced with whatever object is passed
+  in as the subject of a `await Get(...)`. See the following example:
+
+  @union
+  class UnionBase: pass
+
+  @rule
+  async def get_some_union_type(x: X) -> B:
+    result = await Get(ResultType, UnionBase, x.f())
+    # ...
+
+  If there exists a single path from (whatever type the expression `x.f()` returns) -> `ResultType`
+  in the rule graph, the engine will retrieve and execute that path to produce a `ResultType` from
+  `x.f()`. This requires also that whatever type `x.f()` returns was registered as a union member of
+  `UnionBase` with a `UnionRule`.
+
+  Unions allow @rule bodies to be written without knowledge of what types may eventually be provided
+  as input -- rather, they let the engine check that there is a valid path to the desired result.
+  """
+  # TODO: Check that the union base type is used as a tag and nothing else (e.g. no attributes)!
+  assert isinstance(cls, type)
+  def non_member_error_message(subject):
+    if hasattr(cls, 'non_member_error_message'):
+      return cls.non_member_error_message(subject)
+    desc = f' ("{cls.__doc__}")' if cls.__doc__ else ''
+    return f'Type {type(subject).__name__} is not a member of the {cls.__name__} @union{desc}'
+
+  return union.define_instance_of(cls, non_member_error_message=staticmethod(non_member_error_message))
diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py
index 3154e3b91d0..da03a5de787 100644
--- a/src/python/pants/engine/rules.py
+++ b/src/python/pants/engine/rules.py
@@ -14,6 +14,7 @@
 from twitter.common.collections import OrderedSet
 
 from pants.engine.goal import Goal
+from pants.engine.objects import union
 from pants.engine.selectors import Get
 from pants.util.collections import assert_single_element
 from pants.util.memo import memoized
@@ -250,44 +251,6 @@ def console_rule(*args, **kwargs) -> Callable:
   return inner_rule(*args, **kwargs, cacheable=False)
 
 
-def union(cls):
-  """A class decorator which other classes can specify that they can resolve to with `UnionRule`.
-
-  Annotating a class with @union allows other classes to use a UnionRule() instance to indicate that
-  they can be resolved to this base union class. This class will never be instantiated, and should
-  have no members -- it is used as a tag only, and will be replaced with whatever object is passed
-  in as the subject of a `await Get(...)`. See the following example:
-
-  @union
-  class UnionBase: pass
-
-  @rule
-  def get_some_union_type(x: X) -> B:
-    result = await Get(ResultType, UnionBase, x.f())
-    # ...
-
-  If there exists a single path from (whatever type the expression `x.f()` returns) -> `ResultType`
-  in the rule graph, the engine will retrieve and execute that path to produce a `ResultType` from
-  `x.f()`. This requires also that whatever type `x.f()` returns was registered as a union member of
-  `UnionBase` with a `UnionRule`.
-
-  Unions allow @rule bodies to be written without knowledge of what types may eventually be provided
-  as input -- rather, they let the engine check that there is a valid path to the desired result.
-  """
-  # TODO: Check that the union base type is used as a tag and nothing else (e.g. no attributes)!
-  assert isinstance(cls, type)
-  def non_member_error_message(subject):
-    if hasattr(cls, 'non_member_error_message'):
-      return cls.non_member_error_message(subject)
-    desc = f' ("{cls.__doc__}")' if cls.__doc__ else ''
-    return f'Type {type(subject).__name__} is not a member of the {cls.__name__} @union{desc}'
-
-  return type(cls.__name__, (cls,), {
-    '_is_union': True,
-    'non_member_error_message': non_member_error_message,
-  })
-
-
 @dataclass(frozen=True)
 class UnionRule:
   """Specify that an instance of `union_member` can be substituted wherever `union_base` is used."""
@@ -295,7 +258,7 @@ class UnionRule:
   union_member: Type
 
   def __post_init__(self) -> None:
-    if not getattr(self.union_base, '_is_union', False):
+    if not union.is_instance(self.union_base):
       raise ValueError(
         f'union_base must be a type annotated with @union: was {self.union_base} '
         f'(type {type(self.union_base).__name__})'
@@ -465,7 +428,7 @@ def add_type_transition_rule(union_rule):
       # NB: This does not require that union bases be supplied to `def rules():`, as the union type
       # is never instantiated!
       union_base = union_rule.union_base
-      assert union_base._is_union
+      assert union.is_instance(union_base)
       union_member = union_rule.union_member
       if union_base not in union_rules:
         union_rules[union_base] = OrderedSet()
diff --git a/src/python/pants/engine/scheduler.py b/src/python/pants/engine/scheduler.py
index 84adbeae330..36b230eee0e 100644
--- a/src/python/pants/engine/scheduler.py
+++ b/src/python/pants/engine/scheduler.py
@@ -22,7 +22,7 @@
 )
 from pants.engine.native import Function, TypeId
 from pants.engine.nodes import Return, Throw
-from pants.engine.objects import Collection
+from pants.engine.objects import Collection, union
 from pants.engine.rules import RuleIndex, TaskRule
 from pants.engine.selectors import Params
 from pants.util.contextutil import temporary_file_path
@@ -187,7 +187,7 @@ def add_get_edge(product, subject):
       self._native.lib.tasks_add_get(self._tasks, self._to_type(product), self._to_type(subject))
 
     for the_get in rule.input_gets:
-      if getattr(the_get.subject_declared_type, '_is_union', False):
+      if union.is_instance(the_get.subject_declared_type):
         # If the registered subject type is a union, add Get edges to all registered union members.
         for union_member in union_rules.get(the_get.subject_declared_type, []):
           add_get_edge(the_get.product, union_member)
diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index 7984379a258..416485b2e0d 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -122,6 +122,30 @@ def staticproperty(func: Callable[..., T]) -> T:
   return ClassPropertyDescriptor(func, doc)  # type: ignore[arg-type, return-value]
 
 
+class ClassDecoratorWithSentinelAttribute(Protocol):
+  sentinel_attribute: str
+
+  def __call__(self, cls: Type[T]) -> Type[T]: ...
+
+  def define_instance_of(self, obj: Type[T], **kwargs) -> Type[T]: ...
+
+  def is_instance(self, obj: Any) -> bool: ...
+
+
+def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[Type[T]], Type[T]]], ClassDecoratorWithSentinelAttribute]:
+  def wrapper(func: Callable[[Type[T]], Type[T]]) -> ClassDecoratorWithSentinelAttribute:
+    ret = cast(ClassDecoratorWithSentinelAttribute, func)
+    ret.sentinel_attribute = attribute_name
+    ret.is_instance = lambda obj: hasattr(obj, ret.sentinel_attribute) # type: ignore
+    ret.define_instance_of = lambda obj, **kwargs: type(obj.__name__, (obj,), { # type: ignore
+      ret.sentinel_attribute: True,
+      **kwargs
+    })
+    return ret
+  return wrapper
+
+
+@sentinel_attribute('_frozen_after_init')
 def frozen_after_init(cls: C) -> C:
   """Class decorator to freeze any modifications to the object after __init__() is done.
 
@@ -146,6 +170,7 @@ def new_setattr(self, key: str, value: Any) -> None:
       )
     prev_setattr(self, key, value)  # type: ignore[call-arg]
 
-  cls.__init__ = new_init
-  cls.__setattr__ = new_setattr
-  return cls
+  cls.__init__ = new_init  # type: ignore[assignment]
+  cls.__setattr__ = new_setattr  # type: ignore[assignment]
+
+  return frozen_after_init.define_instance_of(cls)

From 15f8434fcfe606936b1bf59c476673a73f8d535c Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sun, 20 Oct 2019 15:12:08 -0700
Subject: [PATCH 02/10] add tests for @sentinel_attribute

---
 tests/python/pants_test/util/test_meta.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py
index ea7c33c19ee..af64387ff71 100644
--- a/tests/python/pants_test/util/test_meta.py
+++ b/tests/python/pants_test/util/test_meta.py
@@ -6,7 +6,8 @@
 from dataclasses import FrozenInstanceError, dataclass
 
 from pants.testutil.test_base import TestBase
-from pants.util.meta import SingletonMetaclass, classproperty, frozen_after_init, staticproperty
+from pants.util.meta import (SingletonMetaclass, classproperty, frozen_after_init,
+                             sentinel_attribute, staticproperty)
 
 
 class AbstractClassTest(TestBase):
@@ -246,6 +247,22 @@ def f(cls):
     self.assertEqual(Concrete2.f, 'hello')
 
 
+class SentinelAttributeTest(unittest.TestCase):
+
+  def test_sentinel_attribute(self):
+    @sentinel_attribute('_test_attr_name')
+    def f(cls):
+      return f.define_instance_of(cls)
+
+    @f
+    class C:
+      pass
+
+    self.assertEqual(f.sentinel_attribute, '_test_attr_name')
+    self.assertTrue(C._test_attr_name)
+    self.assertTrue(f.is_instance(C))
+
+
 class FrozenAfterInitTest(unittest.TestCase):
 
   def test_no_init(self) -> None:

From 3f6420527c3eb93e6928e3015adc1f08cc38817f Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sun, 17 Nov 2019 02:26:04 -0800
Subject: [PATCH 03/10] refactor implementation

---
 src/python/pants/engine/legacy/structs.py     |  4 +-
 src/python/pants/engine/objects.py            |  2 +
 src/python/pants/rules/core/binary.py         |  3 +-
 .../pants/rules/core/core_test_model.py       |  2 +-
 src/python/pants/rules/core/fmt.py            |  3 +-
 src/python/pants/util/meta.py                 | 41 ++++++++++++-------
 .../pants_test/engine/test_scheduler.py       |  3 +-
 tests/python/pants_test/util/test_meta.py     |  9 +++-
 8 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/src/python/pants/engine/legacy/structs.py b/src/python/pants/engine/legacy/structs.py
index c4606f47788..f77f16aa072 100644
--- a/src/python/pants/engine/legacy/structs.py
+++ b/src/python/pants/engine/legacy/structs.py
@@ -11,8 +11,8 @@
 from pants.build_graph.target import Target
 from pants.engine.addressable import addressable_list
 from pants.engine.fs import GlobExpansionConjunction, PathGlobs
-from pants.engine.objects import Locatable
-from pants.engine.rules import UnionRule, union
+from pants.engine.objects import Locatable, union
+from pants.engine.rules import UnionRule
 from pants.engine.struct import Struct, StructWithDeps
 from pants.source import wrapped_globs
 from pants.util.contextutil import exception_logging
diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py
index eef0e705748..5c762bdeb48 100644
--- a/src/python/pants/engine/objects.py
+++ b/src/python/pants/engine/objects.py
@@ -8,6 +8,8 @@
 from collections.abc import Iterable
 from typing import Generic, Iterator, TypeVar
 
+from pants.util.meta import sentinel_attribute
+
 
 class SerializationError(Exception):
   """Indicates an error serializing an object."""
diff --git a/src/python/pants/rules/core/binary.py b/src/python/pants/rules/core/binary.py
index f6f8e1d9ead..69f46ba3ef5 100644
--- a/src/python/pants/rules/core/binary.py
+++ b/src/python/pants/rules/core/binary.py
@@ -9,7 +9,8 @@
 from pants.engine.fs import Digest, DirectoriesToMerge, DirectoryToMaterialize, Workspace
 from pants.engine.goal import Goal, GoalSubsystem, LineOriented
 from pants.engine.legacy.graph import HydratedTarget
-from pants.engine.rules import console_rule, rule, union
+from pants.engine.objects import union
+from pants.engine.rules import console_rule, rule
 from pants.engine.selectors import Get, MultiGet
 from pants.rules.core.distdir import DistDir
 
diff --git a/src/python/pants/rules/core/core_test_model.py b/src/python/pants/rules/core/core_test_model.py
index 907f631bd47..dbd203bafcc 100644
--- a/src/python/pants/rules/core/core_test_model.py
+++ b/src/python/pants/rules/core/core_test_model.py
@@ -4,7 +4,7 @@
 from dataclasses import dataclass
 
 from pants.engine.isolated_process import FallibleExecuteProcessResult
-from pants.engine.rules import union
+from pants.engine.objects import union
 from pants.util.collections import Enum
 
 
diff --git a/src/python/pants/rules/core/fmt.py b/src/python/pants/rules/core/fmt.py
index 9a8924e4fb0..b90135e5538 100644
--- a/src/python/pants/rules/core/fmt.py
+++ b/src/python/pants/rules/core/fmt.py
@@ -10,7 +10,8 @@
 from pants.engine.isolated_process import ExecuteProcessResult
 from pants.engine.legacy.graph import HydratedTargets
 from pants.engine.legacy.structs import TargetAdaptor
-from pants.engine.rules import UnionMembership, console_rule, union
+from pants.engine.rules import UnionMembership, console_rule
+from pants.engine.objects import union
 from pants.engine.selectors import Get, MultiGet
 
 
diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index 416485b2e0d..a6a8d0d4406 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -1,6 +1,7 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from abc import ABC, abstractmethod, abstractproperty
 from dataclasses import FrozenInstanceError
 from functools import wraps
 from typing import Any, Callable, Optional, Type, TypeVar, Union
@@ -122,26 +123,38 @@ def staticproperty(func: Callable[..., T]) -> T:
   return ClassPropertyDescriptor(func, doc)  # type: ignore[arg-type, return-value]
 
 
-class ClassDecoratorWithSentinelAttribute(Protocol):
-  sentinel_attribute: str
+class ClassDecoratorWithSentinelAttribute(ABC):
+  @abstractproperty
+  def sentinel_attribute(self) -> str: ...
 
+  @abstractmethod
   def __call__(self, cls: Type[T]) -> Type[T]: ...
 
-  def define_instance_of(self, obj: Type[T], **kwargs) -> Type[T]: ...
+  def define_instance_of(self, obj: Type[T], **kwargs) -> Type[T]:
+    return type(obj.__name__, (obj,), {
+      self.sentinel_attribute: True,
+      **kwargs
+    })
 
-  def is_instance(self, obj: Any) -> bool: ...
+  def is_instance(self, obj: Any) -> bool:
+    return hasattr(obj, self.sentinel_attribute)
 
 
-def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[Type[T]], Type[T]]], ClassDecoratorWithSentinelAttribute]:
-  def wrapper(func: Callable[[Type[T]], Type[T]]) -> ClassDecoratorWithSentinelAttribute:
-    ret = cast(ClassDecoratorWithSentinelAttribute, func)
-    ret.sentinel_attribute = attribute_name
-    ret.is_instance = lambda obj: hasattr(obj, ret.sentinel_attribute) # type: ignore
-    ret.define_instance_of = lambda obj, **kwargs: type(obj.__name__, (obj,), { # type: ignore
-      ret.sentinel_attribute: True,
-      **kwargs
-    })
-    return ret
+SentinelAttributeType = TypeVar("SentinelAttributeType", bound=ClassDecoratorWithSentinelAttribute)
+
+
+def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[Type[T]], Type[T]]], Type[SentinelAttributeType]]:
+  def wrapper(func: Callable[[Type[T]], Type[T]]) -> Type[SentinelAttributeType]:
+    class WrappedFunction(ClassDecoratorWithSentinelAttribute):
+      @property
+      def sentinel_attribute(self):
+        return attribute_name
+
+      @wraps(func)
+      def __call__(self, cls: Type[T]) -> Type[T]:
+        return func(cls)
+
+    return WrappedFunction()
   return wrapper
 
 
diff --git a/tests/python/pants_test/engine/test_scheduler.py b/tests/python/pants_test/engine/test_scheduler.py
index db773617bca..564866525d5 100644
--- a/tests/python/pants_test/engine/test_scheduler.py
+++ b/tests/python/pants_test/engine/test_scheduler.py
@@ -10,7 +10,8 @@
 from typing import List
 
 from pants.engine.native import Native
-from pants.engine.rules import RootRule, UnionRule, rule, union
+from pants.engine.objects import union
+from pants.engine.rules import RootRule, UnionRule, rule
 from pants.engine.scheduler import ExecutionError, SchedulerSession
 from pants.engine.selectors import Get, Params
 from pants.testutil.engine.util import assert_equal_with_printing, remove_locations_from_traceback
diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py
index af64387ff71..137dd341634 100644
--- a/tests/python/pants_test/util/test_meta.py
+++ b/tests/python/pants_test/util/test_meta.py
@@ -6,8 +6,13 @@
 from dataclasses import FrozenInstanceError, dataclass
 
 from pants.testutil.test_base import TestBase
-from pants.util.meta import (SingletonMetaclass, classproperty, frozen_after_init,
-                             sentinel_attribute, staticproperty)
+from pants.util.meta import (
+  SingletonMetaclass,
+  classproperty,
+  frozen_after_init,
+  sentinel_attribute,
+  staticproperty,
+)
 
 
 class AbstractClassTest(TestBase):

From f3ad012582d9ff29f26876dda639494248871078 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sun, 17 Nov 2019 14:36:31 -0800
Subject: [PATCH 04/10] add docstrings and make mypy pass!

---
 src/python/pants/engine/objects.py |  4 +++-
 src/python/pants/util/meta.py      | 31 +++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py
index 5c762bdeb48..3066c39e0e4 100644
--- a/src/python/pants/engine/objects.py
+++ b/src/python/pants/engine/objects.py
@@ -204,4 +204,6 @@ def non_member_error_message(subject):
     desc = f' ("{cls.__doc__}")' if cls.__doc__ else ''
     return f'Type {type(subject).__name__} is not a member of the {cls.__name__} @union{desc}'
 
-  return union.define_instance_of(cls, non_member_error_message=staticmethod(non_member_error_message))
+  return union.define_instance_of(
+    cls,
+    non_member_error_message=staticmethod(non_member_error_message))
diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index a6a8d0d4406..a5711b27479 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -123,35 +123,48 @@ def staticproperty(func: Callable[..., T]) -> T:
   return ClassPropertyDescriptor(func, doc)  # type: ignore[arg-type, return-value]
 
 
-class ClassDecoratorWithSentinelAttribute(ABC):
+class _ClassDecoratorWithSentinelAttribute(ABC):
+  """Base class to wrap a class decorator which sets a "sentinel attribute".
+
+  This functionality is exposed via the `@sentinel_attribute(name: str)` decorator.
+  """
+
   @abstractproperty
   def sentinel_attribute(self) -> str: ...
 
   @abstractmethod
-  def __call__(self, cls: Type[T]) -> Type[T]: ...
+  def __call__(self, cls: type) -> type: ...
 
-  def define_instance_of(self, obj: Type[T], **kwargs) -> Type[T]:
+  def define_instance_of(self, obj: type, **kwargs) -> type:
     return type(obj.__name__, (obj,), {
       self.sentinel_attribute: True,
       **kwargs
     })
 
-  def is_instance(self, obj: Any) -> bool:
+  def is_instance(self, obj: type) -> bool:
     return hasattr(obj, self.sentinel_attribute)
 
 
-SentinelAttributeType = TypeVar("SentinelAttributeType", bound=ClassDecoratorWithSentinelAttribute)
+def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[type], type]], _ClassDecoratorWithSentinelAttribute]:
+  """Wraps a class decorator to add a "sentinel attribute" to decorated classes.
 
+  A "sentinel attribute" is an attribute added to the wrapped class decorator's result with
+  `.define_instance_of()`. The wrapped class decorator can then be imported and used to check
+  whether some class object was wrapped with that decorator with `.is_instance()`.
+
+  When used on a class decorator method, the method should return
+  `<method name>.define_instance_of(cls)`, where `cls` is the class object that the decorator would
+  otherwise return.
+  """
 
-def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[Type[T]], Type[T]]], Type[SentinelAttributeType]]:
-  def wrapper(func: Callable[[Type[T]], Type[T]]) -> Type[SentinelAttributeType]:
-    class WrappedFunction(ClassDecoratorWithSentinelAttribute):
+  def wrapper(func: Callable[[type], type]) -> _ClassDecoratorWithSentinelAttribute:
+    class WrappedFunction(_ClassDecoratorWithSentinelAttribute):
       @property
       def sentinel_attribute(self):
         return attribute_name
 
       @wraps(func)
-      def __call__(self, cls: Type[T]) -> Type[T]:
+      def __call__(self, cls: type) -> type:
         return func(cls)
 
     return WrappedFunction()

From 5c8edb4809c403e4733455f1897c2fee0b840891 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Mon, 18 Nov 2019 01:43:41 -0800
Subject: [PATCH 05/10] add BUILD file dep!

---
 tests/python/pants_test/engine/BUILD | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/python/pants_test/engine/BUILD b/tests/python/pants_test/engine/BUILD
index 0b4c78e3b25..26841167ac4 100644
--- a/tests/python/pants_test/engine/BUILD
+++ b/tests/python/pants_test/engine/BUILD
@@ -227,6 +227,7 @@ python_tests(
   sources=['test_objects.py'],
   dependencies=[
     'src/python/pants/engine:objects',
+    'src/python/pants/util:meta',
   ],
   tags = {"partially_type_checked"},
 )

From b4cc0c2a427414b3c0097e662203f43f8372c4d0 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Mon, 18 Nov 2019 13:14:07 -0800
Subject: [PATCH 06/10] respond to review comments and drastically simplify the
 implementation!

---
 src/python/pants/engine/objects.py        |  2 +-
 src/python/pants/util/meta.py             | 31 ++++++++---------------
 tests/python/pants_test/util/test_meta.py |  5 ++--
 3 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py
index 3066c39e0e4..c5b3f5be3a0 100644
--- a/src/python/pants/engine/objects.py
+++ b/src/python/pants/engine/objects.py
@@ -171,7 +171,7 @@ def __bool__(self) -> bool:
     return bool(self.dependencies)
 
 
-@sentinel_attribute('_is_union')
+@sentinel_attribute
 def union(cls):
   """A class decorator which other classes can specify that they can resolve to with `UnionRule`.
 
diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index a5711b27479..eaa37f23bea 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -1,7 +1,7 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-from abc import ABC, abstractmethod, abstractproperty
+from abc import ABC, abstractmethod
 from dataclasses import FrozenInstanceError
 from functools import wraps
 from typing import Any, Callable, Optional, Type, TypeVar, Union
@@ -126,26 +126,23 @@ def staticproperty(func: Callable[..., T]) -> T:
 class _ClassDecoratorWithSentinelAttribute(ABC):
   """Base class to wrap a class decorator which sets a "sentinel attribute".
 
-  This functionality is exposed via the `@sentinel_attribute(name: str)` decorator.
+  This functionality is exposed via the `@sentinel_attribute` decorator.
   """
 
-  @abstractproperty
-  def sentinel_attribute(self) -> str: ...
-
   @abstractmethod
   def __call__(self, cls: type) -> type: ...
 
   def define_instance_of(self, obj: type, **kwargs) -> type:
     return type(obj.__name__, (obj,), {
-      self.sentinel_attribute: True,
+      '_sentinel_attribute_type': type(self),
       **kwargs
     })
 
   def is_instance(self, obj: type) -> bool:
-    return hasattr(obj, self.sentinel_attribute)
+    return (getattr(obj, '_sentinel_attribute_type', None) == type(self))
 
 
-def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[type], type]], _ClassDecoratorWithSentinelAttribute]:
+def sentinel_attribute(decorator: Callable[[type], type]) -> _ClassDecoratorWithSentinelAttribute:
   """Wraps a class decorator to add a "sentinel attribute" to decorated classes.
 
   A "sentinel attribute" is an attribute added to the wrapped class decorator's result with
@@ -157,21 +154,15 @@ def sentinel_attribute(attribute_name: str) -> Callable[[Callable[[type], type]]
   otherwise return.
   """
 
-  def wrapper(func: Callable[[type], type]) -> _ClassDecoratorWithSentinelAttribute:
-    class WrappedFunction(_ClassDecoratorWithSentinelAttribute):
-      @property
-      def sentinel_attribute(self):
-        return attribute_name
-
-      @wraps(func)
-      def __call__(self, cls: type) -> type:
-        return func(cls)
+  class WrappedFunction(_ClassDecoratorWithSentinelAttribute):
+    @wraps(decorator)
+    def __call__(self, cls: type) -> type:
+      return decorator(cls)
 
-    return WrappedFunction()
-  return wrapper
+  return WrappedFunction()
 
 
-@sentinel_attribute('_frozen_after_init')
+@sentinel_attribute
 def frozen_after_init(cls: C) -> C:
   """Class decorator to freeze any modifications to the object after __init__() is done.
 
diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py
index 137dd341634..9c5e831a822 100644
--- a/tests/python/pants_test/util/test_meta.py
+++ b/tests/python/pants_test/util/test_meta.py
@@ -255,7 +255,7 @@ def f(cls):
 class SentinelAttributeTest(unittest.TestCase):
 
   def test_sentinel_attribute(self):
-    @sentinel_attribute('_test_attr_name')
+    @sentinel_attribute
     def f(cls):
       return f.define_instance_of(cls)
 
@@ -263,8 +263,7 @@ def f(cls):
     class C:
       pass
 
-    self.assertEqual(f.sentinel_attribute, '_test_attr_name')
-    self.assertTrue(C._test_attr_name)
+    self.assertEqual(C._sentinel_attribute_type, type(f))
     self.assertTrue(f.is_instance(C))
 
 

From 03d55f90cbeb246c6a01bc3b8b1a59244cd52518 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Mon, 18 Nov 2019 19:55:08 -0800
Subject: [PATCH 07/10] add capital letters to type annotations

---
 src/python/pants/util/meta.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index eaa37f23bea..cd151eb1f7f 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -130,19 +130,19 @@ class _ClassDecoratorWithSentinelAttribute(ABC):
   """
 
   @abstractmethod
-  def __call__(self, cls: type) -> type: ...
+  def __call__(self, cls: Type) -> Type: ...
 
-  def define_instance_of(self, obj: type, **kwargs) -> type:
+  def define_instance_of(self, obj: Type, **kwargs) -> Type:
     return type(obj.__name__, (obj,), {
       '_sentinel_attribute_type': type(self),
       **kwargs
     })
 
-  def is_instance(self, obj: type) -> bool:
-    return (getattr(obj, '_sentinel_attribute_type', None) == type(self))
+  def is_instance(self, obj: Type) -> bool:
+    return getattr(obj, '_sentinel_attribute_type', None) is type(self)
 
 
-def sentinel_attribute(decorator: Callable[[type], type]) -> _ClassDecoratorWithSentinelAttribute:
+def sentinel_attribute(decorator: Callable[[Type], Type]) -> _ClassDecoratorWithSentinelAttribute:
   """Wraps a class decorator to add a "sentinel attribute" to decorated classes.
 
   A "sentinel attribute" is an attribute added to the wrapped class decorator's result with
@@ -156,7 +156,7 @@ def sentinel_attribute(decorator: Callable[[type], type]) -> _ClassDecoratorWith
 
   class WrappedFunction(_ClassDecoratorWithSentinelAttribute):
     @wraps(decorator)
-    def __call__(self, cls: type) -> type:
+    def __call__(self, cls: Type) -> Type:
       return decorator(cls)
 
   return WrappedFunction()

From 38b64ab62433b1d82bd667e15985e76bbc176b03 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sun, 24 Nov 2019 19:36:52 -0800
Subject: [PATCH 08/10] rename @sentinel_attribute -> @decorated_type_checkable

---
 src/python/pants/engine/objects.py        |  4 ++--
 src/python/pants/util/meta.py             | 10 +++++-----
 tests/python/pants_test/util/test_meta.py |  8 ++++----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py
index c5b3f5be3a0..17b0dac1e49 100644
--- a/src/python/pants/engine/objects.py
+++ b/src/python/pants/engine/objects.py
@@ -8,7 +8,7 @@
 from collections.abc import Iterable
 from typing import Generic, Iterator, TypeVar
 
-from pants.util.meta import sentinel_attribute
+from pants.util.meta import decorated_type_checkable
 
 
 class SerializationError(Exception):
@@ -171,7 +171,7 @@ def __bool__(self) -> bool:
     return bool(self.dependencies)
 
 
-@sentinel_attribute
+@decorated_type_checkable
 def union(cls):
   """A class decorator which other classes can specify that they can resolve to with `UnionRule`.
 
diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index cd151eb1f7f..9d66d63af15 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -126,7 +126,7 @@ def staticproperty(func: Callable[..., T]) -> T:
 class _ClassDecoratorWithSentinelAttribute(ABC):
   """Base class to wrap a class decorator which sets a "sentinel attribute".
 
-  This functionality is exposed via the `@sentinel_attribute` decorator.
+  This functionality is exposed via the `@decorated_type_checkable` decorator.
   """
 
   @abstractmethod
@@ -134,15 +134,15 @@ def __call__(self, cls: Type) -> Type: ...
 
   def define_instance_of(self, obj: Type, **kwargs) -> Type:
     return type(obj.__name__, (obj,), {
-      '_sentinel_attribute_type': type(self),
+      '_decorated_type_checkable_type': type(self),
       **kwargs
     })
 
   def is_instance(self, obj: Type) -> bool:
-    return getattr(obj, '_sentinel_attribute_type', None) is type(self)
+    return getattr(obj, '_decorated_type_checkable_type', None) is type(self)
 
 
-def sentinel_attribute(decorator: Callable[[Type], Type]) -> _ClassDecoratorWithSentinelAttribute:
+def decorated_type_checkable(decorator: Callable[[Type], Type]) -> _ClassDecoratorWithSentinelAttribute:
   """Wraps a class decorator to add a "sentinel attribute" to decorated classes.
 
   A "sentinel attribute" is an attribute added to the wrapped class decorator's result with
@@ -162,7 +162,7 @@ def __call__(self, cls: Type) -> Type:
   return WrappedFunction()
 
 
-@sentinel_attribute
+@decorated_type_checkable
 def frozen_after_init(cls: C) -> C:
   """Class decorator to freeze any modifications to the object after __init__() is done.
 
diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py
index 9c5e831a822..2ddf62306d7 100644
--- a/tests/python/pants_test/util/test_meta.py
+++ b/tests/python/pants_test/util/test_meta.py
@@ -9,8 +9,8 @@
 from pants.util.meta import (
   SingletonMetaclass,
   classproperty,
+  decorated_type_checkable,
   frozen_after_init,
-  sentinel_attribute,
   staticproperty,
 )
 
@@ -254,8 +254,8 @@ def f(cls):
 
 class SentinelAttributeTest(unittest.TestCase):
 
-  def test_sentinel_attribute(self):
-    @sentinel_attribute
+  def test_decorated_type_checkable(self):
+    @decorated_type_checkable
     def f(cls):
       return f.define_instance_of(cls)
 
@@ -263,7 +263,7 @@ def f(cls):
     class C:
       pass
 
-    self.assertEqual(C._sentinel_attribute_type, type(f))
+    self.assertEqual(C._decorated_type_checkable_type, type(f))
     self.assertTrue(f.is_instance(C))
 
 

From 481502fb4cf186c859f6af02a268f68a0e47220a Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Tue, 24 Dec 2019 13:41:46 -0500
Subject: [PATCH 09/10] update after rebase

---
 .../pants/backend/awslambda/common/awslambda_common_rules.py  | 3 ++-
 src/python/pants/backend/python/lint/python_format_target.py  | 3 ++-
 src/python/pants/backend/python/lint/python_lint_target.py    | 3 ++-
 src/python/pants/fs/fs.py                                     | 1 -
 src/python/pants/rules/core/fmt.py                            | 2 +-
 src/python/pants/rules/core/lint.py                           | 4 ++--
 .../python/pants_test/build_graph/test_build_configuration.py | 3 ++-
 7 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/python/pants/backend/awslambda/common/awslambda_common_rules.py b/src/python/pants/backend/awslambda/common/awslambda_common_rules.py
index 23d806d3b53..08eba3e1a80 100644
--- a/src/python/pants/backend/awslambda/common/awslambda_common_rules.py
+++ b/src/python/pants/backend/awslambda/common/awslambda_common_rules.py
@@ -9,7 +9,8 @@
 from pants.engine.fs import Digest, DirectoriesToMerge, DirectoryToMaterialize, Workspace
 from pants.engine.goal import Goal, GoalSubsystem, LineOriented
 from pants.engine.legacy.graph import HydratedTarget
-from pants.engine.rules import console_rule, rule, union
+from pants.engine.objects import union
+from pants.engine.rules import console_rule, rule
 from pants.engine.selectors import Get, MultiGet
 from pants.rules.core.distdir import DistDir
 
diff --git a/src/python/pants/backend/python/lint/python_format_target.py b/src/python/pants/backend/python/lint/python_format_target.py
index ef72f271ce6..6471cae1f61 100644
--- a/src/python/pants/backend/python/lint/python_format_target.py
+++ b/src/python/pants/backend/python/lint/python_format_target.py
@@ -12,7 +12,8 @@
   PythonTestsAdaptor,
   TargetAdaptor,
 )
-from pants.engine.rules import UnionMembership, UnionRule, rule, union
+from pants.engine.objects import union
+from pants.engine.rules import UnionMembership, UnionRule, rule
 from pants.engine.selectors import Get
 from pants.rules.core.fmt import AggregatedFmtResults, FmtResult, FormatTarget
 
diff --git a/src/python/pants/backend/python/lint/python_lint_target.py b/src/python/pants/backend/python/lint/python_lint_target.py
index 163625ed227..06bf1e1c37e 100644
--- a/src/python/pants/backend/python/lint/python_lint_target.py
+++ b/src/python/pants/backend/python/lint/python_lint_target.py
@@ -11,7 +11,8 @@
   PythonTestsAdaptor,
   TargetAdaptor,
 )
-from pants.engine.rules import UnionMembership, UnionRule, rule, union
+from pants.engine.objects import union
+from pants.engine.rules import UnionMembership, UnionRule, rule
 from pants.engine.selectors import Get, MultiGet
 from pants.rules.core.lint import LintResult, LintResults, LintTarget
 
diff --git a/src/python/pants/fs/fs.py b/src/python/pants/fs/fs.py
index 4a2b7be9c98..a2473670434 100644
--- a/src/python/pants/fs/fs.py
+++ b/src/python/pants/fs/fs.py
@@ -74,4 +74,3 @@ def expand_path(path):
 def is_child_of(path: Path, directory: Path) -> bool:
   abs_path = path if path.is_absolute() else directory.joinpath(path).resolve()
   return directory == abs_path or directory in abs_path.parents
-
diff --git a/src/python/pants/rules/core/fmt.py b/src/python/pants/rules/core/fmt.py
index b90135e5538..7264791951a 100644
--- a/src/python/pants/rules/core/fmt.py
+++ b/src/python/pants/rules/core/fmt.py
@@ -10,8 +10,8 @@
 from pants.engine.isolated_process import ExecuteProcessResult
 from pants.engine.legacy.graph import HydratedTargets
 from pants.engine.legacy.structs import TargetAdaptor
-from pants.engine.rules import UnionMembership, console_rule
 from pants.engine.objects import union
+from pants.engine.rules import UnionMembership, console_rule
 from pants.engine.selectors import Get, MultiGet
 
 
diff --git a/src/python/pants/rules/core/lint.py b/src/python/pants/rules/core/lint.py
index 9d5d13a8784..966c2547ee3 100644
--- a/src/python/pants/rules/core/lint.py
+++ b/src/python/pants/rules/core/lint.py
@@ -8,8 +8,8 @@
 from pants.engine.isolated_process import FallibleExecuteProcessResult
 from pants.engine.legacy.graph import HydratedTargets
 from pants.engine.legacy.structs import TargetAdaptor
-from pants.engine.objects import Collection
-from pants.engine.rules import UnionMembership, console_rule, union
+from pants.engine.objects import Collection, union
+from pants.engine.rules import UnionMembership, console_rule
 from pants.engine.selectors import Get, MultiGet
 
 
diff --git a/tests/python/pants_test/build_graph/test_build_configuration.py b/tests/python/pants_test/build_graph/test_build_configuration.py
index 733cf5825cc..db4557e19ad 100644
--- a/tests/python/pants_test/build_graph/test_build_configuration.py
+++ b/tests/python/pants_test/build_graph/test_build_configuration.py
@@ -10,7 +10,8 @@
 from pants.build_graph.build_configuration import BuildConfiguration
 from pants.build_graph.build_file_aliases import BuildFileAliases, TargetMacro
 from pants.build_graph.target import Target
-from pants.engine.rules import UnionRule, union
+from pants.engine.objects import union
+from pants.engine.rules import UnionRule
 from pants.util.contextutil import temporary_dir
 from pants.util.dirutil import touch
 

From abb2a5941ae9cca9b99c545be65db9903dbc5379 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Tue, 24 Dec 2019 14:02:15 -0500
Subject: [PATCH 10/10] remove @decorated_type_checkable from
 @frozen_after_init for now

---
 src/python/pants/util/meta.py             |  6 +++---
 tests/python/pants_test/util/test_meta.py | 13 +++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py
index 9d66d63af15..a9a7d2ed4fd 100644
--- a/src/python/pants/util/meta.py
+++ b/src/python/pants/util/meta.py
@@ -187,7 +187,7 @@ def new_setattr(self, key: str, value: Any) -> None:
       )
     prev_setattr(self, key, value)  # type: ignore[call-arg]
 
-  cls.__init__ = new_init  # type: ignore[assignment]
-  cls.__setattr__ = new_setattr  # type: ignore[assignment]
+  cls.__init__ = new_init
+  cls.__setattr__ = new_setattr
 
-  return frozen_after_init.define_instance_of(cls)
+  return cls
diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py
index 2ddf62306d7..dd7ec88a393 100644
--- a/tests/python/pants_test/util/test_meta.py
+++ b/tests/python/pants_test/util/test_meta.py
@@ -266,6 +266,19 @@ class C:
     self.assertEqual(C._decorated_type_checkable_type, type(f))
     self.assertTrue(f.is_instance(C))
 
+    # Check that .is_instance() is only true for exactly the decorator @g used on the class D!
+    @decorated_type_checkable
+    def g(cls):
+      return g.define_instance_of(cls)
+
+    @g
+    class D:
+      pass
+
+    self.assertEqual(D._decorated_type_checkable_type, type(g))
+    self.assertTrue(g.is_instance(D))
+    self.assertFalse(f.is_instance(D))
+
 
 class FrozenAfterInitTest(unittest.TestCase):