-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update parser API to merge lexing and parsing #11494
Conversation
Lexer
lazy (#11244)16473c5
to
bc7dee5
Compare
/// Tokens represents a vector of lexed [`Token`]. | ||
#[derive(Debug)] | ||
pub struct Tokens { | ||
raw: Vec<Token>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Tokens
struct will gain new methods as I understand how the lexer APIs are being used by the downstream tools but two APIs have been added:
up_to_first_unknown
- Slice of tokens up to (and excluding) the first unknown token. This is what the linter sees.tokens_in_range
- This will replace most usages oflex_starts_at
9e13da6
to
c47f892
Compare
9b33c47
to
2fd01aa
Compare
c47f892
to
c48193d
Compare
2fd01aa
to
b62ba85
Compare
c48193d
to
20fd1b1
Compare
b62ba85
to
88a7b6f
Compare
20fd1b1
to
57c5cdc
Compare
88a7b6f
to
aca7393
Compare
57c5cdc
to
c30d34e
Compare
pub(crate) struct TokenSourceCheckpoint<'src> { | ||
lexer: LexerCheckpoint<'src>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the sizse of a TokenSourceCheckpoint
and ParserCheckpoint
at this point? It seems to becoming somewhat big ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is getting quite big:
ParserCheckpoint
- 176 bytesTokenSourceCheckpoint
- 152 bytesLexerCheckpoint
- 136 bytes
TokenSource { | ||
lexer, | ||
tokens: vec![], | ||
comments: vec![], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure if we should collect the comments in the parse phase or if it is actually faster to extract them after by simply looping over the tokens where needed.
Doint it later has the advantage that it doesn't require any rewinding. I'm okay going with this solution but it might be worth testing that the performance difference is between doing it inside of the parser and doing it as a separate pass. I somewhat suspect that both are equally fast, in which case I would probably prefer doing it outside of the parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can add it to my todo list to look at once the code starts to compile :)
It makes sense to keep it outside the parser.
let leading_quote_len = str::leading_quote(expression).unwrap().text_len(); | ||
let trailing_quote_len = str::trailing_quote(expression).unwrap().text_len(); | ||
let range = range |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that this will fail if the string has any prefixes. While uncommon, it is valid python code to e.g. have r"test"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name of the function is misleading, it does match against both prefixes and quotes:
ruff/crates/ruff_python_ast/src/str.rs
Lines 207 to 221 in c30d34e
/// An [`AhoCorasick`] matcher for string and byte literal prefixes. | |
static PREFIX_MATCHER: Lazy<AhoCorasick> = Lazy::new(|| { | |
AhoCorasick::builder() | |
.start_kind(StartKind::Anchored) | |
.match_kind(MatchKind::LeftmostLongest) | |
.kind(Some(AhoCorasickKind::DFA)) | |
.build( | |
TRIPLE_QUOTE_STR_PREFIXES | |
.iter() | |
.chain(TRIPLE_QUOTE_BYTE_PREFIXES) | |
.chain(SINGLE_QUOTE_STR_PREFIXES) | |
.chain(SINGLE_QUOTE_BYTE_PREFIXES), | |
) | |
.unwrap() | |
}); |
parse_tokens(lxr.collect(), source, mode) | ||
/// This is same as the [`parse`] function except that it doesn't check for any [`ParseError`] | ||
/// and returns the [`Program`] as is. | ||
pub fn parse_unchecked(source: &str, mode: Mode) -> Program<Mod> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe parse_with_recovery
to make it clear that this parsing mode performs error recovery.
pub struct Tokens(Vec<LexResult>); | ||
/// Represents the parsed source code. | ||
#[derive(Debug)] | ||
pub struct Program<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You brought it up in Discord that we'll have a conflict with red-knot.
How about Parsed
or SyntaxTree
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I like Parsed
. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this in a follow-up PR.
aca7393
to
28aa1f7
Compare
c30d34e
to
e75902d
Compare
## Summary This PR updates the parser API within the `ruff_python_parser` crate. It doesn't change any of the references in this PR. The final API looks like: ```rs pub fn parse_module(source: &str) -> Result<Program<ModModule>, ParseError> {} pub fn parse_expression(source: &str) -> Result<Program<ModExpression>, ParseError> {} pub fn parse_expression_range( source: &str, range: TextRange, ) -> Result<Program<ModExpression>, ParseError> {} pub fn parse(source: &str, mode: Mode) -> Result<Program<Mod>, ParseError> {} // Temporary. The `parse` will replace this function once we update the downstream // tools to work with programs containing syntax error. pub fn parse_unchecked(source: &str, mode: Mode) -> Program<Mod> {} ``` Following is a detailed list of changes: * Make `Program` generic over `T` which can be either `Mod` (enum), `ModModule` or `ModExpression` * Add helper methods to cast `Mod` into `ModModule` or `ModExpression` * Add helper method `Program::into_result` which converts a `Program<T>` into a `Result<Program<T>, ParseError>` where the `Err` variant contains the first `ParseError` * Update `TokenSource` to store the comment ranges * Parser crate depends on `ruff_python_trivia` because of `CommentRanges`. This struct could possibly be moved in the parser crate itself at the end * Move from `parse_expression_starts_at` to `parse_expression_range` which parses the source code at the given range using `Mode::Expression`. Unlike the `starts_at` variant, this accepts the entire source code * Remove all access to the `Lexer` * Remove all `parse_*` functions which works on the tokens provided by the caller ## Test Plan The good news is that the tests in `ruff_python_parser` can be run. So, ``` cargo insta test --package ruff_python_parser ```
This PR updates the parser API within the `ruff_python_parser` crate. It doesn't change any of the references in this PR. The final API looks like: ```rs pub fn parse_module(source: &str) -> Result<Program<ModModule>, ParseError> {} pub fn parse_expression(source: &str) -> Result<Program<ModExpression>, ParseError> {} pub fn parse_expression_range( source: &str, range: TextRange, ) -> Result<Program<ModExpression>, ParseError> {} pub fn parse(source: &str, mode: Mode) -> Result<Program<Mod>, ParseError> {} // Temporary. The `parse` will replace this function once we update the downstream // tools to work with programs containing syntax error. pub fn parse_unchecked(source: &str, mode: Mode) -> Program<Mod> {} ``` Following is a detailed list of changes: * Make `Program` generic over `T` which can be either `Mod` (enum), `ModModule` or `ModExpression` * Add helper methods to cast `Mod` into `ModModule` or `ModExpression` * Add helper method `Program::into_result` which converts a `Program<T>` into a `Result<Program<T>, ParseError>` where the `Err` variant contains the first `ParseError` * Update `TokenSource` to store the comment ranges * Parser crate depends on `ruff_python_trivia` because of `CommentRanges`. This struct could possibly be moved in the parser crate itself at the end * Move from `parse_expression_starts_at` to `parse_expression_range` which parses the source code at the given range using `Mode::Expression`. Unlike the `starts_at` variant, this accepts the entire source code * Remove all access to the `Lexer` * Remove all `parse_*` functions which works on the tokens provided by the caller The good news is that the tests in `ruff_python_parser` can be run. So, ``` cargo insta test --package ruff_python_parser ```
This PR updates the parser API within the `ruff_python_parser` crate. It doesn't change any of the references in this PR. The final API looks like: ```rs pub fn parse_module(source: &str) -> Result<Program<ModModule>, ParseError> {} pub fn parse_expression(source: &str) -> Result<Program<ModExpression>, ParseError> {} pub fn parse_expression_range( source: &str, range: TextRange, ) -> Result<Program<ModExpression>, ParseError> {} pub fn parse(source: &str, mode: Mode) -> Result<Program<Mod>, ParseError> {} // Temporary. The `parse` will replace this function once we update the downstream // tools to work with programs containing syntax error. pub fn parse_unchecked(source: &str, mode: Mode) -> Program<Mod> {} ``` Following is a detailed list of changes: * Make `Program` generic over `T` which can be either `Mod` (enum), `ModModule` or `ModExpression` * Add helper methods to cast `Mod` into `ModModule` or `ModExpression` * Add helper method `Program::into_result` which converts a `Program<T>` into a `Result<Program<T>, ParseError>` where the `Err` variant contains the first `ParseError` * Update `TokenSource` to store the comment ranges * Parser crate depends on `ruff_python_trivia` because of `CommentRanges`. This struct could possibly be moved in the parser crate itself at the end * Move from `parse_expression_starts_at` to `parse_expression_range` which parses the source code at the given range using `Mode::Expression`. Unlike the `starts_at` variant, this accepts the entire source code * Remove all access to the `Lexer` * Remove all `parse_*` functions which works on the tokens provided by the caller The good news is that the tests in `ruff_python_parser` can be run. So, ``` cargo insta test --package ruff_python_parser ```
This PR updates the parser API within the `ruff_python_parser` crate. It doesn't change any of the references in this PR. The final API looks like: ```rs pub fn parse_module(source: &str) -> Result<Program<ModModule>, ParseError> {} pub fn parse_expression(source: &str) -> Result<Program<ModExpression>, ParseError> {} pub fn parse_expression_range( source: &str, range: TextRange, ) -> Result<Program<ModExpression>, ParseError> {} pub fn parse(source: &str, mode: Mode) -> Result<Program<Mod>, ParseError> {} // Temporary. The `parse` will replace this function once we update the downstream // tools to work with programs containing syntax error. pub fn parse_unchecked(source: &str, mode: Mode) -> Program<Mod> {} ``` Following is a detailed list of changes: * Make `Program` generic over `T` which can be either `Mod` (enum), `ModModule` or `ModExpression` * Add helper methods to cast `Mod` into `ModModule` or `ModExpression` * Add helper method `Program::into_result` which converts a `Program<T>` into a `Result<Program<T>, ParseError>` where the `Err` variant contains the first `ParseError` * Update `TokenSource` to store the comment ranges * Parser crate depends on `ruff_python_trivia` because of `CommentRanges`. This struct could possibly be moved in the parser crate itself at the end * Move from `parse_expression_starts_at` to `parse_expression_range` which parses the source code at the given range using `Mode::Expression`. Unlike the `starts_at` variant, this accepts the entire source code * Remove all access to the `Lexer` * Remove all `parse_*` functions which works on the tokens provided by the caller The good news is that the tests in `ruff_python_parser` can be run. So, ``` cargo insta test --package ruff_python_parser ```
## Summary This PR updates the entire parser stack in multiple ways: ### Make the lexer lazy * #11244 * #11473 Previously, Ruff's lexer would act as an iterator. The parser would collect all the tokens in a vector first and then process the tokens to create the syntax tree. The first task in this project is to update the entire parsing flow to make the lexer lazy. This includes the `Lexer`, `TokenSource`, and `Parser`. For context, the `TokenSource` is a wrapper around the `Lexer` to filter out the trivia tokens[^1]. Now, the parser will ask the token source to get the next token and only then the lexer will continue and emit the token. This means that the lexer needs to be aware of the "current" token. When the `next_token` is called, the current token will be updated with the newly lexed token. The main motivation to make the lexer lazy is to allow re-lexing a token in a different context. This is going to be really useful to make the parser error resilience. For example, currently the emitted tokens remains the same even if the parser can recover from an unclosed parenthesis. This is important because the lexer emits a `NonLogicalNewline` in parenthesized context while a normal `Newline` in non-parenthesized context. This different kinds of newline is also used to emit the indentation tokens which is important for the parser as it's used to determine the start and end of a block. Additionally, this allows us to implement the following functionalities: 1. Checkpoint - rewind infrastructure: The idea here is to create a checkpoint and continue lexing. At a later point, this checkpoint can be used to rewind the lexer back to the provided checkpoint. 2. Remove the `SoftKeywordTransformer` and instead use lookahead or speculative parsing to determine whether a soft keyword is a keyword or an identifier 3. Remove the `Tok` enum. The `Tok` enum represents the tokens emitted by the lexer but it contains owned data which makes it expensive to clone. The new `TokenKind` enum just represents the type of token which is very cheap. This brings up a question as to how will the parser get the owned value which was stored on `Tok`. This will be solved by introducing a new `TokenValue` enum which only contains a subset of token kinds which has the owned value. This is stored on the lexer and is requested by the parser when it wants to process the data. For example: https://github.com/astral-sh/ruff/blob/8196720f809380d8f1fc7651679ff3fc2cb58cd7/crates/ruff_python_parser/src/parser/expression.rs#L1260-L1262 [^1]: Trivia tokens are `NonLogicalNewline` and `Comment` ### Remove `SoftKeywordTransformer` * #11441 * #11459 * #11442 * #11443 * #11474 For context, https://github.com/RustPython/RustPython/pull/4519/files#diff-5de40045e78e794aa5ab0b8aacf531aa477daf826d31ca129467703855408220 added support for soft keywords in the parser which uses infinite lookahead to classify a soft keyword as a keyword or an identifier. This is a brilliant idea as it basically wraps the existing Lexer and works on top of it which means that the logic for lexing and re-lexing a soft keyword remains separate. The change here is to remove `SoftKeywordTransformer` and let the parser determine this based on context, lookahead and speculative parsing. * **Context:** The transformer needs to know the position of the lexer between it being at a statement position or a simple statement position. This is because a `match` token starts a compound statement while a `type` token starts a simple statement. **The parser already knows this.** * **Lookahead:** Now that the parser knows the context it can perform lookahead of up to two tokens to classify the soft keyword. The logic for this is mentioned in the PR implementing it for `type` and `match soft keyword. * **Speculative parsing:** This is where the checkpoint - rewind infrastructure helps. For `match` soft keyword, there are certain cases for which we can't classify based on lookahead. The idea here is to create a checkpoint and keep parsing. Based on whether the parsing was successful and what tokens are ahead we can classify the remaining cases. Refer to #11443 for more details. If the soft keyword is being parsed in an identifier context, it'll be converted to an identifier and the emitted token will be updated as well. Refer https://github.com/astral-sh/ruff/blob/8196720f809380d8f1fc7651679ff3fc2cb58cd7/crates/ruff_python_parser/src/parser/expression.rs#L487-L491. The `case` soft keyword doesn't require any special handling because it'll be a keyword only in the context of a match statement. ### Update the parser API * #11494 * #11505 Now that the lexer is in sync with the parser, and the parser helps to determine whether a soft keyword is a keyword or an identifier, the lexer cannot be used on its own. The reason being that it's not sensitive to the context (which is correct). This means that the parser API needs to be updated to not allow any access to the lexer. Previously, there were multiple ways to parse the source code: 1. Passing the source code itself 2. Or, passing the tokens Now that the lexer and parser are working together, the API corresponding to (2) cannot exists. The final API is mentioned in this PR description: #11494. ### Refactor the downstream tools (linter and formatter) * #11511 * #11515 * #11529 * #11562 * #11592 And, the final set of changes involves updating all references of the lexer and `Tok` enum. This was done in two-parts: 1. Update all the references in a way that doesn't require any changes from this PR i.e., it can be done independently * #11402 * #11406 * #11418 * #11419 * #11420 * #11424 2. Update all the remaining references to use the changes made in this PR For (2), there were various strategies used: 1. Introduce a new `Tokens` struct which wraps the token vector and add methods to query a certain subset of tokens. These includes: 1. `up_to_first_unknown` which replaces the `tokenize` function 2. `in_range` and `after` which replaces the `lex_starts_at` function where the former returns the tokens within the given range while the latter returns all the tokens after the given offset 2. Introduce a new `TokenFlags` which is a set of flags to query certain information from a token. Currently, this information is only limited to any string type token but can be expanded to include other information in the future as needed. #11578 3. Move the `CommentRanges` to the parsed output because this information is common to both the linter and the formatter. This removes the need for `tokens_and_ranges` function. ## Test Plan - [x] Update and verify the test snapshots - [x] Make sure the entire test suite is passing - [x] Make sure there are no changes in the ecosystem checks - [x] Run the fuzzer on the parser - [x] Run this change on dozens of open-source projects ### Running this change on dozens of open-source projects Refer to the PR description to get the list of open source projects used for testing. Now, the following tests were done between `main` and this branch: 1. Compare the output of `--select=E999` (syntax errors) 2. Compare the output of default rule selection 3. Compare the output of `--select=ALL` **Conclusion: all output were same** ## What's next? The next step is to introduce re-lexing logic and update the parser to feed the recovery information to the lexer so that it can emit the correct token. This moves us one step closer to having error resilience in the parser and provides Ruff the possibility to lint even if the source code contains syntax errors.
Summary
This PR updates the parser API within the
ruff_python_parser
crate. It doesn't change any of the references in this PR.The final API looks like:
Following is a detailed list of changes:
Program
generic overT
which can be eitherMod
(enum),ModModule
orModExpression
Mod
intoModModule
orModExpression
Program::into_result
which converts aProgram<T>
into aResult<Program<T>, ParseError>
where theErr
variant contains the firstParseError
TokenSource
to store the comment rangesruff_python_trivia
because ofCommentRanges
. This struct could possibly be moved in the parser crate itself at the endparse_expression_starts_at
toparse_expression_range
which parses the source code at the given range usingMode::Expression
. Unlike thestarts_at
variant, this accepts the entire source codeLexer
parse_*
functions which works on the tokens provided by the callerTest Plan
The good news is that the tests in
ruff_python_parser
can be run. So,