Also take into account the count of subschema errors for relevance.

When sibling subschemas fail, pick the one which produced fewer errors.

Closes: #1002

Author: Julian

CHANGELOG.rst

@@ -4,6 +4,7 @@ v4.27.0
 * Improve ``best_match`` for ``anyOf`` / ``oneOf`` errors: the most relevant error is now found within each subschema separately before the deepest of those is picked (#1257, #1300).
   In particular an applicator with a single subschema now produces the same best match as the bare subschema would.
   ``relevance`` no longer considers an error's position amongst its siblings.
+  When subschemas fail equally deeply, the one which produced fewer errors is now preferred (#1002).
 * Accessing an index with no errors on an ``ErrorTree`` no longer causes that index to appear in the tree (#1328).
   Subtrees also properly propagate lookup errors which were previously missing when accessing indices not in the subtree.
 

jsonschema/exceptions.py

@@ -3,12 +3,10 @@
 """
 from __future__ import annotations
 
-from collections import deque
-from operator import itemgetter
+from collections import defaultdict, deque
 from pprint import pformat
 from textwrap import dedent, indent
-from typing import TYPE_CHECKING, Any, ClassVar
-import heapq
+from typing import TYPE_CHECKING, Any, ClassVar, TypeVar
 import re
 import warnings
 
@@ -18,7 +16,13 @@
 from jsonschema import _utils
 
 if TYPE_CHECKING:
-    from collections.abc import Iterable, Mapping, MutableMapping, Sequence
+    from collections.abc import (
+        Callable,
+        Iterable,
+        Mapping,
+        MutableMapping,
+        Sequence,
+    )
 
     from jsonschema import _types
 
@@ -198,6 +202,9 @@ def _matches_type(self) -> bool:
         )
 
 
+_E = TypeVar("_E", bound=_Error)
+
+
 class ValidationError(_Error):
     """
     An instance was invalid under a provided schema.
@@ -468,9 +475,10 @@ def best_match(errors, key=relevance):
 
     If the resulting match is either :kw:`oneOf` or :kw:`anyOf`, the
     *opposite* assumption is made -- i.e. the deepest error is picked
-    among the most relevant errors in each separate subschema,
-    since these keywords only need to match once, and any other errors
-    may not be relevant.
+    among the most relevant errors in each separate subschema (preferring
+    subschemas which produced fewer errors when tied), since these
+    keywords only need to match once, and any other errors may not be
+    relevant.
 
     Arguments:
         errors (collections.abc.Iterable):
@@ -498,55 +506,57 @@ def best_match(errors, key=relevance):
         set of inputs from version to version if better heuristics are added.
 
     """
-    most_relevant = None
-    for error in errors:
-        most_relevant = _more_relevant(most_relevant, (key(error), error))
-    if most_relevant is None:
+    _, best = _most_relevant(errors, key=key)
+    if best is None:
         return
-    _, best = most_relevant
 
     while best.context:
-        # Find the most relevant error within each separate subschema,
-        # computing each error's key exactly once along the way.
-        best_in_subschemas: dict[Any, tuple[Any, ValidationError]] = {}
+        # Group the errors by the subschema which produced them.
+        by_subschema: dict[Any, list[_Error]] = defaultdict(list)
         for error in best.context:
             index = error.schema_path[0] if error.schema_path else None
-            best_in_subschemas[index] = _more_relevant(
-                best_in_subschemas.get(index),
-                (key(error), error),
-            )
-
-        # Calculate the minimum via nsmallest, because we don't recurse if
-        # all nested errors have the same relevance (i.e. if min == max == all)
-        smallest = heapq.nsmallest(
-            2,
-            best_in_subschemas.values(),
-            key=itemgetter(0),
-        )
-        if len(smallest) == 2 and smallest[0][0] == smallest[1][0]:  # noqa: PLR2004
-            return best
-        _, best = smallest[0]
+            by_subschema[index].append(error)
+
+        # Rank each subschema by how deep its most relevant error is,
+        # and amongst those equally deep, by how few errors it produced
+        # (i.e. how close it was to being valid). Lower ranks are better.
+        best_rank, best_in_subschema, tied = None, None, False
+        for errors_in_subschema in by_subschema.values():
+            error_key, error = _most_relevant(errors_in_subschema, key=key)
+            rank = error_key, len(errors_in_subschema)
+            if best_rank is None or rank < best_rank:
+                best_rank, best_in_subschema, tied = rank, error, False
+            elif rank == best_rank:
+                tied = True
+
+        # If multiple subschemas rank equally we can't tell which was
+        # intended, so we stop here rather than descend into one of them.
+        if tied:
+            break
+        best = best_in_subschema
     return best
 
 
-def _more_relevant(
-    previous: tuple[Any, ValidationError] | None,
-    candidate: tuple[Any, ValidationError],
-) -> tuple[Any, ValidationError]:
+def _most_relevant(
+    errors: Iterable[_E],
+    key: Callable[[_E], Any],
+) -> tuple[Any, _E | None]:
     """
-    Pick the more relevant of two ``(key, error)`` pairs.
+    Find the most relevant error along with its key, computing each key once.
 
     Equally relevant errors are settled by picking the one which appears
     earlier in the instance, which makes the choice independent of the
     order in which the errors happened to be produced.
+
+    Returns ``(None, None)`` if there were no errors.
     """
-    if previous is None:
-        return candidate
-    previous_key, previous_error = previous
-    candidate_key, candidate_error = candidate
-    if candidate_key > previous_key or (
-        candidate_key == previous_key
-        and candidate_error.path < previous_error.path
-    ):
-        return candidate
-    return previous
+    best_key, best = None, None
+    for error in errors:
+        error_key = key(error)
+        if (
+            best is None
+            or error_key > best_key
+            or (error_key == best_key and error.path < best.path)
+        ):
+            best_key, best = error_key, error
+    return best_key, best

jsonschema/tests/test_exceptions.py

@@ -45,6 +45,35 @@ def test_earlier_sibling_errors_are_better_matches(self):
         best = self.best_match_of(instance=[12, 12, 12], schema=schema)
         self.assertEqual(list(best.path), [0])
 
+    def test_earlier_sibling_errors_within_a_subschema_are_better_matches(
+        self,
+    ):
+        """
+        The same holds within an anyOf subschema, even though its errors are
+        always provided in the order they were produced in.
+        """
+
+        schema = {"anyOf": [{"items": {"const": 37}}]}
+        error = next(_LATEST_VERSION(schema).iter_errors([12, 12, 12]))
+        error.context.reverse()
+        best = exceptions.best_match([error])
+        self.assertEqual(list(best.path), [0])
+
+    def test_hand_constructed_errors_with_context(self):
+        """
+        Errors constructed by hand need not have any schema path at all.
+        """
+
+        error = exceptions.ValidationError(
+            "parent",
+            context=[
+                exceptions.ValidationError("child", path=["foo"]),
+                exceptions.ValidationError("grandchild", path=["foo", "bar"]),
+            ],
+        )
+        best = exceptions.best_match([error])
+        self.assertEqual(best.message, "child")
+
     def test_oneOf_and_anyOf_are_weak_matches(self):
         """
         A property you *must* match is probably better than one you have to
@@ -160,10 +189,13 @@ def test_no_anyOf_traversal_for_two_properties_sibling_errors(self):
         best = self.best_match_of(instance={"foo": 1, "bar": 1}, schema=schema)
         self.assertEqual(best.validator, "anyOf")
 
-    def test_no_anyOf_traversal_for_two_items_sibling_errors(self):
+    def test_anyOf_traversal_prefers_subschemas_with_fewer_errors(self):
         """
-        We don't traverse into an anyOf if all of its context errors
-        seem to be equally "wrong" against the instance.
+        When subschemas fail at the same depth, we traverse into the one
+        which produced fewer errors, since it was closer to being valid.
+
+        Here the first subschema rejects every item, whereas the second
+        rejects only the non-integer one.
         """
 
         schema = {
@@ -188,9 +220,40 @@ def test_no_anyOf_traversal_for_two_items_sibling_errors(self):
             },
         }
         best = self.best_match_of(instance=["not an int", 0], schema=schema)
-        self.assertEqual(best.validator, "anyOf")
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(best.instance, "not an int")
         best = self.best_match_of(instance=[0, "not an int"], schema=schema)
-        self.assertEqual(best.validator, "anyOf")
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(best.instance, "not an int")
+
+    def test_anyOf_traversal_for_partially_matching_subschemas(self):
+        """
+        When one subschema matches more of the instance than another,
+        we traverse into it, and report its remaining error.
+
+        See #1002.
+        """
+
+        schema = {
+            "anyOf": [
+                {
+                    "properties": {
+                        "version": {"const": 1},
+                        "description": {"type": "string"},
+                    },
+                },
+                {
+                    "properties": {
+                        "version": {"const": 2},
+                        "description": {"type": "string"},
+                    },
+                },
+            ],
+        }
+        instance = {"version": 1, "description": 0}
+        best = self.best_match_of(instance=instance, schema=schema)
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(list(best.path), ["description"])
 
     def test_anyOf_traversal_for_single_equally_relevant_error(self):
         """
@@ -337,10 +400,13 @@ def test_no_oneOf_traversal_for_two_properties_sibling_errors(self):
         best = self.best_match_of(instance={"foo": 1, "bar": 1}, schema=schema)
         self.assertEqual(best.validator, "oneOf")
 
-    def test_no_oneOf_traversal_for_two_items_sibling_errors(self):
+    def test_oneOf_traversal_prefers_subschemas_with_fewer_errors(self):
         """
-        We don't traverse into an anyOf if all of its context errors
-        seem to be equally "wrong" against the instance.
+        When subschemas fail at the same depth, we traverse into the one
+        which produced fewer errors, since it was closer to being valid.
+
+        Here the first subschema rejects every item, whereas the second
+        rejects only the non-integer one.
         """
 
         schema = {
@@ -365,9 +431,40 @@ def test_no_oneOf_traversal_for_two_items_sibling_errors(self):
             },
         }
         best = self.best_match_of(instance=["not an int", 0], schema=schema)
-        self.assertEqual(best.validator, "oneOf")
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(best.instance, "not an int")
         best = self.best_match_of(instance=[0, "not an int"], schema=schema)
-        self.assertEqual(best.validator, "oneOf")
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(best.instance, "not an int")
+
+    def test_oneOf_traversal_for_partially_matching_subschemas(self):
+        """
+        When one subschema matches more of the instance than another,
+        we traverse into it, and report its remaining error.
+
+        See #1002.
+        """
+
+        schema = {
+            "oneOf": [
+                {
+                    "properties": {
+                        "version": {"const": 1},
+                        "description": {"type": "string"},
+                    },
+                },
+                {
+                    "properties": {
+                        "version": {"const": 2},
+                        "description": {"type": "string"},
+                    },
+                },
+            ],
+        }
+        instance = {"version": 1, "description": 0}
+        best = self.best_match_of(instance=instance, schema=schema)
+        self.assertEqual(best.validator, "type")
+        self.assertEqual(list(best.path), ["description"])
 
     def test_oneOf_traversal_for_single_equally_relevant_error(self):
         """