Skip to content

[red-knot] Silence errors in unreachable type annotations / class bases #17342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sharkdp merged 1 commit into from
Apr 10, 2025
Merged

[red-knot] Silence errors in unreachable type annotations / class bases #17342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions crates/red_knot_python_semantic/resources/mdtest/unreachable.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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): ...
```

Expand Down
21 changes: 14 additions & 7 deletions crates/red_knot_python_semantic/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
100 changes: 62 additions & 38 deletions crates/red_knot_python_semantic/src/types/infer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<AnyNodeRef<'a>>,
{
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()
}
Expand Down Expand Up @@ -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],
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand All @@ -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;
};

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
}
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down
Loading