fix(validate): address review feedback for --path option

Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>

Author: Vaibhav701161

docs/validate.markdown

@@ -11,6 +11,7 @@ jsonschema validate <schema.json|.yaml> <instance.json|.jsonl|.yaml|directory...
   [--benchmark/-b] [--loop <iterations>] [--extension/-e <extension>]
   [--ignore/-i <schemas-or-directories>] [--trace/-t] [--fast/-f]
   [--template/-m <template.json>] [--json/-j] [--entrypoint/-p <pointer|uri>]
+  [--path/-P <pointer>]
 ```
 
 The most popular use case of JSON Schema is to validate JSON documents. The
@@ -190,3 +191,24 @@ jsonschema validate path/to/my/schema.json path/to/instances/ \
 jsonschema validate path/to/my/schema.json path/to/my/instance.json \
   --entrypoint '/$defs/MyType'
 ```
+
+### Extract and validate against a sub-schema using a JSON Pointer
+
+The `--path`/`-P` option extracts a sub-schema from the input document using a
+[JSON Pointer](https://www.rfc-editor.org/rfc/rfc6901) before validation. This
+is useful for validating instances against schemas embedded in larger documents,
+such as OpenAPI specifications.
+
+```sh
+jsonschema validate path/to/openapi.json path/to/instance.json \
+  --path '/components/schemas/User'
+```
+
+The JSON Pointer must resolve to a value in the document that is a valid JSON
+Schema. If the pointer does not resolve, the CLI will report an error with the
+attempted pointer path.
+
+> [!WARNING]
+> Extracting a sub-schema with `--path` may break `$ref` references that point
+> outside the selected subtree, since only the targeted sub-schema is used for
+> validation. This option cannot be used together with `--template`/`-m`.

src/command_validate.cc

@@ -169,27 +169,30 @@ auto sourcemeta::jsonschema::validate(const sourcemeta::core::Options &options)
       read_configuration(options, configuration_path, schema_config_base)};
   const auto dialect{default_dialect(options, configuration)};
 
-  sourcemeta::core::JSON schema{
-      schema_from_stdin ? read_from_stdin().document
-                        : sourcemeta::core::read_yaml_or_json(schema_path)};
+  auto schema = schema_from_stdin
+                    ? read_from_stdin().document
+                    : sourcemeta::core::read_yaml_or_json(schema_path);
 
   if (options.contains("path") && !options.at("path").empty()) {
-    // Invalid pointer syntax is handled by to_pointer(), consistent with
-    // --entrypoint behavior.
-    const auto path_string{std::string{options.at("path").front()}};
-    const auto pointer{sourcemeta::core::to_pointer(path_string)};
-    const auto *const result{sourcemeta::core::try_get(schema, pointer)};
-    // We intentionally reuse NotSchemaError here to align with existing CLI
-    // error semantics without introducing a new error type.
+    sourcemeta::core::Pointer pointer;
+    try {
+      pointer = sourcemeta::core::to_pointer(
+          std::string{options.at("path").front()});
+    } catch (const sourcemeta::core::PointerParseError &) {
+      throw PositionalArgumentError{
+          "The JSON Pointer is not valid",
+          "jsonschema validate path/to/schema.json path/to/instance.json "
+          "--path '/components/schemas/User'"};
+    }
+
+    const auto *const result = sourcemeta::core::try_get(schema, pointer);
     if (!result) {
-      throw NotSchemaError{schema_resolution_base};
+      throw PathResolutionError{schema_resolution_base,
+                                sourcemeta::core::to_string(pointer)};
     }
-    // Note: extracting a sub-schema may break $ref references outside the
-    // selected subtree. This is expected behavior for --path given the current
-    // CLI design.
+
     // `result` points into `schema`, so we must copy before reassigning to
-    // avoid a use-after-free (the copy assignment destroys schema's storage
-    // before reading from other when they alias).
+    // avoid a use-after-free.
     sourcemeta::core::JSON subschema{*result};
     schema = std::move(subschema);
   }

src/error.h

@@ -76,6 +76,26 @@ class NotSchemaError : public std::runtime_error {
   std::filesystem::path path_;
 };
 
+class PathResolutionError : public std::runtime_error {
+public:
+  PathResolutionError(std::filesystem::path path, std::string pointer)
+      : std::runtime_error{"The JSON Pointer does not resolve to a value in "
+                           "the document"},
+        path_{std::move(path)}, pointer_{std::move(pointer)} {}
+
+  [[nodiscard]] auto path() const noexcept -> const std::filesystem::path & {
+    return this->path_;
+  }
+
+  [[nodiscard]] auto pointer() const noexcept -> const std::string & {
+    return this->pointer_;
+  }
+
+private:
+  std::filesystem::path path_;
+  std::string pointer_;
+};
+
 class YAMLInputError : public std::runtime_error {
 public:
   YAMLInputError(std::string message, std::filesystem::path path)
@@ -390,6 +410,16 @@ inline auto print_exception(const bool is_json, const Exception &exception)
     }
   }
 
+  if constexpr (requires(const Exception &current) {
+                  { current.pointer() } -> std::convertible_to<std::string>;
+                }) {
+    if (is_json) {
+      error_json.assign("pointer", sourcemeta::core::JSON{exception.pointer()});
+    } else {
+      std::cerr << "  at path " << exception.pointer() << "\n";
+    }
+  }
+
   if constexpr (requires(const Exception &current) { current.location(); }) {
     if (is_json) {
       error_json.assign("location",
@@ -504,6 +534,10 @@ inline auto try_catch(const sourcemeta::core::Options &options,
     const auto is_json{options.contains("json")};
     print_exception(is_json, error);
     return EXIT_OTHER_INPUT_ERROR;
+  } catch (const PathResolutionError &error) {
+    const auto is_json{options.contains("json")};
+    print_exception(is_json, error);
+    return EXIT_OTHER_INPUT_ERROR;
   } catch (const NotSchemaError &error) {
     const auto is_json{options.contains("json")};
     print_exception(is_json, error);

src/main.cc

@@ -38,7 +38,7 @@ Global Options:
             [--benchmark/-b] [--loop <iterations>] [--extension/-e <extension>]
             [--ignore/-i <schemas-or-directories>] [--trace/-t] [--fast/-f]
             [--template/-m <template.json>] [--entrypoint/-p <pointer|uri>]
-            [--path <pointer>]
+            [--path/-P <pointer>]
 
        Validate one or more instances against the given schema.
 
@@ -53,9 +53,8 @@ Global Options:
        for error reporting purposes. Make sure they match or you will get
        non-sense results.
 
-       Use --path to extract a sub-schema from the input document using
-       a JSON Pointer before validation. This is useful for validating
-       against schemas embedded in larger documents, such as OpenAPI.
+       Use --path/-P to extract a sub-schema using a JSON Pointer
+       before validation.
 
    metaschema [schemas-or-directories...] [--extension/-e <extension>]
               [--ignore/-i <schemas-or-directories>] [--trace/-t]
@@ -187,7 +186,7 @@ auto jsonschema_main(const std::string &program, const std::string &command,
     app.option("template", {"m"});
     app.option("loop", {"l"});
     app.option("entrypoint", {"p"});
-    app.option("path", {});
+    app.option("path", {"P"});
     app.parse(argc, argv, {.skip = 1});
     sourcemeta::jsonschema::validate(app);
     return EXIT_SUCCESS;

test/CMakeLists.txt

@@ -245,6 +245,7 @@ add_jsonschema_test_unix(validate/fail_path_not_found)
 add_jsonschema_test_unix(validate/fail_path_not_schema)
 add_jsonschema_test_unix(validate/fail_path_with_template)
 add_jsonschema_test_unix(validate/fail_path_ref_outside_subtree)
+add_jsonschema_test_unix(validate/fail_path_invalid_pointer)
 add_jsonschema_test_unix(validate/pass_config_ignore)
 add_jsonschema_test_unix(validate/pass_config_ignore_with_cli)
 add_jsonschema_test_unix(validate/pass_stdin_instance)

test/validate/fail_path_invalid_pointer.sh

@@ -0,0 +1,47 @@
+#!/bin/sh
+
+set -o errexit
+set -o nounset
+
+TMP="$(mktemp -d)"
+clean() { rm -rf "$TMP"; }
+trap clean EXIT
+
+cat << 'EOF' > "$TMP/schema.json"
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "type": "object"
+}
+EOF
+
+cat << 'EOF' > "$TMP/instance.json"
+{}
+EOF
+
+"$1" validate "$TMP/schema.json" "$TMP/instance.json" \
+  --path 'invalid~pointer' > "$TMP/output.txt" 2>&1 \
+  && EXIT_CODE="$?" || EXIT_CODE="$?"
+# Invalid CLI arguments
+test "$EXIT_CODE" = "5"
+
+cat << EOF > "$TMP/expected.txt"
+error: The JSON Pointer is not valid
+
+For example: jsonschema validate path/to/schema.json path/to/instance.json --path '/components/schemas/User'
+EOF
+
+diff "$TMP/output.txt" "$TMP/expected.txt"
+
+# JSON error
+"$1" validate "$TMP/schema.json" "$TMP/instance.json" \
+  --path 'invalid~pointer' --json > "$TMP/stdout.txt" 2>&1 \
+  && EXIT_CODE="$?" || EXIT_CODE="$?"
+test "$EXIT_CODE" = "5"
+
+cat << EOF > "$TMP/expected.txt"
+{
+  "error": "The JSON Pointer is not valid"
+}
+EOF
+
+diff "$TMP/stdout.txt" "$TMP/expected.txt"

test/validate/fail_path_not_found.sh

@@ -27,12 +27,29 @@ EOF
 "$1" validate "$TMP/document.json" "$TMP/instance.json" \
   --path "/components/schemas/NonExistent" 2> "$TMP/stderr.txt" \
   && EXIT_CODE="$?" || EXIT_CODE="$?"
-# Schema input error
-test "$EXIT_CODE" = "4"
+# Other input error (path not found)
+test "$EXIT_CODE" = "6"
 
 cat << EOF > "$TMP/expected.txt"
-error: The schema file you provided does not represent a valid JSON Schema
+error: The JSON Pointer does not resolve to a value in the document
   at file path $(realpath "$TMP")/document.json
+  at path /components/schemas/NonExistent
 EOF
 
 diff "$TMP/stderr.txt" "$TMP/expected.txt"
+
+# JSON error
+"$1" validate "$TMP/document.json" "$TMP/instance.json" \
+  --path "/components/schemas/NonExistent" --json > "$TMP/stdout.txt" \
+  && EXIT_CODE="$?" || EXIT_CODE="$?"
+test "$EXIT_CODE" = "6"
+
+cat << EOF > "$TMP/expected.txt"
+{
+  "error": "The JSON Pointer does not resolve to a value in the document",
+  "filePath": "$(realpath "$TMP")/document.json",
+  "pointer": "/components/schemas/NonExistent"
+}
+EOF
+
+diff "$TMP/stdout.txt" "$TMP/expected.txt"

test/validate/fail_path_not_schema.sh

@@ -33,3 +33,18 @@ error: The schema file you provided does not represent a valid JSON Schema
 EOF
 
 diff "$TMP/stderr.txt" "$TMP/expected.txt"
+
+# JSON error
+"$1" validate "$TMP/document.json" "$TMP/instance.json" \
+  --path "/components/schemas/User" --json > "$TMP/stdout.txt" \
+  && EXIT_CODE="$?" || EXIT_CODE="$?"
+test "$EXIT_CODE" = "4"
+
+cat << EOF > "$TMP/expected.txt"
+{
+  "error": "The schema file you provided does not represent a valid JSON Schema",
+  "filePath": "$(realpath "$TMP")/document.json"
+}
+EOF
+
+diff "$TMP/stdout.txt" "$TMP/expected.txt"

test/validate/fail_path_ref_outside_subtree.sh

@@ -59,5 +59,4 @@ cat << EOF > "$TMP/expected.txt"
 }
 EOF
 
-diff "$TMP/stdout.txt" "$TMP/expected.txt"
-
+diff "$TMP/stdout.txt" "$TMP/expected.txt"

test/validate/pass_path_openapi.sh

@@ -31,3 +31,13 @@ EOF
 
 "$1" validate "$TMP/openapi.json" "$TMP/instance.json" \
   --path "/components/schemas/User"
+
+# Verbose run
+"$1" validate "$TMP/openapi.json" "$TMP/instance.json" \
+  --path "/components/schemas/User" --verbose 2> "$TMP/stderr.txt"
+cat "$TMP/stderr.txt" | grep -q "ok"
+
+# JSON run
+"$1" validate "$TMP/openapi.json" "$TMP/instance.json" \
+  --path "/components/schemas/User" --json > "$TMP/stdout.txt"
+cat "$TMP/stdout.txt" | grep -q '"valid"'