Skip to content

Commit

Permalink
[red-knot] Do not attach diagnostics to wrong file (astral-sh#14337)
Browse files Browse the repository at this point in the history
## Summary

Avoid attaching diagnostics to the wrong file. See related issue for
details.

Closes astral-sh#14334

## Test Plan

New regression test.
  • Loading branch information
sharkdp authored Nov 14, 2024
1 parent ec2c7ca commit 9a3001b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Regression test for #14334

Regression test for [this issue](https://github.com/astral-sh/ruff/issues/14334).

```py path=base.py
# error: [invalid-base]
class Base(2): ...
```

```py path=a.py
# No error here
from base import Base
```
24 changes: 15 additions & 9 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,16 +469,22 @@ impl<'db> TypeInferenceBuilder<'db> {
let class_definitions = self
.types
.declarations
.values()
.filter_map(|ty| ty.into_class_literal())
.map(|class_ty| class_ty.class);
.iter()
.filter_map(|(definition, ty)| {
// Filter out class literals that result from imports
if let DefinitionKind::Class(class) = definition.kind(self.db) {
ty.into_class_literal().map(|ty| (ty.class, class.node()))
} else {
None
}
});

// Iterate through all class definitions in this scope.
for class in class_definitions {
for (class, class_node) in class_definitions {
// (1) Check that the class does not have a cyclic definition
if class.is_cyclically_defined(self.db) {
self.diagnostics.add(
class.node(self.db).into(),
class_node.into(),
"cyclic-class-def",
format_args!(
"Cyclic definition of `{}` or bases of `{}` (class cannot inherit from itself)",
Expand All @@ -495,7 +501,7 @@ impl<'db> TypeInferenceBuilder<'db> {
if let Err(mro_error) = class.try_mro(self.db).as_ref() {
match mro_error.reason() {
MroErrorKind::DuplicateBases(duplicates) => {
let base_nodes = class.node(self.db).bases();
let base_nodes = class_node.bases();
for (index, duplicate) in duplicates {
self.diagnostics.add(
(&base_nodes[*index]).into(),
Expand All @@ -505,7 +511,7 @@ impl<'db> TypeInferenceBuilder<'db> {
}
}
MroErrorKind::InvalidBases(bases) => {
let base_nodes = class.node(self.db).bases();
let base_nodes = class_node.bases();
for (index, base_ty) in bases {
self.diagnostics.add(
(&base_nodes[*index]).into(),
Expand All @@ -518,7 +524,7 @@ impl<'db> TypeInferenceBuilder<'db> {
}
}
MroErrorKind::UnresolvableMro { bases_list } => self.diagnostics.add(
class.node(self.db).into(),
class_node.into(),
"inconsistent-mro",
format_args!(
"Cannot create a consistent method resolution order (MRO) for class `{}` with bases list `[{}]`",
Expand All @@ -545,7 +551,7 @@ impl<'db> TypeInferenceBuilder<'db> {
},
candidate1_is_base_class,
} => {
let node = class.node(self.db).into();
let node = class_node.into();
if *candidate1_is_base_class {
self.diagnostics.add(
node,
Expand Down
1 change: 0 additions & 1 deletion crates/ruff_benchmark/benches/red_knot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[
"error[possibly-unresolved-reference] /src/tomllib/_parser.py:580:63 Name `char` used when possibly not defined",
"error[conflicting-declarations] /src/tomllib/_parser.py:590:9 Conflicting declared types for `char`: Unknown, str | None",
"error[possibly-unresolved-reference] /src/tomllib/_parser.py:629:38 Name `datetime_obj` used when possibly not defined",
"error[invalid-base] /src/tomllib/_parser.py:692:8354 Invalid class base with type `GenericAlias` (all bases must be a class, `Any`, `Unknown` or `Todo`)",
];

fn get_test_file(name: &str) -> TestFile {
Expand Down

0 comments on commit 9a3001b

Please sign in to comment.