Introduce InferContext (#14956)

## Summary

I'm currently on the fence about landing the #14760 PR because it's
unclear how we'd support tracking used and unused suppression comments
in a performant way:
* Salsa adds an "untracked" dependency to every query reading
accumulated values. This has the effect that the query re-runs on every
revision. For example, a possible future query
`unused_suppression_comments(db, file)` would re-run on every
incremental change and for every file. I don't expect the operation
itself to be expensive, but it all adds up in a project with 100k+ files
* Salsa collects the accumulated values by traversing the entire query
dependency graph. It can skip over sub-graphs if it is known that they
contain no accumulated values. This makes accumulators a great tool for
when they are rare; diagnostics are a good example. Unfortunately,
suppressions are more common, and they often appear in many different
files, making the "skip over subgraphs" optimization less effective.

Because of that, I want to wait to adopt salsa accumulators for type
check diagnostics (we could start using them for other diagnostics)
until we have very specific reasons that justify regressing incremental
check performance.

This PR does a "small" refactor that brings us closer to what I have in
#14760 but without using accumulators. To emit a diagnostic, a method
needs:

* Access to the db
* Access to the currently checked file

This PR introduces a new `InferContext` that holds on to the db, the
current file, and the reported diagnostics. It replaces the
`TypeCheckDiagnosticsBuilder`. We pass the `InferContext` instead of the
`db` to methods that *might* emit diagnostics. This simplifies some of
the `Outcome` methods, which can now be called with a context instead of
a `db` and the diagnostics builder. Having the `db` and the file on a
single type like this would also be useful when using accumulators.

This PR doesn't solve the issue that the `Outcome` types feel somewhat
complicated nor that it can be annoying when you need to report a
`Diagnostic,` but you don't have access to an `InferContext` (or the
file). However, I also believe that accumulators won't solve these
problems because:

* Even with accumulators, it's necessary to have a reference to the file
that's being checked. The struggle would be to get a reference to that
file rather than getting a reference to `InferContext`.
* Users of the `HasTy` trait (e.g., a linter) don't want to bother
getting the `File` when calling `Type::return_ty` because they aren't
interested in the created diagnostics. They just want to know what
calling the current expression would return (and if it even is a
callable). This is what the different methods of `Outcome` enable today.
I can ask for the return type without needing extra data that's only
relevant for emitting a diagnostic.

A shortcoming of this approach is that it is now a bit confusing when to
pass `db` and when an `InferContext`. An option is that we'd make the
`file` on `InferContext` optional (it won't collect any diagnostics if
`None`) and change all methods on `Type` to take `InferContext` as the
first argument instead of a `db`. I'm interested in your opinion on
this.

Accumulators are definitely harder to use incorrectly because they
remove the need to merge the diagnostics explicitly and there's no risk
that we accidentally merge the diagnostics twice, resulting in
duplicated diagnostics. I still value performance more over making our
life slightly easier.
This commit is contained in:
Micha Reiser 2024-12-18 13:22:33 +01:00 committed by GitHub
parent ac81c72bf3
commit 0fc4e8f795
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 868 additions and 745 deletions

1
Cargo.lock generated
View file

@ -2288,6 +2288,7 @@ dependencies = [
"compact_str",
"countme",
"dir-test",
"drop_bomb",
"hashbrown 0.15.2",
"indexmap",
"insta",

View file

@ -26,6 +26,7 @@ bitflags = { workspace = true }
camino = { workspace = true }
compact_str = { workspace = true }
countme = { workspace = true }
drop_bomb = { workspace = true }
indexmap = { workspace = true }
itertools = { workspace = true }
ordermap = { workspace = true }
@ -58,4 +59,3 @@ serde = ["ruff_db/serde", "dep:serde"]
[lints]
workspace = true

View file

@ -0,0 +1,50 @@
use salsa;
use ruff_db::{files::File, parsed::comment_ranges, source::source_text};
use ruff_index::{newtype_index, IndexVec};
use crate::{lint::LintId, Db};
#[salsa::tracked(return_ref)]
pub(crate) fn suppressions(db: &dyn Db, file: File) -> IndexVec<SuppressionIndex, Suppression> {
let comments = comment_ranges(db.upcast(), file);
let source = source_text(db.upcast(), file);
let mut suppressions = IndexVec::default();
for range in comments {
let text = &source[range];
if text.starts_with("# type: ignore") {
suppressions.push(Suppression {
target: None,
kind: SuppressionKind::TypeIgnore,
});
} else if text.starts_with("# knot: ignore") {
suppressions.push(Suppression {
target: None,
kind: SuppressionKind::KnotIgnore,
});
}
}
suppressions
}
#[newtype_index]
pub(crate) struct SuppressionIndex;
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub(crate) struct Suppression {
target: Option<LintId>,
kind: SuppressionKind,
}
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
pub(crate) enum SuppressionKind {
/// A `type: ignore` comment
TypeIgnore,
/// A `knot: ignore` comment
KnotIgnore,
}

View file

@ -1,5 +1,7 @@
use std::hash::Hash;
use context::InferContext;
use diagnostic::{report_not_iterable, report_not_iterable_possibly_unbound};
use indexmap::IndexSet;
use itertools::Itertools;
use ruff_db::diagnostic::Severity;
@ -29,7 +31,7 @@ use crate::stdlib::{
use crate::symbol::{Boundness, Symbol};
use crate::types::call::{CallDunderResult, CallOutcome};
use crate::types::class_base::ClassBase;
use crate::types::diagnostic::{TypeCheckDiagnosticsBuilder, INVALID_TYPE_FORM};
use crate::types::diagnostic::INVALID_TYPE_FORM;
use crate::types::mro::{Mro, MroError, MroIterator};
use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet, Module, Program, PythonVersion};
@ -37,6 +39,7 @@ use crate::{Db, FxOrderSet, Module, Program, PythonVersion};
mod builder;
mod call;
mod class_base;
mod context;
mod diagnostic;
mod display;
mod infer;
@ -2177,17 +2180,13 @@ pub struct InvalidTypeExpressionError<'db> {
}
impl<'db> InvalidTypeExpressionError<'db> {
fn into_fallback_type(
self,
diagnostics: &mut TypeCheckDiagnosticsBuilder,
node: &ast::Expr,
) -> Type<'db> {
fn into_fallback_type(self, context: &InferContext, node: &ast::Expr) -> Type<'db> {
let InvalidTypeExpressionError {
fallback_type,
invalid_expressions,
} = self;
for error in invalid_expressions {
diagnostics.add_lint(
context.report_lint(
&INVALID_TYPE_FORM,
node.into(),
format_args!("{}", error.reason()),
@ -2827,20 +2826,20 @@ enum IterationOutcome<'db> {
impl<'db> IterationOutcome<'db> {
fn unwrap_with_diagnostic(
self,
context: &InferContext<'db>,
iterable_node: ast::AnyNodeRef,
diagnostics: &mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self {
Self::Iterable { element_ty } => element_ty,
Self::NotIterable { not_iterable_ty } => {
diagnostics.add_not_iterable(iterable_node, not_iterable_ty);
report_not_iterable(context, iterable_node, not_iterable_ty);
Type::Unknown
}
Self::PossiblyUnboundDunderIter {
iterable_ty,
element_ty,
} => {
diagnostics.add_not_iterable_possibly_unbound(iterable_node, iterable_ty);
report_not_iterable_possibly_unbound(context, iterable_node, iterable_ty);
element_ty
}
}

View file

@ -1,4 +1,5 @@
use super::diagnostic::{TypeCheckDiagnosticsBuilder, CALL_NON_CALLABLE};
use super::context::InferContext;
use super::diagnostic::CALL_NON_CALLABLE;
use super::{Severity, Type, TypeArrayDisplay, UnionBuilder};
use crate::Db;
use ruff_db::diagnostic::DiagnosticId;
@ -86,24 +87,23 @@ impl<'db> CallOutcome<'db> {
}
/// Get the return type of the call, emitting default diagnostics if needed.
pub(super) fn unwrap_with_diagnostic<'a>(
pub(super) fn unwrap_with_diagnostic(
&self,
db: &'db dyn Db,
context: &InferContext<'db>,
node: ast::AnyNodeRef,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self.return_ty_result(db, node, diagnostics) {
match self.return_ty_result(context, node) {
Ok(return_ty) => return_ty,
Err(NotCallableError::Type {
not_callable_ty,
return_ty,
}) => {
diagnostics.add_lint(
context.report_lint(
&CALL_NON_CALLABLE,
node,
format_args!(
"Object of type `{}` is not callable",
not_callable_ty.display(db)
not_callable_ty.display(context.db())
),
);
return_ty
@ -113,13 +113,13 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
diagnostics.add_lint(
context.report_lint(
&CALL_NON_CALLABLE,
node,
format_args!(
"Object of type `{}` is not callable (due to union element `{}`)",
called_ty.display(db),
not_callable_ty.display(db),
called_ty.display(context.db()),
not_callable_ty.display(context.db()),
),
);
return_ty
@ -129,13 +129,13 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
diagnostics.add_lint(
context.report_lint(
&CALL_NON_CALLABLE,
node,
format_args!(
"Object of type `{}` is not callable (due to union elements {})",
called_ty.display(db),
not_callable_tys.display(db),
called_ty.display(context.db()),
not_callable_tys.display(context.db()),
),
);
return_ty
@ -144,12 +144,12 @@ impl<'db> CallOutcome<'db> {
callable_ty: called_ty,
return_ty,
}) => {
diagnostics.add_lint(
context.report_lint(
&CALL_NON_CALLABLE,
node,
format_args!(
"Object of type `{}` is not callable (possibly unbound `__call__` method)",
called_ty.display(db)
called_ty.display(context.db())
),
);
return_ty
@ -158,11 +158,10 @@ impl<'db> CallOutcome<'db> {
}
/// Get the return type of the call as a result.
pub(super) fn return_ty_result<'a>(
pub(super) fn return_ty_result(
&self,
db: &'db dyn Db,
context: &InferContext<'db>,
node: ast::AnyNodeRef,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Result<Type<'db>, NotCallableError<'db>> {
match self {
Self::Callable { return_ty } => Ok(*return_ty),
@ -170,11 +169,11 @@ impl<'db> CallOutcome<'db> {
return_ty,
revealed_ty,
} => {
diagnostics.add(
context.report_diagnostic(
node,
DiagnosticId::RevealedType,
Severity::Info,
format_args!("Revealed type is `{}`", revealed_ty.display(db)),
format_args!("Revealed type is `{}`", revealed_ty.display(context.db())),
);
Ok(*return_ty)
}
@ -187,14 +186,16 @@ impl<'db> CallOutcome<'db> {
call_outcome,
} => Err(NotCallableError::PossiblyUnboundDunderCall {
callable_ty: *called_ty,
return_ty: call_outcome.return_ty(db).unwrap_or(Type::Unknown),
return_ty: call_outcome
.return_ty(context.db())
.unwrap_or(Type::Unknown),
}),
Self::Union {
outcomes,
called_ty,
} => {
let mut not_callable = vec![];
let mut union_builder = UnionBuilder::new(db);
let mut union_builder = UnionBuilder::new(context.db());
let mut revealed = false;
for outcome in outcomes {
let return_ty = match outcome {
@ -210,10 +211,10 @@ impl<'db> CallOutcome<'db> {
*return_ty
} else {
revealed = true;
outcome.unwrap_with_diagnostic(db, node, diagnostics)
outcome.unwrap_with_diagnostic(context, node)
}
}
_ => outcome.unwrap_with_diagnostic(db, node, diagnostics),
_ => outcome.unwrap_with_diagnostic(context, node),
};
union_builder = union_builder.add(return_ty);
}

View file

@ -0,0 +1,131 @@
use std::fmt;
use drop_bomb::DebugDropBomb;
use ruff_db::{
diagnostic::{DiagnosticId, Severity},
files::File,
};
use ruff_python_ast::AnyNodeRef;
use ruff_text_size::Ranged;
use crate::{
lint::{LintId, LintMetadata},
Db,
};
use super::{TypeCheckDiagnostic, TypeCheckDiagnostics};
/// Context for inferring the types of a single file.
///
/// One context exists for at least for every inferred region but it's
/// possible that inferring a sub-region, like an unpack assignment, creates
/// a sub-context.
///
/// Tracks the reported diagnostics of the inferred region.
///
/// ## Consuming
/// It's important that the context is explicitly consumed before dropping by calling
/// [`InferContext::finish`] and the returned diagnostics must be stored
/// on the current [`TypeInference`](super::infer::TypeInference) result.
pub(crate) struct InferContext<'db> {
db: &'db dyn Db,
file: File,
diagnostics: std::cell::RefCell<TypeCheckDiagnostics>,
bomb: DebugDropBomb,
}
impl<'db> InferContext<'db> {
pub(crate) fn new(db: &'db dyn Db, file: File) -> Self {
Self {
db,
file,
diagnostics: std::cell::RefCell::new(TypeCheckDiagnostics::default()),
bomb: DebugDropBomb::new("`InferContext` needs to be explicitly consumed by calling `::finish` to prevent accidental loss of diagnostics."),
}
}
/// The file for which the types are inferred.
pub(crate) fn file(&self) -> File {
self.file
}
pub(crate) fn db(&self) -> &'db dyn Db {
self.db
}
pub(crate) fn extend<T>(&mut self, other: &T)
where
T: WithDiagnostics,
{
self.diagnostics
.get_mut()
.extend(other.diagnostics().iter().cloned());
}
/// Reports a lint located at `node`.
pub(super) fn report_lint(
&self,
lint: &'static LintMetadata,
node: AnyNodeRef,
message: std::fmt::Arguments,
) {
// Skip over diagnostics if the rule is disabled.
let Some(severity) = self.db.rule_selection().severity(LintId::of(lint)) else {
return;
};
self.report_diagnostic(node, DiagnosticId::Lint(lint.name()), severity, message);
}
/// Adds a new diagnostic.
///
/// The diagnostic does not get added if the rule isn't enabled for this file.
pub(super) fn report_diagnostic(
&self,
node: AnyNodeRef,
id: DiagnosticId,
severity: Severity,
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.
// * Check for suppression comments, bump a counter if the diagnostic is suppressed.
self.diagnostics.borrow_mut().push(TypeCheckDiagnostic {
file: self.file,
id,
message: message.to_string(),
range: node.range(),
severity,
});
}
#[must_use]
pub(crate) fn finish(mut self) -> TypeCheckDiagnostics {
self.bomb.defuse();
let mut diagnostics = self.diagnostics.into_inner();
diagnostics.shrink_to_fit();
diagnostics
}
}
impl fmt::Debug for InferContext<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_struct("TyContext")
.field("file", &self.file)
.field("diagnostics", &self.diagnostics)
.field("defused", &self.bomb)
.finish()
}
}
pub(crate) trait WithDiagnostics {
fn diagnostics(&self) -> &TypeCheckDiagnostics;
}

View file

@ -1,11 +1,11 @@
use crate::lint::{Level, LintId, LintMetadata, LintRegistryBuilder, LintStatus};
use crate::declare_lint;
use crate::lint::{Level, LintRegistryBuilder, LintStatus};
use crate::types::string_annotation::{
BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION,
IMPLICIT_CONCATENATED_STRING_TYPE_ANNOTATION, INVALID_SYNTAX_IN_FORWARD_ANNOTATION,
RAW_STRING_TYPE_ANNOTATION,
};
use crate::types::{ClassLiteralType, Type};
use crate::{declare_lint, Db};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::files::File;
use ruff_python_ast::{self as ast, AnyNodeRef};
@ -15,6 +15,8 @@ use std::fmt::Formatter;
use std::ops::Deref;
use std::sync::Arc;
use super::context::InferContext;
/// Registers all known type check lints.
pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
registry.register_lint(&CALL_NON_CALLABLE);
@ -564,223 +566,158 @@ impl<'a> IntoIterator for &'a TypeCheckDiagnostics {
}
}
pub(super) struct TypeCheckDiagnosticsBuilder<'db> {
db: &'db dyn Db,
file: File,
diagnostics: TypeCheckDiagnostics,
/// Emit a diagnostic declaring that the object represented by `node` is not iterable
pub(super) fn report_not_iterable(context: &InferContext, node: AnyNodeRef, not_iterable_ty: Type) {
context.report_lint(
&NOT_ITERABLE,
node,
format_args!(
"Object of type `{}` is not iterable",
not_iterable_ty.display(context.db())
),
);
}
impl<'db> TypeCheckDiagnosticsBuilder<'db> {
pub(super) fn new(db: &'db dyn Db, file: File) -> Self {
Self {
db,
file,
diagnostics: TypeCheckDiagnostics::default(),
/// Emit a diagnostic declaring that the object represented by `node` is not iterable
/// because its `__iter__` method is possibly unbound.
pub(super) fn report_not_iterable_possibly_unbound(
context: &InferContext,
node: AnyNodeRef,
element_ty: Type,
) {
context.report_lint(
&NOT_ITERABLE,
node,
format_args!(
"Object of type `{}` is not iterable because its `__iter__` method is possibly unbound",
element_ty.display(context.db())
),
);
}
/// Emit a diagnostic declaring that an index is out of bounds for a tuple.
pub(super) fn report_index_out_of_bounds(
context: &InferContext,
kind: &'static str,
node: AnyNodeRef,
tuple_ty: Type,
length: usize,
index: i64,
) {
context.report_lint(
&INDEX_OUT_OF_BOUNDS,
node,
format_args!(
"Index {index} is out of bounds for {kind} `{}` with length {length}",
tuple_ty.display(context.db())
),
);
}
/// Emit a diagnostic declaring that a type does not support subscripting.
pub(super) fn report_non_subscriptable(
context: &InferContext,
node: AnyNodeRef,
non_subscriptable_ty: Type,
method: &str,
) {
context.report_lint(
&NON_SUBSCRIPTABLE,
node,
format_args!(
"Cannot subscript object of type `{}` with no `{method}` method",
non_subscriptable_ty.display(context.db())
),
);
}
pub(super) fn report_unresolved_module<'db>(
context: &InferContext,
import_node: impl Into<AnyNodeRef<'db>>,
level: u32,
module: Option<&str>,
) {
context.report_lint(
&UNRESOLVED_IMPORT,
import_node.into(),
format_args!(
"Cannot resolve import `{}{}`",
".".repeat(level as usize),
module.unwrap_or_default()
),
);
}
pub(super) fn report_slice_step_size_zero(context: &InferContext, node: AnyNodeRef) {
context.report_lint(
&ZERO_STEPSIZE_IN_SLICE,
node,
format_args!("Slice step size can not be zero"),
);
}
pub(super) fn report_invalid_assignment(
context: &InferContext,
node: AnyNodeRef,
declared_ty: Type,
assigned_ty: Type,
) {
match declared_ty {
Type::ClassLiteral(ClassLiteralType { class }) => {
context.report_lint(&INVALID_ASSIGNMENT, node, format_args!(
"Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
class.name(context.db())));
}
}
/// 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_lint(
&NOT_ITERABLE,
node,
format_args!(
"Object of type `{}` is not iterable",
not_iterable_ty.display(self.db)
),
);
}
/// Emit a diagnostic declaring that the object represented by `node` is not iterable
/// because its `__iter__` method is possibly unbound.
pub(super) fn add_not_iterable_possibly_unbound(
&mut self,
node: AnyNodeRef,
element_ty: Type<'db>,
) {
self.add_lint(
&NOT_ITERABLE,
node,
format_args!(
"Object of type `{}` is not iterable because its `__iter__` method is possibly unbound",
element_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_lint(
&INDEX_OUT_OF_BOUNDS,
node,
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_lint(
&NON_SUBSCRIPTABLE,
node,
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_lint(
&UNRESOLVED_IMPORT,
import_node.into(),
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_lint(
&ZERO_STEPSIZE_IN_SLICE,
node,
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(ClassLiteralType { class }) => {
self.add_lint(&INVALID_ASSIGNMENT, node, format_args!(
"Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
class.name(self.db)));
}
Type::FunctionLiteral(function) => {
self.add_lint(&INVALID_ASSIGNMENT, node, format_args!(
"Implicit shadowing of function `{}`; annotate to make it explicit if this is intentional",
function.name(self.db)));
}
_ => {
self.add_lint(
&INVALID_ASSIGNMENT,
node,
format_args!(
"Object of type `{}` is not assignable to `{}`",
assigned_ty.display(self.db),
declared_ty.display(self.db),
),
);
}
Type::FunctionLiteral(function) => {
context.report_lint(&INVALID_ASSIGNMENT, node, format_args!(
"Implicit shadowing of function `{}`; annotate to make it explicit if this is intentional",
function.name(context.db())));
}
}
pub(super) fn add_possibly_unresolved_reference(&mut self, expr_name_node: &ast::ExprName) {
let ast::ExprName { id, .. } = expr_name_node;
self.add_lint(
&POSSIBLY_UNRESOLVED_REFERENCE,
expr_name_node.into(),
format_args!("Name `{id}` used when possibly not defined"),
);
}
pub(super) fn add_unresolved_reference(&mut self, expr_name_node: &ast::ExprName) {
let ast::ExprName { id, .. } = expr_name_node;
self.add_lint(
&UNRESOLVED_REFERENCE,
expr_name_node.into(),
format_args!("Name `{id}` used when not defined"),
);
}
pub(super) fn add_invalid_exception_caught(&mut self, db: &dyn Db, node: &ast::Expr, ty: Type) {
self.add_lint(
&INVALID_EXCEPTION_CAUGHT,
node.into(),
format_args!(
"Cannot catch object of type `{}` in an exception handler \
(must be a `BaseException` subclass or a tuple of `BaseException` subclasses)",
ty.display(db)
),
);
}
pub(super) fn add_lint(
&mut self,
lint: &'static LintMetadata,
node: AnyNodeRef,
message: std::fmt::Arguments,
) {
// Skip over diagnostics if the rule is disabled.
let Some(severity) = self.db.rule_selection().severity(LintId::of(lint)) else {
return;
};
self.add(node, DiagnosticId::Lint(lint.name()), severity, message);
}
/// Adds a new diagnostic.
///
/// The diagnostic does not get added if the rule isn't enabled for this file.
pub(super) fn add(
&mut self,
node: AnyNodeRef,
id: DiagnosticId,
severity: Severity,
message: std::fmt::Arguments,
) {
if !self.db.is_file_open(self.file) {
return;
_ => {
context.report_lint(
&INVALID_ASSIGNMENT,
node,
format_args!(
"Object of type `{}` is not assignable to `{}`",
assigned_ty.display(context.db()),
declared_ty.display(context.db()),
),
);
}
// 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,
id,
message: message.to_string(),
range: node.range(),
severity,
});
}
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
}
}
pub(super) fn report_possibly_unresolved_reference(
context: &InferContext,
expr_name_node: &ast::ExprName,
) {
let ast::ExprName { id, .. } = expr_name_node;
context.report_lint(
&POSSIBLY_UNRESOLVED_REFERENCE,
expr_name_node.into(),
format_args!("Name `{id}` used when possibly not defined"),
);
}
pub(super) fn report_unresolved_reference(context: &InferContext, expr_name_node: &ast::ExprName) {
let ast::ExprName { id, .. } = expr_name_node;
context.report_lint(
&UNRESOLVED_REFERENCE,
expr_name_node.into(),
format_args!("Name `{id}` used when not defined"),
);
}
pub(super) fn report_invalid_exception_caught(context: &InferContext, node: &ast::Expr, ty: Type) {
context.report_lint(
&INVALID_EXCEPTION_CAUGHT,
node.into(),
format_args!(
"Cannot catch object of type `{}` in an exception handler \
(must be a `BaseException` subclass or a tuple of `BaseException` subclasses)",
ty.display(context.db())
),
);
}

File diff suppressed because it is too large Load diff

View file

@ -1,13 +1,13 @@
use ruff_db::files::File;
use ruff_db::source::source_text;
use ruff_python_ast::str::raw_contents;
use ruff_python_ast::{self as ast, ModExpression, StringFlags};
use ruff_python_parser::{parse_expression_range, Parsed};
use ruff_text_size::Ranged;
use crate::declare_lint;
use crate::lint::{Level, LintStatus};
use crate::types::diagnostic::{TypeCheckDiagnostics, TypeCheckDiagnosticsBuilder};
use crate::{declare_lint, Db};
use super::context::InferContext;
declare_lint! {
/// ## What it does
@ -127,24 +127,23 @@ declare_lint! {
}
}
type AnnotationParseResult = Result<Parsed<ModExpression>, TypeCheckDiagnostics>;
/// Parses the given expression as a string annotation.
pub(crate) fn parse_string_annotation(
db: &dyn Db,
file: File,
context: &InferContext,
string_expr: &ast::ExprStringLiteral,
) -> AnnotationParseResult {
) -> Option<Parsed<ModExpression>> {
let file = context.file();
let db = context.db();
let _span = tracing::trace_span!("parse_string_annotation", string=?string_expr.range(), file=%file.path(db)).entered();
let source = source_text(db.upcast(), file);
let node_text = &source[string_expr.range()];
let mut diagnostics = TypeCheckDiagnosticsBuilder::new(db, file);
if let [string_literal] = string_expr.value.as_slice() {
let prefix = string_literal.flags.prefix();
if prefix.is_raw() {
diagnostics.add_lint(
context.report_lint(
&RAW_STRING_TYPE_ANNOTATION,
string_literal.into(),
format_args!("Type expressions cannot use raw string literal"),
@ -167,8 +166,8 @@ pub(crate) fn parse_string_annotation(
// """ = 1
// ```
match parse_expression_range(source.as_str(), range_excluding_quotes) {
Ok(parsed) => return Ok(parsed),
Err(parse_error) => diagnostics.add_lint(
Ok(parsed) => return Some(parsed),
Err(parse_error) => context.report_lint(
&INVALID_SYNTAX_IN_FORWARD_ANNOTATION,
string_literal.into(),
format_args!("Syntax error in forward annotation: {}", parse_error.error),
@ -177,7 +176,7 @@ pub(crate) fn parse_string_annotation(
} else {
// The raw contents of the string doesn't match the parsed content. This could be the
// case for annotations that contain escape sequences.
diagnostics.add_lint(
context.report_lint(
&ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION,
string_expr.into(),
format_args!("Type expressions cannot contain escape characters"),
@ -185,12 +184,12 @@ pub(crate) fn parse_string_annotation(
}
} else {
// String is implicitly concatenated.
diagnostics.add_lint(
context.report_lint(
&IMPLICIT_CONCATENATED_STRING_TYPE_ANNOTATION,
string_expr.into(),
format_args!("Type expressions cannot span multiple string literals"),
);
}
Err(diagnostics.finish())
None
}

View file

@ -6,30 +6,34 @@ use rustc_hash::FxHashMap;
use crate::semantic_index::ast_ids::{HasScopedExpressionId, ScopedExpressionId};
use crate::semantic_index::symbol::ScopeId;
use crate::types::{todo_type, Type, TypeCheckDiagnostics, TypeCheckDiagnosticsBuilder};
use crate::types::{todo_type, Type, TypeCheckDiagnostics};
use crate::Db;
use super::context::{InferContext, WithDiagnostics};
/// Unpacks the value expression type to their respective targets.
pub(crate) struct Unpacker<'db> {
db: &'db dyn Db,
context: InferContext<'db>,
targets: FxHashMap<ScopedExpressionId, Type<'db>>,
diagnostics: TypeCheckDiagnosticsBuilder<'db>,
}
impl<'db> Unpacker<'db> {
pub(crate) fn new(db: &'db dyn Db, file: File) -> Self {
Self {
db,
context: InferContext::new(db, file),
targets: FxHashMap::default(),
diagnostics: TypeCheckDiagnosticsBuilder::new(db, file),
}
}
fn db(&self) -> &'db dyn Db {
self.context.db()
}
pub(crate) fn unpack(&mut self, target: &ast::Expr, value_ty: Type<'db>, scope: ScopeId<'db>) {
match target {
ast::Expr::Name(target_name) => {
self.targets
.insert(target_name.scoped_expression_id(self.db, scope), value_ty);
.insert(target_name.scoped_expression_id(self.db(), scope), value_ty);
}
ast::Expr::Starred(ast::ExprStarred { value, .. }) => {
self.unpack(value, value_ty, scope);
@ -40,11 +44,11 @@ impl<'db> Unpacker<'db> {
let starred_index = elts.iter().position(ast::Expr::is_starred_expr);
let element_types = if let Some(starred_index) = starred_index {
if tuple_ty.len(self.db) >= elts.len() - 1 {
if tuple_ty.len(self.db()) >= elts.len() - 1 {
let mut element_types = Vec::with_capacity(elts.len());
element_types.extend_from_slice(
// SAFETY: Safe because of the length check above.
&tuple_ty.elements(self.db)[..starred_index],
&tuple_ty.elements(self.db())[..starred_index],
);
// E.g., in `(a, *b, c, d) = ...`, the index of starred element `b`
@ -52,10 +56,10 @@ impl<'db> Unpacker<'db> {
let remaining = elts.len() - (starred_index + 1);
// This index represents the type of the last element that belongs
// to the starred expression, in an exclusive manner.
let starred_end_index = tuple_ty.len(self.db) - remaining;
let starred_end_index = tuple_ty.len(self.db()) - remaining;
// SAFETY: Safe because of the length check above.
let _starred_element_types =
&tuple_ty.elements(self.db)[starred_index..starred_end_index];
&tuple_ty.elements(self.db())[starred_index..starred_end_index];
// TODO: Combine the types into a list type. If the
// starred_element_types is empty, then it should be `List[Any]`.
// combine_types(starred_element_types);
@ -63,11 +67,11 @@ impl<'db> Unpacker<'db> {
element_types.extend_from_slice(
// SAFETY: Safe because of the length check above.
&tuple_ty.elements(self.db)[starred_end_index..],
&tuple_ty.elements(self.db())[starred_end_index..],
);
Cow::Owned(element_types)
} else {
let mut element_types = tuple_ty.elements(self.db).to_vec();
let mut element_types = tuple_ty.elements(self.db()).to_vec();
// Subtract 1 to insert the starred expression type at the correct
// index.
element_types.resize(elts.len() - 1, Type::Unknown);
@ -76,7 +80,7 @@ impl<'db> Unpacker<'db> {
Cow::Owned(element_types)
}
} else {
Cow::Borrowed(tuple_ty.elements(self.db).as_ref())
Cow::Borrowed(tuple_ty.elements(self.db()).as_ref())
};
for (index, element) in elts.iter().enumerate() {
@ -94,9 +98,9 @@ impl<'db> Unpacker<'db> {
// individual character, instead of just an array of `LiteralString`, but
// there would be a cost and it's not clear that it's worth it.
let value_ty = Type::tuple(
self.db,
self.db(),
std::iter::repeat(Type::LiteralString)
.take(string_literal_ty.python_len(self.db)),
.take(string_literal_ty.python_len(self.db())),
);
self.unpack(target, value_ty, scope);
}
@ -105,8 +109,8 @@ impl<'db> Unpacker<'db> {
Type::LiteralString
} else {
value_ty
.iterate(self.db)
.unwrap_with_diagnostic(AnyNodeRef::from(target), &mut self.diagnostics)
.iterate(self.db())
.unwrap_with_diagnostic(&self.context, AnyNodeRef::from(target))
};
for element in elts {
self.unpack(element, value_ty, scope);
@ -120,7 +124,7 @@ impl<'db> Unpacker<'db> {
pub(crate) fn finish(mut self) -> UnpackResult<'db> {
self.targets.shrink_to_fit();
UnpackResult {
diagnostics: self.diagnostics.finish(),
diagnostics: self.context.finish(),
targets: self.targets,
}
}
@ -136,8 +140,10 @@ impl<'db> UnpackResult<'db> {
pub(crate) fn get(&self, expr_id: ScopedExpressionId) -> Option<Type<'db>> {
self.targets.get(&expr_id).copied()
}
}
pub(crate) fn diagnostics(&self) -> &TypeCheckDiagnostics {
impl WithDiagnostics for UnpackResult<'_> {
fn diagnostics(&self) -> &TypeCheckDiagnostics {
&self.diagnostics
}
}