Skip to content

Commit 70e9202

Browse files
cursoragentsimbo1905
andcommitted
Issue #128 Address all PR review comments
Fixes: 1. Remove unused evaluateRecursiveDescentFull() method from JsonPathHelpers 2. Remove unused collectAtPath() method from JsonPathHelpers 3. Fix README example: use 'compiled'/'interpreted' instead of undefined 'path' 4. Clarify in README that only limited script expressions are supported 5. Throw UnsupportedOperationException for unsupported script expressions instead of silently ignoring them All 140 tests pass. Co-authored-by: simbo1905 <simbo1905@60hertz.com>
1 parent 026b157 commit 70e9202

File tree

3 files changed

+5
-50
lines changed

3 files changed

+5
-50
lines changed

json-java21-jsonpath/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ JsonPath compiled = JsonPath.compile(interpreted);
9999
JsonPath sameCompiled = JsonPath.compile(compiled); // Returns same instance
100100

101101
// Check if a path is compiled
102-
boolean isCompiled = path.isCompiled();
103-
boolean isInterpreted = path.isInterpreted();
102+
boolean isCompiled = compiled.isCompiled();
103+
boolean isInterpreted = interpreted.isInterpreted();
104104
```
105105

106106
### Supported Features in Compiled Mode
@@ -109,7 +109,7 @@ All JsonPath features are supported in compiled mode:
109109
- Property access, array indices, slices, wildcards
110110
- Recursive descent (`$..property`)
111111
- Filters with comparisons and logical operators
112-
- Unions and script expressions
112+
- Unions and limited script expressions (e.g., `[(@.length-1)]`)
113113

114114
## Stream-Based Functions (Aggregations)
115115

json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathCompiler.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,10 @@ private static void generateScriptExpression(
469469
sb.append(" }\n");
470470
sb.append(" }\n");
471471
} else {
472-
// Unsupported script - log warning and skip
472+
// Unsupported script - throw exception at runtime for clarity
473473
// Only @.length-1 is supported in compiled mode
474-
sb.append(" // WARNING: Unsupported script expression '%s' - skipped in compiled mode\n"
474+
sb.append(" throw new UnsupportedOperationException(\"Unsupported script expression in compiled mode: '%s'. Consider using slice notation instead (e.g., [-1:] for last element).\");\n"
475475
.formatted(escapeJavaString(script.script())));
476-
sb.append(" // Consider using slice notation instead: [-1:] for last element\n");
477476
}
478477
}
479478

json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathHelpers.java

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -151,30 +151,6 @@ public static void evaluateRecursiveDescent(String propertyName, JsonValue curre
151151
}
152152
}
153153

154-
/// Evaluates recursive descent and then applies subsequent segments.
155-
/// This is a more general version that delegates back to the interpreter for complex cases.
156-
/// @param path the original JsonPath being evaluated
157-
/// @param segmentIndex the index of the recursive descent segment
158-
/// @param current the current value
159-
/// @param root the root document
160-
/// @param results the results list
161-
public static void evaluateRecursiveDescentFull(
162-
JsonPath path,
163-
int segmentIndex,
164-
JsonValue current,
165-
JsonValue root,
166-
List<JsonValue> results) {
167-
168-
// For full recursive descent support, we delegate to the interpreter
169-
// This handles the case where there are segments after the recursive descent
170-
if (path instanceof JsonPathInterpreted interpreted) {
171-
final var ast = interpreted.ast();
172-
JsonPathInterpreted.evaluateSegments(ast.segments(), segmentIndex, current, root, results);
173-
} else if (path.ast() != null) {
174-
JsonPathInterpreted.evaluateSegments(path.ast().segments(), segmentIndex, current, root, results);
175-
}
176-
}
177-
178154
/// Collects all arrays recursively from a JSON value.
179155
/// Used for recursive descent with array index targets like $..book[2].
180156
/// @param current the current JSON value to search
@@ -196,24 +172,4 @@ public static void collectArrays(JsonValue current, List<JsonValue> arrays) {
196172
}
197173
}
198174

199-
/// Collects values at a specific property path recursively.
200-
/// Used for recursive descent patterns like $..book.
201-
/// @param propertyName the property name to search for
202-
/// @param current the current JSON value to search
203-
/// @param results the list to collect results into
204-
public static void collectAtPath(String propertyName, JsonValue current, List<JsonValue> results) {
205-
if (current instanceof JsonObject obj) {
206-
final var value = obj.members().get(propertyName);
207-
if (value != null) {
208-
results.add(value);
209-
}
210-
for (final var child : obj.members().values()) {
211-
collectAtPath(propertyName, child, results);
212-
}
213-
} else if (current instanceof JsonArray array) {
214-
for (final var element : array.elements()) {
215-
collectAtPath(propertyName, element, results);
216-
}
217-
}
218-
}
219175
}

0 commit comments

Comments
 (0)