Fixed the error messages in assert_list().

Add a key field to the error messages in assert_list() function in validation.py

Testing Done:
When calling the assert_list() function in validation.py, the user didn't have access to the keyword in which the specific error happened, thus making it harder to debug. I have added a key argument param to this function and to the error message to make the message more specific.

current error message:
```
Exception message: Expected an object of acceptable type (<type 'list'>, <class 'twitter.common.dirutil.fileset.Fileset'>, <class 'pants.backend.core.wrapped_globs.FilesetWithSpec'>, <class 'twitter.common.collections.orderedset.OrderedSet'>, <type 'set'>, <type 'tuple'>), received examples/src/python/hello/greet instead
```

expected modified error message:
```
Exception message: In key 'sources': Expected an object of acceptable type (<type 'list'>, <class 'twitter.common.dirutil.fileset.Fileset'>, <class 'pants.backend.core.wrapped_globs.FilesetWithSpec'>, <class 'twitter.common.collections.orderedset.OrderedSet'>, <type 'set'>, <type 'tuple'>), received examples/src/python/hello/greet instead
```

(the key argument has been added to the error message)

Error encountered in issue pantsbuild#642

Bugs closed: 1673

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

contrib/cpp/src/python/pants/contrib/cpp/targets/cpp_target.py

@@ -28,6 +28,7 @@ def __init__(self,
     payload = payload or Payload()
     payload.add_fields({
       'sources': self.create_sources_field(sources=sources,
-                                           sources_rel_path=sources_rel_path),
+                                           sources_rel_path=sources_rel_path,
+                                           key_arg='sources'),
     })
     super(CppTarget, self).__init__(address=address, payload=payload, **kwargs)

src/python/pants/backend/core/targets/doc.py

@@ -94,7 +94,8 @@ def __init__(self,
         format = 'md'
     payload.add_fields({
       'sources': self.create_sources_field(sources=[source],
-                                           sources_rel_path=address.spec_path),
+                                           sources_rel_path=address.spec_path,
+                                           key_arg='sources'),
       'format': PrimitiveField(format),
       'links': PrimitiveField(links or []),
       'provides': self.ProvidesTupleField(provides or []),

src/python/pants/backend/core/targets/resources.py

@@ -27,8 +27,8 @@ def __init__(self, address=None, payload=None, sources=None, **kwargs):
     """
     payload = payload or Payload()
     payload.add_fields({
-      'sources': self.create_sources_field(sources=self.assert_list(sources),
-                                           sources_rel_path=address.spec_path),
+      'sources': self.create_sources_field(sources=self.assert_list(sources, key_arg='sources'),
+                                           sources_rel_path=address.spec_path, key_arg='sources'),
     })
     super(Resources, self).__init__(address=address, payload=payload, **kwargs)
 

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

@@ -108,7 +108,7 @@ def __init__(self, user_id=None, name=None, email=None, url=None, organization=N
     self.url = _validate_maybe_string('url', url)
     self.organization = _validate_maybe_string('organization', organization)
     self.organization_url = _validate_maybe_string('organization_url', organization_url)
-    self.roles = assert_list(roles)
+    self.roles = assert_list(roles, key_arg='roles')
 
   @property
   def has_roles(self):
@@ -136,7 +136,7 @@ def __init__(self, description, url, licenses, developers, scm, name=None):
                         will be synthesized.
     """
     def validate_nonempty_list(list_name, item, expected_type):
-      assert_list(item, expected_type=expected_type, can_be_none=False, allowable=(list,))
+      assert_list(item, expected_type=expected_type, can_be_none=False, key_arg='roles', allowable=(list,))
       if not item:
         raise ValueError('At least 1 entry is required in the {} list.'.format(list_name))
       return item

src/python/pants/backend/jvm/targets/jar_dependency.py

@@ -103,7 +103,7 @@ def __init__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
     self.transitive = not intransitive
     self.apidocs = apidocs
     self.mutable = mutable
-    self.artifacts = tuple(assert_list(artifacts, expected_type=IvyArtifact))
+    self.artifacts = tuple(assert_list(artifacts, expected_type=IvyArtifact, key_arg='artifacts'))
 
     if ext or url or type_ or classifier:
       self.append_artifact(name,

src/python/pants/backend/jvm/targets/jar_library.py

@@ -31,7 +31,7 @@ def __init__(self, payload=None, jars=None, **kwargs):
     """
     payload = payload or Payload()
     payload.add_fields({
-      'jars': JarsField(self.assert_list(jars, expected_type=JarDependency)),
+      'jars': JarsField(self.assert_list(jars, expected_type=JarDependency, key_arg='jars')),
       'excludes': ExcludesField([]),
     })
     super(JarLibrary, self).__init__(payload=payload, **kwargs)

src/python/pants/backend/jvm/targets/java_agent.py

@@ -41,10 +41,10 @@ def __init__(self,
 
     super(JavaAgent, self).__init__(
         name=name,
-        sources=self.assert_list(sources),
+        sources=self.assert_list(sources, key_arg='sources'),
         provides=None,
-        excludes=self.assert_list(excludes),
-        resources=self.assert_list(resources),
+        excludes=self.assert_list(excludes, key_arg='excludes'),
+        resources=self.assert_list(resources, key_arg='resources'),
         **kwargs)
 
     if not (premain or agent_class):

src/python/pants/backend/jvm/targets/jvm_binary.py

@@ -167,7 +167,8 @@ def skip_signatures_and_duplicates_concat_well_known_metadata(cls, default_dup_a
     :returns: JarRules
     """
     default_dup_action = Duplicate.validate_action(default_dup_action or Duplicate.SKIP)
-    additional_rules = assert_list(additional_rules, expected_type=(Duplicate, Skip))
+    additional_rules = assert_list(additional_rules,
+                                   expected_type=(Duplicate, Skip))
 
     rules = [Skip(r'^META-INF/[^/]+\.SF$'),  # signature file
              Skip(r'^META-INF/[^/]+\.DSA$'),  # default signature alg. file
@@ -209,7 +210,7 @@ def __init__(self, rules=None, default_dup_action=Duplicate.SKIP):
     self.payload.add_fields({
       'default_dup_action' : PrimitiveField(Duplicate.validate_action(default_dup_action))
     })
-    self._rules = assert_list(rules, expected_type=JarRule)
+    self._rules = assert_list(rules, expected_type=JarRule, key_arg="rules")
 
   @property
   def default_dup_action(self):
@@ -333,7 +334,9 @@ def __init__(self,
     payload = payload or Payload()
     payload.add_fields({
       'basename' : PrimitiveField(basename or name),
-      'deploy_excludes' : ExcludesField(self.assert_list(deploy_excludes, expected_type=Exclude)),
+      'deploy_excludes' : ExcludesField(self.assert_list(deploy_excludes,
+                                                         expected_type=Exclude,
+                                                         key_arg='deploy_excludes')),
       'deploy_jar_rules' :  FingerprintedField(deploy_jar_rules or JarRules.default()),
       'manifest_entries' : FingerprintedField(ManifestEntries(manifest_entries)),
       'main': PrimitiveField(main),
@@ -342,7 +345,7 @@ def __init__(self,
     super(JvmBinary, self).__init__(name=name,
                                     address=address,
                                     payload=payload,
-                                    sources=self.assert_list(sources),
+                                    sources=self.assert_list(sources, key_arg='sources'),
                                     **kwargs)
 
   @property

src/python/pants/backend/jvm/targets/jvm_target.py

@@ -52,12 +52,12 @@ def __init__(self,
       sources_rel_path = address.spec_path
     payload = payload or Payload()
     payload.add_fields({
-      'sources': self.create_sources_field(sources, sources_rel_path, address),
+      'sources': self.create_sources_field(sources, sources_rel_path, address, key_arg='sources'),
       'provides': provides,
-      'excludes': ExcludesField(self.assert_list(excludes, expected_type=Exclude)),
-      'configurations': ConfigurationsField(self.assert_list(configurations)),
+      'excludes': ExcludesField(self.assert_list(excludes, expected_type=Exclude, key_arg='excludes')),
+      'configurations': ConfigurationsField(self.assert_list(configurations, key_arg='configurations')),
     })
-    self._resource_specs = self.assert_list(resources)
+    self._resource_specs = self.assert_list(resources, key_arg='resources')
 
     super(JvmTarget, self).__init__(address=address, payload=payload,
                                     **kwargs)

src/python/pants/backend/jvm/targets/scala_library.py

@@ -37,7 +37,7 @@ def __init__(self, java_sources=None, **kwargs):
     :param resources: An optional list of paths (DEPRECATED) or ``resources``
       targets containing resources that belong on this library's classpath.
     """
-    self._java_sources_specs = self.assert_list(java_sources)
+    self._java_sources_specs = self.assert_list(java_sources, key_arg='java_sources')
     super(ScalaLibrary, self).__init__(**kwargs)
     self.add_labels('scala')
 

src/python/pants/backend/python/targets/python_requirement_library.py

@@ -21,7 +21,7 @@ def __init__(self, payload=None, requirements=None, **kwargs):
     """
     payload = payload or Payload()
 
-    assert_list(requirements, expected_type=PythonRequirement)
+    assert_list(requirements, expected_type=PythonRequirement, key_arg='requirements')
     payload.add_fields({
       'requirements': PythonRequirementsField(requirements or []),
     })

src/python/pants/backend/python/targets/python_target.py

@@ -57,8 +57,8 @@ def __init__(self,
       sources_rel_path = address.spec_path
     payload = payload or Payload()
     payload.add_fields({
-      'sources': self.create_sources_field(sources, sources_rel_path, address),
-      'resources': self.create_sources_field(resources, address.spec_path),
+      'sources': self.create_sources_field(sources, sources_rel_path, address, key_arg='sources'),
+      'resources': self.create_sources_field(resources, address.spec_path, key_arg='resources'),
       'provides': provides,
       'compatibility': PrimitiveField(maybe_list(compatibility or ())),
     })

src/python/pants/base/payload_field.py

@@ -93,7 +93,7 @@ def __init__(self, sources_rel_path, sources, ref_address=None, filespec=None):
     :param filespec: glob and exclude data that generated this set of sources
     """
     self._rel_path = sources_rel_path
-    self._source_paths = assert_list(sources)
+    self._source_paths = assert_list(sources, key_arg='sources')
     self._ref_address = ref_address
     self._filespec = filespec
 
@@ -180,7 +180,7 @@ def populate(self, sources, rel_path=None):
       raise self.AlreadyPopulatedError("Called with rel_path={rel_path} sources={sources}"
       .format(rel_path=rel_path, sources=sources))
     self._rel_path = rel_path
-    self._source_paths = assert_list(sources)
+    self._source_paths = assert_list(sources, key_arg='sources')
     self._populated = True
 
   @property

src/python/pants/base/target.py

@@ -213,8 +213,8 @@ def tags(self):
   def num_chunking_units(self):
     return max(1, len(self.sources_relative_to_buildroot()))
 
-  def assert_list(self, maybe_list, expected_type=string_types):
-    return assert_list(maybe_list, expected_type,
+  def assert_list(self, maybe_list, expected_type=string_types, key_arg=None):
+    return assert_list(maybe_list, expected_type, key_arg=key_arg,
                        raise_type=lambda msg: TargetDefinitionException(self, msg))
 
   def compute_invalidation_hash(self, fingerprint_strategy=None):
@@ -464,7 +464,7 @@ def __repr__(self):
     addr = self.address if hasattr(self, 'address') else 'address not yet set'
     return "{}({})".format(type(self).__name__, addr)
 
-  def create_sources_field(self, sources, sources_rel_path, address=None):
+  def create_sources_field(self, sources, sources_rel_path, address=None, key_arg=None):
     """Factory method to create a SourcesField appropriate for the type of the sources object.
 
     Note that this method is called before the call to Target.__init__ so don't expect fields to
@@ -486,7 +486,7 @@ def create_sources_field(self, sources, sources_rel_path, address=None):
       filespec = sources.filespec
     else:
       sources = sources or []
-      assert_list(sources)
+      assert_list(sources, key_arg=key_arg)
       filespec = {'globs' : [os.path.join(sources_rel_path, src) for src in (sources or [])]}
 
     return SourcesField(sources=sources, sources_rel_path=sources_rel_path, filespec=filespec)

src/python/pants/base/validation.py

@@ -12,35 +12,43 @@
 from pants.backend.core.wrapped_globs import FilesetWithSpec
 
 
-def assert_list(obj, expected_type=string_types, can_be_none=True, default=(),
+def assert_list(obj, expected_type=string_types, can_be_none=True, default=(), key_arg=None,
     allowable=(list, Fileset, FilesetWithSpec, OrderedSet, set, tuple), raise_type=ValueError):
   """
   This function is used to ensure that parameters set by users in BUILD files are of acceptable types.
   :param obj           : the object that may be a list. It will pass if it is of type in allowable.
   :param expected_type : this is the expected type of the returned list contents.
   :param can_be_none   : this defines whether or not the obj can be None. If True, return default.
   :param default       : this is the default to return if can_be_none is True and obj is None.
+  :param key_arg       : this is the name of the key to which obj belongs to
   :param allowable     : the acceptable types for obj. We do not want to allow any iterable (eg string).
   :param raise_type    : the error to throw if the type is not correct.
   """
+  def get_key_msg(key=None):
+    if key is None:
+      return ''
+    else:
+      return "In key '{}': ".format(key)
+
+  key_msg = get_key_msg(key_arg)
   val = obj
   if val is None:
     if can_be_none:
       val = list(default)
     else:
       raise raise_type(
-        'Expected an object of acceptable type {}, received None and can_be_none is False'
-        .format(allowable))
+        '{}Expected an object of acceptable type {}, received None and can_be_none is False'
+          .format(key_msg, allowable))
 
   if isinstance(val, allowable):
     lst = list(val)
     for e in lst:
       if not isinstance(e, expected_type):
         raise raise_type(
-            'Expected a list containing values of type {}, instead got a value {} of {}'
-            .format(expected_type, e, e.__class__))
+            '{}Expected a list containing values of type {}, instead got a value {} of {}'
+            .format(key_msg, expected_type, e, e.__class__))
     return lst
   else:
     raise raise_type(
-        'Expected an object of acceptable type {}, received {} instead'
-        .format(allowable, val))
+        '{}Expected an object of acceptable type {}, received {} instead'
+        .format(key_msg, allowable, val))

tests/python/pants_test/base/test_validation.py

@@ -5,19 +5,32 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-import pytest
+import unittest
 
 from pants.base.validation import assert_list
 
 
-def test_valid_inputs():
-  # list of strings gives list of strings
-  assert assert_list(["file1.txt", "file2.txt"]) == ["file1.txt", "file2.txt"]
-  assert assert_list(None) == []  # None is ok by default
+class ParseValidation(unittest.TestCase):
+  def test_valid_inputs(self):
+    list_result0 = assert_list(["file1.txt"])
+    list_result1 = assert_list(["file1.txt", "file2.txt"])
+    list_result2 = assert_list(None)
+    self.assertEqual(list_result0, ["file1.txt"])  # list of strings gives list of strings
+    self.assertEqual(list_result1, ["file1.txt", "file2.txt"])
+    self.assertEqual(list_result2, []) # None is ok by default
 
+  def test_invalid_inputs(self):
+    with self.assertRaises(ValueError):
+      assert_list({"file2.txt": True})  # Can't pass a dict by default
+    with self.assertRaises(ValueError):
+      assert_list([["file2.txt"], "file2.txt"])  # All values in list must be stringy values
+    with self.assertRaises(ValueError):
+      assert_list(None, can_be_none=False) # The default is ok as None only when can_be_noe is true
 
-def test_invalid_inputs():
-  with pytest.raises(ValueError):
-    assert_list({"file2.txt": True})  # Can't pass a dict by default
-  with pytest.raises(ValueError):
-    assert_list([["file2.txt"], "file2.txt"])  # All values in list must be stringy values
+  def test_invalid_inputs_with_key_arg(self):
+    with self.assertRaisesRegexp(ValueError, "In key 'resources':"):
+      assert_list({"file3.txt":"source"}, key_arg='resources') # Can't pass a dict
+    with self.assertRaisesRegexp(ValueError, "In key 'artifacts':"):
+      assert_list([["file3.txt"]], key_arg='artifacts') # All values most be strings
+    with self.assertRaisesRegexp(ValueError, "In key 'jars':"):
+      assert_list(None, can_be_none=False, key_arg='jars') # The default is ok as None only when can_be_noe is true