From 4aba390838e8528bdb6b4c3909d2f23e3ae47762 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 27 Nov 2024 10:04:16 +0100 Subject: [PATCH 1/2] [red-knot] No panic for illegal self-referential f-string annotations --- .../resources/mdtest/annotations/string.md | 1 - .../resources/mdtest/call/function.md | 2 + .../resources/mdtest/literal/literal.md | 2 + .../src/types/infer.rs | 229 +++++++++--------- crates/red_knot_workspace/tests/check.rs | 3 - crates/ruff_benchmark/benches/red_knot.rs | 4 + 6 files changed, 129 insertions(+), 112 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/annotations/string.md b/crates/red_knot_python_semantic/resources/mdtest/annotations/string.md index 140a9ad0033c7e..c7da9df28fd283 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/annotations/string.md +++ b/crates/red_knot_python_semantic/resources/mdtest/annotations/string.md @@ -125,7 +125,6 @@ def f7() -> "\x69nt": def f8() -> """int""": return 1 -# error: [annotation-byte-string] "Type expressions cannot use bytes literal" def f9() -> "b'int'": return 1 diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/function.md b/crates/red_knot_python_semantic/resources/mdtest/call/function.md index b67b14d5585854..965ab483a3def9 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/call/function.md +++ b/crates/red_knot_python_semantic/resources/mdtest/call/function.md @@ -36,6 +36,8 @@ from typing import Callable def foo() -> int: return 42 +# TODO: This should be resolved once we understand `Callable` annotations +# error: [annotation-with-invalid-expression] def decorator(func) -> Callable[[], int]: return foo diff --git a/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md b/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md index 0fb01df9a7f04a..d6bd8f93a9d063 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md +++ b/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md @@ -75,6 +75,8 @@ Literal: _SpecialForm ```py from other import Literal +# TODO: we should not emit this error once we understand Literal[..] annotations +# error: [annotation-with-invalid-expression] a1: Literal[26] def f(): diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 7224ac0c9dabaa..1ef1b1ff273e92 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -31,6 +31,7 @@ use std::num::NonZeroU32; use itertools::Itertools; use ruff_db::files::File; use ruff_db::parsed::parsed_module; +use ruff_python_ast::visitor::{self, Visitor}; use ruff_python_ast::{self as ast, AnyNodeRef, Expr, ExprContext, UnaryOp}; use rustc_hash::{FxHashMap, FxHashSet}; use salsa; @@ -4214,25 +4215,6 @@ impl<'db> TypeInferenceBuilder<'db> { // Annotation expressions also get special handling for `*args` and `**kwargs`. ast::Expr::Starred(starred) => self.infer_starred_expression(starred), - ast::Expr::BytesLiteral(bytes) => { - self.diagnostics.add( - bytes.into(), - "annotation-byte-string", - format_args!("Type expressions cannot use bytes literal"), - ); - Type::Unknown - } - - ast::Expr::FString(fstring) => { - self.diagnostics.add( - fstring.into(), - "annotation-f-string", - format_args!("Type expressions cannot use f-strings"), - ); - self.infer_fstring_expression(fstring); - Type::Unknown - } - // All other annotation expressions are (possibly) valid type expressions, so handle // them there instead. type_expr => self.infer_type_expression_no_store(type_expr), @@ -4243,6 +4225,72 @@ impl<'db> TypeInferenceBuilder<'db> { annotation_ty } + /// Walk child expressions of the given AST node and store the given type for each of them. + /// Does not store a type for the root expression. Resets to normal type inference for all + /// expressions with a nested scope (lambda, named expression, comprehensions, generators). + fn store_type_for_sub_expressions_of(&mut self, expression: &ast::Expr, ty: Type<'db>) { + struct StoreTypeVisitor<'a, 'db> { + builder: &'a mut TypeInferenceBuilder<'db>, + ty: Type<'db>, + is_root_node: bool, + } + + impl<'a, 'db> StoreTypeVisitor<'a, 'db> { + fn new(builder: &'a mut TypeInferenceBuilder<'db>, ty: Type<'db>) -> Self { + Self { + builder, + ty, + is_root_node: true, + } + } + + fn store(&mut self, expr: &ast::Expr) { + if self.is_root_node { + self.is_root_node = false; + } else { + self.builder.store_expression_type(expr, self.ty); + } + } + } + + impl<'a, 'db, 'ast> Visitor<'ast> for StoreTypeVisitor<'a, 'db> { + fn visit_expr(&mut self, expr: &'ast Expr) { + match expr { + ast::Expr::Lambda(lambda) => { + self.builder.infer_lambda_expression(lambda); + self.store(expr); + } + ast::Expr::Named(named) => { + self.builder.infer_named_expression(named); + self.store(expr); + } + ast::Expr::ListComp(list_comp) => { + self.builder.infer_list_comprehension_expression(list_comp); + self.store(expr); + } + ast::Expr::SetComp(set_comp) => { + self.builder.infer_set_comprehension_expression(set_comp); + self.store(expr); + } + ast::Expr::DictComp(dict_comp) => { + self.builder.infer_dict_comprehension_expression(dict_comp); + self.store(expr); + } + ast::Expr::Generator(generator) => { + self.builder.infer_generator_expression(generator); + self.store(expr); + } + _ => { + self.store(expr); + visitor::walk_expr(self, expr); + } + } + } + } + + StoreTypeVisitor::new(self, ty).visit_expr(expression); + } + /// Infer the type of a string annotation expression. fn infer_string_annotation_expression(&mut self, string: &ast::ExprStringLiteral) -> Type<'db> { match parse_string_annotation(self.db, self.file, string) { @@ -4324,13 +4372,6 @@ impl<'db> TypeInferenceBuilder<'db> { // expressions, but is meaningful in the context of a number of special forms. ast::Expr::EllipsisLiteral(_literal) => todo_type!(), - // Other literals do not have meaningful values in the annotation expression context. - // However, we will we want to handle these differently when working with special forms, - // since (e.g.) `123` is not valid in an annotation expression but `Literal[123]` is. - ast::Expr::BytesLiteral(_literal) => todo_type!(), - ast::Expr::NumberLiteral(_literal) => todo_type!(), - ast::Expr::BooleanLiteral(_literal) => todo_type!(), - ast::Expr::Subscript(subscript) => { let ast::ExprSubscript { value, @@ -4379,90 +4420,62 @@ impl<'db> TypeInferenceBuilder<'db> { // string annotation, as they are not present in the semantic index. _ if self.deferred_state.in_string_annotation() => Type::Unknown, - // Forms which are invalid in the context of annotation expressions: we infer their - // nested expressions as normal expressions, but the type of the top-level expression is - // always `Type::Unknown` in these cases. - ast::Expr::BoolOp(bool_op) => { - self.infer_boolean_expression(bool_op); - Type::Unknown - } - ast::Expr::Named(named) => { - self.infer_named_expression(named); - Type::Unknown - } - ast::Expr::UnaryOp(unary) => { - self.infer_unary_expression(unary); - Type::Unknown - } - ast::Expr::Lambda(lambda_expression) => { - self.infer_lambda_expression(lambda_expression); - Type::Unknown - } - ast::Expr::If(if_expression) => { - self.infer_if_expression(if_expression); - Type::Unknown - } - ast::Expr::Dict(dict) => { - self.infer_dict_expression(dict); - Type::Unknown - } - ast::Expr::Set(set) => { - self.infer_set_expression(set); - Type::Unknown - } - ast::Expr::ListComp(listcomp) => { - self.infer_list_comprehension_expression(listcomp); - Type::Unknown - } - ast::Expr::SetComp(setcomp) => { - self.infer_set_comprehension_expression(setcomp); - Type::Unknown - } - ast::Expr::DictComp(dictcomp) => { - self.infer_dict_comprehension_expression(dictcomp); - Type::Unknown - } - ast::Expr::Generator(generator) => { - self.infer_generator_expression(generator); - Type::Unknown - } - ast::Expr::Await(await_expression) => { - self.infer_await_expression(await_expression); - Type::Unknown - } - ast::Expr::Yield(yield_expression) => { - self.infer_yield_expression(yield_expression); - Type::Unknown - } - ast::Expr::YieldFrom(yield_from) => { - self.infer_yield_from_expression(yield_from); - Type::Unknown - } - ast::Expr::Compare(compare) => { - self.infer_compare_expression(compare); - Type::Unknown - } - ast::Expr::Call(call_expr) => { - self.infer_call_expression(call_expr); - Type::Unknown - } - ast::Expr::FString(fstring) => { - self.infer_fstring_expression(fstring); - Type::Unknown - } - ast::Expr::List(list) => { - self.infer_list_expression(list); - Type::Unknown - } - ast::Expr::Tuple(tuple) => { - self.infer_tuple_expression(tuple); - Type::Unknown - } - ast::Expr::Slice(slice) => { - self.infer_slice_expression(slice); + // Forms which are invalid in the context of annotation expressions: we store a type of + // `Type::Unknown` for these expressions (and their sub-expressions) to avoid problems + // with invalid-syntax examples like `x: f"{x}"` or `x: lambda y: x`, for which we can + // not infer a meaningful type for the inner `x` expression. The top-level expression + // is also `Type::Unknown` in these cases. + ast::Expr::BoolOp(_) + | ast::Expr::Named(_) + | ast::Expr::UnaryOp(_) + | ast::Expr::Lambda(_) + | ast::Expr::If(_) + | ast::Expr::Dict(_) + | ast::Expr::Set(_) + | ast::Expr::ListComp(_) + | ast::Expr::SetComp(_) + | ast::Expr::DictComp(_) + | ast::Expr::Generator(_) + | ast::Expr::Await(_) + | ast::Expr::Yield(_) + | ast::Expr::YieldFrom(_) + | ast::Expr::Compare(_) + | ast::Expr::Call(_) + | ast::Expr::FString(_) + | ast::Expr::List(_) + | ast::Expr::Tuple(_) + | ast::Expr::Slice(_) + | ast::Expr::IpyEscapeCommand(_) + | ast::Expr::BytesLiteral(_) + | ast::Expr::NumberLiteral(_) + | ast::Expr::BooleanLiteral(_) => { + match expression { + ast::Expr::BytesLiteral(bytes) => { + self.diagnostics.add( + bytes.into(), + "annotation-byte-string", + format_args!("Type expressions cannot use bytes literal"), + ); + } + ast::Expr::FString(fstring) => { + self.diagnostics.add( + fstring.into(), + "annotation-f-string", + format_args!("Type expressions cannot use f-strings"), + ); + } + _ => { + self.diagnostics.add( + expression.into(), + "annotation-with-invalid-expression", + format_args!("Invalid expression in type expression"), + ); + } + } + + self.store_type_for_sub_expressions_of(expression, Type::Unknown); Type::Unknown } - ast::Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"), } } diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index f3be6651514138..50c1c9bd9643c6 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -272,7 +272,4 @@ const KNOWN_FAILURES: &[(&str, bool, bool)] = &[ ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP039.py", true, false), // related to circular references in type aliases (salsa cycle panic): ("crates/ruff_python_parser/resources/inline/err/type_alias_invalid_value_expr.py", true, true), - // related to circular references in f-string annotations (invalid syntax) - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_15.py", true, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_14.py", false, true), ]; diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index 60e9845f0e143d..bdebea5918b71d 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -27,13 +27,17 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ // We don't support `*` imports yet: "error[unresolved-import] /src/tomllib/_parser.py:7:29 Module `collections.abc` has no member `Iterable`", // We don't support terminal statements in control flow yet: + "error[annotation-with-invalid-expression] /src/tomllib/_parser.py:57:71 Invalid expression in type expression", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:66:18 Name `s` used when possibly not defined", + "error[annotation-with-invalid-expression] /src/tomllib/_parser.py:69:66 Invalid expression in type expression", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:98:12 Name `char` used when possibly not defined", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:101:12 Name `char` used when possibly not defined", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:104:14 Name `char` used when possibly not defined", "error[conflicting-declarations] /src/tomllib/_parser.py:108:17 Conflicting declared types for `second_char`: Unknown, str | None", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:115:14 Name `char` used when possibly not defined", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:126:12 Name `char` used when possibly not defined", + "error[annotation-with-invalid-expression] /src/tomllib/_parser.py:145:27 Invalid expression in type expression", + "error[annotation-with-invalid-expression] /src/tomllib/_parser.py:196:25 Invalid expression in type expression", "error[conflicting-declarations] /src/tomllib/_parser.py:267:9 Conflicting declared types for `char`: Unknown, str | None", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:348:20 Name `nest` used when possibly not defined", "error[possibly-unresolved-reference] /src/tomllib/_parser.py:353:5 Name `nest` used when possibly not defined", From 36ba9707ca565d54faf2a67838182918f9c6c846 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 27 Nov 2024 13:19:23 +0100 Subject: [PATCH 2/2] Remove TODO message --- .../red_knot_python_semantic/resources/mdtest/literal/literal.md | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md b/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md index d6bd8f93a9d063..e75371a1d592ae 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md +++ b/crates/red_knot_python_semantic/resources/mdtest/literal/literal.md @@ -75,7 +75,6 @@ Literal: _SpecialForm ```py from other import Literal -# TODO: we should not emit this error once we understand Literal[..] annotations # error: [annotation-with-invalid-expression] a1: Literal[26]