Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parser diagnostics about END statement #468

Merged

Conversation

Claes65
Copy link
Contributor

@Claes65 Claes65 commented Apr 10, 2024

No description provided.

@Claes65 Claes65 marked this pull request as draft April 10, 2024 13:26
@Claes65 Claes65 marked this pull request as ready for review April 11, 2024 12:52
@Claes65 Claes65 marked this pull request as draft April 11, 2024 12:53
@Claes65 Claes65 marked this pull request as ready for review April 11, 2024 13:12
@Claes65 Claes65 marked this pull request as draft April 11, 2024 13:13
@Claes65 Claes65 marked this pull request as ready for review April 11, 2024 13:21
@MarkusAmshove
Copy link
Owner

This adds around 132 diagnostics for us.

Most of them are No source code allowed after the END statement, because we don't parse END TRANSACTION yet and it gets recognized as END-Statement.

The others are modules using . as END 🙄

I just took a sample size to look through, but those two are the ones I've in that sample size.

We should get rid of both by supporting the actual stuff. . (SyntaxKind.DOT) could be added as IEndNode, so that we don't have to check for both.
If an analyzer wants to do check the end statement for code style, it can hook into IEndNode and just check the token. No need to have different nodes.

If you want to do either of those, or both, you can implement them in this PR or in new ones. I'd leave this open until we fix them.
I can also do one of those or both, you can decide :)

@Claes65
Copy link
Contributor Author

Claes65 commented Apr 12, 2024

This adds around 132 diagnostics for us.

Most of them are No source code allowed after the END statement, because we don't parse END TRANSACTION yet and it gets recognized as END-Statement.

The others are modules using . as END 🙄

I just took a sample size to look through, but those two are the ones I've in that sample size.

We should get rid of both by supporting the actual stuff. . (SyntaxKind.DOT) could be added as IEndNode, so that we don't have to check for both. If an analyzer wants to do check the end statement for code style, it can hook into IEndNode and just check the token. No need to have different nodes.

If you want to do either of those, or both, you can implement them in this PR or in new ones. I'd leave this open until we fix them. I can also do one of those or both, you can decide :)

Interesting ;) If you can do the SyntaxKind.DOT thing, then I can support ET. Or if you have time, you can do both. I won't be able to work on this until Monday.

@MarkusAmshove
Copy link
Owner

I've added both things and don't get any new diagnostic anymore :)

@Claes65
Copy link
Contributor Author

Claes65 commented Apr 13, 2024

I've added both things and don't get any new diagnostic anymore :)

Cool. I can run it through our source on Monday. :) if you push your changes.

Copy link

@MarkusAmshove MarkusAmshove changed the title END statement parser errors Add parser diagnostics about END statement Apr 15, 2024
@MarkusAmshove MarkusAmshove added the natparse🔎 Parser and project structure label Apr 15, 2024
@MarkusAmshove MarkusAmshove added this to the v0.14 milestone Apr 15, 2024
@MarkusAmshove MarkusAmshove merged commit 22d45ac into MarkusAmshove:main Apr 15, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
natparse🔎 Parser and project structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants