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

Reject trailing tokens after declaration #42

Closed
wants to merge 1 commit into from

Conversation

jcaesar
Copy link
Contributor

@jcaesar jcaesar commented Dec 1, 2022

Current venial parses struct Good {} bad and silently ignores bad or any other trailing junk tokens. I'm not sure what the best way to deal with that is, but I think that should be an error.

(Origin issue: jcaesar/structstruck#4)

Copy link
Collaborator

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! 👍

src/tests.rs Outdated Show resolved Hide resolved
@Bromeon Bromeon added the bug Something isn't working label Dec 1, 2022
@PoignardAzur
Copy link
Owner

At a glance, I think my preferred solution would be to have two separate consume_declaration and parse_declaration functions. Will handle it when I have the time.

@jcaesar
Copy link
Contributor Author

jcaesar commented Dec 7, 2022

@PoignardAzur How about just making parse_declaration_tokens public? (I'd argue that if you hand ownership of a list of tokens, you'd want them to be fully consumed.)

@PoignardAzur
Copy link
Owner

Right, I wasn't up-to-date on that part of the code.

@PoignardAzur
Copy link
Owner

I created #44 based on your PR. I think it addresses all your concerns?

@jcaesar
Copy link
Contributor Author

jcaesar commented Dec 10, 2022

Tested it, does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants