feat: add indentDSL to improve checkDSL parsing reliability

[dbrrt]

This PR introduces indent-dsl utilities to facilitate friendly DSL parsing.

Description

• indent-dsl.ts added with corresponding tests

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

[CLAassistant]

All committers have signed the CLA.

[adriantam]

@dbrrt : thank you for your contribution! Instead of adding the indentDSL in check-dsl.ts, maybe it will be better to update the grammar file https://github.com/openfga/syntax-transformer/blob/main/src/openfga.ne instead. In this way, the line number parsed by the checkDSL's catch statement (https://github.com/openfga/syntax-transformer/blob/f2b3cc0a0fc53531e4d00b1a55484c32eeb46151/src/check-dsl.ts#L137) will be correct. Please let me know if you need further clarifications.

Thanks!

[dbrrt]

@dbrrt : thank you for your contribution! Instead of adding the indentDSL in check-dsl.ts, maybe it will be better to update the grammar file https://github.com/openfga/syntax-transformer/blob/main/src/openfga.ne instead. In this way, the line number parsed by the checkDSL's catch statement (

https://github.com/openfga/syntax-transformer/blob/f2b3cc0a0fc53531e4d00b1a55484c32eeb46151/src/check-dsl.ts#L137

) will be correct. Please let me know if you need further clarifications.
Thanks!

I don't think changing the grammar is fair here. The purpose of my function is to make sure that any non properly indented input (copy pasted from any IDE / text editor, FGA playground, Zoom etc...) will match 1:1 the grammar and make the parser more resilient. It also natively drops the new lines making nearley's job easier.

@adriantam Can you please elaborate on "why adding indenting to the catch statement"? I'm not sure how it could help with raw DSL parsing.

@rhamzeh Any thought?

[rhamzeh]

Apologies for the late reply!

First, thanks for your contribution @dbrrt! This is definitely helpful while we decide whether we want to make indentation optional in the grammar or not.

A few notes:

• Long term, I think what @adriantam mentioned is right - if we want to allow this, we should consider making different forms of indentation part of the grammar, this is especially true if users get trained not to indent properly and let the tool handle it for them, making indentation less important from a grammar perspective, only from a readability perspective.
• Also long term I see the value of this as more than just fixing indentation, but more like prettifying/formatting (think of it like eslint --fix/go fmt)
• I don't think this should run as part of checkDSL - checkDSL is concerned with validating and reporting back markers - if it modified the lines, the markers will be off (as @adriantam alluded)
• Most likely this should be it's own util function that users can run separately if they chose to do so before passing it into checkDSL
• Ideally, this should not affect any of the snapshots as those are related to checkDSL, not formatting
• I would ask that you move this to it's own file: you can call it indent-dsl.ts (we will fix the naming and reorganize it after merge)
• Also I would ask that for that file, you create a test file with a bunch of tests and cases to verify that it works as expected

TLDR:
Please:

• move the indentDSL function into it's own file outside of checkDSL
• add indent-dsl.test.ts with multiple tests for it 🙏🏽

[adriantam]

Thanks @dbrrt . Do you mind running npm run format:fix to update the code so that it passes prettier validation? Otherwise, it looks good to me.

[dbrrt]

Thanks @dbrrt . Do you mind running npm run format:fix to update the code so that it passes prettier validation? Otherwise, it looks good to me.

Sure!

[adriantam]

@dbrrt : It seems to be failing lint test (https://github.com/openfga/syntax-transformer/actions/runs/3251900056/jobs/5338708365#step:8:44). Can you take a look at it?

[dbrrt]

@dbrrt : It seems to be failing lint test (https://github.com/openfga/syntax-transformer/actions/runs/3251900056/jobs/5338708365#step:8:44). Can you take a look at it?

Yes, I missed the inferred type complaint from eslint, I only get warning from other files now, it should be fine.

[adriantam]

Looks like the unit test failed - https://github.com/openfga/syntax-transformer/actions/runs/3252493163/jobs/5340130318#step:9:15

[dbrrt]

Looks like the unit test failed - https://github.com/openfga/syntax-transformer/actions/runs/3252493163/jobs/5340130318#step:9:15

The last test I've added wasn't related to the functionality introduced by indent-dsl, so I've removed it.

[adriantam]

LGTM

[adriantam]

@dbrrt : Thank you for your contribution. The changes have been merged into main.