Repository: coverahealth/dataspec

Issue:
`s(None)` throws an exception
```
>>> from dataspec import s
>>> s(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/christopher/Projects/dataspec/src/dataspec/api.py", line 101, in __call__
    return make_spec(*args, conformer=conformer)
  File "/Users/christopher/Projects/dataspec/src/dataspec/base.py", line 768, in make_spec
    raise TypeError(f"Expected some spec predicate; received type {type(pred)}")
TypeError: Expected some spec predicate; received type <class 'NoneType'>
```

This should be a valid alias for `s(type(None))`.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2672046..cc15b9a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   the value spec tags are derived automatically from the corresponding dict keys (#52)
 - Add documentation built using Sphinx and hosted on ReadTheDocs (#9)
 
+### Fixed
+- Fixed a bug where `s(None)` is not a valid alias for `s(type(None))` (#61)
+
 ## [v0.2.5] - 2020-04-10
 ### Added
 - Add `SpecPredicate` and `tag_maybe` to the public interface (#49)
diff --git a/docs/usage.rst b/docs/usage.rst
index f0bc885..b25cdc8 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -99,6 +99,10 @@ by simply passing a Python type directly to the ``s`` constructor:
    spec.is_valid("a string")  # True
    spec.is_valid(3)           # False
 
+.. note::
+
+   ``s(None)`` is a shortcut for ``s(type(None))`` .
+
 .. _factories_usage:
 
 Factories
diff --git a/src/dataspec/base.py b/src/dataspec/base.py
index f0ca97c..42e4b74 100644
--- a/src/dataspec/base.py
+++ b/src/dataspec/base.py
@@ -494,7 +494,7 @@ class DictSpec(Spec):
                     )
         except TypeError:
             yield ErrorDetails(
-                message=f"Value is not a mapping type",
+                message="Value is not a mapping type",
                 pred=self,
                 value=d,
                 via=[self.tag],
@@ -771,7 +771,7 @@ def type_spec(
     )
 
 
-def make_spec(  # pylint: disable=inconsistent-return-statements
+def make_spec(  # pylint: disable=inconsistent-return-statements  # noqa: MC0001
     *args: Union[Tag, SpecPredicate], conformer: Optional[Conformer] = None
 ) -> Spec:
     """
@@ -784,7 +784,9 @@ def make_spec(  # pylint: disable=inconsistent-return-statements
     be *exactly* ``Iterator[ErrorDetails]`` ).
 
     Specs may be created from Python types, in which case a Spec will be produced
-    that performs an :py:func:`isinstance` check.
+    that performs an :py:func:`isinstance` check. :py:obj:`None` may be provided as
+    a shortcut for ``type(None)`` . To specify a nilable value, you should use
+    :py:meth:`dataspec.SpecAPI.nilable` instead.
 
     Specs may be created for enumerated types using a Python ``set`` or ``frozenset``
     or using Python :py:class:`enum.Enum` types. Specs created for enumerated types
@@ -820,7 +822,11 @@ def make_spec(  # pylint: disable=inconsistent-return-statements
     :return: a :py:class:`dataspec.base.Spec` instance
     """
     tag = args[0] if isinstance(args[0], str) else None
-    pred = args[0] if tag is None else args[1]
+
+    try:
+        pred = args[0] if tag is None else args[1]
+    except IndexError:
+        raise TypeError("Expected some spec predicate; received only a Tag")
 
     if isinstance(pred, (frozenset, set)):
         return SetSpec(tag or "set", pred, conformer=conformer)
@@ -861,5 +867,7 @@ def make_spec(  # pylint: disable=inconsistent-return-statements
             return PredicateSpec(
                 tag or pred.__name__, cast(PredicateFn, pred), conformer=conformer
             )
+    elif pred is None:
+        return type_spec(tag, type(None), conformer=conformer)
     else:
         raise TypeError(f"Expected some spec predicate; received type {type(pred)}")
diff --git a/src/dataspec/factories.py b/src/dataspec/factories.py
index d1b2474..d8ed31e 100644
--- a/src/dataspec/factories.py
+++ b/src/dataspec/factories.py
@@ -510,7 +510,7 @@ def _make_datetime_spec_factory(  # noqa: MC0001
             if type_ is datetime:
 
                 @pred_to_validator(
-                    f"Datetime '{{value}}' is not aware", complement=is_aware
+                    "Datetime '{value}' is not aware", complement=is_aware
                 )
                 def datetime_is_aware(d: datetime) -> bool:
                     return d.tzinfo is not None and d.tzinfo.utcoffset(d) is not None
@@ -519,9 +519,7 @@ def _make_datetime_spec_factory(  # noqa: MC0001
 
             elif type_ is time:
 
-                @pred_to_validator(
-                    f"Time '{{value}}' is not aware", complement=is_aware
-                )
+                @pred_to_validator("Time '{value}' is not aware", complement=is_aware)
                 def time_is_aware(t: time) -> bool:
                     return t.tzinfo is not None and t.tzinfo.utcoffset(None) is not None
 
@@ -632,7 +630,7 @@ else:
 
         tag = tag or "datetime_str"
 
-        @pred_to_validator(f"Value '{{value}}' is not type 'str'", complement=True)
+        @pred_to_validator("Value '{value}' is not type 'str'", complement=True)
         def is_str(x: Any) -> bool:
             return isinstance(x, str)
 
@@ -1079,7 +1077,7 @@ else:
         tag = tag or "phonenumber_str"
         default_conformer = conform_phonenumber
 
-        @pred_to_validator(f"Value '{{value}}' is not type 'str'", complement=True)
+        @pred_to_validator("Value '{value}' is not type 'str'", complement=True)
         def is_str(x: Any) -> bool:
             return isinstance(x, str)
 
@@ -1094,7 +1092,7 @@ else:
             country_code = phonenumbers.country_code_for_region(region)
 
             @pred_to_validator(
-                f"Parsed telephone number regions ({{value}}) does not "
+                "Parsed telephone number regions ({value}) does not "
                 f"match expected region {region}",
                 complement=True,
                 convert_value=lambda p: ", ".join(
@@ -1111,7 +1109,7 @@ else:
         if is_possible:
 
             @pred_to_validator(
-                f"Parsed telephone number '{{value}}' is not possible", complement=True
+                "Parsed telephone number '{value}' is not possible", complement=True
             )
             def validate_phonenumber_is_possible(p: phonenumbers.PhoneNumber) -> bool:
                 return phonenumbers.is_possible_number(p)
@@ -1121,7 +1119,7 @@ else:
         if is_valid:
 
             @pred_to_validator(
-                f"Parsed telephone number '{{value}}' is not valid", complement=True
+                "Parsed telephone number '{value}' is not valid", complement=True
             )
             def validate_phonenumber_is_valid(p: phonenumbers.PhoneNumber) -> bool:
                 return phonenumbers.is_valid_number(p)
@@ -1199,7 +1197,7 @@ def _str_is_uuid(s: str) -> Iterator[ErrorDetails]:
         uuid.UUID(s)
     except ValueError:
         yield ErrorDetails(
-            message=f"String does not contain UUID", pred=_str_is_uuid, value=s
+            message="String does not contain UUID", pred=_str_is_uuid, value=s
         )
 
 
@@ -1211,7 +1209,7 @@ if sys.version_info >= (3, 7):
             date.fromisoformat(s)
         except ValueError:
             yield ErrorDetails(
-                message=f"String does not contain ISO formatted date",
+                message="String does not contain ISO formatted date",
                 pred=_str_is_iso_date,
                 value=s,
             )
@@ -1222,7 +1220,7 @@ if sys.version_info >= (3, 7):
             datetime.fromisoformat(s)
         except ValueError:
             yield ErrorDetails(
-                message=f"String does not contain ISO formatted datetime",
+                message="String does not contain ISO formatted datetime",
                 pred=_str_is_iso_datetime,
                 value=s,
             )
@@ -1233,7 +1231,7 @@ if sys.version_info >= (3, 7):
             time.fromisoformat(s)
         except ValueError:
             yield ErrorDetails(
-                message=f"String does not contain ISO formatted time",
+                message="String does not contain ISO formatted time",
                 pred=_str_is_iso_time,
                 value=s,
             )
@@ -1335,7 +1333,7 @@ def str_spec(  # noqa: MC0001  # pylint: disable=too-many-arguments
     :return: a Spec which validates strings
     """
 
-    @pred_to_validator(f"Value '{{value}}' is not a string", complement=True)
+    @pred_to_validator("Value '{value}' is not a string", complement=True)
     def is_str(s: Any) -> bool:
         return isinstance(s, str)
 
@@ -1516,7 +1514,7 @@ def url_str_spec(
     :return: a Spec which can validate that a string contains a URL
     """
 
-    @pred_to_validator(f"Value '{{value}}' is not a string", complement=True)
+    @pred_to_validator("Value '{value}' is not a string", complement=True)
     def is_str(s: Any) -> bool:
         return isinstance(s, str)
 
@@ -1560,7 +1558,7 @@ def url_str_spec(
         raise ValueError(f"Unused keyword arguments: {kwargs}")
 
     if query_spec is None and not child_validators:
-        raise ValueError(f"URL specs must include at least one validation rule")
+        raise ValueError("URL specs must include at least one validation rule")
 
     def validate_parse_result(v: ParseResult) -> Iterator[ErrorDetails]:
         for validate in child_validators:
@@ -1611,7 +1609,7 @@ def uuid_spec(
     :return: a Spec which validates UUIDs
     """
 
-    @pred_to_validator(f"Value '{{value}}' is not a UUID", complement=True)
+    @pred_to_validator("Value '{value}' is not a UUID", complement=True)
     def is_uuid(v: Any) -> bool:
         return isinstance(v, uuid.UUID)
 
@@ -1622,7 +1620,7 @@ def uuid_spec(
         if not {1, 3, 4, 5}.issuperset(set(versions)):
             raise ValueError("UUID versions must be specified as a set of integers")
 
-        @pred_to_validator(f"UUID '{{value}}' is not RFC 4122 variant", complement=True)
+        @pred_to_validator("UUID '{value}' is not RFC 4122 variant", complement=True)
         def uuid_is_rfc_4122(v: uuid.UUID) -> bool:
             return v.variant is uuid.RFC_4122
 


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index b5c19a5..af391e7 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -7,15 +7,11 @@ from dataspec.base import PredicateSpec, ValidatorSpec, pred_to_validator
 
 
 class TestSpecConstructor:
-    @pytest.mark.parametrize("v", [None, 5, 3.14, 8j])
+    @pytest.mark.parametrize("v", [5, 3.14, 8j, "a value"])
     def test_invalid_specs(self, v):
         with pytest.raises(TypeError):
             s(v)
 
-    def test_spec_with_no_pred(self):
-        with pytest.raises(IndexError):
-            s("string")
-
     def test_validator_spec(self):
         def is_valid(v) -> Iterator[ErrorDetails]:
             if v:
diff --git a/tests/test_base.py b/tests/test_base.py
index f0a45c6..4dc09d6 100644
--- a/tests/test_base.py
+++ b/tests/test_base.py
@@ -665,6 +665,7 @@ class TestTypeSpec:
     @pytest.mark.parametrize(
         "tp,vals",
         [
+            (None, [None]),
             (bool, [True, False]),
             (bytes, [b"", b"a", b"bytes"]),
             (dict, [{}, {"a": "b"}]),
@@ -683,6 +684,7 @@ class TestTypeSpec:
     @pytest.fixture
     def python_vals(self):
         return [
+            None,
             True,
             False,
             b"",