Move BuildFileAliases validation to BuildFileAliases.

Previously, alias mappings were validated by BuildConfiguration.  Doing
the validation in BuilfFileAliases ensures only valid objects are
constructed and never float around in an invalid state and it
centralizes the mostly internal detail that "targets" contains a mix of
types, allowing the mix to be split and presented via seperate
properties.

This adds TargetMacro.Factory coverage to BuildConfigurationTest while
moving its validation tests over to BuilfFileAliasesTest.

The change also forces existing users of BuilfFileAliases.targets to
deal with BuilfFileAliases.target_types and
BuilfFileAliases.target_macro_factories.  To help those users a few
utility methods are added to BuildFileAliases to unify target types
accessed via these two properties.

Testing Done:
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/79719489

Bugs closed: 2124, 2160

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

contrib/cpp/src/python/pants/contrib/cpp/register.py

@@ -17,7 +17,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'cpp_library': CppLibrary,
       'cpp_binary': CppBinary,

contrib/go/src/python/pants/contrib/go/register.py

@@ -20,7 +20,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       GoBinary.alias(): TargetMacro.Factory.wrap(GoBinary.create, GoBinary),
       GoLibrary.alias(): TargetMacro.Factory.wrap(GoLibrary.create, GoLibrary),

contrib/node/src/python/pants/contrib/node/register.py

@@ -15,7 +15,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'node_module': NodeModule,
       'node_remote_module': NodeRemoteModule,

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py

@@ -32,7 +32,7 @@ def task_type(cls):
 
   @property
   def alias_groups(self):
-    return BuildFileAliases.create(targets={'java_thrift_library': JavaThriftLibrary})
+    return BuildFileAliases(targets={'java_thrift_library': JavaThriftLibrary})
 
   def setUp(self):
     super(ScroogeGenTest, self).setUp()

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_thrift_linter.py

@@ -21,7 +21,7 @@ def _prepare_mocks(self, task):
 
   @property
   def alias_groups(self):
-    return BuildFileAliases.create(
+    return BuildFileAliases(
       targets={
         'java_thrift_library': JavaThriftLibrary,
       },

contrib/spindle/src/python/pants/contrib/spindle/register.py

@@ -13,7 +13,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'spindle_thrift_library': SpindleThriftLibrary,
     }

contrib/spindle/tests/python/pants_test/contrib/spindle/tasks/test_spindle_gen.py

@@ -24,7 +24,7 @@ def task_type(cls):
 
   @property
   def alias_groups(self):
-    return BuildFileAliases.create(
+    return BuildFileAliases(
       targets={
         'spindle_thrift_library': SpindleThriftLibrary,
         'jar_library': JarLibrary,

examples/src/java/org/pantsbuild/example/page.md

@@ -179,7 +179,7 @@ have one. In the plugin, define a `Wiki` and register it:
 
     # in register.py:
     def build_file_aliases():
-      return BuildFileAliases.create(
+      return BuildFileAliases(
         # ...
         objects={
           # ...

pants-plugins/src/python/internal_backend/repositories/register.py

@@ -54,7 +54,7 @@ def org_pantsbuild_publication_metadata(description):
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     objects={
       'public': public_repo,  # key 'public' must match name='public' above
       'testing': testing_repo,

pants-plugins/src/python/internal_backend/utilities/register.py

@@ -159,7 +159,7 @@ def create_setup_py(cls, name, description, additional_classifiers=None):
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     objects={
       'pants_setup_py': pants_setup_py,
       'contrib_setup_py': contrib_setup_py

src/python/pants/backend/android/register.py

@@ -20,7 +20,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'android_binary': AndroidBinary,
       'android_dependency': AndroidDependency,

src/python/pants/backend/authentication/register.py

@@ -10,7 +10,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     objects={
       'netrc': Netrc,
     },

src/python/pants/backend/codegen/register.py

@@ -24,7 +24,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'java_antlr_library': JavaAntlrLibrary,
       'java_protobuf_library': JavaProtobufLibrary,

src/python/pants/backend/core/register.py

@@ -53,7 +53,7 @@ def __call__(self):
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       # NB: the 'dependencies' alias is deprecated in favor of the 'target' alias
       'dependencies': DeprecatedDependencies,

src/python/pants/backend/core/tasks/BUILD

@@ -148,12 +148,11 @@ python_library(
   name = 'filter',
   sources = ['filter.py'],
   dependencies = [
-    ':common',
     ':console_task',
-    'src/python/pants/base:address',
+    'src/python/pants/base:address_lookup_error',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:cmd_line_spec_parser',
-    'src/python/pants/base:target',
+    'src/python/pants/base:exceptions',
     'src/python/pants/util:filtering',
   ],
 )

src/python/pants/backend/core/tasks/dependees.py

@@ -7,6 +7,7 @@
 
 import os
 from collections import defaultdict
+from textwrap import dedent
 
 from twitter.common.collections import OrderedSet
 
@@ -36,36 +37,49 @@ def __init__(self, *args, **kwargs):
 
     self._transitive = self.get_options().transitive
     self._closed = self.get_options().closed
-    self._dependees_type = self.get_options().type
+    self._dependees_types = self.get_options().type
     self._spec_excludes = self.get_options().spec_excludes
 
   def console_output(self, _):
     buildfiles = OrderedSet()
     address_mapper = self.context.address_mapper
-    if self._dependees_type:
+    if self._dependees_types:
+      registered_aliases = self.context.build_file_parser.registered_aliases()
       base_paths = OrderedSet()
-      for dependees_type in self._dependees_type:
-        target_aliases = self.context.build_file_parser.registered_aliases().targets
-        if dependees_type not in target_aliases:
-          raise TaskError('Invalid type name: {}'.format(dependees_type))
-        target_type = target_aliases[dependees_type]
-        # Try to find the SourceRoot for the given input type
-        try:
-          roots = SourceRoot.roots(target_type)
-          base_paths.update(roots)
-        except KeyError:
-          pass
-
+      for dependees_type in self._dependees_types:
+        target_types = registered_aliases.target_types_by_alias.get(dependees_type, None)
+        if not target_types:
+          raise TaskError('No Targets with type name: {}'.format(dependees_type))
+        # Try to find the SourceRoots for the given input type alias
+        for target_type in target_types:
+          try:
+            roots = SourceRoot.roots(target_type)
+            base_paths.update(roots)
+          except KeyError:
+            pass
+
+      # TODO(John Sirois): BUG: This should not cause a failure, it should just force a slower full
+      # scan.
+      # TODO(John Sirois): BUG: The --type argument only limited the scn bases, it does no limit the
+      # types of targets found under those bases, ie: we may have just limited our scan to roots
+      # containing java_library, but those same roots likely also contain jvm_binary targets that
+      # we do not wish to have in the results.  So the --type filtering needs to apply to the final
+      # dependees_by_target map as well below.
       if not base_paths:
-        raise TaskError('No SourceRoot set for any target type in {}.'.format(self._dependees_type) +
-                        '\nPlease define a source root in BUILD file as:' +
-                        '\n\tsource_root(\'<src-folder>\', {})'.format(', '.join(self._dependees_type)))
+        raise TaskError(dedent("""\
+                        No SourceRoot set for any of these target types: {}.
+                        Please define a source root in BUILD file as:
+                          source_root('<src-folder>', {})
+                        """.format(' '.join(self._dependees_types),
+                                   ', '.join(self._dependees_types))).strip())
       for base_path in base_paths:
-        buildfiles.update(address_mapper.scan_buildfiles(get_buildroot(),
-                                                    os.path.join(get_buildroot(), base_path),
-                                                    spec_excludes=self._spec_excludes))
+        scanned = address_mapper.scan_buildfiles(get_buildroot(),
+                                                 os.path.join(get_buildroot(), base_path),
+                                                 spec_excludes=self._spec_excludes)
+        buildfiles.update(scanned)
     else:
-      buildfiles = address_mapper.scan_buildfiles(get_buildroot(), spec_excludes=self._spec_excludes)
+      buildfiles = address_mapper.scan_buildfiles(get_buildroot(),
+                                                  spec_excludes=self._spec_excludes)
 
     build_graph = self.context.build_graph
     build_file_parser = self.context.build_file_parser

src/python/pants/backend/core/tasks/filter.py

@@ -12,7 +12,6 @@
 from pants.base.build_environment import get_buildroot
 from pants.base.cmd_line_spec_parser import CmdLineSpecParser
 from pants.base.exceptions import TaskError
-from pants.base.target import Target
 from pants.util.filtering import create_filters, wrap_filters
 
 
@@ -70,22 +69,11 @@ def filter_for_address(spec):
     self._filters.extend(create_filters(self.get_options().target, filter_for_address))
 
     def filter_for_type(name):
-      # FIXME(pl): This should be a standard function provided by the plugin/BuildFileParser
-      # machinery
-      try:
-        # Try to do a fully qualified import 1st for filtering on custom types.
-        from_list, module, type_name = name.rsplit('.', 2)
-        module = __import__('{}.{}'.format(from_list, module), fromlist=[from_list])
-        target_type = getattr(module, type_name)
-      except (ImportError, ValueError):
-        # Fall back on pants provided target types.
-        registered_aliases = self.context.build_file_parser.registered_aliases()
-        if name not in registered_aliases.targets:
-          raise TaskError('Invalid type name: {}'.format(name))
-        target_type = registered_aliases.targets[name]
-      if not issubclass(target_type, Target):
-        raise TaskError('Not a Target type: {}'.format(name))
-      return lambda target: isinstance(target, target_type)
+      registered_aliases = self.context.build_file_parser.registered_aliases()
+      target_types = registered_aliases.target_types_by_alias.get(name, None)
+      if not target_types:
+        raise TaskError('No Targets with type name: {}'.format(name))
+      return lambda target: isinstance(target, tuple(target_types))
     self._filters.extend(create_filters(self.get_options().type, filter_for_type))
 
     def filter_for_ancestor(spec):

src/python/pants/backend/core/tasks/reflect.py

@@ -420,7 +420,14 @@ def map_symbols(symbols):
         syms[sym] = item
 
   aliases = build_file_parser.registered_aliases()
-  map_symbols(aliases.targets)
+  map_symbols(aliases.target_types)
+
+  # TODO(John Sirois): Handle mapping the `Macro.expand` arguments - these are the real arguments
+  # to document and may be different than the set gathered from walking the Target hierarchy.
+  for alias, target_macro_factory in aliases.target_macro_factories.items():
+    for target_type in target_macro_factory.target_types:
+      map_symbols({alias: target_type})
+
   map_symbols(aliases.objects)
   map_symbols(aliases.context_aware_object_factories)
   return syms

src/python/pants/backend/jvm/register.py

@@ -56,7 +56,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'annotation_processor': AnnotationProcessor,
       'benchmark': Benchmark,

src/python/pants/backend/maven_layout/register.py

@@ -10,7 +10,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     context_aware_object_factories={
       'maven_layout': BuildFileAliases.curry_context(maven_layout)
     }

src/python/pants/backend/project_info/tasks/BUILD

@@ -68,17 +68,21 @@ python_library(
   name = 'export',
   sources = ['export.py'],
   dependencies = [
-    ':projectutils',
-    '3rdparty/python:pex',
     '3rdparty/python/twitter/commons:twitter.common.collections',
+    '3rdparty/python:pex',
+    ':projectutils',
     'src/python/pants/backend/core/targets:all',
     'src/python/pants/backend/core/tasks:console_task',
+    'src/python/pants/backend/jvm/subsystems:jvm_platform',
     'src/python/pants/backend/jvm/targets:jvm',
     'src/python/pants/backend/jvm/targets:scala',
     'src/python/pants/backend/jvm:ivy_utils',
+    'src/python/pants/backend/python/targets:python',
     'src/python/pants/backend/python/tasks:python',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:exceptions',
+    'src/python/pants/java/distribution',
+    'src/python/pants/util:memo',
   ],
 )
 

src/python/pants/backend/project_info/tasks/export.py

@@ -27,6 +27,7 @@
 from pants.base.build_environment import get_buildroot
 from pants.base.exceptions import TaskError
 from pants.java.distribution.distribution import DistributionLocator
+from pants.util.memo import memoized_property
 
 
 # Changing the behavior of this task may affect the IntelliJ Pants plugin
@@ -115,7 +116,6 @@ def prepare(cls, options, round_manager):
   def __init__(self, *args, **kwargs):
     super(Export, self).__init__(*args, **kwargs)
     self.format = self.get_options().formatted
-    self.target_aliases_map = None
 
   def console_output(self, targets):
     targets_map = {}
@@ -290,12 +290,18 @@ def _resolve_jars_info(self):
       mapping[self._jar_id(ivy_module_ref)].update(conf_to_jarfile_map)
     return mapping
 
+  @memoized_property
+  def target_aliases_map(self):
+    registered_aliases = self.context.build_file_parser.registered_aliases()
+    map = {}
+    for alias, target_types in registered_aliases.target_types_by_alias.items():
+      # If a target class is registered under multiple aliases returns the last one.
+      for target_type in target_types:
+        map[target_type] = alias
+    return map
+
   def _get_pants_target_alias(self, pants_target_type):
     """Returns the pants target alias for the given target"""
-    if not self.target_aliases_map:
-      target_aliases = self.context.build_file_parser.registered_aliases().targets
-      # If a target class is registered under multiple aliases returns the last one.
-      self.target_aliases_map = {classname: alias for alias, classname in target_aliases.items()}
     if pants_target_type in self.target_aliases_map:
       return self.target_aliases_map.get(pants_target_type)
     else:

src/python/pants/backend/python/register.py

@@ -23,7 +23,7 @@
 
 
 def build_file_aliases():
-  return BuildFileAliases.create(
+  return BuildFileAliases(
     targets={
       'python_binary': PythonBinary,
       'python_library': PythonLibrary,

src/python/pants/base/BUILD

@@ -83,6 +83,9 @@ python_library(
   sources = ['build_file_aliases.py'],
   dependencies = [
     ':build_file_target_factory',
+    ':deprecated',
+    ':target',
+    'src/python/pants/util:memo',
   ]
 )