Skip to content

Commit

Permalink
Include soft keywords for is_keyword check (#11445)
Browse files Browse the repository at this point in the history 
## Summary

This PR updates the `TokenKind::is_keyword` check to include soft
keywords. To account for this change, it adds a new
`is_non_soft_keyword` method.

The usage in logical line rules were updated to use the
`is_non_soft_keyword` method but it'll be updated to use `is_keyword` in
a follow-up PR (#11446).

While, the parser usages were kept as is. And because of that, the
snapshots for two test cases were updated in a better direction.

## Test Plan

`cargo insta test`
  • Loading branch information
@dhruvmanila
dhruvmanila authored May 17, 2024
1 parent 43e8147 commit 83152ff
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 133 deletions.
2 changes: 1 addition & 1 deletion ...ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_after_keyword.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) fn missing_whitespace_after_keyword(
let tok0_kind = tok0.kind();
let tok1_kind = tok1.kind();

if tok0_kind.is_keyword()
if tok0_kind.is_non_soft_keyword()
&& !(tok0_kind.is_singleton()
|| matches!(tok0_kind, TokenKind::Async | TokenKind::Await)
|| tok0_kind == TokenKind::Except && tok1_kind == TokenKind::Star
Expand Down
4 changes: 1 addition & 3 deletions ...ff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,7 @@ pub(crate) fn missing_whitespace_around_operator(
matches!(
prev_kind,
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace
) || !(prev_kind.is_operator()
|| prev_kind.is_keyword()
|| prev_kind.is_soft_keyword())
) || !(prev_kind.is_operator() || prev_kind.is_keyword())
};

if is_binary {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ impl LogicalLinesBuilder {

if matches!(kind, TokenKind::Comma | TokenKind::Semi | TokenKind::Colon) {
line.flags.insert(TokenFlags::PUNCTUATION);
} else if kind.is_keyword() {
} else if kind.is_non_soft_keyword() {
line.flags.insert(TokenFlags::KEYWORD);
}

Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/whitespace_around_keywords.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ pub(crate) fn whitespace_around_keywords(line: &LogicalLine, context: &mut Logic
let mut after_keyword = false;

for token in line.tokens() {
let is_keyword = token.kind().is_keyword();
if is_keyword {
let is_non_soft_keyword = token.kind().is_non_soft_keyword();
if is_non_soft_keyword {
if !after_keyword {
match line.leading_whitespace(token) {
(Whitespace::Tab, offset) => {
Expand Down Expand Up @@ -184,6 +184,6 @@ pub(crate) fn whitespace_around_keywords(line: &LogicalLine, context: &mut Logic
}
}

after_keyword = is_keyword;
after_keyword = is_non_soft_keyword;
}
}
85 changes: 33 additions & 52 deletions crates/ruff_python_parser/src/token.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl fmt::Display for Tok {
///
/// This is a lightweight representation of [`Tok`] which doesn't contain any information
/// about the token itself.
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
pub enum TokenKind {
/// Token value for a name, commonly known as an identifier.
Name,
Expand Down Expand Up @@ -485,12 +485,10 @@ pub enum TokenKind {
/// Token value for ellipsis `...`.
Ellipsis,

// Self documenting.
// Keywords (alphabetically):
False,
None,
True,
// The keywords should be sorted in alphabetical order. If the boundary tokens for the
// "Keywords" and "Soft keywords" group change, update the related methods on `TokenKind`.

// Keywords
And,
As,
Assert,
Expand All @@ -504,6 +502,7 @@ pub enum TokenKind {
Elif,
Else,
Except,
False,
Finally,
For,
From,
Expand All @@ -513,20 +512,24 @@ pub enum TokenKind {
In,
Is,
Lambda,
None,
Nonlocal,
Not,
Or,
Pass,
Raise,
Return,
True,
Try,
While,
Match,
Type,
Case,
With,
Yield,

// Soft keywords
Case,
Match,
Type,

Unknown,
}

Expand All @@ -536,45 +539,28 @@ impl TokenKind {
matches!(self, TokenKind::Newline | TokenKind::NonLogicalNewline)
}

/// Returns `true` if the token is a keyword (including soft keywords).
///
/// See also [`TokenKind::is_soft_keyword`], [`TokenKind::is_non_soft_keyword`].
#[inline]
pub const fn is_keyword(self) -> bool {
matches!(
self,
TokenKind::False
| TokenKind::True
| TokenKind::None
| TokenKind::And
| TokenKind::As
| TokenKind::Assert
| TokenKind::Await
| TokenKind::Break
| TokenKind::Class
| TokenKind::Continue
| TokenKind::Def
| TokenKind::Del
| TokenKind::Elif
| TokenKind::Else
| TokenKind::Except
| TokenKind::Finally
| TokenKind::For
| TokenKind::From
| TokenKind::Global
| TokenKind::If
| TokenKind::Import
| TokenKind::In
| TokenKind::Is
| TokenKind::Lambda
| TokenKind::Nonlocal
| TokenKind::Not
| TokenKind::Or
| TokenKind::Pass
| TokenKind::Raise
| TokenKind::Return
| TokenKind::Try
| TokenKind::While
| TokenKind::With
| TokenKind::Yield
)
pub fn is_keyword(self) -> bool {
TokenKind::And <= self && self <= TokenKind::Type
}

/// Returns `true` if the token is strictly a soft keyword.
///
/// See also [`TokenKind::is_keyword`], [`TokenKind::is_non_soft_keyword`].
#[inline]
pub fn is_soft_keyword(self) -> bool {
TokenKind::Case <= self && self <= TokenKind::Type
}

/// Returns `true` if the token is strictly a non-soft keyword.
///
/// See also [`TokenKind::is_keyword`], [`TokenKind::is_soft_keyword`].
#[inline]
pub fn is_non_soft_keyword(self) -> bool {
TokenKind::And <= self && self <= TokenKind::Yield
}

#[inline]
Expand Down Expand Up @@ -685,11 +671,6 @@ impl TokenKind {
)
}

#[inline]
pub const fn is_soft_keyword(self) -> bool {
matches!(self, TokenKind::Match | TokenKind::Case)
}

/// Returns `true` if the current token is a unary arithmetic operator.
#[inline]
pub const fn is_unary_arithmetic_operator(self) -> bool {
Expand Down
82 changes: 49 additions & 33 deletions ...ython_parser/tests/snapshots/invalid_syntax@ann_assign_stmt_type_alias_annotation.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Module(
body: [
AnnAssign(
StmtAnnAssign {
range: 0..2,
range: 0..7,
target: Name(
ExprName {
range: 0..1,
Expand All @@ -21,26 +21,27 @@ Module(
),
annotation: Name(
ExprName {
range: 2..2,
id: "",
ctx: Invalid,
range: 3..7,
id: "type",
ctx: Load,
},
),
value: None,
simple: true,
},
),
TypeAlias(
StmtTypeAlias {
range: 3..15,
name: Name(
ExprName {
range: 8..9,
id: "X",
ctx: Store,
},
),
type_params: None,
Assign(
StmtAssign {
range: 8..15,
targets: [
Name(
ExprName {
range: 8..9,
id: "X",
ctx: Store,
},
),
],
value: Name(
ExprName {
range: 12..15,
Expand All @@ -52,33 +53,34 @@ Module(
),
Expr(
StmtExpr {
range: 16..23,
range: 16..28,
value: Lambda(
ExprLambda {
range: 16..23,
range: 16..28,
parameters: None,
body: Name(
ExprName {
range: 23..23,
id: "",
ctx: Invalid,
range: 24..28,
id: "type",
ctx: Load,
},
),
},
),
},
),
TypeAlias(
StmtTypeAlias {
range: 24..36,
name: Name(
ExprName {
range: 29..30,
id: "X",
ctx: Store,
},
),
type_params: None,
Assign(
StmtAssign {
range: 29..36,
targets: [
Name(
ExprName {
range: 29..30,
id: "X",
ctx: Store,
},
),
],
value: Name(
ExprName {
range: 33..36,
Expand All @@ -96,13 +98,27 @@ Module(

|
1 | a: type X = int
| ^^^^ Syntax Error: Expected an expression
| ^^^^ Syntax Error: Expected an identifier, but found a keyword 'type' that cannot be used here
2 | lambda: type X = int
|


|
1 | a: type X = int
| ^ Syntax Error: Simple statements must be separated by newlines or semicolons
2 | lambda: type X = int
|


|
1 | a: type X = int
2 | lambda: type X = int
| ^^^^ Syntax Error: Expected an identifier, but found a keyword 'type' that cannot be used here
|


|
1 | a: type X = int
2 | lambda: type X = int
| ^^^^ Syntax Error: Expected an expression
| ^ Syntax Error: Simple statements must be separated by newlines or semicolons
|
46 changes: 6 additions & 40 deletions ...ython_parser/tests/snapshots/invalid_syntax@comprehension_missing_for_after_async.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,11 @@ Module(
Expr(
StmtExpr {
range: 0..7,
value: Generator(
ExprGenerator {
range: 0..7,
elt: Name(
ExprName {
range: 1..1,
id: "",
ctx: Invalid,
},
),
generators: [
Comprehension {
range: 1..6,
target: Name(
ExprName {
range: 6..6,
id: "",
ctx: Store,
},
),
iter: Name(
ExprName {
range: 6..6,
id: "",
ctx: Invalid,
},
),
ifs: [],
is_async: true,
},
],
parenthesized: true,
value: Name(
ExprName {
range: 1..6,
id: "async",
ctx: Load,
},
),
},
Expand Down Expand Up @@ -95,14 +68,7 @@ Module(

|
1 | (async)
| ^^^^^ Syntax Error: Expected an expression
2 | (x async x in iter)
|


|
1 | (async)
| ^ Syntax Error: Expected 'for', found ')'
| ^^^^^ Syntax Error: Expected an identifier, but found a keyword 'async' that cannot be used here
2 | (x async x in iter)
|

Expand Down

0 comments on commit 83152ff

Please sign in to comment.