Skip to content
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.

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

Separate type check diagnostics builder #13978

Merged
merged 4 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 12 additions & 12 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use infer::TypeInferenceBuilder;
use ruff_db::files::File;
use ruff_python_ast as ast;

Expand All @@ -13,6 +12,7 @@ use crate::semantic_index::{
use crate::stdlib::{
builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty, typing_extensions_symbol_ty,
};
use crate::types::diagnostic::TypeCheckDiagnosticsBuilder;
use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet, HasTy, Module, SemanticModel};

Expand Down Expand Up @@ -1456,15 +1456,15 @@ impl<'db> CallOutcome<'db> {
&self,
db: &'db dyn Db,
node: ast::AnyNodeRef,
builder: &'a mut TypeInferenceBuilder<'db>,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self.return_ty_result(db, node, builder) {
match self.return_ty_result(db, node, diagnostics) {
Ok(return_ty) => return_ty,
Err(NotCallableError::Type {
not_callable_ty,
return_ty,
}) => {
builder.add_diagnostic(
diagnostics.add(
node,
"call-non-callable",
format_args!(
Expand All @@ -1479,7 +1479,7 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
builder.add_diagnostic(
diagnostics.add(
node,
"call-non-callable",
format_args!(
Expand All @@ -1495,7 +1495,7 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
builder.add_diagnostic(
diagnostics.add(
node,
"call-non-callable",
format_args!(
Expand All @@ -1514,15 +1514,15 @@ impl<'db> CallOutcome<'db> {
&self,
db: &'db dyn Db,
node: ast::AnyNodeRef,
builder: &'a mut TypeInferenceBuilder<'db>,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Result<Type<'db>, NotCallableError<'db>> {
match self {
Self::Callable { return_ty } => Ok(*return_ty),
Self::RevealType {
return_ty,
revealed_ty,
} => {
builder.add_diagnostic(
diagnostics.add(
node,
"revealed-type",
format_args!("Revealed type is `{}`", revealed_ty.display(db)),
Expand Down Expand Up @@ -1554,10 +1554,10 @@ impl<'db> CallOutcome<'db> {
*return_ty
} else {
revealed = true;
outcome.unwrap_with_diagnostic(db, node, builder)
outcome.unwrap_with_diagnostic(db, node, diagnostics)
}
}
_ => outcome.unwrap_with_diagnostic(db, node, builder),
_ => outcome.unwrap_with_diagnostic(db, node, diagnostics),
};
union_builder = union_builder.add(return_ty);
}
Expand Down Expand Up @@ -1640,12 +1640,12 @@ impl<'db> IterationOutcome<'db> {
fn unwrap_with_diagnostic(
self,
iterable_node: ast::AnyNodeRef,
inference_builder: &mut TypeInferenceBuilder<'db>,
diagnostics: &mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self {
Self::Iterable { element_ty } => element_ty,
Self::NotIterable { not_iterable_ty } => {
inference_builder.not_iterable_diagnostic(iterable_node, not_iterable_ty);
diagnostics.add_not_iterable(iterable_node, not_iterable_ty);
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
Type::Unknown
}
}
Expand Down
155 changes: 155 additions & 0 deletions crates/red_knot_python_semantic/src/types/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use ruff_db::files::File;
use ruff_python_ast::AnyNodeRef;
use ruff_text_size::{Ranged, TextRange};
use std::fmt::Formatter;
use std::ops::Deref;
use std::sync::Arc;

use crate::types::Type;
use crate::Db;

#[derive(Debug, Eq, PartialEq)]
pub struct TypeCheckDiagnostic {
// TODO: Don't use string keys for rules
Expand Down Expand Up @@ -109,3 +113,154 @@ impl<'a> IntoIterator for &'a TypeCheckDiagnostics {
self.inner.iter()
}
}

pub(super) struct TypeCheckDiagnosticsBuilder<'db> {
db: &'db dyn Db,
file: File,
diagnostics: TypeCheckDiagnostics,
}

impl<'db> TypeCheckDiagnosticsBuilder<'db> {
pub(super) fn new(db: &'db dyn Db, file: File) -> Self {
Self {
db,
file,
diagnostics: TypeCheckDiagnostics::new(),
}
}

/// Emit a diagnostic declaring that the object represented by `node` is not iterable
pub(super) fn add_not_iterable(&mut self, node: AnyNodeRef, not_iterable_ty: Type<'db>) {
self.add(
node,
"not-iterable",
format_args!(
"Object of type `{}` is not iterable",
not_iterable_ty.display(self.db)
),
);
}

/// Emit a diagnostic declaring that an index is out of bounds for a tuple.
pub(super) fn add_index_out_of_bounds(
&mut self,
kind: &'static str,
node: AnyNodeRef,
tuple_ty: Type<'db>,
length: usize,
index: i64,
) {
self.add(
node,
"index-out-of-bounds",
format_args!(
"Index {index} is out of bounds for {kind} `{}` with length {length}",
tuple_ty.display(self.db)
),
);
}

/// Emit a diagnostic declaring that a type does not support subscripting.
pub(super) fn add_non_subscriptable(
&mut self,
node: AnyNodeRef,
non_subscriptable_ty: Type<'db>,
method: &str,
) {
self.add(
node,
"non-subscriptable",
format_args!(
"Cannot subscript object of type `{}` with no `{method}` method",
non_subscriptable_ty.display(self.db)
),
);
}

pub(super) fn add_unresolved_module(
&mut self,
import_node: impl Into<AnyNodeRef<'db>>,
level: u32,
module: Option<&str>,
) {
self.add(
import_node.into(),
"unresolved-import",
format_args!(
"Cannot resolve import `{}{}`",
".".repeat(level as usize),
module.unwrap_or_default()
),
);
}

pub(super) fn add_slice_step_size_zero(&mut self, node: AnyNodeRef) {
self.add(
node,
"zero-stepsize-in-slice",
format_args!("Slice step size can not be zero"),
);
}

pub(super) fn add_invalid_assignment(
&mut self,
node: AnyNodeRef,
declared_ty: Type<'db>,
assigned_ty: Type<'db>,
) {
match declared_ty {
Type::ClassLiteral(class) => {
self.add(node, "invalid-assignment", format_args!(
"Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
class.name(self.db)));
}
Type::FunctionLiteral(function) => {
self.add(node, "invalid-assignment", format_args!(
"Implicit shadowing of function `{}`; annotate to make it explicit if this is intentional",
function.name(self.db)));
}
_ => {
self.add(
node,
"invalid-assignment",
format_args!(
"Object of type `{}` is not assignable to `{}`",
assigned_ty.display(self.db),
declared_ty.display(self.db),
),
);
}
}
}

/// Adds a new diagnostic.
///
/// The diagnostic does not get added if the rule isn't enabled for this file.
carljm marked this conversation as resolved.
Show resolved Hide resolved
pub(super) fn add(&mut self, node: AnyNodeRef, rule: &str, message: std::fmt::Arguments) {
if !self.db.is_file_open(self.file) {
return;
}

// TODO: Don't emit the diagnostic if:
// * The enclosing node contains any syntax errors
// * The rule is disabled for this file. We probably want to introduce a new query that
// returns a rule selector for a given file that respects the package's settings,
// any global pragma comments in the file, and any per-file-ignores.

self.diagnostics.push(TypeCheckDiagnostic {
file: self.file,
rule: rule.to_string(),
message: message.to_string(),
range: node.range(),
});
}

pub(super) fn extend(&mut self, diagnostics: &TypeCheckDiagnostics) {
self.diagnostics.extend(diagnostics);
}

pub(super) fn finish(mut self) -> TypeCheckDiagnostics {
self.diagnostics.shrink_to_fit();
self.diagnostics
}
}
Loading
Loading