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

Panic when parsing let declaration with no declarations #154

Closed
jasikpark opened this issue Jun 1, 2024 · 5 comments
Closed

Panic when parsing let declaration with no declarations #154

jasikpark opened this issue Jun 1, 2024 · 5 comments
Labels
bug Something isn't working parser Related to Ezno's syntax parser, AST definitions and output

Comments

@jasikpark
Copy link
Contributor

!!sqve+l//
















l
let
///
oOo�str!er

as input for the module_roundtrip_naive fuzzer produces this error:

Running: fuzz/artifacts/module_roundtrip_naive/crash-cbb815ba074710aad65fc66c094f2e1600e0c80e
thread 'AST parsing' panicked at /Users/calebjasik-personal/Git/ezno/parser/src/declarations/variable.rs:274:63:
called `Option::unwrap()` on a `None` value

VariableDeclaration::LetDeclaration {
position: start.union(declarations.last().unwrap().get_position()),
declarations,
}

@jasikpark
Copy link
Contributor Author

Seems like there needs to be an error thrown when there's a let but no associated declaration?

@kaleidawave
Copy link
Owner

kaleidawave commented Jun 2, 2024

Hmm yes. Could start with let mut declarations = vec![VariableDeclarationItem::from_reader(...)] but yes emitting a ParseError if declarations.is_empty would be better because it could be allowed under partial_syntax so that this works

Will investigate and fix! (also same issue for const below and will check for var)

@kaleidawave kaleidawave added bug Something isn't working parser Related to Ezno's syntax parser, AST definitions and output labels Jun 2, 2024
@kaleidawave kaleidawave changed the title Bug: parsing bug found by module_roundtrip_naive Panic when parsing let declaration with no declarations Jun 2, 2024
@jasikpark
Copy link
Contributor Author

jasikpark commented Jun 3, 2024

I'm thinking I may do work to integrate these mature fuzzers with OSS-Fuzz, where they can be run for longer and catch bugs like these, since it seems like they aren't catching anything on 5m Github Action runs anymore, right?

(https://google.github.io/oss-fuzz/getting-started/continuous-integration/)

kaleidawave added a commit that referenced this issue Jun 3, 2024
- Fix capitalisation in `ForLoopStatementInitialiser`
- Fix for #154
@kaleidawave
Copy link
Owner

There are things being caught! I am unsure how this example hadn't been flagged by CI yet? Maybe just a rare state.

ATM things in the type checker have a lot higher priority so I am quite happy with the current level of parsing fuzzing. If you want to and is simple to change/upgrade sure! I do want to keep it under 2 mins though, it is a bit annoying to wait any longer for results.

kaleidawave added a commit that referenced this issue Jun 24, 2024
- Fix capitalisation in `ForLoopStatementInitialiser`
- Fix for #154
- Renames and fix for #161
- Fix for code_blocks_to_script & performance action
- Add array pretty printing
- Fix spread being allowed not at the end of destructuring (required checker changes)
- Not sure why fuzzing broke?
- Add `LTSI::new_under` public method
- Change `cargo-fuzz` install to fix issue
@kaleidawave
Copy link
Owner

Fixed in #158

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser Related to Ezno's syntax parser, AST definitions and output
Projects
None yet
Development

No branches or pull requests

2 participants