feat: change transformModFileToJSON to return line and column information

openfga / language

Grammar for the OpenFGA modeling language

https://openfga.dev

Apache License 2.0

15 stars 7 forks source link

feat: change transformModFileToJSON to return line and column information #177

Closed ewanharris closed 4 months ago

ewanharris commented 4 months ago

Description

Changes the return type of the transformModFileToJSON to a structure that allows us to return line and column information as part of the data to allow us to improve error reporting without tying us to the underlying yaml parser that we use on each platform.

Currently only has the JS implementation and will follow up with the Go implementation.

References

Resolves #173

Review Checklist

[x] 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]
[x] 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

d-jeffery commented 4 months ago

Source mapping is great: https://github.com/openfga/language/assets/1425457/52935de6-3c3f-46bf-b93e-882dc67205e7

Just questioning who should own validation of whether a file name is valid to avoid whack-a-mole.

rhamzeh commented 4 months ago

Just questioning who should own validation of whether a file name is valid to avoid whack-a-mole.

I think the frontend (so VS Code/CLI should always be the one validating the files, as reading the files in a uniform way is outside the scope of language), e.g. reading the files on web vs native vs code

ewanharris commented 4 months ago

@rhamzeh I think @d-jeffery was referring to the validation the language does currently here where it ensure that it ends with .fga, do you think we should remove that?

Myself and Daniel had discussed potentially moving the .fga check to be a "return file with errors" case and then changing the returned structure to be like { modFile, errors }, that would allow us to potentially extend in future if we ever wanted to have warnings rather than just errors.

d-jeffery commented 4 months ago

@rhamzeh I think @d-jeffery was referring to the validation the language does currently here where it ensure that it ends with .fga, do you think we should remove that?

Yes, this is what I was referencing.

d-jeffery commented 4 months ago

From a quick talk with @rhamzeh

agree on: file loading and validation should be done by the "UI" (VS Code ext, CLI, etc..) and file name validation being in language (this happens before loading)

also agree on language returning a json blob containing errors with a level