fix: correct error locations when type/schema lines have trailing comments

pkg/java/src/main/java/dev/openfga/language/validation/Dsl.java

@@ -9,6 +9,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.function.Predicate;
+import java.util.regex.Pattern;
 import java.util.stream.IntStream;
 
 class Dsl {
@@ -24,7 +25,7 @@ private int findLine(Predicate<String> predicate, int skipIndex) {
             return -1;
         }
 
-        return IntStream.range(skipIndex, lines.length)
+        return IntStream.range(Math.max(skipIndex, 0), lines.length)
                 .filter(index -> predicate.test(lines[index]))
                 .findFirst()
                 .orElse(-1);
@@ -35,23 +36,42 @@ public int getConditionLineNumber(String conditionName) {
     }
 
     public int getConditionLineNumber(String conditionName, int skipIndex) {
-        return findLine(line -> line.trim().startsWith("condition " + conditionName), skipIndex);
+        // Require `(` after the name so a condition name that is a prefix of
+        // another (e.g. `less` vs `less_than`) cannot match the wrong line.
+        return findLine(
+                line -> line.trim().matches("condition " + Pattern.quote(conditionName) + "\\s*\\(.*"), skipIndex);
     }
 
     public int getRelationLineNumber(String relationName, int skipIndex) {
-        return findLine(line -> line.trim().replaceAll(" {2,}", " ").startsWith("define " + relationName), skipIndex);
+        // Require `:` after the name so a relation name that is a prefix of
+        // another (e.g. `writer` vs `writers`) cannot match the wrong line.
+        return findLine(
+                line -> line.trim()
+                        .replaceAll(" {2,}", " ")
+                        .matches("define " + Pattern.quote(relationName) + "\\s*:.*"),
+                skipIndex);
     }
 
     public int getSchemaLineNumber(String schemaVersion) {
-        return findLine(line -> line.trim().replaceAll(" {2,}", " ").startsWith("schema " + schemaVersion), 0);
+        // Allow only whitespace or a trailing comment after the version so
+        // e.g. `1.1` cannot match `schema 1.10`. A comment must be preceded by
+        // whitespace so a `#` glued to the version isn't treated as a comment.
+        return findLine(
+                line -> line.trim()
+                        .replaceAll(" {2,}", " ")
+                        .matches("schema " + Pattern.quote(schemaVersion) + "(\\s+#.*)?"),
+                0);
     }
 
     public int getTypeLineNumber(String typeName) {
         return getTypeLineNumber(typeName, 0);
     }
 
     public int getTypeLineNumber(String typeName, int skipIndex) {
-        return findLine(line -> line.trim().matches("type " + typeName), skipIndex);
+        // Allow an optional trailing comment (e.g. `type page # module: ...`) after the type name.
+        // The comment must be preceded by whitespace so a `#` glued to the name isn't treated as a comment.
+        // Quote the type name so regex metacharacters (e.g. `.`) are matched literally.
+        return findLine(line -> line.trim().matches("type " + Pattern.quote(typeName) + "(\\s+#.*)?"), skipIndex);
     }
 
     public static String getRelationDefName(Userset userset) {

pkg/java/src/main/java/dev/openfga/language/validation/ValidationErrorsBuilder.java

@@ -25,7 +25,7 @@ private ErrorProperties buildErrorProperties(
 
         var properties = new ErrorProperties(null, null, message);
 
-        if (lines != null) {
+        if (lines != null && lineIndex >= 0 && lineIndex < lines.length) {
             var rawLine = lines[lineIndex];
             var regex = Pattern.compile("\\b" + symbol + "\\b");
             var wordIdx = 0;
@@ -97,9 +97,15 @@ public void raiseInvalidRelationError(int lineIndex, String symbol, Collection<S
     }
 
     public void raiseAssignableRelationMustHaveTypes(int lineIndex, String symbol) {
-        var rawLine = lines[lineIndex];
-        var actualValue =
-                rawLine.contains("[") ? rawLine.substring(rawLine.indexOf('['), rawLine.lastIndexOf(']') + 1) : "self";
+        // Mirror the JS behavior: fall back to an empty value when the line
+        // cannot be resolved instead of throwing ArrayIndexOutOfBoundsException.
+        var actualValue = "";
+        if (lines != null && lineIndex >= 0 && lineIndex < lines.length) {
+            var rawLine = lines[lineIndex];
+            actualValue = rawLine.contains("[")
+                    ? rawLine.substring(rawLine.indexOf('['), rawLine.lastIndexOf(']') + 1)
+                    : "self";
+        }
         var message = "assignable relation '" + actualValue + "' must have types";
         var errorProperties = buildErrorProperties(message, lineIndex, symbol);
         var metadata = new ValidationMetadata(symbol, ValidationError.AssignableRelationsMustHaveType);

pkg/js/tests/modules/module-to-model.test.ts

@@ -67,8 +67,10 @@ describe("transformModuleFilesToModel - module test cases", () => {
       testFn = it.skip;
     }
 
-    // Skip schema validation errors
-    if (testCase.dsl.includes("0.9")) {
+    // The rewrite below only converts `model\n  schema 1.1` into a module, so cases using a
+    // different schema version (or an invalid one) cannot be exercised here; they are covered
+    // by the DSL validator tests instead.
+    if (!testCase.dsl.includes("schema 1.1")) {
       testFn = it.skip;
     }
 

pkg/js/util/exceptions.ts

@@ -225,7 +225,7 @@ const createInvalidTypeError = (props: BaseProps, typeName: string) => {
 const createAssignableRelationMustHaveTypesError = (props: BaseProps) => {
   const { errors, lines, lineIndex } = props;
 
-  if (!lines?.length || lineIndex === undefined) {
+  if (!lines?.length || lineIndex === undefined || lineIndex < 0) {
     const actualValue = "";
     errors.push(
       constructValidationError({
@@ -292,7 +292,7 @@ const createDuplicateRelationError = (props: BaseProps, relationName: string, ty
 const createDuplicateRelationshipDefinitionError = (props: BaseProps) => {
   const { errors, lines, lineIndex, symbol, file, module } = props;
 
-  if (!lines?.length || lineIndex === undefined) {
+  if (!lines?.length || lineIndex === undefined || lineIndex < 0) {
     errors.push(
       new ModelValidationSingleError(
         {
@@ -437,7 +437,7 @@ function constructValidationError(props: ValidationErrorProps): ModelValidationS
     offendingType: metadata.offendingType,
   };
 
-  if (lines?.length && lineIndex != undefined) {
+  if (lines?.length && lineIndex != undefined && lineIndex >= 0) {
     const rawLine = lines[lineIndex];
 
     const re = new RegExp("\\b" + metadata.symbol + "\\b");
@@ -749,7 +749,7 @@ interface TransformationErrorProps {
 export function constructTransformationError(props: TransformationErrorProps) {
   const { message, lines, lineIndex, metadata } = props;
 
-  if (!lines?.length || lineIndex === undefined) {
+  if (!lines?.length || lineIndex === undefined || lineIndex < 0) {
     return new ModuleTransformationSingleError(
       {
         msg: message,

pkg/js/util/line-numbers.ts

@@ -1,39 +1,50 @@
 export const getConditionLineNumber = (conditionName: string, lines?: string[], skipIndex?: number) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return (
-    lines.slice(skipIndex).findIndex((line: string) => line.trim().startsWith(`condition ${conditionName}`)) + skipIndex
-  );
+  // Require `(` after the name so a condition name that is a prefix of another
+  // (e.g. `less` vs `less_than`) cannot match the wrong line.
+  const conditionPrefix = `condition ${conditionName}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const trimmed = line.trim();
+    return trimmed.startsWith(conditionPrefix) && /^\s*\(/.test(trimmed.slice(conditionPrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };
 
 export const getTypeLineNumber = (typeName: string, lines?: string[], skipIndex?: number, extension = false) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return (
-    lines
-      .slice(skipIndex)
-      .findIndex((line: string) => line.trim().match(`^${extension ? "extend " : ""}type ${typeName}$`)) + skipIndex
-  );
+  // Allow an optional trailing comment (e.g. `type page # module: ...`) after the type name.
+  // The comment must be preceded by whitespace so a `#` glued to the name isn't treated as a comment.
+  // Match the type name literally (it may contain regex metacharacters like `.`).
+  const typePrefix = `${extension ? "extend " : ""}type ${typeName}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const trimmed = line.trim();
+    return trimmed.startsWith(typePrefix) && /^(\s+#.*)?$/.test(trimmed.slice(typePrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };
 
 export const getRelationLineNumber = (relation: string, lines?: string[], skipIndex?: number) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return (
-    lines
-      .slice(skipIndex)
-      .findIndex((line: string) => line.trim().replace(/ {2,}/g, " ").match(`^define ${relation}\\s*:`)) + skipIndex
-  );
+  // Match the relation name literally (it may contain regex metacharacters like `.`).
+  const relationPrefix = `define ${relation}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const normalized = line.trim().replace(/ {2,}/g, " ");
+    return normalized.startsWith(relationPrefix) && /^\s*:/.test(normalized.slice(relationPrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };

pkg/js/validator/validate-dsl.ts

@@ -292,47 +292,69 @@ function hasEntryPointOrLoop(
 }
 
 const geConditionLineNumber = (conditionName: string, lines?: string[], skipIndex?: number) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return (
-    lines.slice(skipIndex).findIndex((line: string) => line.trim().startsWith(`condition ${conditionName}`)) + skipIndex
-  );
+  // Require `(` after the name so a condition name that is a prefix of another
+  // (e.g. `less` vs `less_than`) cannot match the wrong line.
+  const conditionPrefix = `condition ${conditionName}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const trimmed = line.trim();
+    return trimmed.startsWith(conditionPrefix) && /^\s*\(/.test(trimmed.slice(conditionPrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };
 
 const getTypeLineNumber = (typeName: string, lines?: string[], skipIndex?: number) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return lines.slice(skipIndex).findIndex((line: string) => line.trim().match(`^type ${typeName}$`)) + skipIndex;
+  // Allow an optional trailing comment (e.g. `type page # module: ...`) after the type name.
+  // The comment must be preceded by whitespace so a `#` glued to the name isn't treated as a comment.
+  // Match the type name literally (it may contain regex metacharacters like `.`).
+  const typePrefix = `type ${typeName}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const trimmed = line.trim();
+    return trimmed.startsWith(typePrefix) && /^(\s+#.*)?$/.test(trimmed.slice(typePrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };
 
 const getRelationLineNumber = (relation: string, lines?: string[], skipIndex?: number) => {
-  if (!skipIndex) {
+  if (!skipIndex || skipIndex < 0) {
     skipIndex = 0;
   }
   if (!lines) {
     return undefined;
   }
-  return (
-    lines
-      .slice(skipIndex)
-      .findIndex((line: string) => line.trim().replace(/ {2,}/g, " ").match(`^define ${relation}\\s*:`)) + skipIndex
-  );
+  // Match the relation name literally (it may contain regex metacharacters like `.`).
+  const relationPrefix = `define ${relation}`;
+  const index = lines.slice(skipIndex).findIndex((line: string) => {
+    const normalized = line.trim().replace(/ {2,}/g, " ");
+    return normalized.startsWith(relationPrefix) && /^\s*:/.test(normalized.slice(relationPrefix.length));
+  });
+  return index === -1 ? -1 : index + skipIndex;
 };
 
 const getSchemaLineNumber = (schema: string, lines?: string[]) => {
   if (!lines) {
     return undefined;
   }
 
-  const index = lines.findIndex((line: string) => line.trim().replace(/ {2,}/g, " ").match(`^schema ${schema}$`));
+  // Allow an optional trailing comment (e.g. `schema 1.1 # ...`) after the schema version.
+  // The comment must be preceded by whitespace so a `#` glued to the version isn't treated as a comment.
+  // Match the schema version literally (it contains `.`).
+  const schemaPrefix = `schema ${schema}`;
+  const index = lines.slice(0).findIndex((line: string) => {
+    const normalized = line.trim().replace(/ {2,}/g, " ");
+    return normalized.startsWith(schemaPrefix) && /^(\s+#.*)?$/.test(normalized.slice(schemaPrefix.length));
+  });
 
   // As findIndex returns -1 when it doesn't find the line, we want to return 0 instead
   if (index >= 1) {