[engine] Fix TargetMacro replacements of adapted aliases

In cases where a TargetMacro has been installed as wrapping an existing symbol, it will not exist in the `target_aliases` field, and will thus not be replaced with our adaptor.

"Why yes, this continues to highlight the importance of:"
pantsbuild#3560
pantsbuild#3561

Testing Done:
https://travis-ci.org/pantsbuild/pants/builds/140649055
And tested on internal TargetMacro usecases.

Bugs closed: 3506, 3607

Reviewed at https://rbcommons.com/s/twitter/r/4000/

src/python/pants/bin/engine_initializer.py

@@ -44,18 +44,16 @@ def aliases(cls):
   @classmethod
   @memoized_method
   def table(cls):
-    def target_type(alias):
-      # TODO: The alias matching here is to avoid elevating "TargetAdaptors" into the public
-      # API until until after https://github.com/pantsbuild/pants/issues/3560 has been completed.
-      # These should likely move onto Target subclasses as the engine gets deeper into beta
-      # territory.
-      if alias == 'jvm_app':
-        return JvmAppAdaptor
-      elif alias in ('python_library', 'python_tests', 'python_binary'):
-        return PythonTargetAdaptor
-      else:
-        return TargetAdaptor
-    return {alias: target_type(alias) for alias in cls.aliases().target_types}
+    aliases = {alias: TargetAdaptor for alias in cls.aliases().target_types}
+    # TODO: The alias replacement here is to avoid elevating "TargetAdaptors" into the public
+    # API until after https://github.com/pantsbuild/pants/issues/3560 has been completed.
+    # These should likely move onto Target subclasses as the engine gets deeper into beta
+    # territory.
+    aliases['jvm_app'] = JvmAppAdaptor
+    for alias in ('python_library', 'python_tests', 'python_binary'):
+      aliases[alias] = PythonTargetAdaptor
+
+    return aliases
 
 
 class LegacyGraphHelper(namedtuple('LegacyGraphHelper', ['scheduler',
@@ -86,17 +84,19 @@ def parse_commandline_to_spec_roots(options=None, args=None, build_root=None):
     return spec_roots
 
   @staticmethod
-  def setup_legacy_graph(path_ignore_patterns):
+  def setup_legacy_graph(path_ignore_patterns, symbol_table_cls=None):
     """Construct and return the components necessary for LegacyBuildGraph construction.
 
     :param list path_ignore_patterns: A list of path ignore patterns for FileSystemProjectTree,
                                       usually taken from the `--pants-ignore` global option.
+    :param SymbolTable symbol_table_cls: A SymbolTable class to use for build file parsing, or
+                                         None to use the default.
     :returns: A tuple of (scheduler, engine, symbol_table_cls, build_graph_cls).
     """
 
     build_root = get_buildroot()
     project_tree = FileSystemProjectTree(build_root, path_ignore_patterns)
-    symbol_table_cls = LegacySymbolTable
+    symbol_table_cls = symbol_table_cls or LegacySymbolTable
 
     # Register "literal" subjects required for these tasks.
     # TODO: Replace with `Subsystems`.
@@ -118,7 +118,7 @@ def setup_legacy_graph(path_ignore_patterns):
 
   @classmethod
   @contextmanager
-  def open_legacy_graph(cls, options=None, path_ignore_patterns=None):
+  def open_legacy_graph(cls, options=None, path_ignore_patterns=None, symbol_table_cls=None):
     """A context manager that yields a usable, legacy LegacyBuildGraph by way of the v2 scheduler.
 
     This is used primarily for testing and non-daemon runs.
@@ -127,14 +127,16 @@ def open_legacy_graph(cls, options=None, path_ignore_patterns=None):
     :param list path_ignore_patterns: A list of path ignore patterns for FileSystemProjectTree,
                                       usually taken from the `--pants-ignore` global option.
                                       Defaults to: ['.*']
+    :param SymbolTable symbol_table_cls: A SymbolTable class to use for build file parsing, or
+                                         None to use the default.
     :yields: A tuple of (graph, addresses, scheduler).
     """
     path_ignore_patterns = path_ignore_patterns or ['.*']
     spec_roots = cls.parse_commandline_to_spec_roots(options=options)
     (scheduler,
      engine,
      symbol_table_cls,
-     build_graph_cls) = cls.setup_legacy_graph(path_ignore_patterns)
+     build_graph_cls) = cls.setup_legacy_graph(path_ignore_patterns, symbol_table_cls=symbol_table_cls)
 
     engine.start()
     try:

tests/python/pants_test/engine/legacy/test_graph.py

@@ -11,29 +11,31 @@
 
 import mock
 
-from pants.bin.goal_runner import EngineInitializer
+from pants.bin.engine_initializer import EngineInitializer, LegacySymbolTable
 from pants.build_graph.address import Address
+from pants.build_graph.build_file_aliases import BuildFileAliases, TargetMacro
+from pants.build_graph.target import Target
 
 
 class GraphInvalidationTest(unittest.TestCase):
-  def _make_setup_args(self, *specs):
+  def _make_setup_args(self, specs, symbol_table_cls=None):
     options = mock.Mock()
     options.target_specs = specs
-    return dict(options=options)
+    return dict(options=options, symbol_table_cls=symbol_table_cls)
 
   @contextmanager
-  def open_scheduler(self, *specs):
-    kwargs = self._make_setup_args(*specs)
+  def open_scheduler(self, specs, symbol_table_cls=None):
+    kwargs = self._make_setup_args(specs, symbol_table_cls=symbol_table_cls)
     with EngineInitializer.open_legacy_graph(**kwargs) as triple:
       yield triple
 
   @contextmanager
-  def open_pg(self, *specs):
-    with self.open_scheduler(*specs) as (_, _, scheduler):
+  def open_pg(self, specs):
+    with self.open_scheduler(specs) as (_, _, scheduler):
       yield scheduler.product_graph
 
   def test_invalidate_fsnode(self):
-    with self.open_pg('3rdparty/python::') as product_graph:
+    with self.open_pg(['3rdparty/python::']) as product_graph:
       initial_node_count = len(product_graph)
       self.assertGreater(initial_node_count, 0)
 
@@ -42,7 +44,7 @@ def test_invalidate_fsnode(self):
       self.assertLess(len(product_graph), initial_node_count)
 
   def test_invalidate_fsnode_incremental(self):
-    with self.open_pg('3rdparty::') as product_graph:
+    with self.open_pg(['3rdparty::']) as product_graph:
       node_count = len(product_graph)
       self.assertGreater(node_count, 0)
 
@@ -56,8 +58,39 @@ def test_invalidate_fsnode_incremental(self):
 
   def test_sources_ordering(self):
     spec = 'testprojects/src/resources/org/pantsbuild/testproject/ordering'
-    with self.open_scheduler(spec) as (graph, _, _):
+    with self.open_scheduler([spec]) as (graph, _, _):
       target = graph.get_target(Address.parse(spec))
       sources = [os.path.basename(s) for s in target.sources_relative_to_buildroot()]
       self.assertEquals(['p', 'a', 'n', 't', 's', 'b', 'u', 'i', 'l', 'd', 'p', 'a', 'n', 't', 's'],
                         sources)
+
+  def test_target_macro_override(self):
+    """Tests that we can "wrap" an existing target type with additional functionality.
+
+    Installs an additional TargetMacro that wraps `target` aliases to add a tag to all definitions.
+    """
+    tag = 'tag_added_by_macro'
+    target_cls = Target
+    spec = 'testprojects/tests/python/pants/build_parsing:'
+
+    # Macro that adds the specified tag.
+    def macro(parse_context, tags=None, **kwargs):
+      tags = tags or set()
+      tags.add(tag)
+      parse_context.create_object(target_cls, tags=tags, **kwargs)
+
+    # SymbolTable that extends the legacy table to apply the macro.
+    class TaggingSymbolTable(LegacySymbolTable):
+      @classmethod
+      def aliases(cls):
+        return super(TaggingSymbolTable, cls).aliases().merge(
+            BuildFileAliases(
+              targets={'target': TargetMacro.Factory.wrap(macro, target_cls),}
+            )
+          )
+
+    # Confirm that python_tests in a small directory are marked.
+    with self.open_scheduler([spec], symbol_table_cls=TaggingSymbolTable) as (graph, addresses, _):
+      self.assertTrue(len(addresses) > 0, 'No targets matched by {}'.format(addresses))
+      for address in addresses:
+        self.assertIn(tag, graph.get_target(address).tags)