From 1a3b73720c3c88b63ff150a2bcbd0fb2424e2dce Mon Sep 17 00:00:00 2001 From: David Peter Date: Thu, 10 Apr 2025 22:47:47 +0200 Subject: [PATCH] [red-knot] Silence errors in unreachable type annotations / class bases (#17342) ## Summary For silencing `invalid-type-form` diagnostics in unreachable code, we use the same approach that we use before and check the reachability that we already record. For silencing `invalid-bases`, we simply check if the type of the base is `Never`. If so, we silence the diagnostic with the argument that the class construction would never happen. ## Test Plan Updated Markdown tests. --- .../resources/mdtest/unreachable.md | 13 +-- crates/red_knot_python_semantic/src/types.rs | 21 ++-- .../src/types/infer.rs | 100 +++++++++++------- 3 files changed, 83 insertions(+), 51 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md b/crates/red_knot_python_semantic/resources/mdtest/unreachable.md index affacb8581..e5f22c2642 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md +++ b/crates/red_knot_python_semantic/resources/mdtest/unreachable.md @@ -447,15 +447,16 @@ We should not show any diagnostics in type annotations inside unreachable sectio ```py def _(): - class C: ... + class C: + class Inner: ... + return - # TODO - # error: [invalid-type-form] "Variable of type `Never` is not allowed in a type expression" - c: C = C() + c1: C = C() + c2: C.Inner = C.Inner() + c3: tuple[C, C] = (C(), C()) + c4: tuple[C.Inner, C.Inner] = (C.Inner(), C.Inner()) - # TODO - # error: [invalid-base] "Invalid class base with type `Never` (all bases must be a class, `Any`, `Unknown` or `Todo`)" class Sub(C): ... ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 93e6852f9a..4d3b05f28f 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -4475,17 +4475,24 @@ pub struct InvalidTypeExpressionError<'db> { } impl<'db> InvalidTypeExpressionError<'db> { - fn into_fallback_type(self, context: &InferContext, node: &ast::Expr) -> Type<'db> { + fn into_fallback_type( + self, + context: &InferContext, + node: &ast::Expr, + is_reachable: bool, + ) -> Type<'db> { let InvalidTypeExpressionError { fallback_type, invalid_expressions, } = self; - for error in invalid_expressions { - context.report_lint_old( - &INVALID_TYPE_FORM, - node, - format_args!("{}", error.reason(context.db())), - ); + if is_reachable { + for error in invalid_expressions { + context.report_lint_old( + &INVALID_TYPE_FORM, + node, + format_args!("{}", error.reason(context.db())), + ); + } } fallback_type } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index bf20c6ddf2..25f888a886 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -600,6 +600,22 @@ impl<'db> TypeInferenceBuilder<'db> { } } + /// Check if a given AST node is reachable. + /// + /// Note that this only works if reachability is explicitly tracked for this specific + /// type of node (see `node_reachability` in the use-def map). + fn is_reachable<'a, N>(&self, node: N) -> bool + where + N: Into>, + { + let file_scope_id = self.scope().file_scope_id(self.db()); + self.index.is_node_reachable( + self.db(), + file_scope_id, + self.enclosing_node_key(node.into()), + ) + } + fn in_stub(&self) -> bool { self.context.in_stub() } @@ -781,6 +797,12 @@ impl<'db> TypeInferenceBuilder<'db> { MroErrorKind::InvalidBases(bases) => { let base_nodes = class_node.bases(); for (index, base_ty) in bases { + if base_ty.is_never() { + // A class base of type `Never` can appear in unreachable code. It + // does not indicate a problem, since the actual construction of the + // class will never happen. + continue; + } self.context.report_lint_old( &INVALID_BASE, &base_nodes[*index], @@ -802,7 +824,7 @@ impl<'db> TypeInferenceBuilder<'db> { ) } } - Ok(_) => check_class_slots(&self.context, class, class_node) + Ok(_) => check_class_slots(&self.context, class, class_node), } // (4) Check that the class's metaclass can be determined without error. @@ -3120,15 +3142,12 @@ impl<'db> TypeInferenceBuilder<'db> { fn report_unresolved_import( &self, - import_node: NodeKey, + import_node: AnyNodeRef<'_>, range: TextRange, level: u32, module: Option<&str>, ) { - let file_scope_id = self.scope().file_scope_id(self.db()); - let is_import_reachable = - self.index - .is_node_reachable(self.db(), file_scope_id, import_node); + let is_import_reachable = self.is_reachable(import_node); if !is_import_reachable { return; @@ -3166,7 +3185,7 @@ impl<'db> TypeInferenceBuilder<'db> { // Resolve the module being imported. let Some(full_module_ty) = self.module_type_from_name(&full_module_name) else { - self.report_unresolved_import(NodeKey::from_node(node), alias.range(), 0, Some(name)); + self.report_unresolved_import(node.into(), alias.range(), 0, Some(name)); self.add_unknown_declaration_with_binding(alias.into(), definition); return; }; @@ -3280,8 +3299,6 @@ impl<'db> TypeInferenceBuilder<'db> { ) -> Option<(ModuleName, Type<'db>)> { let ast::StmtImportFrom { module, level, .. } = import_from; - let node_key = NodeKey::from_node(import_from); - // For diagnostics, we want to highlight the unresolvable // module and not the entire `from ... import ...` statement. let module_ref = module @@ -3310,7 +3327,12 @@ impl<'db> TypeInferenceBuilder<'db> { "Relative module resolution `{}` failed: too many leading dots", format_import_from_module(*level, module), ); - self.report_unresolved_import(node_key, module_ref.range(), *level, module); + self.report_unresolved_import( + import_from.into(), + module_ref.range(), + *level, + module, + ); return None; } Err(ModuleNameResolutionError::UnknownCurrentModule) => { @@ -3319,13 +3341,18 @@ impl<'db> TypeInferenceBuilder<'db> { format_import_from_module(*level, module), self.file().path(self.db()) ); - self.report_unresolved_import(node_key, module_ref.range(), *level, module); + self.report_unresolved_import( + import_from.into(), + module_ref.range(), + *level, + module, + ); return None; } }; let Some(module_ty) = self.module_type_from_name(&module_name) else { - self.report_unresolved_import(node_key, module_ref.range(), *level, module); + self.report_unresolved_import(import_from.into(), module_ref.range(), *level, module); return None; }; @@ -3410,12 +3437,7 @@ impl<'db> TypeInferenceBuilder<'db> { } if &alias.name != "*" { - let file_scope_id = self.scope().file_scope_id(self.db()); - let is_import_reachable = self.index.is_node_reachable( - self.db(), - file_scope_id, - NodeKey::from_node(import_from), - ); + let is_import_reachable = self.is_reachable(import_from); if is_import_reachable { self.context.report_lint_old( @@ -4503,24 +4525,16 @@ impl<'db> TypeInferenceBuilder<'db> { }) }); - let report_unresolved_usage = || { - self.index.is_node_reachable( - db, - file_scope_id, - self.enclosing_node_key(name_node.into()), - ) - }; - symbol .unwrap_with_diagnostic(|lookup_error| match lookup_error { LookupError::Unbound(qualifiers) => { - if report_unresolved_usage() { + if self.is_reachable(name_node) { report_unresolved_reference(&self.context, name_node); } TypeAndQualifiers::new(Type::unknown(), qualifiers) } LookupError::PossiblyUnbound(type_when_bound) => { - if report_unresolved_usage() { + if self.is_reachable(name_node) { report_possibly_unresolved_reference(&self.context, name_node); } type_when_bound @@ -4553,13 +4567,7 @@ impl<'db> TypeInferenceBuilder<'db> { .member(db, &attr.id) .unwrap_with_diagnostic(|lookup_error| match lookup_error { LookupError::Unbound(_) => { - let report_unresolved_attribute = self - .index - .is_node_reachable( - db, - self.scope().file_scope_id(db), - self.enclosing_node_key(attribute.into()), - ); + let report_unresolved_attribute = self.is_reachable(attribute); if report_unresolved_attribute { let bound_on_instance = match value_type { @@ -6334,7 +6342,11 @@ impl<'db> TypeInferenceBuilder<'db> { _ => name_expr_ty .in_type_expression(self.db()) .unwrap_or_else(|error| { - error.into_fallback_type(&self.context, annotation) + error.into_fallback_type( + &self.context, + annotation, + self.is_reachable(annotation), + ) }) .into(), } @@ -6499,7 +6511,13 @@ impl<'db> TypeInferenceBuilder<'db> { ast::ExprContext::Load => self .infer_name_expression(name) .in_type_expression(self.db()) - .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), + .unwrap_or_else(|error| { + error.into_fallback_type( + &self.context, + expression, + self.is_reachable(expression), + ) + }), ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => { todo_type!("Name expression annotation in Store/Del context") @@ -6510,7 +6528,13 @@ impl<'db> TypeInferenceBuilder<'db> { ast::ExprContext::Load => self .infer_attribute_expression(attribute_expression) .in_type_expression(self.db()) - .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), + .unwrap_or_else(|error| { + error.into_fallback_type( + &self.context, + expression, + self.is_reachable(expression), + ) + }), ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => { todo_type!("Attribute expression annotation in Store/Del context")