Skip to content

Commit

Permalink
[red-knot] Unresolved imports lint rule
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Apr 27, 2024
1 parent 8491e8f commit f3d7ea9
Show file tree
Hide file tree
Showing 9 changed files with 271 additions and 54 deletions.
2 changes: 1 addition & 1 deletion crates/red_knot/src/cancellation.rs
Expand Up @@ -12,7 +12,7 @@ impl CancellationTokenSource {
}
}

#[tracing::instrument(level = "trace")]
#[tracing::instrument(level = "trace", skip_all)]
pub fn cancel(&self) {
let (cancelled, condvar) = &*self.signal;

Expand Down
28 changes: 18 additions & 10 deletions crates/red_knot/src/db.rs
Expand Up @@ -2,7 +2,7 @@ use std::path::Path;
use std::sync::Arc;

use crate::files::FileId;
use crate::lint::{Diagnostics, LintSyntaxStorage};
use crate::lint::{Diagnostics, LintSemanticStorage, LintSyntaxStorage};
use crate::module::{Module, ModuleData, ModuleName, ModuleResolver, ModuleSearchPath};
use crate::parse::{Parsed, ParsedStorage};
use crate::source::{Source, SourceStorage};
Expand Down Expand Up @@ -32,13 +32,15 @@ pub trait SemanticDb: SourceDb {

fn symbol_table(&self, file_id: FileId) -> Arc<SymbolTable>;

fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> Type;

fn lint_semantic(&self, file_id: FileId) -> Diagnostics;

// mutations

fn add_module(&mut self, path: &Path) -> Option<(Module, Vec<Arc<ModuleData>>)>;

fn set_module_search_paths(&mut self, paths: Vec<ModuleSearchPath>);

fn infer_symbol_type(&mut self, file_id: FileId, symbol_id: SymbolId) -> Type;
}

pub trait Db: SemanticDb {}
Expand All @@ -55,6 +57,7 @@ pub struct SemanticJar {
pub module_resolver: ModuleResolver,
pub symbol_tables: SymbolTablesStorage,
pub type_store: TypeStore,
pub lint_semantic: LintSemanticStorage,
}

/// Gives access to a specific jar in the database.
Expand All @@ -79,9 +82,12 @@ pub trait HasJar<T> {

#[cfg(test)]
pub(crate) mod tests {
use std::path::Path;
use std::sync::Arc;

use crate::db::{HasJar, SourceDb, SourceJar};
use crate::files::{FileId, Files};
use crate::lint::{lint_syntax, Diagnostics};
use crate::lint::{lint_semantic, lint_syntax, Diagnostics};
use crate::module::{
add_module, file_to_module, path_to_module, resolve_module, set_module_search_paths,
Module, ModuleData, ModuleName, ModuleSearchPath,
Expand All @@ -90,8 +96,6 @@ pub(crate) mod tests {
use crate::source::{source_text, Source};
use crate::symbols::{symbol_table, SymbolId, SymbolTable};
use crate::types::{infer_symbol_type, Type};
use std::path::Path;
use std::sync::Arc;

use super::{SemanticDb, SemanticJar};

Expand Down Expand Up @@ -163,16 +167,20 @@ pub(crate) mod tests {
symbol_table(self, file_id)
}

fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> Type {
infer_symbol_type(self, file_id, symbol_id)
}

fn lint_semantic(&self, file_id: FileId) -> Diagnostics {
lint_semantic(self, file_id)
}

fn add_module(&mut self, path: &Path) -> Option<(Module, Vec<Arc<ModuleData>>)> {
add_module(self, path)
}

fn set_module_search_paths(&mut self, paths: Vec<ModuleSearchPath>) {
set_module_search_paths(self, paths);
}

fn infer_symbol_type(&mut self, file_id: FileId, symbol_id: SymbolId) -> Type {
infer_symbol_type(self, file_id, symbol_id)
}
}
}
7 changes: 7 additions & 0 deletions crates/red_knot/src/lib.rs
@@ -1,3 +1,4 @@
use std::fmt::Formatter;
use std::hash::BuildHasherDefault;
use std::ops::Deref;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -100,3 +101,9 @@ where
Self(value.into())
}
}

impl std::fmt::Display for Name {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}
130 changes: 127 additions & 3 deletions crates/red_knot/src/lint.rs
@@ -1,12 +1,17 @@
use std::cell::RefCell;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;

use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::StringLiteral;
use ruff_python_ast::{ModModule, StringLiteral};

use crate::cache::KeyValueCache;
use crate::db::{HasJar, SourceDb, SourceJar};
use crate::db::{HasJar, SemanticDb, SemanticJar, SourceDb, SourceJar};
use crate::files::FileId;
use crate::parse::Parsed;
use crate::source::Source;
use crate::symbols::{Definition, SymbolId, SymbolTable};
use crate::types::Type;

#[tracing::instrument(level = "debug", skip(db))]
pub(crate) fn lint_syntax<Db>(db: &Db, file_id: FileId) -> Diagnostics
Expand Down Expand Up @@ -40,7 +45,7 @@ where
})
}

pub(crate) fn lint_lines(source: &str, diagnostics: &mut Vec<String>) {
fn lint_lines(source: &str, diagnostics: &mut Vec<String>) {
for (line_number, line) in source.lines().enumerate() {
if line.len() < 88 {
continue;
Expand All @@ -57,6 +62,108 @@ pub(crate) fn lint_lines(source: &str, diagnostics: &mut Vec<String>) {
}
}

#[tracing::instrument(level = "debug", skip(db))]
pub(crate) fn lint_semantic<Db>(db: &Db, file_id: FileId) -> Diagnostics
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let storage = &db.jar().lint_semantic;

storage.get(&file_id, |file_id| {
let source = db.source(*file_id);
let parsed = db.parse(*file_id);
let symbols = db.symbol_table(*file_id);

let context = SemanticLintContext {
file_id: *file_id,
source,
parsed,
symbols,
db,
diagnostics: RefCell::new(Vec::new()),
};

lint_unresolved_imports(&context);

Diagnostics::from(context.diagnostics.take())
})
}

fn lint_unresolved_imports(context: &SemanticLintContext) {
for (symbol, definition) in context.symbols().all_definitions() {
match definition {
Definition::Import(import) => {
let ty = context.eval_symbol(symbol);

if ty.is_unknown() {
context.push_diagnostic(format!("Unresolved module {}", import.module));
}
}
Definition::ImportFrom(import) => {
let ty = context.eval_symbol(symbol);

if ty.is_unknown() {
let message = if let Some(module) = import.module() {
format!(
"Unresolved relative import '{}' from '{}{module}'",
import.name(),
".".repeat(import.level() as usize)
)
} else {
format!(
"Unresolved import '{}' from {}",
import.name(),
".".repeat(import.level() as usize)
)
};

context.push_diagnostic(message);
}
}
_ => {}
}
}
}

pub struct SemanticLintContext<'a> {
file_id: FileId,
source: Source,
parsed: Parsed,
symbols: Arc<SymbolTable>,
db: &'a dyn SemanticDb,
diagnostics: RefCell<Vec<String>>,
}

impl<'a> SemanticLintContext<'a> {
pub fn source_text(&self) -> &str {
self.source.text()
}

pub fn file_id(&self) -> FileId {
self.file_id
}

pub fn ast(&self) -> &ModModule {
self.parsed.ast()
}

pub fn symbols(&self) -> &SymbolTable {
&self.symbols
}

pub fn eval_symbol(&self, symbol_id: SymbolId) -> Type {
self.db.infer_symbol_type(self.file_id, symbol_id)
}

pub fn push_diagnostic(&self, diagnostic: String) {
self.diagnostics.borrow_mut().push(diagnostic);
}

pub fn extend_diagnostics(&mut self, diagnostics: impl IntoIterator<Item = String>) {
self.diagnostics.get_mut().extend(diagnostics);
}
}

#[derive(Debug)]
struct SyntaxLintVisitor<'a> {
diagnostics: Vec<String>,
Expand Down Expand Up @@ -123,3 +230,20 @@ impl DerefMut for LintSyntaxStorage {
&mut self.0
}
}

#[derive(Default, Debug)]
pub struct LintSemanticStorage(KeyValueCache<FileId, Diagnostics>);

impl Deref for LintSemanticStorage {
type Target = KeyValueCache<FileId, Diagnostics>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl DerefMut for LintSemanticStorage {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
9 changes: 6 additions & 3 deletions crates/red_knot/src/program/check.rs
@@ -1,11 +1,13 @@
use std::num::NonZeroUsize;

use rayon::max_num_threads;
use rustc_hash::FxHashSet;

use crate::cancellation::CancellationToken;
use crate::db::{SemanticDb, SourceDb};
use crate::files::FileId;
use crate::lint::Diagnostics;
use crate::program::Program;
use rayon::max_num_threads;
use rustc_hash::FxHashSet;
use std::num::NonZeroUsize;

impl Program {
/// Checks all open files in the workspace and its dependencies.
Expand Down Expand Up @@ -71,6 +73,7 @@ impl Program {

if self.workspace().is_file_open(file) {
diagnostics.extend_from_slice(&self.lint_syntax(file));
diagnostics.extend_from_slice(&self.lint_semantic(file));
}

Ok(Diagnostics::from(diagnostics))
Expand Down
17 changes: 11 additions & 6 deletions crates/red_knot/src/program/mod.rs
Expand Up @@ -5,7 +5,7 @@ use std::sync::Arc;

use crate::db::{Db, HasJar, SemanticDb, SemanticJar, SourceDb, SourceJar};
use crate::files::{FileId, Files};
use crate::lint::{lint_syntax, Diagnostics, LintSyntaxStorage};
use crate::lint::{lint_semantic, lint_syntax, Diagnostics, LintSyntaxStorage};
use crate::module::{
add_module, file_to_module, path_to_module, resolve_module, set_module_search_paths, Module,
ModuleData, ModuleName, ModuleResolver, ModuleSearchPath,
Expand Down Expand Up @@ -36,6 +36,7 @@ impl Program {
module_resolver: ModuleResolver::new(module_search_paths),
symbol_tables: SymbolTablesStorage::default(),
type_store: TypeStore::default(),
lint_semantic: LintSemanticStorage::default(),
},
files: Files::default(),
workspace,
Expand All @@ -56,6 +57,7 @@ impl Program {
self.source.lint_syntax.remove(&change.id);
// TODO: remove all dependent modules as well
self.semantic.type_store.remove_module(change.id);
self.semantic.lint_semantic.remove(&change.id);
}
}

Expand Down Expand Up @@ -111,19 +113,22 @@ impl SemanticDb for Program {
symbol_table(self, file_id)
}

// Mutations
fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> Type {
infer_symbol_type(self, file_id, symbol_id)
}

fn lint_semantic(&self, file_id: FileId) -> Diagnostics {
lint_semantic(self, file_id)
}

// Mutations
fn add_module(&mut self, path: &Path) -> Option<(Module, Vec<Arc<ModuleData>>)> {
add_module(self, path)
}

fn set_module_search_paths(&mut self, paths: Vec<ModuleSearchPath>) {
set_module_search_paths(self, paths);
}

fn infer_symbol_type(&mut self, file_id: FileId, symbol_id: SymbolId) -> Type {
infer_symbol_type(self, file_id, symbol_id)
}
}

impl Db for Program {}
Expand Down

0 comments on commit f3d7ea9

Please sign in to comment.