Skip to content

Read quoted string when expecting an import#3554

Merged
oldergod merged 1 commit intomasterfrom
bquenaudon.2026-03-23.parsing
Mar 23, 2026
Merged

Read quoted string when expecting an import#3554
oldergod merged 1 commit intomasterfrom
bquenaudon.2026-03-23.parsing

Conversation

@oldergod
Copy link
Copy Markdown
Member

Fixes #3545

We were not parsing quoted strings but the parsing would fail anyway because the parser would be in a broken state after having read a word. So we're making the error more correct now.

Comment on lines +83 to +85
check(startQuote == '"' || startQuote == '\'') {
unexpected("expected quoted string", location())
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This was throwing an exception while throwing an exception. Plus the location is already the default value of the second parameter.

There's a helper that does all this for you:

Suggested change
check(startQuote == '"' || startQuote == '\'') {
unexpected("expected quoted string", location())
}
expect(startQuote == '"' || startQuote == '\'') {
"expected quoted string"
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did miss it, thanks a lot!

@oldergod oldergod force-pushed the bquenaudon.2026-03-23.parsing branch from 5d3a6a6 to 8d73b78 Compare March 23, 2026 17:43
@oldergod oldergod enabled auto-merge March 23, 2026 17:45
@oldergod oldergod merged commit fafd987 into master Mar 23, 2026
17 checks passed
@oldergod oldergod deleted the bquenaudon.2026-03-23.parsing branch March 23, 2026 18:27
@JesseWeinstein
Copy link
Copy Markdown

Was it really broken before? Signal was using unquoted imports, and they seemed to be working.

@oldergod
Copy link
Copy Markdown
Member Author

It would break if there was a / in the path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire allows unquoted imports

3 participants