Restrict --validator-class to types (no functions)

In order to better guarantee that `--validator-class` provides the
name of a `jsonschema.protocols.Validator` class, require that the
value passed is a type and not an alternative function.

This requirement does not lose significant expressive power and allows
us to more safely assume that the validator really is an
implementation of the protocol which can be extended with
`--fill-defaults` and other validator-related hooks.

There is no real way to guarantee the safety of using the user's
Validator. After all -- it could always raise an error at any time --
but this matches check-jsonschema's expectations about the argument
better.

This change can always be reverted in the future if there is
significant demand for a function-as-a-validator-class option.

Author: sirosen

src/check_jsonschema/cli/param_types.py

@@ -3,7 +3,6 @@
 import importlib
 import re
 import typing as t
-import warnings
 
 import click
 import jsonschema
@@ -101,17 +100,7 @@ def convert(
                 ctx,
             )
 
-        if not callable(result):
-            self.fail(
-                f"'{classname}' in '{pkg}' is not a class or callable", param, ctx
-            )
-
         if not isinstance(result, type):
-            warnings.warn(
-                f"'{classname}' in '{pkg}' is not a class. If it is a function "
-                f"returning a Validator, it still might work, but this usage "
-                "is not recommended.",
-                stacklevel=1,
-            )
+            self.fail(f"'{classname}' in '{pkg}' is not a class", param, ctx)
 
         return t.cast(t.Type[jsonschema.protocols.Validator], result)

tests/unit/test_cli_parse.py

@@ -307,34 +307,9 @@ def test_can_specify_custom_validator_class(runner, mock_parse_result, mock_modu
     assert mock_parse_result.validator_class == foo.MyValidator
 
 
-def test_warns_on_validator_function(runner, mock_parse_result, mock_module):
-    mock_module(
-        "foo/bar.py",
-        """\
-class MyValidator: pass
-
-def validator(*args, **kwargs):
-    return MyValidator(*args, **kwargs)
-""",
-    )
-    import foo.bar
-
-    with pytest.warns(UserWarning, match="'validator' in 'foo.bar' is not a class"):
-        result = runner.invoke(
-            cli_main,
-            [
-                "--schemafile",
-                "schema.json",
-                "foo.json",
-                "--validator-class",
-                "foo.bar:validator",
-            ],
-        )
-        assert result.exit_code == 0
-    assert mock_parse_result.validator_class == foo.bar.validator
-
-
-@pytest.mark.parametrize("failmode", ("syntax", "import", "attr", "callability"))
+@pytest.mark.parametrize(
+    "failmode", ("syntax", "import", "attr", "function", "non_callable")
+)
 def test_can_custom_validator_class_fails(
     runner, mock_parse_result, mock_module, failmode
 ):
@@ -343,7 +318,7 @@ def test_can_custom_validator_class_fails(
         """\
 class MyValidator: pass
 
-def validator(*args, **kwargs):
+def validator_func(*args, **kwargs):
     return MyValidator(*args, **kwargs)
 
 other_thing = 100
@@ -356,7 +331,9 @@ def validator(*args, **kwargs):
         arg = "foo.bar:MyValidator"
     elif failmode == "attr":
         arg = "foo:no_such_attr"
-    elif failmode == "callability":
+    elif failmode == "function":
+        arg = "foo:validator_func"
+    elif failmode == "non_callable":
         arg = "foo:other_thing"
     else:
         raise NotImplementedError
@@ -373,7 +350,7 @@ def validator(*args, **kwargs):
         assert "was not an importable module" in result.stderr
     elif failmode == "attr":
         assert "was not resolvable to a class" in result.stderr
-    elif failmode == "callability":
-        assert "is not a class or callable" in result.stderr
+    elif failmode in ("function", "non_callable"):
+        assert "is not a class" in result.stderr
     else:
         raise NotImplementedError