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

Enable token-based rules on source with syntax errors #11950

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jun 20, 2024

Summary

This PR updates the linter, specifically the token-based rules, to work on the tokens that come after a syntax error.

For context, the token-based rules only diagnose the tokens up to the first lexical error. This PR builds up an error resilience by introducing a TokenIterWithContext which updates the nesting level and tries to reflect it with what the lexer is seeing. This isn't 100% accurate because if the parser recovered from an unclosed parenthesis in the middle of the line, the context won't reduce the nesting level until it sees the newline token at the end of the line.

resolves: #11915

Test Plan

@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label Jun 20, 2024
@dhruvmanila dhruvmanila force-pushed the dhruv/token-rules-with-syntax-errors branch from b7134c9 to 7db979b Compare June 20, 2024 12:16
Comment on lines -96 to 97
for token in tokens.up_to_first_unknown() {
for token in tokens {
pylint::rules::invalid_string_characters(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is looking at string tokens and the lexer doesn't emit them if it's unterminated. So, we might get away with not doing anything in this case for now.

Comment on lines 24 to 29
impl<'a> DocLines<'a> {
fn new(tokens: &'a Tokens) -> Self {
Self {
inner: tokens.up_to_first_unknown().iter(),
inner: tokens.iter(),
prev: TextSize::default(),
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This extracts a specific set of comments so it doesn't require any specific update.

Comment on lines 256 to 260
_ => {
kind => {
if matches!(kind, TokenKind::Newline if fstrings > 0) {
// The parser recovered from an unterminated f-string.
fstrings = 0;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should work as the newline tokens within f-strings are actually NonLogicalNewline, I'll move this into TokenIterWithContext. I'll test this a lot because f-strings are complex.

@dhruvmanila dhruvmanila force-pushed the dhruv/token-rules-with-syntax-errors branch from 7db979b to 1961406 Compare June 28, 2024 05:58
Comment on lines +110 to -115
for token in tokens {
match token.kind() {
TokenKind::EndOfFile => {
break;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

The token stream doesn't contain the EndOfFile token.

Copy link
Contributor

github-actions bot commented Jun 28, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

demisto/content (error)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'unfixable' -> 'lint.unfixable'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
  Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

demisto/content (error)

ruff format --preview --exclude Packs/ThreatQ/Integrations/ThreatQ/ThreatQ.py

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'unfixable' -> 'lint.unfixable'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
  Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.

@dhruvmanila dhruvmanila force-pushed the dhruv/token-rules-with-syntax-errors branch from 6e96839 to f3bbacd Compare June 28, 2024 11:26
@dhruvmanila dhruvmanila changed the base branch from main to dhruv/revert June 28, 2024 11:26
@dhruvmanila dhruvmanila force-pushed the dhruv/token-rules-with-syntax-errors branch from f3bbacd to 7b42997 Compare June 28, 2024 11:27
dhruvmanila added a commit that referenced this pull request Jun 28, 2024
This PR reverts #12016 with a
small change where the error location points to the continuation
character only. Earlier, it would also highlight the whitespace that
came before it.

The motivation for this change is to avoid panic in
#11950. For example:

```py
\)
```

Playground: https://play.ruff.rs/87711071-1b54-45a3-b45a-81a336a1ea61

The range of `Unknown` token and `Rpar` is the same. Once #11950 is
enabled, the indexer would panic. It won't panic in the stable version
because we stop at the first `Unknown` token.
Base automatically changed from dhruv/revert to main June 28, 2024 12:40
@dhruvmanila dhruvmanila force-pushed the dhruv/token-rules-with-syntax-errors branch from 7b42997 to eeb24b1 Compare June 28, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow token-based rules to work on source code with syntax errors
1 participant