Surface structured parse diagnostics from PolicySet.parsePolicies

CedarJava/CHANGELOG.md

@@ -8,6 +8,7 @@
 * Added Offset function support [#331](https://github.com/cedar-policy/cedar-java/pull/331)
 * Added PolicySet to JSON conversion API [#329](https://github.com/cedar-policy/cedar-java/pull/329)
 * Added Cedar Schema support for Entity Validation [#332](https://github.com/cedar-policy/cedar-java/pull/332)
+* Added `PolicyParseException`, thrown by `PolicySet.parsePolicies` when policy text fails to parse. It is a subclass of `InternalException`, so existing `catch` blocks and `getMessage()` are unaffected, and adds `getDetailedErrors()` returning Cedar's structured diagnostics - source span, expected tokens, and help text - for each error. `getErrors()` now carries one entry per parse error rather than a single entry for the whole document [#367](https://github.com/cedar-policy/cedar-java/pull/367)
 
 ## 4.3.1
 ### Added

CedarJava/src/main/java/com/cedarpolicy/model/DetailedError.java

@@ -38,7 +38,7 @@ public class DetailedError {
     /** Severity */
     @JsonProperty("severity")
     public final Optional<Severity> severity;
-    /** Source labels (ranges) */
+    /** Source labels (ranges); see {@link SourceLabel} for how to index them */
     @JsonProperty("sourceLocations")
     public final ImmutableList<SourceLabel> sourceLocations;
     /** Related errors */
@@ -84,14 +84,44 @@ public enum Severity {
         Error,
     }
 
+    /**
+     * A region of the source text an error refers to, so callers can underline it.
+     *
+     * <p><b>The offsets are UTF-8 byte offsets, not {@code String} indices.</b> Cedar produces
+     * them by counting bytes, while {@link String#substring(int, int)} counts UTF-16 chars. The
+     * two coincide only while the source is pure ASCII; a single non-ASCII character anywhere
+     * earlier in the document - an accented identifier, a non-Latin string literal, an emoji in
+     * a comment - shifts them apart, and slicing the {@code String} directly then either
+     * extracts the wrong region or throws {@link StringIndexOutOfBoundsException}. Slice the
+     * source's UTF-8 bytes instead:
+     *
+     * <pre>{@code
+     * byte[] bytes = source.getBytes(StandardCharsets.UTF_8);
+     * String offending = new String(bytes, label.start, label.end - label.start, StandardCharsets.UTF_8);
+     * }</pre>
+     *
+     * <p>Offsets are absolute within the whole text that was parsed, not relative to the policy
+     * containing the error, so they remain directly usable when several policies are parsed
+     * together. They carry no policy identity of their own: mapping an offset back to a
+     * particular policy statement is left to the caller.
+     *
+     * <p>A region may be empty ({@code start == end}), which happens when there is no extent to
+     * highlight - an unterminated string literal, for instance, reports the position the lexer
+     * stopped at. Renderers should treat zero width as a single caret rather than assuming at
+     * least one character to underline.
+     *
+     * <p>For a parse error the region covers the unexpected token, which is not always where a
+     * reader would place the mistake: a missing operand is reported at the token that followed
+     * it, possibly on a later line.
+     */
     public static final class SourceLabel {
         /** Text of the label (if any) */
         @JsonProperty("label")
         public final Optional<String> label;
-        /** Start of the source location (in bytes) */
+        /** Start of the source location, as a UTF-8 byte offset, inclusive. See {@link SourceLabel}. */
         @JsonProperty("start")
         public final int start;
-        /** End of the source location (in bytes) */
+        /** End of the source location, as a UTF-8 byte offset, exclusive. See {@link SourceLabel}. */
         @JsonProperty("end")
         public final int end;
 

CedarJava/src/main/java/com/cedarpolicy/model/exception/InternalException.java

@@ -41,6 +41,22 @@ public InternalException(String[] errors) {
         this.errors = new ArrayList<>(Arrays.asList(errors));
     }
 
+    /**
+     * Internal exception whose message is not derived from its error list.
+     *
+     * <p>The other constructors build the message by joining {@code errors}, which ties the
+     * two together: splitting the list more finely necessarily changes the message. Subclasses
+     * that report each underlying error separately, but summarise them differently in the
+     * message, use this constructor to set the two independently.
+     *
+     * @param error the message, prefixed as in {@link #InternalException(String)}
+     * @param errors the individual error messages, for {@link #getErrors()}
+     */
+    protected InternalException(String error, List<String> errors) {
+        super("Internal error: " + error);
+        this.errors = new ArrayList<>(errors);
+    }
+
     /**
      * Get errors.
      *

CedarJava/src/main/java/com/cedarpolicy/model/exception/PolicyParseException.java

@@ -0,0 +1,92 @@
+/*
+ * Copyright Cedar Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.cedarpolicy.model.exception;
+
+import com.cedarpolicy.CedarJson;
+import com.cedarpolicy.model.DetailedError;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.type.TypeReference;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Thrown when Cedar policy text fails to parse, carrying the structured diagnostics Cedar
+ * produced for each error.
+ *
+ * <p>Cedar reports a parse failure as one or more {@code miette} diagnostics: a message, the
+ * source span of the offending token, the tokens the parser expected there, and often help
+ * text. A single document may fail in several places, and every failure is reported.
+ *
+ * <p>Three accessors describe the same failure at increasing fidelity:
+ *
+ * <ul>
+ *   <li>{@link #getMessage()} - one human-readable line, describing the first error only. Its
+ *       wording is treated as part of this class's compatibility surface, so it is the least
+ *       informative of the three and the safest to match on.
+ *   <li>{@link #getErrors()} - one message per parse error, in the order Cedar reported them.
+ *       These are Cedar's messages alone, without the prefix {@link #getMessage()} carries.
+ *   <li>{@link #getDetailedErrors()} - the full diagnostic for each error, and the only
+ *       accessor that reports where in the source the error occurred. See {@link DetailedError}.
+ * </ul>
+ *
+ * <p>Extends {@link InternalException}, so callers that catch the general parse-or-evaluate
+ * failure are unaffected and need not know this type exists.
+ */
+public final class PolicyParseException extends InternalException {
+
+    private static final TypeReference<List<DetailedError>> ERROR_LIST =
+            new TypeReference<List<DetailedError>>() { };
+
+    private final transient List<DetailedError> detailedErrors;
+
+    /**
+     * Construct from the JSON array of {@code DetailedError} the native layer serialises.
+     *
+     * @param message the value {@link #getMessage()} reports
+     * @param messages one message per parse error, for {@link #getErrors()}
+     * @param detailedErrorsJson JSON array of {@code DetailedError}; if it cannot be read,
+     *     the exception still carries {@code message} and {@code messages}, and
+     *     {@link #getDetailedErrors()} returns empty, so a serialisation change can never
+     *     turn a parse error into a different failure
+     */
+    public PolicyParseException(String message, String[] messages, String detailedErrorsJson) {
+        super(message, Arrays.asList(messages));
+        this.detailedErrors = readDetailedErrors(detailedErrorsJson);
+    }
+
+    private static List<DetailedError> readDetailedErrors(String json) {
+        if (json == null || json.isEmpty()) {
+            return List.of();
+        }
+        try {
+            List<DetailedError> parsed = CedarJson.objectReader().forType(ERROR_LIST).readValue(json);
+            return parsed == null ? List.of() : List.copyOf(parsed);
+        } catch (JsonProcessingException | RuntimeException e) {
+            return List.of();
+        }
+    }
+
+    /**
+     * The structured diagnostics for each parse error, including source spans and help text.
+     *
+     * @return the diagnostics, or an empty list if none could be recovered
+     */
+    public List<DetailedError> getDetailedErrors() {
+        return Collections.unmodifiableList(detailedErrors);
+    }
+}

CedarJava/src/test/java/com/cedarpolicy/PolicyParseDiagnosticsTests.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright Cedar Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.cedarpolicy;
+
+import com.cedarpolicy.model.DetailedError;
+import com.cedarpolicy.model.exception.InternalException;
+import com.cedarpolicy.model.exception.PolicyParseException;
+import com.cedarpolicy.model.policy.PolicySet;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/** Parse failures carry Cedar's structured diagnostics, not just a flattened string. */
+public class PolicyParseDiagnosticsTests {
+
+    @Test
+    public void parseFailureCarriesSourceSpanAndExpectedTokens() {
+        // An entity literal in the action slot: the scope needs `action == ...`.
+        String src = "forbid(principal, Foo::Action::\"Read\", resource);";
+        PolicyParseException e =
+                assertThrows(PolicyParseException.class, () -> PolicySet.parsePolicies(src));
+
+        List<DetailedError> details = e.getDetailedErrors();
+        assertEquals(1, details.size());
+        DetailedError error = details.get(0);
+        assertTrue(error.message.contains("unexpected token `::`"), error.message);
+
+        assertEquals(1, error.sourceLocations.size());
+        DetailedError.SourceLabel span = error.sourceLocations.get(0);
+        // The span must cover the offending `::`, so callers can underline it.
+        assertEquals(src.indexOf("::"), span.start);
+        assertEquals(src.indexOf("::") + 2, span.end);
+        assertTrue(span.label.orElse("").contains("expected"), span.label.toString());
+    }
+
+    @Test
+    public void everyParseErrorIsReportedNotJustTheFirst() {
+        // ParseErrors' Display prints only the first error, so the flattened path reported
+        // one string for the whole document. Both accessors now carry all of them.
+        String src = "forbid(principal, Foo::Action::\"A\", resource);\n"
+                + "permit(principal, action, resource) when { 1 + };";
+        PolicyParseException e =
+                assertThrows(PolicyParseException.class, () -> PolicySet.parsePolicies(src));
+
+        assertEquals(2, e.getDetailedErrors().size());
+        assertEquals(2, e.getErrors().size());
+        // The message stays what Display gave it - the first error alone - so populating the
+        // list cannot widen the string that existing callers match on.
+        assertEquals("Internal error: Internal JNI Error: " + e.getErrors().get(0), e.getMessage());
+    }
+
+    @Test
+    public void messageIsUnchangedForBackCompat() {
+        // Callers branch on getMessage() and match it with anchored regexes, so the string
+        // stays exactly as the generic error path wrote it, "Internal JNI Error: " and all.
+        // The added detail is reached through the accessors instead.
+        PolicyParseException e = assertThrows(PolicyParseException.class,
+                () -> PolicySet.parsePolicies("forbid(principal, Foo::Action::\"Read\", resource);"));
+
+        assertEquals("Internal error: Internal JNI Error: unexpected token `::`", e.getMessage());
+        // getErrors() entries are the bare Cedar messages: the prefix described the binding
+        // rather than any one error, and is meaningless once the list is per-error.
+        assertEquals(List.of("unexpected token `::`"), e.getErrors());
+    }
+
+    @Test
+    public void helpTextSurvivesWhenCedarSuppliesIt() {
+        PolicyParseException e = assertThrows(PolicyParseException.class,
+                () -> PolicySet.parsePolicies("permit(principle, action, resource);"));
+
+        DetailedError error = e.getDetailedErrors().get(0);
+        assertTrue(error.help.isPresent(), "expected help text for an invalid scope variable");
+        assertTrue(error.help.get().contains("principal"), error.help.get());
+    }
+
+    @Test
+    public void remainsCatchableAsInternalException() {
+        // PolicyParseException extends InternalException so existing callers keep working.
+        InternalException e = assertThrows(InternalException.class,
+                () -> PolicySet.parsePolicies("permit(principal, action, resource)"));
+        assertFalse(e.getErrors().isEmpty());
+    }
+
+    @Test
+    public void validPolicySetStillParses() {
+        assertDoesNotThrow(() -> PolicySet.parsePolicies("permit(principal, action, resource);"));
+    }
+}