Add API to emit type-checking diagnostics (#12988)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
Micha Reiser 2024-08-20 09:22:30 +02:00 committed by GitHub
parent 38c19fb96e
commit c65e3310d5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 337 additions and 142 deletions

View file

@ -1,15 +1,18 @@
use ruff_db::files::File;
use ruff_db::{Db as SourceDb, Upcast}; use ruff_db::{Db as SourceDb, Upcast};
/// Database giving access to semantic information about a Python program. /// Database giving access to semantic information about a Python program.
#[salsa::db] #[salsa::db]
pub trait Db: SourceDb + Upcast<dyn SourceDb> {} pub trait Db: SourceDb + Upcast<dyn SourceDb> {
fn is_file_open(&self, file: File) -> bool;
}
#[cfg(test)] #[cfg(test)]
pub(crate) mod tests { pub(crate) mod tests {
use std::sync::Arc; use std::sync::Arc;
use crate::module_resolver::vendored_typeshed_stubs; use crate::module_resolver::vendored_typeshed_stubs;
use ruff_db::files::Files; use ruff_db::files::{File, Files};
use ruff_db::system::{DbWithTestSystem, System, TestSystem}; use ruff_db::system::{DbWithTestSystem, System, TestSystem};
use ruff_db::vendored::VendoredFileSystem; use ruff_db::vendored::VendoredFileSystem;
use ruff_db::{Db as SourceDb, Upcast}; use ruff_db::{Db as SourceDb, Upcast};
@ -91,7 +94,11 @@ pub(crate) mod tests {
} }
#[salsa::db] #[salsa::db]
impl Db for TestDb {} impl Db for TestDb {
fn is_file_open(&self, file: File) -> bool {
!file.path(self).is_vendored_path()
}
}
#[salsa::db] #[salsa::db]
impl salsa::Database for TestDb { impl salsa::Database for TestDb {

View file

@ -154,6 +154,10 @@ impl<'db> SemanticIndex<'db> {
&self.scopes[id] &self.scopes[id]
} }
pub(crate) fn scope_ids(&self) -> impl Iterator<Item = ScopeId> {
self.scope_ids_by_scope.iter().copied()
}
/// Returns the id of the parent scope. /// Returns the id of the parent scope.
pub(crate) fn parent_scope_id(&self, scope_id: FileScopeId) -> Option<FileScopeId> { pub(crate) fn parent_scope_id(&self, scope_id: FileScopeId) -> Option<FileScopeId> {
let scope = self.scope(scope_id); let scope = self.scope(scope_id);

View file

@ -5,21 +5,37 @@ use crate::builtins::builtins_scope;
use crate::semantic_index::definition::Definition; use crate::semantic_index::definition::Definition;
use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId}; use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId};
use crate::semantic_index::{ use crate::semantic_index::{
global_scope, symbol_table, use_def_map, DefinitionWithConstraints, global_scope, semantic_index, symbol_table, use_def_map, DefinitionWithConstraints,
DefinitionWithConstraintsIterator, DefinitionWithConstraintsIterator,
}; };
use crate::types::narrow::narrowing_constraint; use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet}; use crate::{Db, FxOrderSet};
pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
pub(crate) use self::diagnostic::TypeCheckDiagnostics;
pub(crate) use self::infer::{
infer_definition_types, infer_expression_types, infer_scope_types, TypeInference,
};
mod builder; mod builder;
mod diagnostic;
mod display; mod display;
mod infer; mod infer;
mod narrow; mod narrow;
pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder}; pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics {
pub(crate) use self::infer::{ let _span = tracing::trace_span!("check_types", file=?file.path(db)).entered();
infer_definition_types, infer_expression_types, infer_scope_types, TypeInference,
}; let index = semantic_index(db, file);
let mut diagnostics = TypeCheckDiagnostics::new();
for scope_id in index.scope_ids() {
let result = infer_scope_types(db, scope_id);
diagnostics.extend(result.diagnostics());
}
diagnostics
}
/// Infer the public type of a symbol (its type as seen from outside its scope). /// Infer the public type of a symbol (its type as seen from outside its scope).
pub(crate) fn symbol_ty<'db>( pub(crate) fn symbol_ty<'db>(
@ -333,3 +349,48 @@ pub struct IntersectionType<'db> {
/// directly in intersections rather than as a separate type. /// directly in intersections rather than as a separate type.
negative: FxOrderSet<Type<'db>>, negative: FxOrderSet<Type<'db>>,
} }
#[cfg(test)]
mod tests {
use anyhow::Context;
use ruff_db::files::system_path_to_file;
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
use crate::db::tests::TestDb;
use crate::{Program, ProgramSettings, PythonVersion, SearchPathSettings};
#[test]
fn check_types() -> anyhow::Result<()> {
let mut db = TestDb::new();
db.write_file("src/foo.py", "import bar\n")
.context("Failed to write foo.py")?;
Program::from_settings(
&db,
ProgramSettings {
target_version: PythonVersion::default(),
search_paths: SearchPathSettings {
extra_paths: Vec::new(),
src_root: SystemPathBuf::from("/src"),
site_packages: vec![],
custom_typeshed: None,
},
},
)
.expect("Valid search path settings");
let foo = system_path_to_file(&db, "src/foo.py").context("Failed to resolve foo.py")?;
let diagnostics = super::check_types(&db, foo);
assert_eq!(diagnostics.len(), 1);
assert_eq!(
diagnostics[0].message(),
"Import 'bar' could not be resolved."
);
Ok(())
}
}

View file

@ -0,0 +1,111 @@
use ruff_db::files::File;
use ruff_text_size::{Ranged, TextRange};
use std::fmt::Formatter;
use std::ops::Deref;
use std::sync::Arc;
#[derive(Debug, Eq, PartialEq)]
pub struct TypeCheckDiagnostic {
// TODO: Don't use string keys for rules
pub(super) rule: String,
pub(super) message: String,
pub(super) range: TextRange,
pub(super) file: File,
}
impl TypeCheckDiagnostic {
pub fn rule(&self) -> &str {
&self.rule
}
pub fn message(&self) -> &str {
&self.message
}
pub fn file(&self) -> File {
self.file
}
}
impl Ranged for TypeCheckDiagnostic {
fn range(&self) -> TextRange {
self.range
}
}
/// A collection of type check diagnostics.
///
/// The diagnostics are wrapped in an `Arc` because they need to be cloned multiple times
/// when going from `infer_expression` to `check_file`. We could consider
/// making [`TypeCheckDiagnostic`] a Salsa struct to have them Arena-allocated (once the Tables refactor is done).
/// Using Salsa struct does have the downside that it leaks the Salsa dependency into diagnostics and
/// each Salsa-struct comes with an overhead.
#[derive(Default, Eq, PartialEq)]
pub struct TypeCheckDiagnostics {
inner: Vec<std::sync::Arc<TypeCheckDiagnostic>>,
}
impl TypeCheckDiagnostics {
pub fn new() -> Self {
Self { inner: Vec::new() }
}
pub(super) fn push(&mut self, diagnostic: TypeCheckDiagnostic) {
self.inner.push(Arc::new(diagnostic));
}
pub(crate) fn shrink_to_fit(&mut self) {
self.inner.shrink_to_fit();
}
}
impl Extend<TypeCheckDiagnostic> for TypeCheckDiagnostics {
fn extend<T: IntoIterator<Item = TypeCheckDiagnostic>>(&mut self, iter: T) {
self.inner.extend(iter.into_iter().map(std::sync::Arc::new));
}
}
impl Extend<std::sync::Arc<TypeCheckDiagnostic>> for TypeCheckDiagnostics {
fn extend<T: IntoIterator<Item = Arc<TypeCheckDiagnostic>>>(&mut self, iter: T) {
self.inner.extend(iter);
}
}
impl<'a> Extend<&'a std::sync::Arc<TypeCheckDiagnostic>> for TypeCheckDiagnostics {
fn extend<T: IntoIterator<Item = &'a Arc<TypeCheckDiagnostic>>>(&mut self, iter: T) {
self.inner
.extend(iter.into_iter().map(std::sync::Arc::clone));
}
}
impl std::fmt::Debug for TypeCheckDiagnostics {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
self.inner.fmt(f)
}
}
impl Deref for TypeCheckDiagnostics {
type Target = [std::sync::Arc<TypeCheckDiagnostic>];
fn deref(&self) -> &Self::Target {
&self.inner
}
}
impl IntoIterator for TypeCheckDiagnostics {
type Item = Arc<TypeCheckDiagnostic>;
type IntoIter = std::vec::IntoIter<std::sync::Arc<TypeCheckDiagnostic>>;
fn into_iter(self) -> Self::IntoIter {
self.inner.into_iter()
}
}
impl<'a> IntoIterator for &'a TypeCheckDiagnostics {
type Item = &'a Arc<TypeCheckDiagnostic>;
type IntoIter = std::slice::Iter<'a, std::sync::Arc<TypeCheckDiagnostic>>;
fn into_iter(self) -> Self::IntoIter {
self.inner.iter()
}
}

View file

@ -29,7 +29,8 @@ use salsa::plumbing::AsId;
use ruff_db::files::File; use ruff_db::files::File;
use ruff_db::parsed::parsed_module; use ruff_db::parsed::parsed_module;
use ruff_python_ast as ast; use ruff_python_ast as ast;
use ruff_python_ast::{Expr, ExprContext}; use ruff_python_ast::{AnyNodeRef, ExprContext};
use ruff_text_size::Ranged;
use crate::builtins::builtins_scope; use crate::builtins::builtins_scope;
use crate::module_name::ModuleName; use crate::module_name::ModuleName;
@ -40,6 +41,7 @@ use crate::semantic_index::expression::Expression;
use crate::semantic_index::semantic_index; use crate::semantic_index::semantic_index;
use crate::semantic_index::symbol::{FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId}; use crate::semantic_index::symbol::{FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId};
use crate::semantic_index::SemanticIndex; use crate::semantic_index::SemanticIndex;
use crate::types::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics};
use crate::types::{ use crate::types::{
builtins_symbol_ty_by_name, definitions_ty, global_symbol_ty_by_name, ClassType, FunctionType, builtins_symbol_ty_by_name, definitions_ty, global_symbol_ty_by_name, ClassType, FunctionType,
Name, Type, UnionBuilder, Name, Type, UnionBuilder,
@ -123,13 +125,16 @@ pub(crate) enum InferenceRegion<'db> {
} }
/// The inferred types for a single region. /// The inferred types for a single region.
#[derive(Debug, Eq, PartialEq, Default, Clone)] #[derive(Debug, Eq, PartialEq, Default)]
pub(crate) struct TypeInference<'db> { pub(crate) struct TypeInference<'db> {
/// The types of every expression in this region. /// The types of every expression in this region.
expressions: FxHashMap<ScopedExpressionId, Type<'db>>, expressions: FxHashMap<ScopedExpressionId, Type<'db>>,
/// The types of every definition in this region. /// The types of every definition in this region.
definitions: FxHashMap<Definition<'db>, Type<'db>>, definitions: FxHashMap<Definition<'db>, Type<'db>>,
/// The diagnostics for this region.
diagnostics: TypeCheckDiagnostics,
} }
impl<'db> TypeInference<'db> { impl<'db> TypeInference<'db> {
@ -142,9 +147,14 @@ impl<'db> TypeInference<'db> {
self.definitions[&definition] self.definitions[&definition]
} }
pub(crate) fn diagnostics(&self) -> &[std::sync::Arc<TypeCheckDiagnostic>] {
&self.diagnostics
}
fn shrink_to_fit(&mut self) { fn shrink_to_fit(&mut self) {
self.expressions.shrink_to_fit(); self.expressions.shrink_to_fit();
self.definitions.shrink_to_fit(); self.definitions.shrink_to_fit();
self.diagnostics.shrink_to_fit();
} }
} }
@ -235,6 +245,7 @@ impl<'db> TypeInferenceBuilder<'db> {
fn extend(&mut self, inference: &TypeInference<'db>) { fn extend(&mut self, inference: &TypeInference<'db>) {
self.types.definitions.extend(inference.definitions.iter()); self.types.definitions.extend(inference.definitions.iter());
self.types.expressions.extend(inference.expressions.iter()); self.types.expressions.extend(inference.expressions.iter());
self.types.diagnostics.extend(&inference.diagnostics);
} }
/// Infers types in the given [`InferenceRegion`]. /// Infers types in the given [`InferenceRegion`].
@ -855,7 +866,7 @@ impl<'db> TypeInferenceBuilder<'db> {
asname: _, asname: _,
} = alias; } = alias;
let module_ty = self.module_ty_from_name(ModuleName::new(name)); let module_ty = self.module_ty_from_name(ModuleName::new(name), alias.into());
self.types.definitions.insert(definition, module_ty); self.types.definitions.insert(definition, module_ty);
} }
@ -953,7 +964,7 @@ impl<'db> TypeInferenceBuilder<'db> {
ModuleName::new(module_name) ModuleName::new(module_name)
}; };
let module_ty = self.module_ty_from_name(module_name); let module_ty = self.module_ty_from_name(module_name, import_from.into());
let ast::Alias { let ast::Alias {
range: _, range: _,
@ -984,10 +995,26 @@ impl<'db> TypeInferenceBuilder<'db> {
} }
} }
fn module_ty_from_name(&self, module_name: Option<ModuleName>) -> Type<'db> { fn module_ty_from_name(
module_name &mut self,
.and_then(|module_name| resolve_module(self.db, module_name)) module_name: Option<ModuleName>,
.map_or(Type::Unknown, |module| Type::Module(module.file())) node: AnyNodeRef,
) -> Type<'db> {
let Some(module_name) = module_name else {
return Type::Unknown;
};
if let Some(module) = resolve_module(self.db, module_name.clone()) {
Type::Module(module.file())
} else {
self.add_diagnostic(
node,
"unresolved-import",
format_args!("Import '{module_name}' could not be resolved."),
);
Type::Unknown
}
} }
fn infer_decorator(&mut self, decorator: &ast::Decorator) -> Type<'db> { fn infer_decorator(&mut self, decorator: &ast::Decorator) -> Type<'db> {
@ -1059,7 +1086,7 @@ impl<'db> TypeInferenceBuilder<'db> {
ast::Expr::Yield(yield_expression) => self.infer_yield_expression(yield_expression), ast::Expr::Yield(yield_expression) => self.infer_yield_expression(yield_expression),
ast::Expr::YieldFrom(yield_from) => self.infer_yield_from_expression(yield_from), ast::Expr::YieldFrom(yield_from) => self.infer_yield_from_expression(yield_from),
ast::Expr::Await(await_expression) => self.infer_await_expression(await_expression), ast::Expr::Await(await_expression) => self.infer_await_expression(await_expression),
Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"), ast::Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"),
}; };
let expr_id = expression.scoped_ast_id(self.db, self.scope); let expr_id = expression.scoped_ast_id(self.db, self.scope);
@ -1706,6 +1733,28 @@ impl<'db> TypeInferenceBuilder<'db> {
} }
} }
/// Adds a new diagnostic.
///
/// The diagnostic does not get added if the rule isn't enabled for this file.
fn add_diagnostic(&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.types.diagnostics.push(TypeCheckDiagnostic {
file: self.file,
rule: rule.to_string(),
message: message.to_string(),
range: node.range(),
});
}
pub(super) fn finish(mut self) -> TypeInference<'db> { pub(super) fn finish(mut self) -> TypeInference<'db> {
self.infer_region(); self.infer_region();
self.types.shrink_to_fit(); self.types.shrink_to_fit();

View file

@ -109,7 +109,7 @@ impl Workspace {
pub fn check_file(&self, file_id: &FileHandle) -> Result<Vec<String>, Error> { pub fn check_file(&self, file_id: &FileHandle) -> Result<Vec<String>, Error> {
let result = self.db.check_file(file_id.file).map_err(into_error)?; let result = self.db.check_file(file_id.file).map_err(into_error)?;
Ok(result.to_vec()) Ok(result.clone())
} }
/// Checks all open files /// Checks all open files

View file

@ -17,5 +17,8 @@ fn check() {
let result = workspace.check_file(&test).expect("Check to succeed"); let result = workspace.check_file(&test).expect("Check to succeed");
assert_eq!(result, vec!["/test.py:1:8: Unresolved import 'random22'"]); assert_eq!(
result,
vec!["/test.py:1:8: Import 'random22' could not be resolved.",]
);
} }

View file

@ -11,7 +11,6 @@ use ruff_db::{Db as SourceDb, Upcast};
use salsa::plumbing::ZalsaDatabase; use salsa::plumbing::ZalsaDatabase;
use salsa::{Cancelled, Event}; use salsa::{Cancelled, Event};
use crate::lint::Diagnostics;
use crate::workspace::{check_file, Workspace, WorkspaceMetadata}; use crate::workspace::{check_file, Workspace, WorkspaceMetadata};
mod changes; mod changes;
@ -61,7 +60,7 @@ impl RootDatabase {
self.with_db(|db| db.workspace().check(db)) self.with_db(|db| db.workspace().check(db))
} }
pub fn check_file(&self, file: File) -> Result<Diagnostics, Cancelled> { pub fn check_file(&self, file: File) -> Result<Vec<String>, Cancelled> {
self.with_db(|db| check_file(db, file)) self.with_db(|db| check_file(db, file))
} }
@ -115,7 +114,15 @@ impl Upcast<dyn SourceDb> for RootDatabase {
} }
#[salsa::db] #[salsa::db]
impl SemanticDb for RootDatabase {} impl SemanticDb for RootDatabase {
fn is_file_open(&self, file: File) -> bool {
let Some(workspace) = &self.workspace else {
return false;
};
workspace.is_file_open(self, file)
}
}
#[salsa::db] #[salsa::db]
impl SourceDb for RootDatabase { impl SourceDb for RootDatabase {
@ -242,7 +249,12 @@ pub(crate) mod tests {
} }
#[salsa::db] #[salsa::db]
impl red_knot_python_semantic::Db for TestDb {} impl red_knot_python_semantic::Db for TestDb {
fn is_file_open(&self, file: ruff_db::files::File) -> bool {
!file.path(self).is_vendored_path()
}
}
#[salsa::db] #[salsa::db]
impl Db for TestDb {} impl Db for TestDb {}

View file

@ -1,5 +1,4 @@
use std::cell::RefCell; use std::cell::RefCell;
use std::ops::Deref;
use std::time::Duration; use std::time::Duration;
use tracing::debug_span; use tracing::debug_span;
@ -22,7 +21,7 @@ use crate::db::Db;
pub(crate) fn unwind_if_cancelled(db: &dyn Db) {} pub(crate) fn unwind_if_cancelled(db: &dyn Db) {}
#[salsa::tracked(return_ref)] #[salsa::tracked(return_ref)]
pub(crate) fn lint_syntax(db: &dyn Db, file_id: File) -> Diagnostics { pub(crate) fn lint_syntax(db: &dyn Db, file_id: File) -> Vec<String> {
#[allow(clippy::print_stdout)] #[allow(clippy::print_stdout)]
if std::env::var("RED_KNOT_SLOW_LINT").is_ok() { if std::env::var("RED_KNOT_SLOW_LINT").is_ok() {
for i in 0..10 { for i in 0..10 {
@ -64,7 +63,7 @@ pub(crate) fn lint_syntax(db: &dyn Db, file_id: File) -> Diagnostics {
})); }));
} }
Diagnostics::from(diagnostics) diagnostics
} }
fn lint_lines(source: &str, diagnostics: &mut Vec<String>) { fn lint_lines(source: &str, diagnostics: &mut Vec<String>) {
@ -86,7 +85,7 @@ fn lint_lines(source: &str, diagnostics: &mut Vec<String>) {
#[allow(unreachable_pub)] #[allow(unreachable_pub)]
#[salsa::tracked(return_ref)] #[salsa::tracked(return_ref)]
pub fn lint_semantic(db: &dyn Db, file_id: File) -> Diagnostics { pub fn lint_semantic(db: &dyn Db, file_id: File) -> Vec<String> {
let _span = debug_span!("lint_semantic", file=%file_id.path(db)).entered(); let _span = debug_span!("lint_semantic", file=%file_id.path(db)).entered();
let source = source_text(db.upcast(), file_id); let source = source_text(db.upcast(), file_id);
@ -94,7 +93,7 @@ pub fn lint_semantic(db: &dyn Db, file_id: File) -> Diagnostics {
let semantic = SemanticModel::new(db.upcast(), file_id); let semantic = SemanticModel::new(db.upcast(), file_id);
if !parsed.is_valid() { if !parsed.is_valid() {
return Diagnostics::Empty; return vec![];
} }
let context = SemanticLintContext { let context = SemanticLintContext {
@ -106,7 +105,7 @@ pub fn lint_semantic(db: &dyn Db, file_id: File) -> Diagnostics {
SemanticVisitor { context: &context }.visit_body(parsed.suite()); SemanticVisitor { context: &context }.visit_body(parsed.suite());
Diagnostics::from(context.diagnostics.take()) context.diagnostics.take()
} }
fn format_diagnostic(context: &SemanticLintContext, message: &str, start: TextSize) -> String { fn format_diagnostic(context: &SemanticLintContext, message: &str, start: TextSize) -> String {
@ -116,48 +115,13 @@ fn format_diagnostic(context: &SemanticLintContext, message: &str, start: TextSi
.source_location(start, context.source_text()); .source_location(start, context.source_text());
format!( format!(
"{}:{}:{}: {}", "{}:{}:{}: {}",
context.semantic.file_path().as_str(), context.semantic.file_path(),
source_location.row, source_location.row,
source_location.column, source_location.column,
message, message,
) )
} }
fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef) {
// TODO: this treats any symbol with `Type::Unknown` as an unresolved import,
// which isn't really correct: if it exists but has `Type::Unknown` in the
// module we're importing it from, we shouldn't really emit a diagnostic here,
// but currently do.
match import {
AnyImportRef::Import(import) => {
for alias in &import.names {
let ty = alias.ty(&context.semantic);
if ty.is_unknown() {
context.push_diagnostic(format_diagnostic(
context,
&format!("Unresolved import '{}'", &alias.name),
alias.start(),
));
}
}
}
AnyImportRef::ImportFrom(import) => {
for alias in &import.names {
let ty = alias.ty(&context.semantic);
if ty.is_unknown() {
context.push_diagnostic(format_diagnostic(
context,
&format!("Unresolved import '{}'", &alias.name),
alias.start(),
));
}
}
}
}
}
fn lint_maybe_undefined(context: &SemanticLintContext, name: &ast::ExprName) { fn lint_maybe_undefined(context: &SemanticLintContext, name: &ast::ExprName) {
if !matches!(name.ctx, ast::ExprContext::Load) { if !matches!(name.ctx, ast::ExprContext::Load) {
return; return;
@ -280,18 +244,9 @@ struct SemanticVisitor<'a> {
impl Visitor<'_> for SemanticVisitor<'_> { impl Visitor<'_> for SemanticVisitor<'_> {
fn visit_stmt(&mut self, stmt: &ast::Stmt) { fn visit_stmt(&mut self, stmt: &ast::Stmt) {
match stmt { if let ast::Stmt::ClassDef(class) = stmt {
ast::Stmt::ClassDef(class) => {
lint_bad_override(self.context, class); lint_bad_override(self.context, class);
} }
ast::Stmt::Import(import) => {
lint_unresolved_imports(self.context, AnyImportRef::Import(import));
}
ast::Stmt::ImportFrom(import) => {
lint_unresolved_imports(self.context, AnyImportRef::ImportFrom(import));
}
_ => {}
}
walk_stmt(self, stmt); walk_stmt(self, stmt);
} }
@ -308,53 +263,6 @@ impl Visitor<'_> for SemanticVisitor<'_> {
} }
} }
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Diagnostics {
Empty,
List(Vec<String>),
}
impl Diagnostics {
pub fn as_slice(&self) -> &[String] {
match self {
Diagnostics::Empty => &[],
Diagnostics::List(list) => list.as_slice(),
}
}
}
impl Deref for Diagnostics {
type Target = [String];
fn deref(&self) -> &Self::Target {
self.as_slice()
}
}
impl From<Vec<String>> for Diagnostics {
fn from(value: Vec<String>) -> Self {
if value.is_empty() {
Diagnostics::Empty
} else {
Diagnostics::List(value)
}
}
}
#[derive(Copy, Clone, Debug)]
enum AnyImportRef<'a> {
Import(&'a ast::StmtImport),
ImportFrom(&'a ast::StmtImportFrom),
}
impl Ranged for AnyImportRef<'_> {
fn range(&self) -> ruff_text_size::TextRange {
match self {
AnyImportRef::Import(import) => import.range(),
AnyImportRef::ImportFrom(import) => import.range(),
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use red_knot_python_semantic::{Program, ProgramSettings, PythonVersion, SearchPathSettings}; use red_knot_python_semantic::{Program, ProgramSettings, PythonVersion, SearchPathSettings};
@ -363,7 +271,7 @@ mod tests {
use crate::db::tests::TestDb; use crate::db::tests::TestDb;
use super::{lint_semantic, Diagnostics}; use super::lint_semantic;
fn setup_db() -> TestDb { fn setup_db() -> TestDb {
setup_db_with_root(SystemPathBuf::from("/src")) setup_db_with_root(SystemPathBuf::from("/src"))
@ -409,9 +317,9 @@ mod tests {
.unwrap(); .unwrap();
let file = system_path_to_file(&db, "/src/a.py").expect("file to exist"); let file = system_path_to_file(&db, "/src/a.py").expect("file to exist");
let Diagnostics::List(messages) = lint_semantic(&db, file) else { let messages = lint_semantic(&db, file);
panic!("expected some diagnostics");
}; assert_ne!(messages, &[] as &[String], "expected some diagnostics");
assert_eq!( assert_eq!(
*messages, *messages,

View file

@ -4,17 +4,19 @@ use rustc_hash::{FxBuildHasher, FxHashSet};
use salsa::{Durability, Setter as _}; use salsa::{Durability, Setter as _};
pub use metadata::{PackageMetadata, WorkspaceMetadata}; pub use metadata::{PackageMetadata, WorkspaceMetadata};
use ruff_db::source::{source_text, SourceDiagnostic}; use red_knot_python_semantic::types::check_types;
use ruff_db::source::{line_index, source_text, SourceDiagnostic};
use ruff_db::{ use ruff_db::{
files::{system_path_to_file, File}, files::{system_path_to_file, File},
system::{walk_directory::WalkState, SystemPath, SystemPathBuf}, system::{walk_directory::WalkState, SystemPath, SystemPathBuf},
}; };
use ruff_python_ast::{name::Name, PySourceType}; use ruff_python_ast::{name::Name, PySourceType};
use ruff_text_size::Ranged;
use crate::workspace::files::{Index, Indexed, PackageFiles}; use crate::workspace::files::{Index, Indexed, PackageFiles};
use crate::{ use crate::{
db::Db, db::Db,
lint::{lint_semantic, lint_syntax, Diagnostics}, lint::{lint_semantic, lint_syntax},
}; };
mod files; mod files;
@ -92,8 +94,8 @@ pub struct Package {
root_buf: SystemPathBuf, root_buf: SystemPathBuf,
/// The files that are part of this package. /// The files that are part of this package.
#[return_ref]
#[default] #[default]
#[return_ref]
file_set: PackageFiles, file_set: PackageFiles,
// TODO: Add the loaded settings. // TODO: Add the loaded settings.
} }
@ -249,6 +251,23 @@ impl Workspace {
FxHashSet::default() FxHashSet::default()
} }
} }
/// Returns `true` if the file is open in the workspace.
///
/// A file is considered open when:
/// * explicitly set as an open file using [`open_file`](Self::open_file)
/// * It has a [`SystemPath`] and belongs to a package's `src` files
/// * It has a [`SystemVirtualPath`](ruff_db::system::SystemVirtualPath)
pub fn is_file_open(self, db: &dyn Db, file: File) -> bool {
if let Some(open_files) = self.open_files(db) {
open_files.contains(&file)
} else if let Some(system_path) = file.path(db).as_system_path() {
self.package(db, system_path)
.map_or(false, |package| package.contains_file(db, file))
} else {
file.path(db).is_system_virtual_path()
}
}
} }
#[salsa::tracked] #[salsa::tracked]
@ -309,8 +328,12 @@ impl Package {
let _entered = let _entered =
tracing::debug_span!("index_package_files", package = %self.name(db)).entered(); tracing::debug_span!("index_package_files", package = %self.name(db)).entered();
tracing::debug!("Indexing files for package {}", self.name(db));
let files = discover_package_files(db, self.root(db)); let files = discover_package_files(db, self.root(db));
tracing::info!(
"Indexed {} files for package '{}'",
files.len(),
self.name(db)
);
vacant.set(files) vacant.set(files)
} }
Index::Indexed(indexed) => indexed, Index::Indexed(indexed) => indexed,
@ -348,7 +371,7 @@ impl Package {
} }
#[salsa::tracked] #[salsa::tracked]
pub(super) fn check_file(db: &dyn Db, file: File) -> Diagnostics { pub(super) fn check_file(db: &dyn Db, file: File) -> Vec<String> {
let path = file.path(db); let path = file.path(db);
let _span = tracing::debug_span!("check_file", file=%path).entered(); let _span = tracing::debug_span!("check_file", file=%path).entered();
tracing::debug!("Checking file {path}"); tracing::debug!("Checking file {path}");
@ -364,13 +387,25 @@ pub(super) fn check_file(db: &dyn Db, file: File) -> Diagnostics {
); );
// Abort checking if there are IO errors. // Abort checking if there are IO errors.
if source_text(db.upcast(), file).has_read_error() { let source = source_text(db.upcast(), file);
return Diagnostics::from(diagnostics);
if source.has_read_error() {
return diagnostics;
}
for diagnostic in check_types(db.upcast(), file) {
let index = line_index(db.upcast(), diagnostic.file());
let location = index.source_location(diagnostic.start(), source.as_str());
diagnostics.push(format!(
"{path}:{location}: {message}",
path = file.path(db),
message = diagnostic.message()
));
} }
diagnostics.extend_from_slice(lint_syntax(db, file)); diagnostics.extend_from_slice(lint_syntax(db, file));
diagnostics.extend_from_slice(lint_semantic(db, file)); diagnostics.extend_from_slice(lint_semantic(db, file));
Diagnostics::from(diagnostics) diagnostics
} }
fn discover_package_files(db: &dyn Db, path: &SystemPath) -> FxHashSet<File> { fn discover_package_files(db: &dyn Db, path: &SystemPath) -> FxHashSet<File> {
@ -424,7 +459,7 @@ mod tests {
use ruff_db::testing::assert_function_query_was_not_run; use ruff_db::testing::assert_function_query_was_not_run;
use crate::db::tests::TestDb; use crate::db::tests::TestDb;
use crate::lint::{lint_syntax, Diagnostics}; use crate::lint::lint_syntax;
use crate::workspace::check_file; use crate::workspace::check_file;
#[test] #[test]
@ -442,9 +477,7 @@ mod tests {
assert_eq!(source_text(&db, file).as_str(), ""); assert_eq!(source_text(&db, file).as_str(), "");
assert_eq!( assert_eq!(
check_file(&db, file), check_file(&db, file),
Diagnostics::List(vec![ vec!["Failed to read file: No such file or directory".to_string()]
"Failed to read file: No such file or directory".to_string()
])
); );
let events = db.take_salsa_events(); let events = db.take_salsa_events();
@ -455,7 +488,7 @@ mod tests {
db.write_file(path, "").unwrap(); db.write_file(path, "").unwrap();
assert_eq!(source_text(&db, file).as_str(), ""); assert_eq!(source_text(&db, file).as_str(), "");
assert_eq!(check_file(&db, file), Diagnostics::Empty); assert_eq!(check_file(&db, file), vec![] as Vec<String>);
Ok(()) Ok(())
} }

View file

@ -18,6 +18,7 @@ struct Case {
} }
const TOMLLIB_312_URL: &str = "https://raw.githubusercontent.com/python/cpython/8e8a4baf652f6e1cee7acde9d78c4b6154539748/Lib/tomllib"; const TOMLLIB_312_URL: &str = "https://raw.githubusercontent.com/python/cpython/8e8a4baf652f6e1cee7acde9d78c4b6154539748/Lib/tomllib";
const EXPECTED_DIAGNOSTICS: usize = 27;
fn get_test_file(name: &str) -> TestFile { fn get_test_file(name: &str) -> TestFile {
let path = format!("tomllib/{name}"); let path = format!("tomllib/{name}");
@ -89,7 +90,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
let Case { db, parser, .. } = case; let Case { db, parser, .. } = case;
let result = db.check_file(*parser).unwrap(); let result = db.check_file(*parser).unwrap();
assert_eq!(result.len(), 34); assert_eq!(result.len(), EXPECTED_DIAGNOSTICS);
}, },
BatchSize::SmallInput, BatchSize::SmallInput,
); );
@ -104,7 +105,7 @@ fn benchmark_cold(criterion: &mut Criterion) {
let Case { db, parser, .. } = case; let Case { db, parser, .. } = case;
let result = db.check_file(*parser).unwrap(); let result = db.check_file(*parser).unwrap();
assert_eq!(result.len(), 34); assert_eq!(result.len(), EXPECTED_DIAGNOSTICS);
}, },
BatchSize::SmallInput, BatchSize::SmallInput,
); );

View file

@ -254,6 +254,12 @@ impl Debug for SourceLocation {
} }
} }
impl std::fmt::Display for SourceLocation {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "{row}:{column}", row = self.row, column = self.column)
}
}
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub enum SourceRow { pub enum SourceRow {
/// A row within a cell in a Jupyter Notebook. /// A row within a cell in a Jupyter Notebook.