From 014abe1ee19e617c13ac498d09de5ae2f932f6e8 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 25 Dec 2025 16:05:47 +0100 Subject: [PATCH] [ty] Fix completion in decorators with missing declaration (#22177) --- .../err/decorator_missing_expression.py | 3 + .../src/parser/expression.rs | 33 +++-- .../src/parser/statement.rs | 45 ++++-- ...yntax@decorator_missing_expression.py.snap | 138 +++++++++++++----- ..._syntax@decorator_unexpected_token.py.snap | 77 +++++++++- crates/ty_ide/src/completion.rs | 13 ++ 6 files changed, 248 insertions(+), 61 deletions(-) diff --git a/crates/ruff_python_parser/resources/inline/err/decorator_missing_expression.py b/crates/ruff_python_parser/resources/inline/err/decorator_missing_expression.py index d12890eddc..4dca3b4534 100644 --- a/crates/ruff_python_parser/resources/inline/err/decorator_missing_expression.py +++ b/crates/ruff_python_parser/resources/inline/err/decorator_missing_expression.py @@ -3,3 +3,6 @@ def foo(): ... @@ def foo(): ... +@test +@ +class Test diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index f0e930461a..b176d1e319 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -477,6 +477,17 @@ impl<'src> Parser<'src> { } } + pub(super) fn parse_missing_name(&mut self) -> ast::ExprName { + let identifier = self.parse_missing_identifier(); + + ast::ExprName { + range: identifier.range, + id: identifier.id, + ctx: ExprContext::Invalid, + node_index: AtomicNodeIndex::NONE, + } + } + /// Parses an identifier. /// /// For an invalid identifier, the `id` field will be an empty string. @@ -524,16 +535,20 @@ impl<'src> Parser<'src> { node_index: AtomicNodeIndex::NONE, } } else { - self.add_error( - ParseErrorType::OtherError("Expected an identifier".into()), - range, - ); + self.parse_missing_identifier() + } + } - ast::Identifier { - id: Name::empty(), - range: self.missing_node_range(), - node_index: AtomicNodeIndex::NONE, - } + fn parse_missing_identifier(&mut self) -> ast::Identifier { + self.add_error( + ParseErrorType::OtherError("Expected an identifier".into()), + self.current_token_range(), + ); + + ast::Identifier { + id: Name::empty(), + range: self.missing_node_range(), + node_index: AtomicNodeIndex::NONE, } } diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 07e5816a9c..0a56768df5 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -2782,13 +2782,20 @@ impl<'src> Parser<'src> { // def foo(): ... // @@ // def foo(): ... + // @test + // @ + // class Test while self.at(TokenKind::At) { progress.assert_progressing(self); let decorator_start = self.node_start(); self.bump(TokenKind::At); - let parsed_expr = self.parse_named_expression_or_higher(ExpressionContext::default()); + let parsed_expr = if self.at(TokenKind::Def) || self.at(TokenKind::Class) { + Expr::Name(self.parse_missing_name()).into() + } else { + self.parse_named_expression_or_higher(ExpressionContext::default()) + }; if self.options.target_version < PythonVersion::PY39 { // test_ok decorator_expression_dotted_ident_py38 @@ -2914,21 +2921,27 @@ impl<'src> Parser<'src> { self.current_token_range(), ); - // TODO(dhruvmanila): It seems that this recovery drops all the parsed - // decorators. Maybe we could convert them into statement expression - // with a flag indicating that this expression is part of a decorator. - // It's only possible to keep them if it's a function or class definition. - // We could possibly keep them if there's indentation error: - // - // ```python - // @decorator - // @decorator - // def foo(): ... - // ``` - // - // Or, parse it as a binary expression where the left side is missing. - // We would need to convert each decorator into a binary expression. - self.parse_statement() + let range = self.node_range(start); + + ast::StmtFunctionDef { + node_index: AtomicNodeIndex::default(), + range, + is_async: false, + decorator_list: decorators, + name: ast::Identifier { + id: Name::empty(), + range: self.missing_node_range(), + node_index: AtomicNodeIndex::NONE, + }, + type_params: None, + parameters: Box::new(ast::Parameters { + range: self.missing_node_range(), + ..ast::Parameters::default() + }), + returns: None, + body: vec![], + } + .into() } } } diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_missing_expression.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_missing_expression.py.snap index dd7225493f..5411ed313a 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_missing_expression.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_missing_expression.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs -input_file: crates/ruff_python_parser/resources/inline/err/decorator_missing_expression.py --- ## AST @@ -8,40 +7,57 @@ input_file: crates/ruff_python_parser/resources/inline/err/decorator_missing_exp Module( ModModule { node_index: NodeIndex(None), - range: 0..51, + range: 0..70, body: [ - AnnAssign( - StmtAnnAssign { + FunctionDef( + StmtFunctionDef { node_index: NodeIndex(None), - range: 5..15, - target: Call( - ExprCall { + range: 0..15, + is_async: false, + decorator_list: [ + Decorator { + range: 0..1, node_index: NodeIndex(None), - range: 5..10, - func: Name( + expression: Name( ExprName { node_index: NodeIndex(None), - range: 5..8, - id: Name("foo"), - ctx: Load, + range: 1..1, + id: Name(""), + ctx: Invalid, }, ), - arguments: Arguments { - range: 8..10, + }, + ], + name: Identifier { + id: Name("foo"), + range: 5..8, + node_index: NodeIndex(None), + }, + type_params: None, + parameters: Parameters { + range: 8..10, + node_index: NodeIndex(None), + posonlyargs: [], + args: [], + vararg: None, + kwonlyargs: [], + kwarg: None, + }, + returns: None, + body: [ + Expr( + StmtExpr { node_index: NodeIndex(None), - args: [], - keywords: [], + range: 12..15, + value: EllipsisLiteral( + ExprEllipsisLiteral { + node_index: NodeIndex(None), + range: 12..15, + }, + ), }, - }, - ), - annotation: EllipsisLiteral( - ExprEllipsisLiteral { - node_index: NodeIndex(None), - range: 12..15, - }, - ), - value: None, - simple: false, + ), + ], }, ), FunctionDef( @@ -161,6 +177,46 @@ Module( ], }, ), + ClassDef( + StmtClassDef { + node_index: NodeIndex(None), + range: 51..69, + decorator_list: [ + Decorator { + range: 51..56, + node_index: NodeIndex(None), + expression: Name( + ExprName { + node_index: NodeIndex(None), + range: 52..56, + id: Name("test"), + ctx: Load, + }, + ), + }, + Decorator { + range: 57..58, + node_index: NodeIndex(None), + expression: Name( + ExprName { + node_index: NodeIndex(None), + range: 58..58, + id: Name(""), + ctx: Invalid, + }, + ), + }, + ], + name: Identifier { + id: Name("Test"), + range: 65..69, + node_index: NodeIndex(None), + }, + type_params: None, + arguments: None, + body: [], + }, + ), ], }, ) @@ -169,15 +225,7 @@ Module( | 1 | @def foo(): ... - | ^^^ Syntax Error: Expected an identifier, but found a keyword `def` that cannot be used here -2 | @ -3 | def foo(): ... - | - - - | -1 | @def foo(): ... - | ^^^ Syntax Error: Expected newline, found name + | ^^^ Syntax Error: Expected an identifier 2 | @ 3 | def foo(): ... | @@ -199,6 +247,7 @@ Module( 4 | @@ | ^ Syntax Error: Expected an expression 5 | def foo(): ... +6 | @test | @@ -208,4 +257,23 @@ Module( 4 | @@ | ^ Syntax Error: Expected an expression 5 | def foo(): ... +6 | @test +7 | @ + | + + + | +5 | def foo(): ... +6 | @test +7 | @ + | ^ Syntax Error: Expected an expression +8 | class Test + | + + + | +6 | @test +7 | @ +8 | class Test + | ^ Syntax Error: Expected `:`, found newline | diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_unexpected_token.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_unexpected_token.py.snap index a2e7b51517..83fd68468b 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_unexpected_token.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@decorator_unexpected_token.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs -input_file: crates/ruff_python_parser/resources/inline/err/decorator_unexpected_token.py --- ## AST @@ -10,6 +9,44 @@ Module( node_index: NodeIndex(None), range: 0..34, body: [ + FunctionDef( + StmtFunctionDef { + node_index: NodeIndex(None), + range: 0..4, + is_async: false, + decorator_list: [ + Decorator { + range: 0..4, + node_index: NodeIndex(None), + expression: Name( + ExprName { + node_index: NodeIndex(None), + range: 1..4, + id: Name("foo"), + ctx: Load, + }, + ), + }, + ], + name: Identifier { + id: Name(""), + range: 4..4, + node_index: NodeIndex(None), + }, + type_params: None, + parameters: Parameters { + range: 4..4, + node_index: NodeIndex(None), + posonlyargs: [], + args: [], + vararg: None, + kwonlyargs: [], + kwarg: None, + }, + returns: None, + body: [], + }, + ), With( StmtWith { node_index: NodeIndex(None), @@ -46,6 +83,44 @@ Module( ], }, ), + FunctionDef( + StmtFunctionDef { + node_index: NodeIndex(None), + range: 23..27, + is_async: false, + decorator_list: [ + Decorator { + range: 23..27, + node_index: NodeIndex(None), + expression: Name( + ExprName { + node_index: NodeIndex(None), + range: 24..27, + id: Name("foo"), + ctx: Load, + }, + ), + }, + ], + name: Identifier { + id: Name(""), + range: 27..27, + node_index: NodeIndex(None), + }, + type_params: None, + parameters: Parameters { + range: 27..27, + node_index: NodeIndex(None), + posonlyargs: [], + args: [], + vararg: None, + kwonlyargs: [], + kwarg: None, + }, + returns: None, + body: [], + }, + ), Assign( StmtAssign { node_index: NodeIndex(None), diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index d8dabbf413..71c7952fcd 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -6686,6 +6686,19 @@ def func(): .not_contains("False"); } + #[test] + fn decorator_without_class_or_function() { + completion_test_builder( + "\ +from dataclasses import dataclass + +@dataclass(froz +", + ) + .build() + .contains("frozen"); + } + #[test] fn statement_keywords_in_if_body() { completion_test_builder(