Add syntactic highlights to the error explanations (#11661)

BenjaminSchubert · web-flow · commit 88ae27da085d · 2023-12-21T17:11:56.000Z
* Put a 'reset' color in front of the highlighting

When doing the highlighting, some lexers will not set the initial color
explicitly, which may lead to the red from the errors being propagated
to the start of the expression

* Add syntactic highlighting to the error explanations

This updates the various error reporting to highlight python code when
displayed, to increase readability and make it easier to understand
diff --git a/changelog/11520.improvement.rst b/changelog/11520.improvement.rst
@@ -1,3 +1,5 @@
 Improved very verbose diff output to color it as a diff instead of only red.
 
 Improved the error reporting to better separate each section.
+
+Improved the error reporting to syntax-highlight Python code when Pygments is available.
diff --git a/src/_pytest/_io/terminalwriter.py b/src/_pytest/_io/terminalwriter.py
@@ -223,7 +223,15 @@ def _highlight(
                         style=os.getenv("PYTEST_THEME"),
                     ),
                 )
-                return highlighted
+                # pygments terminal formatter may add a newline when there wasn't one.
+                # We don't want this, remove.
+                if highlighted[-1] == "\n" and source[-1] != "\n":
+                    highlighted = highlighted[:-1]
+
+                # Some lexers will not set the initial color explicitly
+                # which may lead to the previous color being propagated to the
+                # start of the expression, so reset first.
+                return "\x1b[0m" + highlighted
             except pygments.util.ClassNotFound:
                 raise UsageError(
                     "PYTEST_THEME environment variable had an invalid value: '{}'. "
diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py
@@ -192,12 +192,12 @@ def assertrepr_compare(
         right_repr = saferepr(right, maxsize=maxsize, use_ascii=use_ascii)
 
     summary = f"{left_repr} {op} {right_repr}"
+    highlighter = config.get_terminal_writer()._highlight
 
     explanation = None
     try:
         if op == "==":
-            writer = config.get_terminal_writer()
-            explanation = _compare_eq_any(left, right, writer._highlight, verbose)
+            explanation = _compare_eq_any(left, right, highlighter, verbose)
         elif op == "not in":
             if istext(left) and istext(right):
                 explanation = _notin_text(left, right, verbose)
@@ -206,16 +206,16 @@ def assertrepr_compare(
                 explanation = ["Both sets are equal"]
         elif op == ">=":
             if isset(left) and isset(right):
-                explanation = _compare_gte_set(left, right, verbose)
+                explanation = _compare_gte_set(left, right, highlighter, verbose)
         elif op == "<=":
             if isset(left) and isset(right):
-                explanation = _compare_lte_set(left, right, verbose)
+                explanation = _compare_lte_set(left, right, highlighter, verbose)
         elif op == ">":
             if isset(left) and isset(right):
-                explanation = _compare_gt_set(left, right, verbose)
+                explanation = _compare_gt_set(left, right, highlighter, verbose)
         elif op == "<":
             if isset(left) and isset(right):
-                explanation = _compare_lt_set(left, right, verbose)
+                explanation = _compare_lt_set(left, right, highlighter, verbose)
 
     except outcomes.Exit:
         raise
@@ -259,11 +259,11 @@ def _compare_eq_any(
             # used in older code bases before dataclasses/attrs were available.
             explanation = _compare_eq_cls(left, right, highlighter, verbose)
         elif issequence(left) and issequence(right):
-            explanation = _compare_eq_sequence(left, right, verbose)
+            explanation = _compare_eq_sequence(left, right, highlighter, verbose)
         elif isset(left) and isset(right):
-            explanation = _compare_eq_set(left, right, verbose)
+            explanation = _compare_eq_set(left, right, highlighter, verbose)
         elif isdict(left) and isdict(right):
-            explanation = _compare_eq_dict(left, right, verbose)
+            explanation = _compare_eq_dict(left, right, highlighter, verbose)
 
         if isiterable(left) and isiterable(right):
             expl = _compare_eq_iterable(left, right, highlighter, verbose)
@@ -350,7 +350,10 @@ def _compare_eq_iterable(
 
 
 def _compare_eq_sequence(
-    left: Sequence[Any], right: Sequence[Any], verbose: int = 0
+    left: Sequence[Any],
+    right: Sequence[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
     comparing_bytes = isinstance(left, bytes) and isinstance(right, bytes)
     explanation: List[str] = []
@@ -373,7 +376,10 @@ def _compare_eq_sequence(
                 left_value = left[i]
                 right_value = right[i]
 
-            explanation += [f"At index {i} diff: {left_value!r} != {right_value!r}"]
+            explanation.append(
+                f"At index {i} diff:"
+                f" {highlighter(repr(left_value))} != {highlighter(repr(right_value))}"
+            )
             break
 
     if comparing_bytes:
@@ -393,68 +399,91 @@ def _compare_eq_sequence(
             extra = saferepr(right[len_left])
 
         if len_diff == 1:
-            explanation += [f"{dir_with_more} contains one more item: {extra}"]
+            explanation += [
+                f"{dir_with_more} contains one more item: {highlighter(extra)}"
+            ]
         else:
             explanation += [
                 "%s contains %d more items, first extra item: %s"
-                % (dir_with_more, len_diff, extra)
+                % (dir_with_more, len_diff, highlighter(extra))
             ]
     return explanation
 
 
 def _compare_eq_set(
-    left: AbstractSet[Any], right: AbstractSet[Any], verbose: int = 0
+    left: AbstractSet[Any],
+    right: AbstractSet[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
     explanation = []
-    explanation.extend(_set_one_sided_diff("left", left, right))
-    explanation.extend(_set_one_sided_diff("right", right, left))
+    explanation.extend(_set_one_sided_diff("left", left, right, highlighter))
+    explanation.extend(_set_one_sided_diff("right", right, left, highlighter))
     return explanation
 
 
 def _compare_gt_set(
-    left: AbstractSet[Any], right: AbstractSet[Any], verbose: int = 0
+    left: AbstractSet[Any],
+    right: AbstractSet[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
-    explanation = _compare_gte_set(left, right, verbose)
+    explanation = _compare_gte_set(left, right, highlighter)
     if not explanation:
         return ["Both sets are equal"]
     return explanation
 
 
 def _compare_lt_set(
-    left: AbstractSet[Any], right: AbstractSet[Any], verbose: int = 0
+    left: AbstractSet[Any],
+    right: AbstractSet[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
-    explanation = _compare_lte_set(left, right, verbose)
+    explanation = _compare_lte_set(left, right, highlighter)
     if not explanation:
         return ["Both sets are equal"]
     return explanation
 
 
 def _compare_gte_set(
-    left: AbstractSet[Any], right: AbstractSet[Any], verbose: int = 0
+    left: AbstractSet[Any],
+    right: AbstractSet[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
-    return _set_one_sided_diff("right", right, left)
+    return _set_one_sided_diff("right", right, left, highlighter)
 
 
 def _compare_lte_set(
-    left: AbstractSet[Any], right: AbstractSet[Any], verbose: int = 0
+    left: AbstractSet[Any],
+    right: AbstractSet[Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
-    return _set_one_sided_diff("left", left, right)
+    return _set_one_sided_diff("left", left, right, highlighter)
 
 
 def _set_one_sided_diff(
-    posn: str, set1: AbstractSet[Any], set2: AbstractSet[Any]
+    posn: str,
+    set1: AbstractSet[Any],
+    set2: AbstractSet[Any],
+    highlighter: _HighlightFunc,
 ) -> List[str]:
     explanation = []
     diff = set1 - set2
     if diff:
         explanation.append(f"Extra items in the {posn} set:")
         for item in diff:
-            explanation.append(saferepr(item))
+            explanation.append(highlighter(saferepr(item)))
     return explanation
 
 
 def _compare_eq_dict(
-    left: Mapping[Any, Any], right: Mapping[Any, Any], verbose: int = 0
+    left: Mapping[Any, Any],
+    right: Mapping[Any, Any],
+    highlighter: _HighlightFunc,
+    verbose: int = 0,
 ) -> List[str]:
     explanation: List[str] = []
     set_left = set(left)
@@ -465,12 +494,16 @@ def _compare_eq_dict(
         explanation += ["Omitting %s identical items, use -vv to show" % len(same)]
     elif same:
         explanation += ["Common items:"]
-        explanation += pprint.pformat(same).splitlines()
+        explanation += highlighter(pprint.pformat(same)).splitlines()
     diff = {k for k in common if left[k] != right[k]}
     if diff:
         explanation += ["Differing items:"]
         for k in diff:
-            explanation += [saferepr({k: left[k]}) + " != " + saferepr({k: right[k]})]
+            explanation += [
+                highlighter(saferepr({k: left[k]}))
+                + " != "
+                + highlighter(saferepr({k: right[k]}))
+            ]
     extra_left = set_left - set_right
     len_extra_left = len(extra_left)
     if len_extra_left:
@@ -479,7 +512,7 @@ def _compare_eq_dict(
             % (len_extra_left, "" if len_extra_left == 1 else "s")
         )
         explanation.extend(
-            pprint.pformat({k: left[k] for k in extra_left}).splitlines()
+            highlighter(pprint.pformat({k: left[k] for k in extra_left})).splitlines()
         )
     extra_right = set_right - set_left
     len_extra_right = len(extra_right)
@@ -489,7 +522,7 @@ def _compare_eq_dict(
             % (len_extra_right, "" if len_extra_right == 1 else "s")
         )
         explanation.extend(
-            pprint.pformat({k: right[k] for k in extra_right}).splitlines()
+            highlighter(pprint.pformat({k: right[k] for k in extra_right})).splitlines()
         )
     return explanation
 
@@ -528,17 +561,17 @@ def _compare_eq_cls(
         explanation.append("Omitting %s identical items, use -vv to show" % len(same))
     elif same:
         explanation += ["Matching attributes:"]
-        explanation += pprint.pformat(same).splitlines()
+        explanation += highlighter(pprint.pformat(same)).splitlines()
     if diff:
         explanation += ["Differing attributes:"]
-        explanation += pprint.pformat(diff).splitlines()
+        explanation += highlighter(pprint.pformat(diff)).splitlines()
         for field in diff:
             field_left = getattr(left, field)
             field_right = getattr(right, field)
             explanation += [
                 "",
-                "Drill down into differing attribute %s:" % field,
-                ("%s%s: %r != %r") % (indent, field, field_left, field_right),
+                f"Drill down into differing attribute {field}:",
+                f"{indent}{field}: {highlighter(repr(field_left))} != {highlighter(repr(field_right))}",
             ]
             explanation += [
                 indent + line
diff --git a/testing/io/test_terminalwriter.py b/testing/io/test_terminalwriter.py
@@ -254,7 +254,7 @@ def test_combining(self) -> None:
         pytest.param(
             True,
             True,
-            "{kw}assert{hl-reset} {number}0{hl-reset}{endline}\n",
+            "{reset}{kw}assert{hl-reset} {number}0{hl-reset}{endline}\n",
             id="with markup and code_highlight",
         ),
         pytest.param(
diff --git a/testing/test_assertion.py b/testing/test_assertion.py
@@ -20,7 +20,7 @@
 
 def mock_config(verbose: int = 0, assertion_override: Optional[int] = None):
     class TerminalWriter:
-        def _highlight(self, source, lexer):
+        def _highlight(self, source, lexer="python"):
             return source
 
     class Config:
@@ -1933,6 +1933,7 @@ def test():
                 assert [0, 1] == [0, 2]
             """,
             [
+                "{bold}{red}E         At index 1 diff: {reset}{number}1{hl-reset}{endline} != {reset}{number}2*",
                 "{bold}{red}E         {light-red}-     2,{hl-reset}{endline}{reset}",
                 "{bold}{red}E         {light-green}+     1,{hl-reset}{endline}{reset}",
             ],
@@ -1945,7 +1946,13 @@ def test():
                 }
             """,
             [
-                "{bold}{red}E         {light-gray} {hl-reset} {{{endline}{reset}",
+                "{bold}{red}E         Common items:{reset}",
+                "{bold}{red}E         {reset}{{{str}'{hl-reset}{str}number-is-1{hl-reset}{str}'{hl-reset}: {number}1*",
+                "{bold}{red}E         Left contains 1 more item:{reset}",
+                "{bold}{red}E         {reset}{{{str}'{hl-reset}{str}number-is-5{hl-reset}{str}'{hl-reset}: {number}5*",
+                "{bold}{red}E         Right contains 1 more item:{reset}",
+                "{bold}{red}E         {reset}{{{str}'{hl-reset}{str}number-is-0{hl-reset}{str}'{hl-reset}: {number}0*",
+                "{bold}{red}E         {reset}{light-gray} {hl-reset} {{{endline}{reset}",
                 "{bold}{red}E         {light-gray} {hl-reset}     'number-is-1': 1,{endline}{reset}",
                 "{bold}{red}E         {light-green}+     'number-is-5': 5,{hl-reset}{endline}{reset}",
             ],
diff --git a/testing/test_terminal.py b/testing/test_terminal.py
@@ -1268,13 +1268,13 @@ def test_this():
                 "=*= FAILURES =*=",
                 "{red}{bold}_*_ test_this _*_{reset}",
                 "",
-                "    {kw}def{hl-reset} {function}test_this{hl-reset}():{endline}",
+                "    {reset}{kw}def{hl-reset} {function}test_this{hl-reset}():{endline}",
                 ">       fail(){endline}",
                 "",
                 "{bold}{red}test_color_yes.py{reset}:5: ",
                 "_ _ * _ _*",
                 "",
-                "    {kw}def{hl-reset} {function}fail{hl-reset}():{endline}",
+                "    {reset}{kw}def{hl-reset} {function}fail{hl-reset}():{endline}",
                 ">       {kw}assert{hl-reset} {number}0{hl-reset}{endline}",
                 "{bold}{red}E       assert 0{reset}",
                 "",
@@ -1295,9 +1295,9 @@ def test_this():
                 "=*= FAILURES =*=",
                 "{red}{bold}_*_ test_this _*_{reset}",
                 "{bold}{red}test_color_yes.py{reset}:5: in test_this",
-                "    fail(){endline}",
+                "    {reset}fail(){endline}",
                 "{bold}{red}test_color_yes.py{reset}:2: in fail",
-                "    {kw}assert{hl-reset} {number}0{hl-reset}{endline}",
+                "    {reset}{kw}assert{hl-reset} {number}0{hl-reset}{endline}",
                 "{bold}{red}E   assert 0{reset}",
                 "{red}=*= {red}{bold}1 failed{reset}{red} in *s{reset}{red} =*={reset}",
             ]
@@ -2507,7 +2507,7 @@ def test_foo():
         result.stdout.fnmatch_lines(
             color_mapping.format_for_fnmatch(
                 [
-                    "    {kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
+                    "    {reset}{kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
                     ">       {kw}assert{hl-reset} {number}1{hl-reset} == {number}10{hl-reset}{endline}",
                     "{bold}{red}E       assert 1 == 10{reset}",
                 ]
@@ -2529,7 +2529,7 @@ def test_foo():
         result.stdout.fnmatch_lines(
             color_mapping.format_for_fnmatch(
                 [
-                    "    {kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
+                    "    {reset}{kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
                     "        {print}print{hl-reset}({str}'''{hl-reset}{str}{hl-reset}",
                     ">   {str}    {hl-reset}{str}'''{hl-reset}); {kw}assert{hl-reset} {number}0{hl-reset}{endline}",
                     "{bold}{red}E       assert 0{reset}",
@@ -2552,7 +2552,7 @@ def test_foo():
         result.stdout.fnmatch_lines(
             color_mapping.format_for_fnmatch(
                 [
-                    "    {kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
+                    "    {reset}{kw}def{hl-reset} {function}test_foo{hl-reset}():{endline}",
                     ">       {kw}assert{hl-reset} {number}1{hl-reset} == {number}10{hl-reset}{endline}",
                     "{bold}{red}E       assert 1 == 10{reset}",
                 ]
