mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 10:48:32 +00:00
[red-knot] Silence errors in unreachable type annotations / class bases (#17342)
Some checks are pending
CI / test ruff-lsp (push) Blocked by required conditions
CI / cargo clippy (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / check playground (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions
[Knot Playground] Release / publish (push) Waiting to run
Some checks are pending
CI / test ruff-lsp (push) Blocked by required conditions
CI / cargo clippy (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / check playground (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions
[Knot Playground] Release / publish (push) Waiting to run
## 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.
This commit is contained in:
parent
8b2727cf67
commit
1a3b73720c
3 changed files with 83 additions and 51 deletions
|
@ -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): ...
|
||||
```
|
||||
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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()
|
||||
}
|
||||
|
@ -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")
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue