Replace SingletonRule with zero-parameter @rules (#7479)

### Problem

In #6170, it became possible for a `@rule` to take no `Params`, meaning that its identity did not include any of its context, and it could act as a singleton. Before that change, it had always been the case that a `@rule` would have a "subject" in its identity, necessitating `SingletonRule` in order to avoid creating a new copy of the rule in the graph for each distinct subject.

### Solution

Remove `SingletonRule` in favor of definition of zero-parameter `@rules`, which reduces the total number of concepts involved in the engine. Because `SingletonRule` used to allow a declaration to lie about the exact type it was providing (and give an instance of a subclass instead), usage of `SymbolTable` first needed to be cleaned up to remove subclassing.

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

@@ -7,7 +7,7 @@
 import os
 from abc import abstractmethod, abstractproperty
 
-from pants.engine.rules import SingletonRule
+from pants.engine.rules import rule
 from pants.util.memo import memoized_classproperty
 from pants.util.meta import AbstractClass
 from pants.util.objects import datatype, enum
@@ -300,7 +300,12 @@ class CppToolchain(datatype([('cpp_compiler', CppCompiler), ('cpp_linker', Linke
 class HostLibcDev(datatype(['crti_object', 'fingerprint'])): pass
 
 
+@rule(Platform, [])
+def platform_singleton():
+  return Platform.current
+
+
 def create_native_environment_rules():
   return [
-    SingletonRule(Platform, Platform.current),
+    platform_singleton,
   ]

src/python/pants/bin/local_pants_runner.py

@@ -119,7 +119,6 @@ def _maybe_init_target_roots(target_roots, graph_session, options, build_root):
       options=options,
       build_root=build_root,
       session=graph_session.scheduler_session,
-      symbol_table=graph_session.symbol_table,
       exclude_patterns=tuple(global_options.exclude_target_regexp),
       tags=tuple(global_options.tag)
     )

src/python/pants/engine/README.md

@@ -177,20 +177,18 @@ dependencies.
 Currently, it is only possible to load rules into the pants scheduler in two ways: by importing and
 using them in `src/python/pants/bin/engine_initializer.py`, or by adding them to the list returned
 by a `rules()` method defined in `src/python/backend/<backend_name>/register.py`. Plugins cannot add
-new rules yet. Unit tests, however, can mix in `SchedulerTestBase` from
-`tests/python/pants_test/engine/scheduler_test_base.py` to generate and execute a scheduler from a
-given set of rules.
+new rules yet. Unit tests, however, can mix in `TestBase` from
+`tests/python/pants_test/test_base.py` to generate and execute a scheduler from a given set of
+rules.
 
-In general, there are three types of rules you can define:
+In general, there are two types of rules that you can define:
 
 1. an `@rule`, which has a single product type and selects its inputs as described above.
-2. a `SingletonRule`, which matches a product type with a value so the type can then be selected
-   as a parameter to an `@rule`.
-3. a `RootRule`, which declares a type that can be used as a *subject*, which means it can be
+2. a `RootRule`, which declares a type that can be used as a *subject*, which means it can be
    provided as an input to a `product_request()`.
 
-In more depth, a `RootRule` for some type is required when no other rule provides that type (i.e. it
-is not provided with a `SingletonRule` or as the product of any `@rule`). In the absence of a
+In more depth, a `RootRule` for some type is required when no other rule might provide that
+type (i.e. it is not provided as the product of any `@rule`) in some context. In the absence of a
 `RootRule`, any subject type involved in a request "at runtime" (i.e. via `product_request()`),
 would show up as an an unused or impossible path in the rule graph. Another potential name for
 `RootRule` might be `ParamRule`, or something similar, as it can be thought of as saying that the

src/python/pants/engine/build_files.py

@@ -21,7 +21,7 @@
 from pants.engine.mapper import AddressFamily, AddressMap, AddressMapper, ResolveError
 from pants.engine.objects import Locatable, SerializableFactory, Validatable
 from pants.engine.parser import TargetAdaptorContainer
-from pants.engine.rules import RootRule, SingletonRule, rule
+from pants.engine.rules import RootRule, rule
 from pants.engine.selectors import Get
 from pants.engine.struct import Struct
 from pants.util.collections_abc_backport import MutableMapping, MutableSequence
@@ -263,9 +263,13 @@ def create_graph_rules(address_mapper):
   :param address_mapper_key: The subject key for an AddressMapper instance.
   :param symbol_table: A SymbolTable instance to provide symbols for Address lookups.
   """
+
+  @rule(AddressMapper, [])
+  def address_mapper_singleton():
+    return address_mapper
+
   return [
-    # A singleton to provide the AddressMapper.
-    SingletonRule(AddressMapper, address_mapper),
+    address_mapper_singleton,
     # Support for resolving Structs from Addresses.
     hydrate_struct,
     resolve_unhydrated_struct,

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

@@ -19,6 +19,7 @@
 from pants.build_graph.address import Address
 from pants.build_graph.address_lookup_error import AddressLookupError
 from pants.build_graph.app_base import AppBase, Bundle
+from pants.build_graph.build_configuration import BuildConfiguration
 from pants.build_graph.build_graph import BuildGraph
 from pants.build_graph.remote_sources import RemoteSources
 from pants.engine.addressable import BuildFileAddresses
@@ -28,7 +29,7 @@
                                          SourcesField, TargetAdaptor)
 from pants.engine.mapper import AddressMapper
 from pants.engine.objects import Collection
-from pants.engine.parser import SymbolTable, TargetAdaptorContainer
+from pants.engine.parser import TargetAdaptorContainer
 from pants.engine.rules import RootRule, rule
 from pants.engine.selectors import Get
 from pants.option.global_options import GlobMatchErrorBehavior
@@ -40,9 +41,8 @@
 logger = logging.getLogger(__name__)
 
 
-def target_types_from_symbol_table(symbol_table):
-  """Given a LegacySymbolTable, return the concrete target types constructed for each alias."""
-  aliases = symbol_table.aliases()
+def target_types_from_build_file_aliases(aliases):
+  """Given BuildFileAliases, return the concrete target types constructed for each alias."""
   target_types = dict(aliases.target_types)
   for alias, factory in aliases.target_macro_factories.items():
     target_type, = factory.target_types
@@ -64,16 +64,15 @@ class LegacyBuildGraph(BuildGraph):
   """
 
   @classmethod
-  def create(cls, scheduler, symbol_table):
-    """Construct a graph given a Scheduler, Engine, and a SymbolTable class."""
-    return cls(scheduler, target_types_from_symbol_table(symbol_table))
+  def create(cls, scheduler, build_file_aliases):
+    """Construct a graph given a Scheduler and BuildFileAliases."""
+    return cls(scheduler, target_types_from_build_file_aliases(build_file_aliases))
 
   def __init__(self, scheduler, target_types):
-    """Construct a graph given a Scheduler, Engine, and a SymbolTable class.
+    """Construct a graph given a Scheduler, and set of target type aliases.
 
     :param scheduler: A Scheduler that is configured to be able to resolve TransitiveHydratedTargets.
-    :param symbol_table: A SymbolTable instance used to instantiate Target objects. Must match
-      the symbol table installed in the scheduler (TODO: see comment in `_instantiate_target`).
+    :param target_types: A dict mapping aliases to target types.
     """
     self._scheduler = scheduler
     self._target_types = target_types
@@ -146,13 +145,7 @@ def _index_target(self, target_adaptor):
     return target
 
   def _instantiate_target(self, target_adaptor):
-    """Given a TargetAdaptor struct previously parsed from a BUILD file, instantiate a Target.
-
-    TODO: This assumes that the SymbolTable used for parsing matches the SymbolTable passed
-    to this graph. Would be good to make that more explicit, but it might be better to nuke
-    the Target subclassing pattern instead, and lean further into the "configuration composition"
-    model explored in the `exp` package.
-    """
+    """Given a TargetAdaptor struct previously parsed from a BUILD file, instantiate a Target."""
     target_cls = self._target_types[target_adaptor.type_alias]
     try:
       # Pop dependencies, which were already consumed during construction.
@@ -409,8 +402,8 @@ class OwnersRequest(datatype([
   """
 
 
-@rule(BuildFileAddresses, [SymbolTable, AddressMapper, OwnersRequest])
-def find_owners(symbol_table, address_mapper, owners_request):
+@rule(BuildFileAddresses, [BuildConfiguration, AddressMapper, OwnersRequest])
+def find_owners(build_configuration, address_mapper, owners_request):
   sources_set = OrderedSet(owners_request.sources)
   dirs_set = OrderedSet(dirname(source) for source in sources_set)
 
@@ -449,7 +442,8 @@ def owns_any_source(legacy_target):
     all_structs = yield [Get(TargetAdaptorContainer, Address, a.to_address()) for a in all_addresses]
     all_structs = [s.value for s in all_structs]
 
-    graph = _DependentGraph.from_iterable(target_types_from_symbol_table(symbol_table),
+    bfa = build_configuration.registered_aliases()
+    graph = _DependentGraph.from_iterable(target_types_from_build_file_aliases(bfa),
                                           address_mapper,
                                           all_structs)
     if owners_request.include_dependees == 'direct':

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

@@ -91,7 +91,7 @@ def __call__(self, *args, **kwargs):
         else:
           return self._object_type(*args, **kwargs)
 
-    for alias, symbol in symbol_table.table().items():
+    for alias, symbol in symbol_table.table.items():
       registrar = Registrar(parse_context, alias, symbol)
       symbols[alias] = registrar
       symbols[symbol] = registrar

src/python/pants/engine/parser.py

@@ -7,25 +7,17 @@
 from abc import abstractmethod
 
 from pants.util.meta import AbstractClass
-from pants.util.objects import Exactly, datatype
+from pants.util.objects import datatype
 
 
 class ParseError(Exception):
   """Indicates an error parsing BUILD configuration."""
 
 
-class SymbolTable(AbstractClass):
-  """A one-classmethod interface exposing a symbol table dict."""
-
-  @abstractmethod
-  def table(self):
-    """Returns a dict of name to implementation class."""
-
-  def constraint(self):
-    """Returns the typeconstraint for the symbol table"""
-    # NB Sort types so that multiple calls get the same tuples.
-    symbol_table_types = sorted(set(self.table().values()), key=repr)
-    return Exactly(*symbol_table_types, description='symbol table types')
+class SymbolTable(datatype([
+  ('table', dict),
+])):
+  """A symbol table dict mapping symbol name to implementation class."""
 
 
 class TargetAdaptorContainer(datatype(["value"])):
@@ -36,11 +28,6 @@ class TargetAdaptorContainer(datatype(["value"])):
   """
 
 
-class EmptyTable(SymbolTable):
-  def table(self):
-    return {}
-
-
 class Parser(AbstractClass):
 
   @abstractmethod

src/python/pants/engine/rules.py

@@ -425,14 +425,6 @@ def __str__(self):
                     self.dependency_optionables))
 
 
-class SingletonRule(datatype([('output_type', _type_field), 'value']), Rule):
-  """A default rule for a product, which is thus a singleton for that product."""
-
-  @classmethod
-  def from_instance(cls, obj):
-    return cls(type(obj), obj)
-
-
 class RootRule(datatype([('output_type', _type_field)]), Rule):
   """Represents a root input to an execution of a rule graph.
 

src/python/pants/engine/scheduler.py

@@ -19,7 +19,7 @@
 from pants.engine.native import Function, TypeId
 from pants.engine.nodes import Return, Throw
 from pants.engine.objects import Collection
-from pants.engine.rules import RuleIndex, SingletonRule, TaskRule
+from pants.engine.rules import RuleIndex, TaskRule
 from pants.engine.selectors import Params
 from pants.rules.core.exceptions import GracefulTerminationException
 from pants.util.contextutil import temporary_file_path
@@ -181,22 +181,11 @@ def _register_rules(self, rule_index):
           continue
         registered.add(key)
 
-        if type(rule) is SingletonRule:
-          self._register_singleton(output_type, rule)
-        elif type(rule) is TaskRule:
+        if type(rule) is TaskRule:
           self._register_task(output_type, rule, rule_index.union_rules)
         else:
           raise ValueError('Unexpected Rule type: {}'.format(rule))
 
-  def _register_singleton(self, output_type, rule):
-    """Register the given SingletonRule.
-
-    A SingletonRule installed for a type will be the only provider for that type.
-    """
-    self._native.lib.tasks_singleton_add(self._tasks,
-                                         self._to_value(rule.value),
-                                         TypeId(self._to_id(output_type)))
-
   def _register_task(self, output_type, rule, union_rules):
     """Register the given TaskRule with the native scheduler."""
     func = Function(self._to_key(rule.func))