Fix all compilation errors #11612
Merged
Fix all compilation errors #11612
Conversation
This file contains 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
CodSpeed Performance ReportMerging #11612 will not alter performanceComparing Summary
|
dhruvmanila
force-pushed
the
dhruv/bugfixes
branch
from
7040795
to
a40012c
Compare
dhruvmanila
force-pushed
the
dhruv/compile
branch
from
37faa87
to
68e781e
Compare
dhruvmanila
force-pushed
the
dhruv/bugfixes
branch
from
a40012c
to
c137200
Compare
dhruvmanila
force-pushed
the
dhruv/compile
branch
from
68e781e
to
dc0ed20
Compare
dhruvmanila
force-pushed
the
dhruv/bugfixes
branch
from
c137200
to
b356439
Compare
dhruvmanila
force-pushed
the
dhruv/compile
branch
from
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() |
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.
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
| ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type); | ||
| ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type); |
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()) | ||
| } | ||
|
|
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() |
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); | ||
|
|
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.
dhruvmanila
force-pushed
the
dhruv/compile
branch
from
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() |
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
force-pushed
the
dhruv/compile
branch
from
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`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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