Remove last vestiges of "special" deptypes.

- Remove `special_types` dict in spec.py, as only 'all' is still used
- Still allow 'all' to be used as a deptype
- Simplify `canonical_deptype` function
- Clean up args in spack graph
- Add tests

lib/spack/spack/cmd/graph.py

@@ -90,7 +90,8 @@ def graph(parser, args):
     deptype = alldeps
     if args.deptype:
         deptype = tuple(args.deptype.split(','))
-        validate_deptype(deptype)
+        if deptype == ('all',):
+            deptype = 'all'
         deptype = canonical_deptype(deptype)
 
     if args.dot:  # Dot graph only if asked for.

lib/spack/spack/directives.py

@@ -241,8 +241,7 @@ def _depends_on(pkg, spec, when=None, type=None):
         #               but is most backwards-compatible.
         type = ('build', 'link')
 
-    if isinstance(type, str):
+    type = spack.spec.canonical_deptype(type)
-        type = spack.spec.special_types.get(type, (type,))
 
     for deptype in type:
         if deptype not in spack.spec.alldeps:

lib/spack/spack/spec.py

@@ -137,7 +137,6 @@
     'Spec',
     'alldeps',
     'canonical_deptype',
-    'validate_deptype',
     'parse',
     'parse_anonymous_spec',
     'SpecError',
@@ -150,7 +149,6 @@
     'DuplicateArchitectureError',
     'InconsistentSpecError',
     'InvalidDependencyError',
-    'InvalidDependencyTypeError',
     'NoProviderError',
     'MultipleProviderError',
     'UnsatisfiableSpecError',
@@ -197,44 +195,27 @@
    every time we call str()"""
 _any_version = VersionList([':'])
 
-# Special types of dependencies.
+"""Types of dependencies that Spack understands."""
 alldeps = ('build', 'link', 'run')
-norun   = ('link', 'build')
-special_types = {
-    'alldeps': alldeps,
-    'all': alldeps,  # allow "all" as string but not symbol.
-    'norun': norun,
-}
-
-legal_deps = tuple(special_types) + alldeps
 
 """Max integer helps avoid passing too large a value to cyaml."""
 maxint = 2 ** (ctypes.sizeof(ctypes.c_int) * 8 - 1) - 1
 
 
-def validate_deptype(deptype):
-    if isinstance(deptype, str):
-        if deptype not in legal_deps:
-            raise InvalidDependencyTypeError(
-                "Invalid dependency type: %s" % deptype)
-
-    elif isinstance(deptype, (list, tuple)):
-        for t in deptype:
-            validate_deptype(t)
-
-    elif deptype is None:
-        raise InvalidDependencyTypeError("deptype cannot be None!")
-
-
 def canonical_deptype(deptype):
-    if deptype is None:
+    if deptype in (None, 'all', all):
         return alldeps
 
     elif isinstance(deptype, string_types):
-        return special_types.get(deptype, (deptype,))
+        if deptype not in alldeps:
+            raise ValueError('Invalid dependency type: %s' % deptype)
+        return (deptype,)
 
     elif isinstance(deptype, (tuple, list)):
-        return (sum((canonical_deptype(d) for d in deptype), ()))
+        invalid = next((d for d in deptype if d not in alldeps), None)
+        if invalid:
+            raise ValueError('Invalid dependency type: %s' % invalid)
+        return tuple(sorted(deptype))
 
     return deptype
 
@@ -3338,10 +3319,6 @@ class InvalidDependencyError(SpecError):
        of the package."""
 
 
-class InvalidDependencyTypeError(SpecError):
-    """Raised when a dependency type is not a legal Spack dep type."""
-
-
 class NoProviderError(SpecError):
     """Raised when there is no package that provides a particular
        virtual dependency.

lib/spack/spack/test/spec_dag.py

@@ -30,7 +30,7 @@
 import spack.architecture
 import spack.package
 
-from spack.spec import Spec
+from spack.spec import Spec, canonical_deptype, alldeps
 
 
 def check_links(spec_to_check):
@@ -737,3 +737,54 @@ def test_getitem_exceptional_paths(self):
 
         with pytest.raises(AttributeError):
             q.libs
+
+    def test_canonical_deptype(self):
+        # special values
+        assert canonical_deptype(all) == alldeps
+        assert canonical_deptype('all') == alldeps
+        assert canonical_deptype(None) == alldeps
+
+        # everything in alldeps is canonical
+        for v in alldeps:
+            assert canonical_deptype(v) == (v,)
+
+        # tuples
+        assert canonical_deptype(('build',)) == ('build',)
+        assert canonical_deptype(
+            ('build', 'link', 'run')) == ('build', 'link', 'run')
+        assert canonical_deptype(
+            ('build', 'link')) == ('build', 'link')
+        assert canonical_deptype(
+            ('build', 'run')) == ('build', 'run')
+
+        # lists
+        assert canonical_deptype(
+            ['build', 'link', 'run']) == ('build', 'link', 'run')
+        assert canonical_deptype(
+            ['build', 'link']) == ('build', 'link')
+        assert canonical_deptype(
+            ['build', 'run']) == ('build', 'run')
+
+        # sorting
+        assert canonical_deptype(
+            ('run', 'build', 'link')) == ('build', 'link', 'run')
+        assert canonical_deptype(
+            ('run', 'link', 'build')) == ('build', 'link', 'run')
+        assert canonical_deptype(
+            ('run', 'link')) == ('link', 'run')
+        assert canonical_deptype(
+            ('link', 'build')) == ('build', 'link')
+
+        # can't put 'all' in tuple or list
+        with pytest.raises(ValueError):
+            canonical_deptype(['all'])
+        with pytest.raises(ValueError):
+            canonical_deptype(('all',))
+
+        # invalid values
+        with pytest.raises(ValueError):
+            canonical_deptype('foo')
+        with pytest.raises(ValueError):
+            canonical_deptype(('foo', 'bar'))
+        with pytest.raises(ValueError):
+            canonical_deptype(('foo',))