Unambiguously quote and escape properties in JSON path rendering

[kurtmckee]

This PR introduces the following change:

• Unambiguously quote and escape properties in JSON path rendering

Using the script from #1389, this is the new output with the changes in this branch:

$ python demo.py 
property=''             json_path="$['']"
property='['            json_path="$['[']"
property='.'            json_path="$['.']"
property='\\'           json_path="$['\\\\']"
property="'"            json_path="$['\\'']"
property=' '            json_path="$[' ']"
property='"'            json_path='$[\'"\']'
property='a[0]'         json_path="$['a[0]']"

Fixes #1389

---

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

[Julian]

Thanks for the PR (and issue).

I think we should consider and look at jsonpath-ng at very least to start with for testing. It's possible in the future we should deprecate the attribute and replace it with one that returns an object from that library, but even before we do that we can at least use it in our test cases to:

• ensure all the tests you're adding actually are guarded to be valid JSON Paths
• ensure we have covered at least the cases that that library itself tests

Could you have a look at doing that? As I say I'm good with just adding it as a test dependency for the moment and evaluating the deprecation/changing the actual interface down the line.

[Julian]

Please make sure this is private even if we don't use jsonpath-ng in the real code yet. Though if it turns out that a regex isn't good enough for fully accurate generation of paths then I think we should think carefully about adding the dep even now perhaps.

[Julian]

Let's add a self.error_with_path(...) -> ValidationError test helper here so the duplication is removed when all we really care about is the one attribute essentially.

And as I said in the top-level comment I think adding an assertion that jsonpath-ng considers our output valid is a good immediate safeguard.

[kurtmckee]

I'm also dissatisfied with the repetitive code, so I'm glad you called this out.

I'm accustomed to using pytests test parametrization feature, and found something similar in unittest just now: subTest.

I can introduce an error_with_path() method, but would you be open to something like:

class TestJsonPathRendering(TestCase):
    def test_strange_properties(self):
        parameters = (
            ("", "$[' ']"),
            (".", "$['.']"),
            # ...
        )
        for property, expected_path in parameters:
            with self.subTest(property=property):
                e = exceptions.ValidationError(
                    path=[property],
                    message="1",
                    validator="foo",
                    instance="i1",
                )
                self.assertEqual(e.json_path, expected_path)

(Wrote that in this comment, so I don't know if it'll actually run as-is.)

Let me know if that's appealing, otherwise I'll go with the test helper you proposed.

Either way, I'll add an assertion that jsonpath-ng can parse the resulting JSON path. 👍

[Julian]

Yeah using subTest is also fine with me.

[kurtmckee]

Super, I'll put together those changes.