From 91c712cdb122beea76deece80e172bc2691e3c20 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 29 Nov 2024 15:36:26 +0530 Subject: [PATCH] Use `Invalid` context when keyword used as identifier --- .../src/parser/expression.rs | 69 +++++++++++++------ .../ruff_python_parser/src/parser/pattern.rs | 8 +-- .../src/parser/statement.rs | 38 +++++----- ...id_syntax@assert_invalid_test_expr.py.snap | 3 +- ...tax@aug_assign_stmt_invalid_target.py.snap | 3 +- ...prehension_missing_for_after_async.py.snap | 3 +- ...ressions__attribute__multiple_dots.py.snap | 5 +- ...@expressions__attribute__no_member.py.snap | 5 +- ...xpressions__compare__invalid_order.py.snap | 3 +- ...s__dict__double_star_comprehension.py.snap | 3 +- ...ons__dict__missing_closing_brace_0.py.snap | 5 +- ...sions__named__missing_expression_2.py.snap | 5 +- ...sions__subscript__unclosed_slice_1.py.snap | 5 +- 13 files changed, 85 insertions(+), 70 deletions(-) diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index 62b42b6c5e..4982b8112e 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -7,7 +7,7 @@ use rustc_hash::{FxBuildHasher, FxHashSet}; use ruff_python_ast::name::Name; use ruff_python_ast::{ self as ast, BoolOp, CmpOp, ConversionFlag, Expr, ExprContext, FStringElement, FStringElements, - IpyEscapeKind, Number, Operator, UnaryOp, + Identifier, IpyEscapeKind, Number, Operator, UnaryOp, }; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -446,18 +446,13 @@ impl<'src> Parser<'src> { /// Parses a name. /// - /// For an invalid name, the `id` field will be an empty string and the `ctx` - /// field will be [`ExprContext::Invalid`]. + /// For an invalid name, the `id` field will be an empty string and if a keyword is used, + /// the `id` field will be the keyword name. In both cases, the `ctx` field will be + /// [`ExprContext::Invalid`]. /// /// See: pub(super) fn parse_name(&mut self) -> ast::ExprName { - let identifier = self.parse_identifier(); - - let ctx = if identifier.is_valid() { - ExprContext::Load - } else { - ExprContext::Invalid - }; + let (identifier, ctx) = self.parse_identifier().unwrap_with_context(); ast::ExprName { range: identifier.range, @@ -471,20 +466,20 @@ impl<'src> Parser<'src> { /// For an invalid identifier, the `id` field will be an empty string. /// /// See: - pub(super) fn parse_identifier(&mut self) -> ast::Identifier { + pub(super) fn parse_identifier(&mut self) -> IdentifierParseResult { let range = self.current_token_range(); if self.at(TokenKind::Name) { let TokenValue::Name(name) = self.bump_value(TokenKind::Name) else { unreachable!(); }; - return ast::Identifier { id: name, range }; + return IdentifierParseResult::ok(name, range); } if self.current_token_kind().is_soft_keyword() { - let id = Name::new(self.src_text(range)); + let name = Name::new(self.src_text(range)); self.bump_soft_keyword_as_name(); - return ast::Identifier { id, range }; + return IdentifierParseResult::ok(name, range); } if self.current_token_kind().is_keyword() { @@ -497,19 +492,16 @@ impl<'src> Parser<'src> { range, ); - let id = Name::new(self.src_text(range)); + let name = Name::new(self.src_text(range)); self.bump_any(); - ast::Identifier { id, range } + IdentifierParseResult::err(name, range) } else { self.add_error( ParseErrorType::OtherError("Expected an identifier".into()), range, ); - ast::Identifier { - id: Name::empty(), - range: self.missing_node_range(), - } + IdentifierParseResult::err(Name::empty(), self.missing_node_range()) } } @@ -942,12 +934,12 @@ impl<'src> Parser<'src> { ) -> ast::ExprAttribute { self.bump(TokenKind::Dot); - let attr = self.parse_identifier(); + let (attr, ctx) = self.parse_identifier().unwrap_with_context(); ast::ExprAttribute { value: Box::new(value), attr, - ctx: ExprContext::Load, + ctx, range: self.node_range(start), } } @@ -2587,3 +2579,36 @@ impl ExpressionContext { } } } + +/// A result-like type that represents the result of parsing an identifier. +pub(super) struct IdentifierParseResult(Result); + +impl IdentifierParseResult { + fn ok(name: Name, range: TextRange) -> Self { + IdentifierParseResult(Ok(Identifier { id: name, range })) + } + + fn err(name: Name, range: TextRange) -> Self { + IdentifierParseResult(Err(Identifier { id: name, range })) + } + + /// Unwraps the result and returns the inner [`Identifier`]. + pub(super) fn into_inner(self) -> Identifier { + match self.0 { + Ok(identifier) | Err(identifier) => identifier, + } + } + + /// Unwraps the result and returns the inner [`Identifier`] along with the [`ExprContext`]. + /// + /// If the result is [`Ok`], the context is [`Load`], otherwise it's [`Invalid`]. + /// + /// [`Load`]: ExprContext::Load + /// [`Invalid`]: ExprContext::Invalid + fn unwrap_with_context(self) -> (Identifier, ExprContext) { + match self.0 { + Ok(identifier) => (identifier, ExprContext::Load), + Err(identifier) => (identifier, ExprContext::Invalid), + } + } +} diff --git a/crates/ruff_python_parser/src/parser/pattern.rs b/crates/ruff_python_parser/src/parser/pattern.rs index 80b6b294f0..35869f7cf6 100644 --- a/crates/ruff_python_parser/src/parser/pattern.rs +++ b/crates/ruff_python_parser/src/parser/pattern.rs @@ -120,7 +120,7 @@ impl Parser<'_> { self.add_error(ParseErrorType::InvalidStarPatternUsage, &lhs); } - let ident = self.parse_identifier(); + let ident = self.parse_identifier().into_inner(); lhs = Pattern::MatchAs(ast::PatternMatchAs { range: self.node_range(start), name: Some(ident), @@ -183,7 +183,7 @@ impl Parser<'_> { let mapping_item_start = parser.node_start(); if parser.eat(TokenKind::DoubleStar) { - let identifier = parser.parse_identifier(); + let identifier = parser.parse_identifier().into_inner(); if rest.is_some() { parser.add_error( ParseErrorType::OtherError( @@ -258,7 +258,7 @@ impl Parser<'_> { let start = self.node_start(); self.bump(TokenKind::Star); - let ident = self.parse_identifier(); + let ident = self.parse_identifier().into_inner(); ast::PatternMatchStar { range: self.node_range(start), @@ -490,7 +490,7 @@ impl Parser<'_> { // case case: ... // case match: ... // case type: ... - let ident = self.parse_identifier(); + let ident = self.parse_identifier().into_inner(); // test_ok match_as_pattern // match foo: diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index e10487e18c..1bab26a7bf 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -635,7 +635,7 @@ impl<'src> Parser<'src> { let name = match style { ImportStyle::Import => self.parse_dotted_name(), - ImportStyle::ImportFrom => self.parse_identifier(), + ImportStyle::ImportFrom => self.parse_identifier().into_inner(), }; let asname = if self.eat(TokenKind::As) { @@ -644,7 +644,7 @@ impl<'src> Parser<'src> { // import foo as match // import bar as case // import baz as type - Some(self.parse_identifier()) + Some(self.parse_identifier().into_inner()) } else { // test_err import_alias_missing_asname // import x as @@ -671,7 +671,7 @@ impl<'src> Parser<'src> { fn parse_dotted_name(&mut self) -> ast::Identifier { let start = self.node_start(); - let mut dotted_name: CompactString = self.parse_identifier().id.into(); + let mut dotted_name: CompactString = self.parse_identifier().into_inner().id.into(); let mut progress = ParserProgress::default(); while self.eat(TokenKind::Dot) { @@ -681,7 +681,7 @@ impl<'src> Parser<'src> { // import a..b // import a...b dotted_name.push('.'); - dotted_name.push_str(&self.parse_identifier()); + dotted_name.push_str(&self.parse_identifier().into_inner()); } // test_ok dotted_name_normalized_spaces @@ -805,10 +805,10 @@ impl<'src> Parser<'src> { // test_err global_stmt_expression // global x + 1 - let names = self.parse_comma_separated_list_into_vec( - RecoveryContextKind::Identifiers, - Parser::parse_identifier, - ); + let names = self + .parse_comma_separated_list_into_vec(RecoveryContextKind::Identifiers, |p| { + p.parse_identifier().into_inner() + }); if names.is_empty() { // test_err global_stmt_empty @@ -843,10 +843,10 @@ impl<'src> Parser<'src> { // test_err nonlocal_stmt_expression // nonlocal x + 1 - let names = self.parse_comma_separated_list_into_vec( - RecoveryContextKind::Identifiers, - Parser::parse_identifier, - ); + let names = self + .parse_comma_separated_list_into_vec(RecoveryContextKind::Identifiers, |p| { + p.parse_identifier().into_inner() + }); if names.is_empty() { // test_err nonlocal_stmt_empty @@ -1516,7 +1516,7 @@ impl<'src> Parser<'src> { // except Exception as match: ... // except Exception as case: ... // except Exception as type: ... - Some(self.parse_identifier()) + Some(self.parse_identifier().into_inner()) } else { // test_err except_stmt_missing_as_name // try: @@ -1712,7 +1712,7 @@ impl<'src> Parser<'src> { // test_err function_def_missing_identifier // def (): ... // def () -> int: ... - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); // test_err function_def_unclosed_type_param_list // def foo[T1, *T2(a, b): @@ -1826,7 +1826,7 @@ impl<'src> Parser<'src> { // class : ... // class (): ... // class (metaclass=ABC): ... - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); // test_err class_def_unclosed_type_param_list // class Foo[T1, *T2(a, b): @@ -2631,7 +2631,7 @@ impl<'src> Parser<'src> { function_kind: FunctionKind, allow_star_annotation: AllowStarAnnotation, ) -> ast::Parameter { - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); // Annotations are only allowed for function definition. For lambda expression, // the `:` token would indicate its body. @@ -3068,7 +3068,7 @@ impl<'src> Parser<'src> { // type X[T, *Ts] = int // type X[T, *Ts = int] = int if self.eat(TokenKind::Star) { - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); let default = if self.eat(TokenKind::Equal) { if self.at_expr() { @@ -3112,7 +3112,7 @@ impl<'src> Parser<'src> { // type X[T, **P] = int // type X[T, **P = int] = int } else if self.eat(TokenKind::DoubleStar) { - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); let default = if self.eat(TokenKind::Equal) { if self.at_expr() { @@ -3151,7 +3151,7 @@ impl<'src> Parser<'src> { // type X[T: (int, int) = int] = int // type X[T: int = int, U: (int, int) = int] = int } else { - let name = self.parse_identifier(); + let name = self.parse_identifier().into_inner(); let bound = if self.eat(TokenKind::Colon) { if self.at_expr() { diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@assert_invalid_test_expr.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@assert_invalid_test_expr.py.snap index afe4aca2ba..346263bf4f 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@assert_invalid_test_expr.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@assert_invalid_test_expr.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/inline/err/assert_invalid_test_expr.py -snapshot_kind: text --- ## AST @@ -36,7 +35,7 @@ Module( ExprName { range: 17..23, id: Name("assert"), - ctx: Load, + ctx: Invalid, }, ), msg: None, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@aug_assign_stmt_invalid_target.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@aug_assign_stmt_invalid_target.py.snap index 17b749427a..729efe2fe4 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@aug_assign_stmt_invalid_target.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@aug_assign_stmt_invalid_target.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/inline/err/aug_assign_stmt_invalid_target.py -snapshot_kind: text --- ## AST @@ -134,7 +133,7 @@ Module( ExprName { range: 41..45, id: Name("pass"), - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@comprehension_missing_for_after_async.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@comprehension_missing_for_after_async.py.snap index 8713ae5f80..8d507c7567 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@comprehension_missing_for_after_async.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@comprehension_missing_for_after_async.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/inline/err/comprehension_missing_for_after_async.py -snapshot_kind: text --- ## AST @@ -17,7 +16,7 @@ Module( ExprName { range: 1..6, id: Name("async"), - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__multiple_dots.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__multiple_dots.py.snap index a6c1f3f614..a912699843 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__multiple_dots.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__multiple_dots.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/attribute/multiple_dots.py -snapshot_kind: text --- ## AST @@ -30,7 +29,7 @@ Module( id: Name(""), range: 6..6, }, - ctx: Load, + ctx: Invalid, }, ), attr: Identifier { @@ -104,7 +103,7 @@ Module( id: Name(""), range: 40..40, }, - ctx: Load, + ctx: Invalid, }, ), attr: Identifier { diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__no_member.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__no_member.py.snap index 9de860a7e9..d3e073d60b 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__no_member.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__attribute__no_member.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/attribute/no_member.py -snapshot_kind: text --- ## AST @@ -27,7 +26,7 @@ Module( id: Name(""), range: 93..93, }, - ctx: Load, + ctx: Invalid, }, ), }, @@ -61,7 +60,7 @@ Module( id: Name(""), range: 141..141, }, - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__compare__invalid_order.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__compare__invalid_order.py.snap index bb9cf68340..4efc6a21f2 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__compare__invalid_order.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__compare__invalid_order.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/compare/invalid_order.py -snapshot_kind: text --- ## AST @@ -106,7 +105,7 @@ Module( ExprName { range: 126..128, id: Name("is"), - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__double_star_comprehension.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__double_star_comprehension.py.snap index a64d76004a..c3df1875e8 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__double_star_comprehension.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__double_star_comprehension.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/dict/double_star_comprehension.py -snapshot_kind: text --- ## AST @@ -41,7 +40,7 @@ Module( ExprName { range: 130..133, id: Name("for"), - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__missing_closing_brace_0.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__missing_closing_brace_0.py.snap index 1772420cc3..c98fa6510d 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__missing_closing_brace_0.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__dict__missing_closing_brace_0.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/dict/missing_closing_brace_0.py -snapshot_kind: text --- ## AST @@ -31,7 +30,7 @@ Module( ExprName { range: 5..8, id: Name("def"), - ctx: Load, + ctx: Invalid, }, ), }, @@ -59,7 +58,7 @@ Module( ExprName { range: 20..24, id: Name("pass"), - ctx: Load, + ctx: Invalid, }, ), }, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__named__missing_expression_2.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__named__missing_expression_2.py.snap index 56f90d1fe0..2a54f8c990 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__named__missing_expression_2.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__named__missing_expression_2.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/named/missing_expression_2.py -snapshot_kind: text --- ## AST @@ -27,7 +26,7 @@ Module( ExprName { range: 68..71, id: Name("def"), - ctx: Load, + ctx: Invalid, }, ), }, @@ -58,7 +57,7 @@ Module( ExprName { range: 83..87, id: Name("pass"), - ctx: Load, + ctx: Invalid, }, ), value: None, diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__subscript__unclosed_slice_1.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__subscript__unclosed_slice_1.py.snap index bd258eb055..0aceda3b59 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__subscript__unclosed_slice_1.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__subscript__unclosed_slice_1.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/invalid/expressions/subscript/unclosed_slice_1.py -snapshot_kind: text --- ## AST @@ -33,7 +32,7 @@ Module( ExprName { range: 6..9, id: Name("def"), - ctx: Load, + ctx: Invalid, }, ), ), @@ -68,7 +67,7 @@ Module( ExprName { range: 21..25, id: Name("pass"), - ctx: Load, + ctx: Invalid, }, ), value: None,