fix(pkg/go/utils): match declarations by full name, not by prefix

[SoulPancake]

The line-number helpers in pkg/go/utils matched a declaration with strings.HasPrefix on the name alone, so a name that is a prefix of another resolved to the wrong line: asking for document matched type documentation, owner matched define owner_group:, and less matched condition less_than(.

Description

What problem is being solved?

GetTypeLineNumber, GetRelationLineNumber, GetConditionLineNumber and GetExtendedTypeLineNumber each tested only that a trimmed line started with "<keyword> <name>", with nothing checking what followed the name:

func GetTypeLineNumber(typeName string, lines []string) int {
	return slices.IndexFunc(lines, func(line string) bool {
		return strings.HasPrefix(strings.TrimSpace(line), "type "+typeName)
	})
}

Reproduced against develop — every one of these returns the line of the longer declaration that happens to come first:

query	matched	returned	correct
type document	type documentation	2	6
relation owner	define owner_group:	4	5
condition less	condition less_than(	7	8

The helpers have four non-test callers, all in pkg/go/transformer/module-to-model.go: the duplicate-type, duplicate-condition, extended-type and duplicate-relation errors. In a module set where one name is a prefix of another, those errors point at the wrong declaration.

This is user-visible in a released package. pkg/go/utils/line-numbers.go and pkg/go/transformer/module-to-model.go both ship in pkg/go/v0.3.1, which is the tag openfga/cli pins.

This is the Go half of #255, which listed all three implementations' files. #260 closed it after fixing only the JS and Java copies — pkg/go/utils/line-numbers.go was named in the issue but never in the diff, so the Go bug has been live since.

How is it being solved?

By requiring the name to end where the prefix ends. The four helpers now share one matcher, and the terminator rule is derived from the lexer grammar rather than hand-written per helper:

// IDENTIFIER admits letters, digits, `_` and MINUS; EXTENDED_IDENTIFIER adds
// SLASH and DOT (OpenFGALexer.g4).
func IsNameByte(b byte) bool

declarationIndex finds the first line starting with the prefix whose next byte is not a name byte (or which ends there). Because the prefix always spans whole words — type <name>, define <name>, condition <name> — that single check is what stops document matching documentation.

Taking the terminator set from the grammar rather than requiring a specific punctuation character is what makes type core.doc a declaration of core.doc and not of core, since . is a name byte in EXTENDED_IDENTIFIER. A rule that instead demanded : or ( after the name would get that case wrong.

Only the relation helper collapses runs of spaces, mirroring the reference implementation, where {2,} normalization is applied in getRelationLineNumber alone — so define owner: still matches owner.

What changes are made to solve it?

• pkg/go/utils/line-numbers.go: adds IsNameByte and the shared declarationIndex; the four exported helpers become one-line calls into it. Signatures, the int return and the -1 miss sentinel are all unchanged.
• pkg/go/validation/name_validation.go: GetConditionLineNumber had its own inline terminator check requiring ( after the name. It now calls utils.IsNameByte, so the rule lives in one place. This is a deliberate loosening — see the note below.
• tests/data/transformer-module/07-prefix-collisions/: a module fixture where documentation/document and less/less_than collide across two files, asserting the duplicate-condition error lands on dup.fga line 10 rather than an earlier less_than declaration.
• pkg/go/utils/line-numbers_test.go: 47 cases covering each helper, including the trailing-comment form the module fixtures depend on (type other # module: core, file: core.fga), collapsed spaces, dotted names, and the three collisions above.

The only visible behaviour change is that the four transformer errors point at the correct line in collision cases. No error identity or message text changes.

A note on what this does not tighten

The matcher accepts anything that is not a name byte as a terminator, which is looser than the punctuation each declaration form actually requires. All of these match today:

type doc#x            queried as `doc`     -> matches
type doc:x            queried as `doc`     -> matches
define owner [user]   queried as `owner`   -> matches   (no colon)
condition less {      queried as `less`    -> matches   (no parameter list)

None of these are valid DSL, and the helpers are only ever called with a name the parser has already accepted from a model that parsed — so a malformed line cannot reach them in practice. Deriving the rule from the grammar was preferred over four hand-written punctuation checks because it is the same rule in every helper and it handles extended identifiers correctly. Worth flagging explicitly rather than leaving it to be discovered.

For the same reason, the two Go copies are now closer but not identical. pkg/go/validation's type and relation helpers parse the line (strings.Fields, then an exact compare; or split on :), so they reject type doc#x, type doc:x and define owner [user] where utils accepts them. Both agree on every well-formed input, including all three collision cases and the trailing-comment form. Unifying the two copies is a follow-up; this PR makes them agree on the cases that occur, not on malformed input.

Testing

From pkg/go: go build ./... clean, and go test ./... -count=1 green — utils, transformer and validation all pass, with the new module fixture exercised through the transformer path.

The three collisions were verified as failing on develop and fixed on this branch by building the helpers standalone against both versions, rather than only through the test suite.

References

closes #255

• fix: incorrectly assigned error #260 — closed Incorrect line for type when looking up the typeIndex, when type is a substring #255 after fixing the JS and Java copies only; this is the Go half it missed
• fix: correct error locations when type/schema lines have trailing comments #628 — the JS/Java trailing-comment fix, whose behaviour these helpers are matched against

Review Checklist

• I have clicked on "allow edits by maintainers".
• I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
• The correct base branch is being used, if not main
• I have added tests to validate that the change in functionality is working as expected

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 159e1900-8248-409b-bd4a-82b45ad78647

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR fixes how the Go pkg/go/utils line-number helpers locate DSL declarations by ensuring they match full identifiers (and required trailing syntax) rather than using prefix-only matching, which could previously return the wrong line when names were prefixes of other names.

Changes:

• Introduces a shared declarationIndex helper and precise “remainder” validation to avoid prefix collisions for type, extend type, and condition declarations.
• Tightens matching rules to mirror the JS validator behavior (e.g., types allow only EOL or trailing # comment; conditions require (; relations require : and normalize repeated spaces).
• Adds comprehensive unit tests covering collision cases, module comments, and whitespace variations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pkg/go/utils/line-numbers.go	Reworks declaration matching to validate what follows the name, preventing prefix-based false matches and aligning with JS behavior.
pkg/go/utils/line-numbers_test.go	Adds focused tests to ensure correct line selection across name-collision and whitespace/comment edge cases.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[senojj]

@SoulPancake why force the caller to validate the expected tail of the string? You could simply get the index immediately following the provided prefix -- if one exists -- and check for a word termination character.

It seems that an invariant of this function is that prefix be a full word or words, and not just a partial word. Therefore, it makes more sense to assert that any matching prefix be followed by a word terminator. There is no need for the rest argument.

[SoulPancake]

Shared fixtures causing the npm audit failure to show up in this PR, checking that.

Edit: Letting that remain as a separate fix PR to not digress from the topic here.

[Siddhant-K-code]

Tests/lint seems to be failing - can i send a commit to fix it or you wanna address it, @SoulPancake ?

[SoulPancake]

@Siddhant-K-code
#652 (comment)

[Siddhant-K-code]

Thanks for addressing the review comments! Changes look good to me; the prefix matching fix is clear & well tested. % address the lint failing please before merge!

I'd also wait for review from @senojj