Add annotations for _Error

[DanielNoord]

To show what I mean with putting a class "in between" the main _Error class and the classes that actually have the attribute that is being used in __str__. This satisfies type checkers and has minimal runtime impact, while making the code a bit cleaner.

Let me know if this is something you would consider!

---

📚 Documentation preview 📚: https://python-jsonschema--1022.org.readthedocs.build/en/1022/

[DanielNoord]

Oops, I saw some black auto formatting slipped in. I can revert those changes as well. The general idea of the PR is still clear though I think.

[codecov-commenter]

Codecov Report

Base: 98.21% // Head: 98.19% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (19ed819) compared to base (2815178).
Patch coverage: 93.75% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1022      +/-   ##
==========================================
- Coverage   98.21%   98.19%   -0.03%     
==========================================
  Files          20       20              
  Lines        3529     3536       +7     
  Branches      537      537              
==========================================
+ Hits         3466     3472       +6     
- Misses         47       48       +1     
  Partials       16       16              

Impacted Files	Coverage Δ
jsonschema/exceptions.py	97.61% <93.75%> (-0.52%)	⬇️
jsonschema/_format.py	94.39% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Julian]

Thanks for the PR! This is more or less what I was imagining you were talking about, though I don't see from reading it why it couldn't be done to _Error. I haven't run it though, so will have to play around to convince myself. Will try to set aside some time in the next few days. Appreciated!

[Julian]

I took a quick look just now at this again -- though only a minute or two -- but in that minute, yeah, I can't see why the intermediate class is helping. The relevant part of this PR seems like just adding the two ClassVars, and mypy is happy even just adding them to _Error, i.e. simply:

diff --git a/jsonschema/exceptions.py b/jsonschema/exceptions.py
index 55c3ae737..2bc85df43 100644
--- a/jsonschema/exceptions.py
+++ b/jsonschema/exceptions.py
@@ -6,6 +6,7 @@ from __future__ import annotations
 from collections import defaultdict, deque
 from pprint import pformat
 from textwrap import dedent, indent
+from typing import ClassVar
 import heapq
 import itertools
 
@@ -20,9 +21,13 @@ _unset = _utils.Unset()
 
 
 class _Error(Exception):
+
+    _word_for_schema_in_error_message: ClassVar[str]
+    _word_for_instance_in_error_message: ClassVar[str]
+
     def __init__(
         self,
-        message,
+        message: str,
         validator=_unset,
         path=(),
         cause=None,

What am I missing on the point here?

[DanielNoord]

I did that in the other PR, but technically the code is still "incorrectly" typed. Using _word_for_schema_in_error_message in _Error.__str__ is unsafe as for many of its subclasses the ClassVar won't be set. By using an inbetween subclass this gets fixed. _Error.__str__ only uses attributes (none actually) that are always set for any of its subclasses and the actual Errors that use _word_for_schema_in_error_message have their own __str__.

[Julian]

I'm still missing what you mean.

_Error.str is unsafe as for many of its subclasses the ClassVar won't be set.

_Error has only 2 subclasses, both with the attribute -- which subclasses do you mean?

Also how can I reproduce whatever error you're trying to fix by adding this?

[DanielNoord]

My bad, I hadn't seen these were the only subclasses.

Then again, the issue kind of remains:

a = _Error()
a.validator = 1
a.validator_value = 1
a.instance = 1
a.schema = 1
str(a)

This will try to access the ClassVar attribute that isn't set on the base class. This won't happen in the real world due to the way the library works, but with the subclassing it wouldn't even be possible to misuse the code like this.

However, I think that our time is best spent on fixing actual bugs rather than on what-ifs. I'll amend the PR to use the ClassVar.

[Julian]

Thanks, merging.