Fix all compilation errors#11612
Merged
dhruvmanila merged 2 commits intodhruv/parser-phase-2from May 30, 2024
Merged
Conversation
CodSpeed Performance ReportMerging #11612 will not alter performanceComparing Summary
|
7040795 to
a40012c
Compare
37faa87 to
68e781e
Compare
a40012c to
c137200
Compare
68e781e to
dc0ed20
Compare
c137200 to
b356439
Compare
dc0ed20 to
5f634e1
Compare
dhruvmanila
commented
May 30, 2024
crates/ruff_linter/src/linter.rs
Outdated
Comment on lines
+209
to
+211
| pycodestyle::rules::syntax_error(&mut diagnostics, &parse_error, locator); | ||
| error = Some(parse_error); | ||
| CommentRanges::default() |
Member
Author
There was a problem hiding this comment.
I'm not sure if it makes sense to consume the Program and include CommentRanges to LinterResult. I'm thinking of just cloning the ParseError here, it shouldn't affect performance at all.
Member
There was a problem hiding this comment.
Yeah we could do that too. Or you could consider adding a take_errors method to Program.
dhruvmanila
commented
May 30, 2024
|
|
||
| let program = | ||
| ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type); | ||
| ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type); |
Member
Author
There was a problem hiding this comment.
Um, this is actually a bug. Sorry, it's difficult to separate all this changes very carefully
dhruvmanila
commented
May 30, 2024
Comment on lines
+1872
to
+1876
| /// Create a new [`Lexer`] for the given source code and [`Mode`]. | ||
| pub fn lex(source: &str, mode: Mode) -> Lexer { | ||
| Lexer::new(source, mode, TextSize::default()) | ||
| } | ||
|
|
Member
Author
There was a problem hiding this comment.
We need to expose the lexer because the benchmark crate requires it.
Comment on lines
24
to
26
| fn tokenize_reverse(&self) -> Vec<SimpleToken> { | ||
| let program = parse_module(self.source).expect("Input to be a valid Python program"); | ||
| let program = parse_unchecked(self.source, Mode::Module); | ||
| BackwardsTokenizer::new(self.source, self.range, program.comment_ranges()).collect() |
Member
Author
There was a problem hiding this comment.
We're only interested in the comment ranges and the source can contain syntax errors.
Comment on lines
263
to
265
| pub fn tokens(&self, contents: &str) -> Result<String, Error> { | ||
| let program = ruff_python_parser::parse_module(contents)?; | ||
| let program = parse_unchecked(contents, Mode::Module); | ||
|
|
Member
Author
There was a problem hiding this comment.
This is just maintaining the existing behavior where the playground displays the token stream even if it contains syntax errors.
5f634e1 to
b422554
Compare
MichaReiser
approved these changes
May 30, 2024
crates/ruff_linter/src/linter.rs
Outdated
Comment on lines
+209
to
+211
| pycodestyle::rules::syntax_error(&mut diagnostics, &parse_error, locator); | ||
| error = Some(parse_error); | ||
| CommentRanges::default() |
Member
There was a problem hiding this comment.
Yeah we could do that too. Or you could consider adding a take_errors method to Program.
7aef3c9 to
39a9845
Compare
dhruvmanila
added a commit
that referenced
this pull request
May 31, 2024
## Summary This PR fixes all the compilation error as raised by `clippy`. It doesn't look at the tests. Apart from that it also does the following: * Expose a `lex` method which creates the `Lexer` object. This is used in the benchmark crate. Ideally, we shouldn't expose the lexer and in the future we should move the lexer benchmark in the parser crate * Update the parser reference in `red_knot` crate * Change all references of `&[LexResult]` to `&Tokens` * Add `CommentRanges` to the `LinterResult` as it's required after the linter run. The `Program` is consumed because it requires an owned `ParseError`
dhruvmanila
added a commit
that referenced
this pull request
Jun 3, 2024
This PR fixes all the compilation error as raised by `clippy`. It doesn't look at the tests. Apart from that it also does the following: * Expose a `lex` method which creates the `Lexer` object. This is used in the benchmark crate. Ideally, we shouldn't expose the lexer and in the future we should move the lexer benchmark in the parser crate * Update the parser reference in `red_knot` crate * Change all references of `&[LexResult]` to `&Tokens` * Add `CommentRanges` to the `LinterResult` as it's required after the linter run. The `Program` is consumed because it requires an owned `ParseError`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes all the compilation error as raised by
clippy. It doesn't look at the tests. Apart from that it also does the following:lexmethod which creates theLexerobject. This is used in the benchmark crate. Ideally, we shouldn't expose the lexer and in the future we should move the lexer benchmark in the parser cratered_knotcrate&[LexResult]to&TokensCommentRangesto theLinterResultas it's required after the linter run. TheProgramis consumed because it requires an ownedParseError