Compare commits

..

2 Commits

Author SHA1 Message Date
Dhruv Manilawala
ea48d1068d Use usize, simplify is_eof 2024-05-01 09:46:31 +05:30
Dhruv Manilawala
afe4cdb691 Update Cursor to use position instead of iterator 2024-05-01 09:24:43 +05:30
198 changed files with 2910 additions and 3025 deletions

View File

@@ -59,6 +59,7 @@ jobs:
- "!crates/ruff_python_formatter/**"
- "!crates/ruff_formatter/**"
- "!crates/ruff_dev/**"
- "!crates/ruff_shrinking/**"
- scripts/*
- python/**
- .github/workflows/ci.yaml

View File

@@ -1,53 +1,5 @@
# Changelog
## 0.4.3
### Enhancements
- Add support for PEP 696 syntax ([#11120](https://github.com/astral-sh/ruff/pull/11120))
### Preview features
- \[`refurb`\] Use function range for `reimplemented-operator` diagnostics ([#11271](https://github.com/astral-sh/ruff/pull/11271))
- \[`refurb`\] Ignore methods in `reimplemented-operator` (`FURB118`) ([#11270](https://github.com/astral-sh/ruff/pull/11270))
- \[`refurb`\] Implement `fstring-number-format` (`FURB116`) ([#10921](https://github.com/astral-sh/ruff/pull/10921))
- \[`ruff`\] Implement `redirected-noqa` (`RUF101`) ([#11052](https://github.com/astral-sh/ruff/pull/11052))
- \[`pyflakes`\] Distinguish between first-party and third-party imports for fix suggestions ([#11168](https://github.com/astral-sh/ruff/pull/11168))
### Rule changes
- \[`flake8-bugbear`\] Ignore non-abstract class attributes when enforcing `B024` ([#11210](https://github.com/astral-sh/ruff/pull/11210))
- \[`flake8-logging`\] Include inline instantiations when detecting loggers ([#11154](https://github.com/astral-sh/ruff/pull/11154))
- \[`pylint`\] Also emit `PLR0206` for properties with variadic parameters ([#11200](https://github.com/astral-sh/ruff/pull/11200))
- \[`ruff`\] Detect duplicate codes as part of `unused-noqa` (`RUF100`) ([#10850](https://github.com/astral-sh/ruff/pull/10850))
### Formatter
- Avoid multiline expression if format specifier is present ([#11123](https://github.com/astral-sh/ruff/pull/11123))
### LSP
- Write `ruff server` setup guide for Helix ([#11183](https://github.com/astral-sh/ruff/pull/11183))
- `ruff server` no longer hangs after shutdown ([#11222](https://github.com/astral-sh/ruff/pull/11222))
- `ruff server` reads from a configuration TOML file in the user configuration directory if no local configuration exists ([#11225](https://github.com/astral-sh/ruff/pull/11225))
- `ruff server` respects `per-file-ignores` configuration ([#11224](https://github.com/astral-sh/ruff/pull/11224))
- `ruff server`: Support a custom TOML configuration file ([#11140](https://github.com/astral-sh/ruff/pull/11140))
- `ruff server`: Support setting to prioritize project configuration over editor configuration ([#11086](https://github.com/astral-sh/ruff/pull/11086))
### Bug fixes
- Avoid debug assertion around NFKC renames ([#11249](https://github.com/astral-sh/ruff/pull/11249))
- \[`pyflakes`\] Prioritize `redefined-while-unused` over `unused-import` ([#11173](https://github.com/astral-sh/ruff/pull/11173))
- \[`ruff`\] Respect `async` expressions in comprehension bodies ([#11219](https://github.com/astral-sh/ruff/pull/11219))
- \[`pygrep_hooks`\] Fix `blanket-noqa` panic when last line has noqa with no newline (`PGH004`) ([#11108](https://github.com/astral-sh/ruff/pull/11108))
- \[`perflint`\] Ignore list-copy recommendations for async `for` loops ([#11250](https://github.com/astral-sh/ruff/pull/11250))
- \[`pyflakes`\] Improve `invalid-print-syntax` documentation ([#11171](https://github.com/astral-sh/ruff/pull/11171))
### Performance
- Avoid allocations for isort module names ([#11251](https://github.com/astral-sh/ruff/pull/11251))
- Build a separate ARM wheel for macOS ([#11149](https://github.com/astral-sh/ruff/pull/11149))
## 0.4.2
### Rule changes

63
Cargo.lock generated
View File

@@ -501,19 +501,6 @@ dependencies = [
"itertools 0.10.5",
]
[[package]]
name = "crossbeam"
version = "0.8.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1137cd7e7fc0fb5d3c5a8678be38ec56e819125d8d7907411fe24ccb943faca8"
dependencies = [
"crossbeam-channel",
"crossbeam-deque",
"crossbeam-epoch",
"crossbeam-queue",
"crossbeam-utils",
]
[[package]]
name = "crossbeam-channel"
version = "0.5.12"
@@ -542,15 +529,6 @@ dependencies = [
"crossbeam-utils",
]
[[package]]
name = "crossbeam-queue"
version = "0.3.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "df0346b5d5e76ac2fe4e327c5fd1118d6be7c51dfb18f9b7922923f287471e35"
dependencies = [
"crossbeam-utils",
]
[[package]]
name = "crossbeam-utils"
version = "0.8.19"
@@ -1457,6 +1435,16 @@ dependencies = [
"autocfg",
]
[[package]]
name = "num_cpus"
version = "1.16.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4161fcb6d602d4d2081af7c3a45852d875a03dd337a6bfdd6e06407b61342a43"
dependencies = [
"hermit-abi",
"libc",
]
[[package]]
name = "number_prefix"
version = "0.4.0"
@@ -1816,7 +1804,7 @@ version = "0.1.0"
dependencies = [
"anyhow",
"bitflags 2.5.0",
"crossbeam",
"crossbeam-channel",
"ctrlc",
"dashmap",
"hashbrown 0.14.5",
@@ -1943,7 +1931,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.4.3"
version = "0.4.2"
dependencies = [
"anyhow",
"argfile",
@@ -1964,6 +1952,7 @@ dependencies = [
"log",
"mimalloc",
"notify",
"num_cpus",
"path-absolutize",
"rayon",
"regex",
@@ -2104,7 +2093,7 @@ dependencies = [
[[package]]
name = "ruff_linter"
version = "0.4.3"
version = "0.4.2"
dependencies = [
"aho-corasick",
"annotate-snippets 0.9.2",
@@ -2352,7 +2341,7 @@ name = "ruff_server"
version = "0.2.2"
dependencies = [
"anyhow",
"crossbeam",
"crossbeam-channel",
"insta",
"jod-thread",
"libc",
@@ -2377,6 +2366,22 @@ dependencies = [
"walkdir",
]
[[package]]
name = "ruff_shrinking"
version = "0.4.2"
dependencies = [
"anyhow",
"clap",
"fs-err",
"regex",
"ruff_python_ast",
"ruff_python_parser",
"ruff_text_size",
"shlex",
"tracing",
"tracing-subscriber",
]
[[package]]
name = "ruff_source_file"
version = "0.0.0"
@@ -2702,6 +2707,12 @@ dependencies = [
"dirs 5.0.1",
]
[[package]]
name = "shlex"
version = "1.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64"
[[package]]
name = "similar"
version = "2.5.0"

View File

@@ -30,7 +30,7 @@ console_error_panic_hook = { version = "0.1.7" }
console_log = { version = "1.0.0" }
countme = { version = "3.0.1" }
criterion = { version = "0.5.1", default-features = false }
crossbeam = { version = "0.8.4" }
crossbeam-channel = { version = "0.5.12" }
dashmap = { version = "5.5.3" }
dirs = { version = "5.0.0" }
drop_bomb = { version = "0.1.5" }
@@ -66,6 +66,7 @@ memchr = { version = "2.7.1" }
mimalloc = { version = "0.1.39" }
natord = { version = "1.0.9" }
notify = { version = "6.1.1" }
num_cpus = { version = "1.16.0" }
once_cell = { version = "1.19.0" }
path-absolutize = { version = "3.1.1" }
path-slash = { version = "0.2.1" }
@@ -90,6 +91,7 @@ serde_json = { version = "1.0.113" }
serde_test = { version = "1.0.152" }
serde_with = { version = "3.6.0", default-features = false, features = ["macros"] }
shellexpand = { version = "3.0.0" }
shlex = { version = "1.3.0" }
similar = { version = "2.4.0", features = ["inline"] }
smallvec = { version = "1.13.2" }
static_assertions = "1.1.0"

View File

@@ -152,7 +152,7 @@ Ruff can also be used as a [pre-commit](https://pre-commit.com/) hook via [`ruff
```yaml
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.4.3
rev: v0.4.2
hooks:
# Run the linter.
- id: ruff

View File

@@ -3,10 +3,5 @@ doc-valid-idents = [
"CodeQL",
"IPython",
"NumPy",
"LibCST",
"SCREAMING_SNAKE_CASE",
"SQLAlchemy",
"McCabe",
"FastAPI",
"..",
]

View File

@@ -22,7 +22,7 @@ ruff_notebook = { path = "../ruff_notebook" }
anyhow = { workspace = true }
bitflags = { workspace = true }
ctrlc = "3.4.4"
crossbeam = { workspace = true }
crossbeam-channel = { workspace = true }
dashmap = { workspace = true }
hashbrown = { workspace = true }
indexmap = { workspace = true }

View File

@@ -2,7 +2,6 @@ use std::fmt::Formatter;
use std::hash::Hash;
use std::sync::atomic::{AtomicUsize, Ordering};
use crate::db::QueryResult;
use dashmap::mapref::entry::Entry;
use crate::FxDashMap;
@@ -28,11 +27,11 @@ where
}
}
pub fn get<F>(&self, key: &K, compute: F) -> QueryResult<V>
pub fn get<F>(&self, key: &K, compute: F) -> V
where
F: FnOnce(&K) -> QueryResult<V>,
F: FnOnce(&K) -> V,
{
Ok(match self.map.entry(key.clone()) {
match self.map.entry(key.clone()) {
Entry::Occupied(cached) => {
self.statistics.hit();
@@ -41,11 +40,11 @@ where
Entry::Vacant(vacant) => {
self.statistics.miss();
let value = compute(key)?;
let value = compute(key);
vacant.insert(value.clone());
value
}
})
}
}
pub fn set(&mut self, key: K, value: V) {
@@ -118,29 +117,23 @@ pub type CacheStatistics = DebugStatistics;
#[cfg(not(debug_assertions))]
pub type CacheStatistics = ReleaseStatistics;
pub trait StatisticsRecorder {
fn hit(&self);
fn miss(&self);
fn to_statistics(&self) -> Option<Statistics>;
}
#[derive(Debug, Default)]
pub struct DebugStatistics {
hits: AtomicUsize,
misses: AtomicUsize,
}
impl StatisticsRecorder for DebugStatistics {
impl DebugStatistics {
// TODO figure out appropriate Ordering
fn hit(&self) {
pub fn hit(&self) {
self.hits.fetch_add(1, Ordering::SeqCst);
}
fn miss(&self) {
pub fn miss(&self) {
self.misses.fetch_add(1, Ordering::SeqCst);
}
fn to_statistics(&self) -> Option<Statistics> {
pub fn to_statistics(&self) -> Option<Statistics> {
let hits = self.hits.load(Ordering::SeqCst);
let misses = self.misses.load(Ordering::SeqCst);
@@ -151,15 +144,15 @@ impl StatisticsRecorder for DebugStatistics {
#[derive(Debug, Default)]
pub struct ReleaseStatistics;
impl StatisticsRecorder for ReleaseStatistics {
impl ReleaseStatistics {
#[inline]
fn hit(&self) {}
pub const fn hit(&self) {}
#[inline]
fn miss(&self) {}
pub const fn miss(&self) {}
#[inline]
fn to_statistics(&self) -> Option<Statistics> {
pub const fn to_statistics(&self) -> Option<Statistics> {
None
}
}

View File

@@ -1,25 +1,35 @@
use std::sync::atomic::AtomicBool;
use std::sync::Arc;
use std::sync::{Arc, Condvar, Mutex};
#[derive(Debug, Clone, Default)]
#[derive(Debug, Default)]
pub struct CancellationTokenSource {
signal: Arc<AtomicBool>,
signal: Arc<(Mutex<bool>, Condvar)>,
}
impl CancellationTokenSource {
pub fn new() -> Self {
Self {
signal: Arc::new(AtomicBool::new(false)),
signal: Arc::new((Mutex::new(false), Condvar::default())),
}
}
#[tracing::instrument(level = "trace", skip_all)]
pub fn cancel(&self) {
self.signal.store(true, std::sync::atomic::Ordering::SeqCst);
let (cancelled, condvar) = &*self.signal;
let mut cancelled = cancelled.lock().unwrap();
if *cancelled {
return;
}
*cancelled = true;
condvar.notify_all();
}
pub fn is_cancelled(&self) -> bool {
self.signal.load(std::sync::atomic::Ordering::SeqCst)
let (cancelled, _) = &*self.signal;
*cancelled.lock().unwrap()
}
pub fn token(&self) -> CancellationToken {
@@ -31,12 +41,26 @@ impl CancellationTokenSource {
#[derive(Clone, Debug)]
pub struct CancellationToken {
signal: Arc<AtomicBool>,
signal: Arc<(Mutex<bool>, Condvar)>,
}
impl CancellationToken {
/// Returns `true` if cancellation has been requested.
pub fn is_cancelled(&self) -> bool {
self.signal.load(std::sync::atomic::Ordering::SeqCst)
let (cancelled, _) = &*self.signal;
*cancelled.lock().unwrap()
}
pub fn wait(&self) {
let (bool, condvar) = &*self.signal;
let lock = condvar
.wait_while(bool.lock().unwrap(), |bool| !*bool)
.unwrap();
debug_assert!(*lock);
drop(lock);
}
}

View File

@@ -1,11 +1,6 @@
use std::path::Path;
use std::sync::Arc;
pub use jars::{HasJar, HasJars};
pub use query::{QueryError, QueryResult};
pub use runtime::DbRuntime;
pub use storage::JarsStorage;
use crate::files::FileId;
use crate::lint::{Diagnostics, LintSemanticStorage, LintSyntaxStorage};
use crate::module::{Module, ModuleData, ModuleName, ModuleResolver, ModuleSearchPath};
@@ -14,112 +9,32 @@ use crate::source::{Source, SourceStorage};
use crate::symbols::{SymbolId, SymbolTable, SymbolTablesStorage};
use crate::types::{Type, TypeStore};
mod jars;
mod query;
mod runtime;
mod storage;
pub trait Database {
/// Returns a reference to the runtime of the current worker.
fn runtime(&self) -> &DbRuntime;
/// Returns a mutable reference to the runtime. Only one worker can hold a mutable reference to the runtime.
fn runtime_mut(&mut self) -> &mut DbRuntime;
/// Returns `Ok` if the queries have not been cancelled and `Err(QueryError::Cancelled)` otherwise.
fn cancelled(&self) -> QueryResult<()> {
self.runtime().cancelled()
}
/// Returns `true` if the queries have been cancelled.
fn is_cancelled(&self) -> bool {
self.runtime().is_cancelled()
}
}
/// Database that supports running queries from multiple threads.
pub trait ParallelDatabase: Database + Send {
/// Creates a snapshot of the database state that can be used to query the database in another thread.
///
/// The snapshot is a read-only view of the database but query results are shared between threads.
/// All queries will be automatically cancelled when applying any mutations (calling [`HasJars::jars_mut`])
/// to the database (not the snapshot, because they're readonly).
///
/// ## Creating a snapshot
///
/// Creating a snapshot of the database's jars is cheap but creating a snapshot of
/// other state stored on the database might require deep-cloning data. That's why you should
/// avoid creating snapshots in a hot function (e.g. don't create a snapshot for each file, instead
/// create a snapshot when scheduling the check of an entire program).
///
/// ## Salsa compatibility
/// Salsa prohibits creating a snapshot while running a local query (it's fine if other workers run a query) [[source](https://github.com/salsa-rs/salsa/issues/80)].
/// We should avoid creating snapshots while running a query because we might want to adopt Salsa in the future (if we can figure out persistent caching).
/// Unfortunately, the infrastructure doesn't provide an automated way of knowing when a query is run, that's
/// why we have to "enforce" this constraint manually.
#[must_use]
fn snapshot(&self) -> Snapshot<Self>;
}
/// Readonly snapshot of a database.
///
/// ## Dead locks
/// A snapshot should always be dropped as soon as it is no longer necessary to run queries.
/// Storing the snapshot without running a query or periodically checking if cancellation was requested
/// can lead to deadlocks because mutating the [`Database`] requires cancels all pending queries
/// and waiting for all [`Snapshot`]s to be dropped.
#[derive(Debug)]
pub struct Snapshot<DB: ?Sized>
where
DB: ParallelDatabase,
{
db: DB,
}
impl<DB> Snapshot<DB>
where
DB: ParallelDatabase,
{
pub fn new(db: DB) -> Self {
Snapshot { db }
}
}
impl<DB> std::ops::Deref for Snapshot<DB>
where
DB: ParallelDatabase,
{
type Target = DB;
fn deref(&self) -> &DB {
&self.db
}
}
// Red knot specific databases code.
pub trait SourceDb: Database {
pub trait SourceDb {
// queries
fn file_id(&self, path: &std::path::Path) -> FileId;
fn file_path(&self, file_id: FileId) -> Arc<std::path::Path>;
fn source(&self, file_id: FileId) -> QueryResult<Source>;
fn source(&self, file_id: FileId) -> Source;
fn parse(&self, file_id: FileId) -> QueryResult<Parsed>;
fn parse(&self, file_id: FileId) -> Parsed;
fn lint_syntax(&self, file_id: FileId) -> Diagnostics;
}
pub trait SemanticDb: SourceDb {
// queries
fn resolve_module(&self, name: ModuleName) -> QueryResult<Option<Module>>;
fn resolve_module(&self, name: ModuleName) -> Option<Module>;
fn file_to_module(&self, file_id: FileId) -> QueryResult<Option<Module>>;
fn file_to_module(&self, file_id: FileId) -> Option<Module>;
fn path_to_module(&self, path: &Path) -> QueryResult<Option<Module>>;
fn path_to_module(&self, path: &Path) -> Option<Module>;
fn symbol_table(&self, file_id: FileId) -> QueryResult<Arc<SymbolTable>>;
fn symbol_table(&self, file_id: FileId) -> Arc<SymbolTable>;
fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> QueryResult<Type>;
fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> Type;
fn lint_semantic(&self, file_id: FileId) -> Diagnostics;
// mutations
@@ -128,18 +43,13 @@ pub trait SemanticDb: SourceDb {
fn set_module_search_paths(&mut self, paths: Vec<ModuleSearchPath>);
}
pub trait LintDb: SemanticDb {
fn lint_syntax(&self, file_id: FileId) -> QueryResult<Diagnostics>;
fn lint_semantic(&self, file_id: FileId) -> QueryResult<Diagnostics>;
}
pub trait Db: LintDb {}
pub trait Db: SemanticDb {}
#[derive(Debug, Default)]
pub struct SourceJar {
pub sources: SourceStorage,
pub parsed: ParsedStorage,
pub lint_syntax: LintSyntaxStorage,
}
#[derive(Debug, Default)]
@@ -147,12 +57,27 @@ pub struct SemanticJar {
pub module_resolver: ModuleResolver,
pub symbol_tables: SymbolTablesStorage,
pub type_store: TypeStore,
pub lint_semantic: LintSemanticStorage,
}
#[derive(Debug, Default)]
pub struct LintJar {
pub lint_syntax: LintSyntaxStorage,
pub lint_semantic: LintSemanticStorage,
/// Gives access to a specific jar in the database.
///
/// Nope, the terminology isn't borrowed from Java but from Salsa <https://salsa-rs.github.io/salsa/>,
/// which is an analogy to storing the salsa in different jars.
///
/// The basic idea is that each crate can define its own jar and the jars can be combined to a single
/// database in the top level crate. Each crate also defines its own `Database` trait. The combination of
/// `Database` trait and the jar allows to write queries in isolation without having to know how they get composed at the upper levels.
///
/// Salsa further defines a `HasIngredient` trait which slices the jar to a specific storage (e.g. a specific cache).
/// We don't need this just jet because we write our queries by hand. We may want a similar trait if we decide
/// to use a macro to generate the queries.
pub trait HasJar<T> {
/// Gives a read-only reference to the jar.
fn jar(&self) -> &T;
/// Gives a mutable reference to the jar.
fn jar_mut(&mut self) -> &mut T;
}
#[cfg(test)]
@@ -160,10 +85,7 @@ pub(crate) mod tests {
use std::path::Path;
use std::sync::Arc;
use crate::db::{
Database, DbRuntime, HasJar, HasJars, JarsStorage, LintDb, LintJar, QueryResult, SourceDb,
SourceJar,
};
use crate::db::{HasJar, SourceDb, SourceJar};
use crate::files::{FileId, Files};
use crate::lint::{lint_semantic, lint_syntax, Diagnostics};
use crate::module::{
@@ -182,36 +104,27 @@ pub(crate) mod tests {
#[derive(Debug, Default)]
pub(crate) struct TestDb {
files: Files,
jars: JarsStorage<Self>,
source: SourceJar,
semantic: SemanticJar,
}
impl HasJar<SourceJar> for TestDb {
fn jar(&self) -> QueryResult<&SourceJar> {
Ok(&self.jars()?.0)
fn jar(&self) -> &SourceJar {
&self.source
}
fn jar_mut(&mut self) -> &mut SourceJar {
&mut self.jars_mut().0
&mut self.source
}
}
impl HasJar<SemanticJar> for TestDb {
fn jar(&self) -> QueryResult<&SemanticJar> {
Ok(&self.jars()?.1)
fn jar(&self) -> &SemanticJar {
&self.semantic
}
fn jar_mut(&mut self) -> &mut SemanticJar {
&mut self.jars_mut().1
}
}
impl HasJar<LintJar> for TestDb {
fn jar(&self) -> QueryResult<&LintJar> {
Ok(&self.jars()?.2)
}
fn jar_mut(&mut self) -> &mut LintJar {
&mut self.jars_mut().2
&mut self.semantic
}
}
@@ -224,36 +137,44 @@ pub(crate) mod tests {
self.files.path(file_id)
}
fn source(&self, file_id: FileId) -> QueryResult<Source> {
fn source(&self, file_id: FileId) -> Source {
source_text(self, file_id)
}
fn parse(&self, file_id: FileId) -> QueryResult<Parsed> {
fn parse(&self, file_id: FileId) -> Parsed {
parse(self, file_id)
}
fn lint_syntax(&self, file_id: FileId) -> Diagnostics {
lint_syntax(self, file_id)
}
}
impl SemanticDb for TestDb {
fn resolve_module(&self, name: ModuleName) -> QueryResult<Option<Module>> {
fn resolve_module(&self, name: ModuleName) -> Option<Module> {
resolve_module(self, name)
}
fn file_to_module(&self, file_id: FileId) -> QueryResult<Option<Module>> {
fn file_to_module(&self, file_id: FileId) -> Option<Module> {
file_to_module(self, file_id)
}
fn path_to_module(&self, path: &Path) -> QueryResult<Option<Module>> {
fn path_to_module(&self, path: &Path) -> Option<Module> {
path_to_module(self, path)
}
fn symbol_table(&self, file_id: FileId) -> QueryResult<Arc<SymbolTable>> {
fn symbol_table(&self, file_id: FileId) -> Arc<SymbolTable> {
symbol_table(self, file_id)
}
fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> QueryResult<Type> {
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)
}
@@ -262,36 +183,4 @@ pub(crate) mod tests {
set_module_search_paths(self, paths);
}
}
impl LintDb for TestDb {
fn lint_syntax(&self, file_id: FileId) -> QueryResult<Diagnostics> {
lint_syntax(self, file_id)
}
fn lint_semantic(&self, file_id: FileId) -> QueryResult<Diagnostics> {
lint_semantic(self, file_id)
}
}
impl HasJars for TestDb {
type Jars = (SourceJar, SemanticJar, LintJar);
fn jars(&self) -> QueryResult<&Self::Jars> {
self.jars.jars()
}
fn jars_mut(&mut self) -> &mut Self::Jars {
self.jars.jars_mut()
}
}
impl Database for TestDb {
fn runtime(&self) -> &DbRuntime {
self.jars.runtime()
}
fn runtime_mut(&mut self) -> &mut DbRuntime {
self.jars.runtime_mut()
}
}
}

View File

@@ -1,37 +0,0 @@
use crate::db::query::QueryResult;
/// Gives access to a specific jar in the database.
///
/// Nope, the terminology isn't borrowed from Java but from Salsa <https://salsa-rs.github.io/salsa/>,
/// which is an analogy to storing the salsa in different jars.
///
/// The basic idea is that each crate can define its own jar and the jars can be combined to a single
/// database in the top level crate. Each crate also defines its own `Database` trait. The combination of
/// `Database` trait and the jar allows to write queries in isolation without having to know how they get composed at the upper levels.
///
/// Salsa further defines a `HasIngredient` trait which slices the jar to a specific storage (e.g. a specific cache).
/// We don't need this just jet because we write our queries by hand. We may want a similar trait if we decide
/// to use a macro to generate the queries.
pub trait HasJar<T> {
/// Gives a read-only reference to the jar.
fn jar(&self) -> QueryResult<&T>;
/// Gives a mutable reference to the jar.
fn jar_mut(&mut self) -> &mut T;
}
/// Gives access to the jars in a database.
pub trait HasJars {
/// A type storing the jars.
///
/// Most commonly, this is a tuple where each jar is a tuple element.
type Jars: Default;
/// Gives access to the underlying jars but tests if the queries have been cancelled.
///
/// Returns `Err(QueryError::Cancelled)` if the queries have been cancelled.
fn jars(&self) -> QueryResult<&Self::Jars>;
/// Gives mutable access to the underlying jars.
fn jars_mut(&mut self) -> &mut Self::Jars;
}

View File

@@ -1,20 +0,0 @@
use std::fmt::{Display, Formatter};
/// Reason why a db query operation failed.
#[derive(Debug, Clone, Copy)]
pub enum QueryError {
/// The query was cancelled because the DB was mutated or the query was cancelled by the host (e.g. on a file change or when pressing CTRL+C).
Cancelled,
}
impl Display for QueryError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
QueryError::Cancelled => f.write_str("query was cancelled"),
}
}
}
impl std::error::Error for QueryError {}
pub type QueryResult<T> = Result<T, QueryError>;

View File

@@ -1,41 +0,0 @@
use crate::cancellation::CancellationTokenSource;
use crate::db::{QueryError, QueryResult};
/// Holds the jar agnostic state of the database.
#[derive(Debug, Default)]
pub struct DbRuntime {
/// The cancellation token source used to signal other works that the queries should be aborted and
/// exit at the next possible point.
cancellation_token: CancellationTokenSource,
}
impl DbRuntime {
pub(super) fn snapshot(&self) -> Self {
Self {
cancellation_token: self.cancellation_token.clone(),
}
}
/// Cancels the pending queries of other workers. The current worker cannot have any pending
/// queries because we're holding a mutable reference to the runtime.
pub(super) fn cancel_other_workers(&mut self) {
self.cancellation_token.cancel();
// Set a new cancellation token so that we're in a non-cancelled state again when running the next
// query.
self.cancellation_token = CancellationTokenSource::default();
}
/// Returns `Ok` if the queries have not been cancelled and `Err(QueryError::Cancelled)` otherwise.
pub(super) fn cancelled(&self) -> QueryResult<()> {
if self.cancellation_token.is_cancelled() {
Err(QueryError::Cancelled)
} else {
Ok(())
}
}
/// Returns `true` if the queries have been cancelled.
pub(super) fn is_cancelled(&self) -> bool {
self.cancellation_token.is_cancelled()
}
}

View File

@@ -1,117 +0,0 @@
use std::fmt::Formatter;
use std::sync::Arc;
use crossbeam::sync::WaitGroup;
use crate::db::query::QueryResult;
use crate::db::runtime::DbRuntime;
use crate::db::{HasJars, ParallelDatabase};
/// Stores the jars of a database and the state for each worker.
///
/// Today, all state is shared across all workers, but it may be desired to store data per worker in the future.
pub struct JarsStorage<T>
where
T: HasJars + Sized,
{
// It's important that `jars_wait_group` is declared after `jars` to ensure that `jars` is dropped first.
// See https://doc.rust-lang.org/reference/destructors.html
/// Stores the jars of the database.
jars: Arc<T::Jars>,
/// Used to count the references to `jars`. Allows implementing `jars_mut` without requiring to clone `jars`.
jars_wait_group: WaitGroup,
/// The data agnostic state.
runtime: DbRuntime,
}
impl<Db> JarsStorage<Db>
where
Db: HasJars,
{
pub(super) fn new() -> Self {
Self {
jars: Arc::new(Db::Jars::default()),
jars_wait_group: WaitGroup::default(),
runtime: DbRuntime::default(),
}
}
/// Creates a snapshot of the jars.
///
/// Creating the snapshot is cheap because it doesn't clone the jars, it only increments a ref counter.
#[must_use]
pub fn snapshot(&self) -> JarsStorage<Db>
where
Db: ParallelDatabase,
{
Self {
jars: self.jars.clone(),
jars_wait_group: self.jars_wait_group.clone(),
runtime: self.runtime.snapshot(),
}
}
pub(crate) fn jars(&self) -> QueryResult<&Db::Jars> {
self.runtime.cancelled()?;
Ok(&self.jars)
}
/// Returns a mutable reference to the jars without cloning their content.
///
/// The method cancels any pending queries of other works and waits for them to complete so that
/// this instance is the only instance holding a reference to the jars.
pub(crate) fn jars_mut(&mut self) -> &mut Db::Jars {
// We have a mutable ref here, so no more workers can be spawned between calling this function and taking the mut ref below.
self.cancel_other_workers();
// Now all other references to `self.jars` should have been released. We can now safely return a mutable reference
// to the Arc's content.
let jars =
Arc::get_mut(&mut self.jars).expect("All references to jars should have been released");
jars
}
pub(crate) fn runtime(&self) -> &DbRuntime {
&self.runtime
}
pub(crate) fn runtime_mut(&mut self) -> &mut DbRuntime {
// Note: This method may need to use a similar trick to `jars_mut` if `DbRuntime` is ever to store data that is shared between workers.
&mut self.runtime
}
#[tracing::instrument(level = "trace", skip(self))]
fn cancel_other_workers(&mut self) {
self.runtime.cancel_other_workers();
// Wait for all other works to complete.
let existing_wait = std::mem::take(&mut self.jars_wait_group);
existing_wait.wait();
}
}
impl<Db> Default for JarsStorage<Db>
where
Db: HasJars,
{
fn default() -> Self {
Self::new()
}
}
impl<T> std::fmt::Debug for JarsStorage<T>
where
T: HasJars,
<T as HasJars>::Jars: std::fmt::Debug,
{
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.debug_struct("SharedStorage")
.field("jars", &self.jars)
.field("jars_wait_group", &self.jars_wait_group)
.field("runtime", &self.runtime)
.finish()
}
}

View File

@@ -15,9 +15,9 @@ type Map<K, V> = hashbrown::HashMap<K, V, ()>;
pub struct FileId;
// TODO we'll need a higher level virtual file system abstraction that allows testing if a file exists
// or retrieving its content (ideally lazily and in a way that the memory can be retained later)
// I suspect that we'll end up with a FileSystem trait and our own Path abstraction.
#[derive(Default)]
// or retrieving its content (ideally lazily and in a way that the memory can be retained later)
// I suspect that we'll end up with a FileSystem trait and our own Path abstraction.
#[derive(Clone, Default)]
pub struct Files {
inner: Arc<RwLock<FilesInner>>,
}
@@ -36,16 +36,6 @@ impl Files {
pub fn path(&self, id: FileId) -> Arc<Path> {
self.inner.read().path(id)
}
/// Snapshots files for a new database snapshot.
///
/// This method should not be used outside a database snapshot.
#[must_use]
pub fn snapshot(&self) -> Files {
Files {
inner: self.inner.clone(),
}
}
}
impl Debug for Files {
@@ -73,7 +63,7 @@ struct FilesInner {
by_path: Map<FileId, ()>,
// TODO should we use a map here to reclaim the space for removed files?
// TODO I think we should use our own path abstraction here to avoid having to normalize paths
// and dealing with non-utf paths everywhere.
// and dealing with non-utf paths everywhere.
by_id: IndexVec<FileId, Arc<Path>>,
}

View File

@@ -27,7 +27,7 @@ pub(crate) type FxDashMap<K, V> = dashmap::DashMap<K, V, BuildHasherDefault<FxHa
pub(crate) type FxDashSet<V> = dashmap::DashSet<V, BuildHasherDefault<FxHasher>>;
pub(crate) type FxIndexSet<V> = indexmap::set::IndexSet<V, BuildHasherDefault<FxHasher>>;
#[derive(Debug, Clone)]
#[derive(Debug)]
pub struct Workspace {
/// TODO this should be a resolved path. We should probably use a newtype wrapper that guarantees that
/// PATH is a UTF-8 path and is normalized.

View File

@@ -1,13 +1,12 @@
use std::cell::RefCell;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Duration;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{ModModule, StringLiteral};
use crate::cache::KeyValueCache;
use crate::db::{HasJar, LintDb, LintJar, QueryResult, SemanticDb};
use crate::db::{HasJar, SemanticDb, SemanticJar, SourceDb, SourceJar};
use crate::files::FileId;
use crate::parse::Parsed;
use crate::source::Source;
@@ -15,28 +14,19 @@ 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) -> QueryResult<Diagnostics>
pub(crate) fn lint_syntax<Db>(db: &Db, file_id: FileId) -> Diagnostics
where
Db: LintDb + HasJar<LintJar>,
Db: SourceDb + HasJar<SourceJar>,
{
let storage = &db.jar()?.lint_syntax;
#[allow(clippy::print_stdout)]
if std::env::var("RED_KNOT_SLOW_LINT").is_ok() {
for i in 0..10 {
db.cancelled()?;
println!("RED_KNOT_SLOW_LINT is set, sleeping for {i}/10 seconds");
std::thread::sleep(Duration::from_secs(1));
}
}
let storage = &db.jar().lint_syntax;
storage.get(&file_id, |file_id| {
let mut diagnostics = Vec::new();
let source = db.source(*file_id)?;
let source = db.source(*file_id);
lint_lines(source.text(), &mut diagnostics);
let parsed = db.parse(*file_id)?;
let parsed = db.parse(*file_id);
if parsed.errors().is_empty() {
let ast = parsed.ast();
@@ -51,7 +41,7 @@ where
diagnostics.extend(parsed.errors().iter().map(std::string::ToString::to_string));
}
Ok(Diagnostics::from(diagnostics))
Diagnostics::from(diagnostics)
})
}
@@ -73,16 +63,16 @@ 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) -> QueryResult<Diagnostics>
pub(crate) fn lint_semantic<Db>(db: &Db, file_id: FileId) -> Diagnostics
where
Db: LintDb + HasJar<LintJar>,
Db: SemanticDb + HasJar<SemanticJar>,
{
let storage = &db.jar()?.lint_semantic;
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 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,
@@ -93,25 +83,25 @@ where
diagnostics: RefCell::new(Vec::new()),
};
lint_unresolved_imports(&context)?;
lint_unresolved_imports(&context);
Ok(Diagnostics::from(context.diagnostics.take()))
Diagnostics::from(context.diagnostics.take())
})
}
fn lint_unresolved_imports(context: &SemanticLintContext) -> QueryResult<()> {
fn lint_unresolved_imports(context: &SemanticLintContext) {
// TODO: Consider iterating over the dependencies (imports) only instead of all definitions.
for (symbol, definition) in context.symbols().all_definitions() {
match definition {
Definition::Import(import) => {
let ty = context.infer_symbol_type(symbol)?;
let ty = context.eval_symbol(symbol);
if ty.is_unknown() {
context.push_diagnostic(format!("Unresolved module {}", import.module));
}
}
Definition::ImportFrom(import) => {
let ty = context.infer_symbol_type(symbol)?;
let ty = context.eval_symbol(symbol);
if ty.is_unknown() {
let module_name = import.module().map(Deref::deref).unwrap_or_default();
@@ -136,8 +126,6 @@ fn lint_unresolved_imports(context: &SemanticLintContext) -> QueryResult<()> {
_ => {}
}
}
Ok(())
}
pub struct SemanticLintContext<'a> {
@@ -166,7 +154,7 @@ impl<'a> SemanticLintContext<'a> {
&self.symbols
}
pub fn infer_symbol_type(&self, symbol_id: SymbolId) -> QueryResult<Type> {
pub fn eval_symbol(&self, symbol_id: SymbolId) -> Type {
self.db.infer_symbol_type(self.file_id, symbol_id)
}

View File

@@ -1,9 +1,10 @@
#![allow(clippy::dbg_macro)]
use std::collections::hash_map::Entry;
use std::path::Path;
use std::sync::Mutex;
use crossbeam::channel as crossbeam_channel;
use rustc_hash::FxHashMap;
use tracing::subscriber::Interest;
use tracing::{Level, Metadata};
use tracing_subscriber::filter::LevelFilter;
@@ -11,10 +12,12 @@ use tracing_subscriber::layer::{Context, Filter, SubscriberExt};
use tracing_subscriber::{Layer, Registry};
use tracing_tree::time::Uptime;
use red_knot::db::{HasJar, ParallelDatabase, QueryError, SemanticDb, SourceDb, SourceJar};
use red_knot::cancellation::CancellationTokenSource;
use red_knot::db::{HasJar, SourceDb, SourceJar};
use red_knot::files::FileId;
use red_knot::module::{ModuleSearchPath, ModuleSearchPathKind};
use red_knot::program::check::ExecutionMode;
use red_knot::program::{FileWatcherChange, Program};
use red_knot::program::check::{CheckError, RayonCheckScheduler};
use red_knot::program::{FileChange, FileChangeKind, Program};
use red_knot::watch::FileWatcher;
use red_knot::Workspace;
@@ -48,8 +51,7 @@ fn main() -> anyhow::Result<()> {
workspace.root().to_path_buf(),
ModuleSearchPathKind::FirstParty,
);
let mut program = Program::new(workspace);
program.set_module_search_paths(vec![workspace_search_path]);
let mut program = Program::new(workspace, vec![workspace_search_path]);
let entry_id = program.file_id(entry_point);
program.workspace_mut().open_file(entry_id);
@@ -69,15 +71,18 @@ fn main() -> anyhow::Result<()> {
let file_changes_notifier = main_loop.file_changes_notifier();
// Watch for file changes and re-trigger the analysis.
let mut file_watcher = FileWatcher::new(move |changes| {
file_changes_notifier.notify(changes);
})?;
let mut file_watcher = FileWatcher::new(
move |changes| {
file_changes_notifier.notify(changes);
},
program.files().clone(),
)?;
file_watcher.watch_folder(workspace_folder)?;
main_loop.run(&mut program);
let source_jar: &SourceJar = program.jar().unwrap();
let source_jar: &SourceJar = program.jar();
dbg!(source_jar.parsed.statistics());
dbg!(source_jar.sources.statistics());
@@ -96,9 +101,10 @@ impl MainLoop {
let (main_loop_sender, main_loop_receiver) = crossbeam_channel::bounded(1);
let mut orchestrator = Orchestrator {
pending_analysis: None,
receiver: orchestrator_receiver,
sender: main_loop_sender.clone(),
revision: 0,
aggregated_changes: AggregatedChanges::default(),
};
std::thread::spawn(move || {
@@ -131,27 +137,35 @@ impl MainLoop {
tracing::trace!("Main Loop: Tick");
match message {
MainLoopMessage::CheckProgram { revision } => {
let program = program.snapshot();
let sender = self.orchestrator_sender.clone();
MainLoopMessage::CheckProgram => {
// Remove mutability from program.
let program = &*program;
let run_cancellation_token_source = CancellationTokenSource::new();
let run_cancellation_token = run_cancellation_token_source.token();
let sender = &self.orchestrator_sender;
// Spawn a new task that checks the program. This needs to be done in a separate thread
// to prevent blocking the main loop here.
rayon::spawn(move || match program.check(ExecutionMode::ThreadPool) {
Ok(result) => {
sender
.send(OrchestratorMessage::CheckProgramCompleted {
diagnostics: result,
revision,
})
.unwrap();
sender
.send(OrchestratorMessage::CheckProgramStarted {
cancellation_token: run_cancellation_token_source,
})
.unwrap();
rayon::in_place_scope(|scope| {
let scheduler = RayonCheckScheduler::new(program, scope);
let result = program.check(&scheduler, run_cancellation_token);
match result {
Ok(result) => sender
.send(OrchestratorMessage::CheckProgramCompleted(result))
.unwrap(),
Err(CheckError::Cancelled) => sender
.send(OrchestratorMessage::CheckProgramCancelled)
.unwrap(),
}
Err(QueryError::Cancelled) => {}
});
}
MainLoopMessage::ApplyChanges(changes) => {
// Automatically cancels any pending queries and waits for them to complete.
program.apply_changes(changes);
program.apply_changes(changes.iter());
}
MainLoopMessage::CheckCompleted(diagnostics) => {
dbg!(diagnostics);
@@ -178,7 +192,7 @@ struct FileChangesNotifier {
}
impl FileChangesNotifier {
fn notify(&self, changes: Vec<FileWatcherChange>) {
fn notify(&self, changes: Vec<FileChange>) {
self.sender
.send(OrchestratorMessage::FileChanges(changes))
.unwrap();
@@ -197,11 +211,13 @@ impl MainLoopCancellationToken {
}
struct Orchestrator {
aggregated_changes: AggregatedChanges,
pending_analysis: Option<PendingAnalysisState>,
/// Sends messages to the main loop.
sender: crossbeam_channel::Sender<MainLoopMessage>,
/// Receives messages from the main loop.
receiver: crossbeam_channel::Receiver<OrchestratorMessage>,
revision: usize,
}
impl Orchestrator {
@@ -209,33 +225,51 @@ impl Orchestrator {
while let Ok(message) = self.receiver.recv() {
match message {
OrchestratorMessage::Run => {
self.pending_analysis = None;
self.sender.send(MainLoopMessage::CheckProgram).unwrap();
}
OrchestratorMessage::CheckProgramStarted { cancellation_token } => {
debug_assert!(self.pending_analysis.is_none());
self.pending_analysis = Some(PendingAnalysisState { cancellation_token });
}
OrchestratorMessage::CheckProgramCompleted(diagnostics) => {
self.pending_analysis
.take()
.expect("Expected a pending analysis.");
self.sender
.send(MainLoopMessage::CheckProgram {
revision: self.revision,
})
.send(MainLoopMessage::CheckCompleted(diagnostics))
.unwrap();
}
OrchestratorMessage::CheckProgramCompleted {
diagnostics,
revision,
} => {
// Only take the diagnostics if they are for the latest revision.
if self.revision == revision {
self.sender
.send(MainLoopMessage::CheckCompleted(diagnostics))
.unwrap();
} else {
tracing::debug!("Discarding diagnostics for outdated revision {revision} (current: {}).", self.revision);
}
OrchestratorMessage::CheckProgramCancelled => {
self.pending_analysis
.take()
.expect("Expected a pending analysis.");
self.debounce_changes();
}
OrchestratorMessage::FileChanges(changes) => {
// Request cancellation, but wait until all analysis tasks have completed to
// avoid stale messages in the next main loop.
let pending = if let Some(pending_state) = self.pending_analysis.as_ref() {
pending_state.cancellation_token.cancel();
true
} else {
false
};
self.revision += 1;
self.debounce_changes(changes);
self.aggregated_changes.extend(changes);
// If there are no pending analysis tasks, apply the file changes. Otherwise
// keep running until all file checks have completed.
if !pending {
self.debounce_changes();
}
}
OrchestratorMessage::Shutdown => {
return self.shutdown();
@@ -244,7 +278,9 @@ impl Orchestrator {
}
}
fn debounce_changes(&self, mut changes: Vec<FileWatcherChange>) {
fn debounce_changes(&mut self) {
debug_assert!(self.pending_analysis.is_none());
loop {
// Consume possibly incoming file change messages before running a new analysis, but don't wait for more than 100ms.
crossbeam_channel::select! {
@@ -254,12 +290,10 @@ impl Orchestrator {
return self.shutdown();
}
Ok(OrchestratorMessage::FileChanges(file_changes)) => {
changes.extend(file_changes);
self.aggregated_changes.extend(file_changes);
}
Ok(OrchestratorMessage::CheckProgramCompleted { .. })=> {
// disregard any outdated completion message.
}
Ok(OrchestratorMessage::CheckProgramStarted {..}| OrchestratorMessage::CheckProgramCompleted(_) | OrchestratorMessage::CheckProgramCancelled) => unreachable!("No program check should be running while debouncing changes."),
Ok(OrchestratorMessage::Run) => unreachable!("The orchestrator is already running."),
Err(_) => {
@@ -268,10 +302,10 @@ impl Orchestrator {
}
}
},
default(std::time::Duration::from_millis(10)) => {
// No more file changes after 10 ms, send the changes and schedule a new analysis
self.sender.send(MainLoopMessage::ApplyChanges(changes)).unwrap();
self.sender.send(MainLoopMessage::CheckProgram { revision: self.revision}).unwrap();
default(std::time::Duration::from_millis(100)) => {
// No more file changes after 100 ms, send the changes and schedule a new analysis
self.sender.send(MainLoopMessage::ApplyChanges(std::mem::take(&mut self.aggregated_changes))).unwrap();
self.sender.send(MainLoopMessage::CheckProgram).unwrap();
return;
}
}
@@ -284,12 +318,17 @@ impl Orchestrator {
}
}
#[derive(Debug)]
struct PendingAnalysisState {
cancellation_token: CancellationTokenSource,
}
/// Message sent from the orchestrator to the main loop.
#[derive(Debug)]
enum MainLoopMessage {
CheckProgram { revision: usize },
CheckProgram,
CheckCompleted(Vec<String>),
ApplyChanges(Vec<FileWatcherChange>),
ApplyChanges(AggregatedChanges),
Exit,
}
@@ -298,12 +337,83 @@ enum OrchestratorMessage {
Run,
Shutdown,
CheckProgramCompleted {
diagnostics: Vec<String>,
revision: usize,
CheckProgramStarted {
cancellation_token: CancellationTokenSource,
},
CheckProgramCompleted(Vec<String>),
CheckProgramCancelled,
FileChanges(Vec<FileWatcherChange>),
FileChanges(Vec<FileChange>),
}
#[derive(Default, Debug)]
struct AggregatedChanges {
changes: FxHashMap<FileId, FileChangeKind>,
}
impl AggregatedChanges {
fn add(&mut self, change: FileChange) {
match self.changes.entry(change.file_id()) {
Entry::Occupied(mut entry) => {
let merged = entry.get_mut();
match (merged, change.kind()) {
(FileChangeKind::Created, FileChangeKind::Deleted) => {
// Deletion after creations means that ruff never saw the file.
entry.remove();
}
(FileChangeKind::Created, FileChangeKind::Modified) => {
// No-op, for ruff, modifying a file that it doesn't yet know that it exists is still considered a creation.
}
(FileChangeKind::Modified, FileChangeKind::Created) => {
// Uhh, that should probably not happen. Continue considering it a modification.
}
(FileChangeKind::Modified, FileChangeKind::Deleted) => {
*entry.get_mut() = FileChangeKind::Deleted;
}
(FileChangeKind::Deleted, FileChangeKind::Created) => {
*entry.get_mut() = FileChangeKind::Modified;
}
(FileChangeKind::Deleted, FileChangeKind::Modified) => {
// That's weird, but let's consider it a modification.
*entry.get_mut() = FileChangeKind::Modified;
}
(FileChangeKind::Created, FileChangeKind::Created)
| (FileChangeKind::Modified, FileChangeKind::Modified)
| (FileChangeKind::Deleted, FileChangeKind::Deleted) => {
// No-op transitions. Some of them should be impossible but we handle them anyway.
}
}
}
Entry::Vacant(entry) => {
entry.insert(change.kind());
}
}
}
fn extend<I>(&mut self, changes: I)
where
I: IntoIterator<Item = FileChange>,
I::IntoIter: ExactSizeIterator,
{
let iter = changes.into_iter();
self.changes.reserve(iter.len());
for change in iter {
self.add(change);
}
}
fn iter(&self) -> impl Iterator<Item = FileChange> + '_ {
self.changes
.iter()
.map(|(id, kind)| FileChange::new(*id, *kind))
}
}
fn setup_tracing() {

View File

@@ -7,7 +7,7 @@ use std::sync::Arc;
use dashmap::mapref::entry::Entry;
use smol_str::SmolStr;
use crate::db::{HasJar, QueryResult, SemanticDb, SemanticJar};
use crate::db::{HasJar, SemanticDb, SemanticJar};
use crate::files::FileId;
use crate::symbols::Dependency;
use crate::FxDashMap;
@@ -17,48 +17,44 @@ use crate::FxDashMap;
pub struct Module(u32);
impl Module {
pub fn name<Db>(&self, db: &Db) -> QueryResult<ModuleName>
pub fn name<Db>(&self, db: &Db) -> ModuleName
where
Db: HasJar<SemanticJar>,
{
let modules = &db.jar()?.module_resolver;
let modules = &db.jar().module_resolver;
Ok(modules.modules.get(self).unwrap().name.clone())
modules.modules.get(self).unwrap().name.clone()
}
pub fn path<Db>(&self, db: &Db) -> QueryResult<ModulePath>
pub fn path<Db>(&self, db: &Db) -> ModulePath
where
Db: HasJar<SemanticJar>,
{
let modules = &db.jar()?.module_resolver;
let modules = &db.jar().module_resolver;
Ok(modules.modules.get(self).unwrap().path.clone())
modules.modules.get(self).unwrap().path.clone()
}
pub fn kind<Db>(&self, db: &Db) -> QueryResult<ModuleKind>
pub fn kind<Db>(&self, db: &Db) -> ModuleKind
where
Db: HasJar<SemanticJar>,
{
let modules = &db.jar()?.module_resolver;
let modules = &db.jar().module_resolver;
Ok(modules.modules.get(self).unwrap().kind)
modules.modules.get(self).unwrap().kind
}
pub fn resolve_dependency<Db>(
&self,
db: &Db,
dependency: &Dependency,
) -> QueryResult<Option<ModuleName>>
pub fn resolve_dependency<Db>(&self, db: &Db, dependency: &Dependency) -> Option<ModuleName>
where
Db: HasJar<SemanticJar>,
{
let (level, module) = match dependency {
Dependency::Module(module) => return Ok(Some(module.clone())),
Dependency::Module(module) => return Some(module.clone()),
Dependency::Relative { level, module } => (*level, module.as_deref()),
};
let name = self.name(db)?;
let kind = self.kind(db)?;
let name = self.name(db);
let kind = self.kind(db);
let mut components = name.components().peekable();
@@ -71,9 +67,7 @@ impl Module {
// Skip over the relative parts.
for _ in start..level.get() {
if components.next_back().is_none() {
return Ok(None);
}
components.next_back()?;
}
let mut name = String::new();
@@ -86,11 +80,11 @@ impl Module {
name.push_str(part);
}
Ok(if name.is_empty() {
if name.is_empty() {
None
} else {
Some(ModuleName(SmolStr::new(name)))
})
}
}
}
@@ -244,25 +238,20 @@ pub struct ModuleData {
/// TODO: This would not work with Salsa because `ModuleName` isn't an ingredient and, therefore, cannot be used as part of a query.
/// For this to work with salsa, it would be necessary to intern all `ModuleName`s.
#[tracing::instrument(level = "debug", skip(db))]
pub fn resolve_module<Db>(db: &Db, name: ModuleName) -> QueryResult<Option<Module>>
pub fn resolve_module<Db>(db: &Db, name: ModuleName) -> Option<Module>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let jar = db.jar();
let modules = &jar?.module_resolver;
let modules = &jar.module_resolver;
let entry = modules.by_name.entry(name.clone());
match entry {
Entry::Occupied(entry) => Ok(Some(*entry.get())),
Entry::Occupied(entry) => Some(*entry.get()),
Entry::Vacant(entry) => {
let Some((root_path, absolute_path, kind)) = resolve_name(&name, &modules.search_paths)
else {
return Ok(None);
};
let Ok(normalized) = absolute_path.canonicalize() else {
return Ok(None);
};
let (root_path, absolute_path, kind) = resolve_name(&name, &modules.search_paths)?;
let normalized = absolute_path.canonicalize().ok()?;
let file_id = db.file_id(&normalized);
let path = ModulePath::new(root_path.clone(), file_id);
@@ -284,76 +273,59 @@ where
// ```
// Here, both `foo` and `bar` resolve to the same module but through different paths.
// That's why we need to insert the absolute path and not the normalized path here.
let absolute_id = if absolute_path == normalized {
file_id
} else {
db.file_id(&absolute_path)
};
modules.by_file.insert(absolute_id, id);
modules.by_path.insert(absolute_path, id);
entry.insert_entry(id);
Ok(Some(id))
Some(id)
}
}
}
/// Resolves the module id for the given path.
///
/// Returns `None` if the path is not a module in `sys.path`.
#[tracing::instrument(level = "debug", skip(db))]
pub fn path_to_module<Db>(db: &Db, path: &Path) -> QueryResult<Option<Module>>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let file = db.file_id(path);
file_to_module(db, file)
}
/// Resolves the module id for the file with the given id.
///
/// Returns `None` if the file is not a module in `sys.path`.
#[tracing::instrument(level = "debug", skip(db))]
pub fn file_to_module<Db>(db: &Db, file: FileId) -> QueryResult<Option<Module>>
pub fn file_to_module<Db>(db: &Db, file: FileId) -> Option<Module>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let jar = db.jar()?;
let modules = &jar.module_resolver;
if let Some(existing) = modules.by_file.get(&file) {
return Ok(Some(*existing));
}
let path = db.file_path(file);
path_to_module(db, &path)
}
/// Resolves the module id for the given path.
///
/// Returns `None` if the path is not a module in `sys.path`.
#[tracing::instrument(level = "debug", skip(db))]
pub fn path_to_module<Db>(db: &Db, path: &Path) -> Option<Module>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let jar = db.jar();
let modules = &jar.module_resolver;
debug_assert!(path.is_absolute());
let Some((root_path, relative_path)) = modules.search_paths.iter().find_map(|root| {
if let Some(existing) = modules.by_path.get(path) {
return Some(*existing);
}
let (root_path, relative_path) = modules.search_paths.iter().find_map(|root| {
let relative_path = path.strip_prefix(root.path()).ok()?;
Some((root.clone(), relative_path))
}) else {
return Ok(None);
};
})?;
let Some(module_name) = ModuleName::from_relative_path(relative_path) else {
return Ok(None);
};
let module_name = ModuleName::from_relative_path(relative_path)?;
// Resolve the module name to see if Python would resolve the name to the same path.
// If it doesn't, then that means that multiple modules have the same in different
// root paths, but that the module corresponding to the past path is in a lower priority path,
// in which case we ignore it.
let Some(module_id) = resolve_module(db, module_name)? else {
return Ok(None);
};
let module_path = module_id.path(db)?;
let module_id = resolve_module(db, module_name)?;
let module_path = module_id.path(db);
if module_path.root() == &root_path {
let Ok(normalized) = path.canonicalize() else {
return Ok(None);
};
let normalized = path.canonicalize().ok()?;
let interned_normalized = db.file_id(&normalized);
if interned_normalized != module_path.file() {
@@ -364,15 +336,15 @@ where
// ```
// The module name of `src/foo.py` is `foo`, but the module loaded by Python is `src/foo/__init__.py`.
// That means we need to ignore `src/foo.py` even though it resolves to the same module name.
return Ok(None);
return None;
}
// Path has been inserted by `resolved`
Ok(Some(module_id))
Some(module_id)
} else {
// This path is for a module with the same name but in a module search path with a lower priority.
// Ignore it.
Ok(None)
None
}
}
@@ -406,7 +378,7 @@ where
// TODO This needs tests
// Note: Intentionally by-pass caching here. Module should not be in the cache yet.
let module = path_to_module(db, path).ok()??;
let module = path_to_module(db, path)?;
// The code below is to handle the addition of `__init__.py` files.
// When an `__init__.py` file is added, we need to remove all modules that are part of the same package.
@@ -420,7 +392,7 @@ where
return Some((module, Vec::new()));
}
let Some(parent_name) = module.name(db).ok()?.parent() else {
let Some(parent_name) = module.name(db).parent() else {
return Some((module, Vec::new()));
};
@@ -429,7 +401,7 @@ where
let jar = db.jar_mut();
let modules = &mut jar.module_resolver;
modules.by_file.retain(|_, id| {
modules.by_path.retain(|_, id| {
if modules
.modules
.get(id)
@@ -468,7 +440,7 @@ pub struct ModuleResolver {
/// Lookup from absolute path to module.
/// The same module might be reachable from different paths when symlinks are involved.
by_file: FxDashMap<FileId, Module>,
by_path: FxDashMap<PathBuf, Module>,
next_module_id: AtomicU32,
}
@@ -478,14 +450,14 @@ impl ModuleResolver {
search_paths,
modules: FxDashMap::default(),
by_name: FxDashMap::default(),
by_file: FxDashMap::default(),
by_path: FxDashMap::default(),
next_module_id: AtomicU32::new(0),
}
}
pub(crate) fn remove_module(&mut self, file_id: FileId) {
pub fn remove_module(&mut self, path: &Path) {
// No locking is required because we're holding a mutable reference to `self`.
let Some((_, id)) = self.by_file.remove(&file_id) else {
let Some((_, id)) = self.by_path.remove(path) else {
return;
};
@@ -498,7 +470,7 @@ impl ModuleResolver {
self.by_name.remove(&module.name).unwrap();
// It's possible that multiple paths map to the same id. Search all other paths referencing the same module id.
self.by_file.retain(|_, current_id| *current_id != id);
self.by_path.retain(|_, current_id| *current_id != id);
module
}
@@ -719,7 +691,7 @@ mod tests {
}
#[test]
fn first_party_module() -> anyhow::Result<()> {
fn first_party_module() -> std::io::Result<()> {
let TestCase {
db,
src,
@@ -730,22 +702,22 @@ mod tests {
let foo_path = src.path().join("foo.py");
std::fs::write(&foo_path, "print('Hello, world!')")?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
assert_eq!(Some(foo_module), db.resolve_module(ModuleName::new("foo"))?);
assert_eq!(Some(foo_module), db.resolve_module(ModuleName::new("foo")));
assert_eq!(ModuleName::new("foo"), foo_module.name(&db)?);
assert_eq!(&src, foo_module.path(&db)?.root());
assert_eq!(ModuleKind::Module, foo_module.kind(&db)?);
assert_eq!(&foo_path, &*db.file_path(foo_module.path(&db)?.file()));
assert_eq!(ModuleName::new("foo"), foo_module.name(&db));
assert_eq!(&src, foo_module.path(&db).root());
assert_eq!(ModuleKind::Module, foo_module.kind(&db));
assert_eq!(&foo_path, &*db.file_path(foo_module.path(&db).file()));
assert_eq!(Some(foo_module), db.path_to_module(&foo_path)?);
assert_eq!(Some(foo_module), db.path_to_module(&foo_path));
Ok(())
}
#[test]
fn resolve_package() -> anyhow::Result<()> {
fn resolve_package() -> std::io::Result<()> {
let TestCase {
src,
db,
@@ -758,22 +730,22 @@ mod tests {
std::fs::create_dir(&foo_dir)?;
std::fs::write(&foo_path, "print('Hello, world!')")?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
assert_eq!(ModuleName::new("foo"), foo_module.name(&db)?);
assert_eq!(&src, foo_module.path(&db)?.root());
assert_eq!(&foo_path, &*db.file_path(foo_module.path(&db)?.file()));
assert_eq!(ModuleName::new("foo"), foo_module.name(&db));
assert_eq!(&src, foo_module.path(&db).root());
assert_eq!(&foo_path, &*db.file_path(foo_module.path(&db).file()));
assert_eq!(Some(foo_module), db.path_to_module(&foo_path)?);
assert_eq!(Some(foo_module), db.path_to_module(&foo_path));
// Resolving by directory doesn't resolve to the init file.
assert_eq!(None, db.path_to_module(&foo_dir)?);
assert_eq!(None, db.path_to_module(&foo_dir));
Ok(())
}
#[test]
fn package_priority_over_module() -> anyhow::Result<()> {
fn package_priority_over_module() -> std::io::Result<()> {
let TestCase {
db,
temp_dir: _temp_dir,
@@ -789,20 +761,20 @@ mod tests {
let foo_py = src.path().join("foo.py");
std::fs::write(&foo_py, "print('Hello, world!')")?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
assert_eq!(&src, foo_module.path(&db)?.root());
assert_eq!(&foo_init, &*db.file_path(foo_module.path(&db)?.file()));
assert_eq!(ModuleKind::Package, foo_module.kind(&db)?);
assert_eq!(&src, foo_module.path(&db).root());
assert_eq!(&foo_init, &*db.file_path(foo_module.path(&db).file()));
assert_eq!(ModuleKind::Package, foo_module.kind(&db));
assert_eq!(Some(foo_module), db.path_to_module(&foo_init)?);
assert_eq!(None, db.path_to_module(&foo_py)?);
assert_eq!(Some(foo_module), db.path_to_module(&foo_init));
assert_eq!(None, db.path_to_module(&foo_py));
Ok(())
}
#[test]
fn typing_stub_over_module() -> anyhow::Result<()> {
fn typing_stub_over_module() -> std::io::Result<()> {
let TestCase {
db,
src,
@@ -815,19 +787,19 @@ mod tests {
std::fs::write(&foo_stub, "x: int")?;
std::fs::write(&foo_py, "print('Hello, world!')")?;
let foo = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let foo = db.resolve_module(ModuleName::new("foo")).unwrap();
assert_eq!(&src, foo.path(&db)?.root());
assert_eq!(&foo_stub, &*db.file_path(foo.path(&db)?.file()));
assert_eq!(&src, foo.path(&db).root());
assert_eq!(&foo_stub, &*db.file_path(foo.path(&db).file()));
assert_eq!(Some(foo), db.path_to_module(&foo_stub)?);
assert_eq!(None, db.path_to_module(&foo_py)?);
assert_eq!(Some(foo), db.path_to_module(&foo_stub));
assert_eq!(None, db.path_to_module(&foo_py));
Ok(())
}
#[test]
fn sub_packages() -> anyhow::Result<()> {
fn sub_packages() -> std::io::Result<()> {
let TestCase {
db,
src,
@@ -844,18 +816,18 @@ mod tests {
std::fs::write(bar.join("__init__.py"), "")?;
std::fs::write(&baz, "print('Hello, world!')")?;
let baz_module = db.resolve_module(ModuleName::new("foo.bar.baz"))?.unwrap();
let baz_module = db.resolve_module(ModuleName::new("foo.bar.baz")).unwrap();
assert_eq!(&src, baz_module.path(&db)?.root());
assert_eq!(&baz, &*db.file_path(baz_module.path(&db)?.file()));
assert_eq!(&src, baz_module.path(&db).root());
assert_eq!(&baz, &*db.file_path(baz_module.path(&db).file()));
assert_eq!(Some(baz_module), db.path_to_module(&baz)?);
assert_eq!(Some(baz_module), db.path_to_module(&baz));
Ok(())
}
#[test]
fn namespace_package() -> anyhow::Result<()> {
fn namespace_package() -> std::io::Result<()> {
let TestCase {
db,
temp_dir: _,
@@ -891,21 +863,21 @@ mod tests {
std::fs::write(&two, "print('Hello, world!')")?;
let one_module = db
.resolve_module(ModuleName::new("parent.child.one"))?
.resolve_module(ModuleName::new("parent.child.one"))
.unwrap();
assert_eq!(Some(one_module), db.path_to_module(&one)?);
assert_eq!(Some(one_module), db.path_to_module(&one));
let two_module = db
.resolve_module(ModuleName::new("parent.child.two"))?
.resolve_module(ModuleName::new("parent.child.two"))
.unwrap();
assert_eq!(Some(two_module), db.path_to_module(&two)?);
assert_eq!(Some(two_module), db.path_to_module(&two));
Ok(())
}
#[test]
fn regular_package_in_namespace_package() -> anyhow::Result<()> {
fn regular_package_in_namespace_package() -> std::io::Result<()> {
let TestCase {
db,
temp_dir: _,
@@ -942,20 +914,17 @@ mod tests {
std::fs::write(two, "print('Hello, world!')")?;
let one_module = db
.resolve_module(ModuleName::new("parent.child.one"))?
.resolve_module(ModuleName::new("parent.child.one"))
.unwrap();
assert_eq!(Some(one_module), db.path_to_module(&one)?);
assert_eq!(Some(one_module), db.path_to_module(&one));
assert_eq!(
None,
db.resolve_module(ModuleName::new("parent.child.two"))?
);
assert_eq!(None, db.resolve_module(ModuleName::new("parent.child.two")));
Ok(())
}
#[test]
fn module_search_path_priority() -> anyhow::Result<()> {
fn module_search_path_priority() -> std::io::Result<()> {
let TestCase {
db,
src,
@@ -969,20 +938,20 @@ mod tests {
std::fs::write(&foo_src, "")?;
std::fs::write(&foo_site_packages, "")?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
assert_eq!(&src, foo_module.path(&db)?.root());
assert_eq!(&foo_src, &*db.file_path(foo_module.path(&db)?.file()));
assert_eq!(&src, foo_module.path(&db).root());
assert_eq!(&foo_src, &*db.file_path(foo_module.path(&db).file()));
assert_eq!(Some(foo_module), db.path_to_module(&foo_src)?);
assert_eq!(None, db.path_to_module(&foo_site_packages)?);
assert_eq!(Some(foo_module), db.path_to_module(&foo_src));
assert_eq!(None, db.path_to_module(&foo_site_packages));
Ok(())
}
#[test]
#[cfg(target_family = "unix")]
fn symlink() -> anyhow::Result<()> {
fn symlink() -> std::io::Result<()> {
let TestCase {
db,
src,
@@ -996,28 +965,28 @@ mod tests {
std::fs::write(&foo, "")?;
std::os::unix::fs::symlink(&foo, &bar)?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let bar_module = db.resolve_module(ModuleName::new("bar"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
let bar_module = db.resolve_module(ModuleName::new("bar")).unwrap();
assert_ne!(foo_module, bar_module);
assert_eq!(&src, foo_module.path(&db)?.root());
assert_eq!(&foo, &*db.file_path(foo_module.path(&db)?.file()));
assert_eq!(&src, foo_module.path(&db).root());
assert_eq!(&foo, &*db.file_path(foo_module.path(&db).file()));
// Bar has a different name but it should point to the same file.
assert_eq!(&src, bar_module.path(&db)?.root());
assert_eq!(foo_module.path(&db)?.file(), bar_module.path(&db)?.file());
assert_eq!(&foo, &*db.file_path(bar_module.path(&db)?.file()));
assert_eq!(&src, bar_module.path(&db).root());
assert_eq!(foo_module.path(&db).file(), bar_module.path(&db).file());
assert_eq!(&foo, &*db.file_path(bar_module.path(&db).file()));
assert_eq!(Some(foo_module), db.path_to_module(&foo)?);
assert_eq!(Some(bar_module), db.path_to_module(&bar)?);
assert_eq!(Some(foo_module), db.path_to_module(&foo));
assert_eq!(Some(bar_module), db.path_to_module(&bar));
Ok(())
}
#[test]
fn resolve_dependency() -> anyhow::Result<()> {
fn resolve_dependency() -> std::io::Result<()> {
let TestCase {
src,
db,
@@ -1033,8 +1002,8 @@ mod tests {
std::fs::write(foo_path, "from .bar import test")?;
std::fs::write(bar_path, "test = 'Hello world'")?;
let foo_module = db.resolve_module(ModuleName::new("foo"))?.unwrap();
let bar_module = db.resolve_module(ModuleName::new("foo.bar"))?.unwrap();
let foo_module = db.resolve_module(ModuleName::new("foo")).unwrap();
let bar_module = db.resolve_module(ModuleName::new("foo.bar")).unwrap();
// `from . import bar` in `foo/__init__.py` resolves to `foo`
assert_eq!(
@@ -1045,13 +1014,13 @@ mod tests {
level: NonZeroU32::new(1).unwrap(),
module: None,
}
)?
)
);
// `from baz import bar` in `foo/__init__.py` should resolve to `baz.py`
assert_eq!(
Some(ModuleName::new("baz")),
foo_module.resolve_dependency(&db, &Dependency::Module(ModuleName::new("baz")))?
foo_module.resolve_dependency(&db, &Dependency::Module(ModuleName::new("baz")))
);
// from .bar import test in `foo/__init__.py` should resolve to `foo/bar.py`
@@ -1063,7 +1032,7 @@ mod tests {
level: NonZeroU32::new(1).unwrap(),
module: Some(ModuleName::new("bar"))
}
)?
)
);
// from .. import test in `foo/__init__.py` resolves to `` which is not a module
@@ -1075,7 +1044,7 @@ mod tests {
level: NonZeroU32::new(2).unwrap(),
module: None
}
)?
)
);
// `from . import test` in `foo/bar.py` resolves to `foo`
@@ -1087,13 +1056,13 @@ mod tests {
level: NonZeroU32::new(1).unwrap(),
module: None
}
)?
)
);
// `from baz import test` in `foo/bar.py` resolves to `baz`
assert_eq!(
Some(ModuleName::new("baz")),
bar_module.resolve_dependency(&db, &Dependency::Module(ModuleName::new("baz")))?
bar_module.resolve_dependency(&db, &Dependency::Module(ModuleName::new("baz")))
);
// `from .baz import test` in `foo/bar.py` resolves to `foo.baz`.
@@ -1105,7 +1074,7 @@ mod tests {
level: NonZeroU32::new(1).unwrap(),
module: Some(ModuleName::new("baz"))
}
)?
)
);
Ok(())

View File

@@ -6,7 +6,7 @@ use ruff_python_parser::{Mode, ParseError};
use ruff_text_size::{Ranged, TextRange};
use crate::cache::KeyValueCache;
use crate::db::{HasJar, QueryResult, SourceDb, SourceJar};
use crate::db::{HasJar, SourceDb, SourceJar};
use crate::files::FileId;
#[derive(Debug, Clone, PartialEq)]
@@ -64,16 +64,16 @@ impl Parsed {
}
#[tracing::instrument(level = "debug", skip(db))]
pub(crate) fn parse<Db>(db: &Db, file_id: FileId) -> QueryResult<Parsed>
pub(crate) fn parse<Db>(db: &Db, file_id: FileId) -> Parsed
where
Db: SourceDb + HasJar<SourceJar>,
{
let parsed = db.jar()?;
let parsed = db.jar();
parsed.parsed.get(&file_id, |file_id| {
let source = db.source(*file_id)?;
let source = db.source(*file_id);
Ok(Parsed::from_text(source.text()))
Parsed::from_text(source.text())
})
}

View File

@@ -1,7 +1,10 @@
use rayon::{current_num_threads, yield_local};
use std::num::NonZeroUsize;
use rayon::max_num_threads;
use rustc_hash::FxHashSet;
use crate::db::{Database, LintDb, QueryError, QueryResult, SemanticDb};
use crate::cancellation::CancellationToken;
use crate::db::{SemanticDb, SourceDb};
use crate::files::FileId;
use crate::lint::Diagnostics;
use crate::program::Program;
@@ -10,28 +13,42 @@ use crate::symbols::Dependency;
impl Program {
/// Checks all open files in the workspace and its dependencies.
#[tracing::instrument(level = "debug", skip_all)]
pub fn check(&self, mode: ExecutionMode) -> QueryResult<Vec<String>> {
self.cancelled()?;
pub fn check(
&self,
scheduler: &dyn CheckScheduler,
cancellation_token: CancellationToken,
) -> Result<Vec<String>, CheckError> {
let check_loop = CheckFilesLoop::new(scheduler, cancellation_token);
let mut context = CheckContext::new(self);
check_loop.run(self.workspace().open_files.iter().copied())
}
match mode {
ExecutionMode::SingleThreaded => SingleThreadedExecutor.run(&mut context)?,
ExecutionMode::ThreadPool => ThreadPoolExecutor.run(&mut context)?,
};
/// Checks a single file and its dependencies.
#[tracing::instrument(level = "debug", skip(self, scheduler, cancellation_token))]
pub fn check_file(
&self,
file: FileId,
scheduler: &dyn CheckScheduler,
cancellation_token: CancellationToken,
) -> Result<Vec<String>, CheckError> {
let check_loop = CheckFilesLoop::new(scheduler, cancellation_token);
Ok(context.finish())
check_loop.run([file].into_iter())
}
#[tracing::instrument(level = "debug", skip(self, context))]
fn check_file(&self, file: FileId, context: &CheckFileContext) -> QueryResult<Diagnostics> {
self.cancelled()?;
fn do_check_file(
&self,
file: FileId,
context: &CheckContext,
) -> Result<Diagnostics, CheckError> {
context.cancelled_ok()?;
let symbol_table = self.symbol_table(file)?;
let symbol_table = self.symbol_table(file);
let dependencies = symbol_table.dependencies();
if !dependencies.is_empty() {
let module = self.file_to_module(file)?;
let module = self.file_to_module(file);
// TODO scheduling all dependencies here is wasteful if we don't infer any types on them
// but I think that's unlikely, so it is okay?
@@ -40,19 +57,18 @@ impl Program {
for dependency in dependencies {
let dependency_name = match dependency {
Dependency::Module(name) => Some(name.clone()),
Dependency::Relative { .. } => match &module {
Some(module) => module.resolve_dependency(self, dependency)?,
None => None,
},
Dependency::Relative { .. } => module
.as_ref()
.and_then(|module| module.resolve_dependency(self, dependency)),
};
if let Some(dependency_name) = dependency_name {
// TODO We may want to have a different check functions for non-first-party
// files because we only need to index them and not check them.
// Supporting non-first-party code also requires supporting typing stubs.
if let Some(dependency) = self.resolve_module(dependency_name)? {
if dependency.path(self)?.root().kind().is_first_party() {
context.schedule_dependency(dependency.path(self)?.file());
if let Some(dependency) = self.resolve_module(dependency_name) {
if dependency.path(self).root().kind().is_first_party() {
context.schedule_check_file(dependency.path(self).file());
}
}
}
@@ -62,351 +78,238 @@ impl Program {
let mut diagnostics = Vec::new();
if self.workspace().is_file_open(file) {
diagnostics.extend_from_slice(&self.lint_syntax(file)?);
diagnostics.extend_from_slice(&self.lint_semantic(file)?);
diagnostics.extend_from_slice(&self.lint_syntax(file));
diagnostics.extend_from_slice(&self.lint_semantic(file));
}
Ok(Diagnostics::from(diagnostics))
}
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum ExecutionMode {
SingleThreaded,
ThreadPool,
}
/// Context that stores state information about the entire check operation.
struct CheckContext<'a> {
/// IDs of the files that have been queued for checking.
/// Schedules checks for files.
pub trait CheckScheduler {
/// Schedules a check for a file.
///
/// Used to avoid queuing the same file twice.
scheduled_files: FxHashSet<FileId>,
/// The check can either be run immediately on the current thread or the check can be queued
/// in a thread pool and ran asynchronously.
///
/// The order in which scheduled checks are executed is not guaranteed.
///
/// The implementation should call [`CheckFileTask::run`] to execute the check.
fn check_file(&self, file_task: CheckFileTask);
/// Reference to the program that is checked.
program: &'a Program,
/// The aggregated diagnostics
diagnostics: Vec<String>,
/// The maximum number of checks that can be run concurrently.
///
/// Returns `None` if the checks run on the current thread (no concurrency).
fn max_concurrency(&self) -> Option<NonZeroUsize>;
}
impl<'a> CheckContext<'a> {
fn new(program: &'a Program) -> Self {
Self {
scheduled_files: FxHashSet::default(),
program,
diagnostics: Vec::new(),
}
}
/// Scheduler that runs checks on a rayon thread pool.
pub struct RayonCheckScheduler<'program, 'scope_ref, 'scope> {
program: &'program Program,
scope: &'scope_ref rayon::Scope<'scope>,
}
/// Returns the tasks to check all open files in the workspace.
fn check_open_files(&mut self) -> Vec<CheckOpenFileTask> {
self.scheduled_files
.extend(self.program.workspace().open_files());
self.program
.workspace()
.open_files()
.map(|file_id| CheckOpenFileTask { file_id })
.collect()
}
/// Returns the task to check a dependency.
fn check_dependency(&mut self, file_id: FileId) -> Option<CheckDependencyTask> {
if self.scheduled_files.insert(file_id) {
Some(CheckDependencyTask { file_id })
} else {
None
}
}
/// Pushes the result for a single file check operation
fn push_diagnostics(&mut self, diagnostics: &Diagnostics) {
self.diagnostics.extend_from_slice(diagnostics);
}
/// Returns a reference to the program that is being checked.
fn program(&self) -> &'a Program {
self.program
}
/// Creates a task context that is used to check a single file.
fn task_context<'b, S>(&self, dependency_scheduler: &'b S) -> CheckTaskContext<'a, 'b, S>
where
S: ScheduleDependency,
{
CheckTaskContext {
program: self.program,
dependency_scheduler,
}
}
fn finish(self) -> Vec<String> {
self.diagnostics
impl<'program, 'scope_ref, 'scope> RayonCheckScheduler<'program, 'scope_ref, 'scope> {
pub fn new(program: &'program Program, scope: &'scope_ref rayon::Scope<'scope>) -> Self {
Self { program, scope }
}
}
/// Trait that abstracts away how a dependency of a file gets scheduled for checking.
trait ScheduleDependency {
/// Schedules the file with the given ID for checking.
fn schedule(&self, file_id: FileId);
}
impl<T> ScheduleDependency for T
impl<'program, 'scope_ref, 'scope> CheckScheduler
for RayonCheckScheduler<'program, 'scope_ref, 'scope>
where
T: Fn(FileId),
'program: 'scope,
{
fn schedule(&self, file_id: FileId) {
let f = self;
f(file_id);
fn check_file(&self, check_file_task: CheckFileTask) {
let child_span =
tracing::trace_span!("check_file", file_id = check_file_task.file_id.as_u32());
let program = self.program;
self.scope
.spawn(move |_| child_span.in_scope(|| check_file_task.run(program)));
}
fn max_concurrency(&self) -> Option<NonZeroUsize> {
Some(NonZeroUsize::new(max_num_threads()).unwrap_or(NonZeroUsize::MIN))
}
}
/// Context that is used to run a single file check task.
///
/// The task is generic over `S` because it is passed across thread boundaries and
/// we don't want to add the requirement that [`ScheduleDependency`] must be [`Send`].
struct CheckTaskContext<'a, 'scheduler, S>
where
S: ScheduleDependency,
{
dependency_scheduler: &'scheduler S,
/// Scheduler that runs all checks on the current thread.
pub struct SameThreadCheckScheduler<'a> {
program: &'a Program,
}
impl<'a, 'scheduler, S> CheckTaskContext<'a, 'scheduler, S>
where
S: ScheduleDependency,
{
fn as_file_context(&self) -> CheckFileContext<'scheduler> {
CheckFileContext {
dependency_scheduler: self.dependency_scheduler,
}
impl<'a> SameThreadCheckScheduler<'a> {
pub fn new(program: &'a Program) -> Self {
Self { program }
}
}
/// Context passed when checking a single file.
///
/// This is a trimmed down version of [`CheckTaskContext`] with the type parameter `S` erased
/// to avoid monomorphization of [`Program:check_file`].
struct CheckFileContext<'a> {
dependency_scheduler: &'a dyn ScheduleDependency,
impl CheckScheduler for SameThreadCheckScheduler<'_> {
fn check_file(&self, task: CheckFileTask) {
task.run(self.program);
}
fn max_concurrency(&self) -> Option<NonZeroUsize> {
None
}
}
impl<'a> CheckFileContext<'a> {
fn schedule_dependency(&self, file_id: FileId) {
self.dependency_scheduler.schedule(file_id);
}
#[derive(Debug, Clone)]
pub enum CheckError {
Cancelled,
}
#[derive(Debug)]
enum CheckFileTask {
OpenFile(CheckOpenFileTask),
Dependency(CheckDependencyTask),
pub struct CheckFileTask {
file_id: FileId,
context: CheckContext,
}
impl CheckFileTask {
/// Runs the task and returns the results for checking this file.
fn run<S>(&self, context: &CheckTaskContext<S>) -> QueryResult<Diagnostics>
where
S: ScheduleDependency,
{
match self {
Self::OpenFile(task) => task.run(context),
Self::Dependency(task) => task.run(context),
}
}
fn file_id(&self) -> FileId {
match self {
CheckFileTask::OpenFile(task) => task.file_id,
CheckFileTask::Dependency(task) => task.file_id,
/// Runs the check and communicates the result to the orchestrator.
pub fn run(self, program: &Program) {
match program.do_check_file(self.file_id, &self.context) {
Ok(diagnostics) => self
.context
.sender
.send(CheckFileMessage::Completed(diagnostics))
.unwrap(),
Err(CheckError::Cancelled) => self
.context
.sender
.send(CheckFileMessage::Cancelled)
.unwrap(),
}
}
}
/// Task to check an open file.
#[derive(Debug)]
struct CheckOpenFileTask {
file_id: FileId,
#[derive(Clone, Debug)]
struct CheckContext {
cancellation_token: CancellationToken,
sender: crossbeam_channel::Sender<CheckFileMessage>,
}
impl CheckOpenFileTask {
fn run<S>(&self, context: &CheckTaskContext<S>) -> QueryResult<Diagnostics>
where
S: ScheduleDependency,
{
context
.program
.check_file(self.file_id, &context.as_file_context())
impl CheckContext {
fn new(
cancellation_token: CancellationToken,
sender: crossbeam_channel::Sender<CheckFileMessage>,
) -> Self {
Self {
cancellation_token,
sender,
}
}
/// Queues a new file for checking using the [`CheckScheduler`].
#[allow(unused)]
fn schedule_check_file(&self, file_id: FileId) {
self.sender.send(CheckFileMessage::Queue(file_id)).unwrap();
}
/// Returns `true` if the check has been cancelled.
fn is_cancelled(&self) -> bool {
self.cancellation_token.is_cancelled()
}
fn cancelled_ok(&self) -> Result<(), CheckError> {
if self.is_cancelled() {
Err(CheckError::Cancelled)
} else {
Ok(())
}
}
}
/// Task to check a dependency file.
#[derive(Debug)]
struct CheckDependencyTask {
file_id: FileId,
struct CheckFilesLoop<'a> {
scheduler: &'a dyn CheckScheduler,
cancellation_token: CancellationToken,
pending: usize,
queued_files: FxHashSet<FileId>,
}
impl CheckDependencyTask {
fn run<S>(&self, context: &CheckTaskContext<S>) -> QueryResult<Diagnostics>
where
S: ScheduleDependency,
{
context
.program
.check_file(self.file_id, &context.as_file_context())
impl<'a> CheckFilesLoop<'a> {
fn new(scheduler: &'a dyn CheckScheduler, cancellation_token: CancellationToken) -> Self {
Self {
scheduler,
cancellation_token,
queued_files: FxHashSet::default(),
pending: 0,
}
}
}
/// Executor that schedules the checking of individual program files.
trait CheckExecutor {
fn run(self, context: &mut CheckContext) -> QueryResult<()>;
}
fn run(mut self, files: impl Iterator<Item = FileId>) -> Result<Vec<String>, CheckError> {
let (sender, receiver) = if let Some(max_concurrency) = self.scheduler.max_concurrency() {
crossbeam_channel::bounded(max_concurrency.get())
} else {
// The checks run on the current thread. That means it is necessary to store all messages
// or we risk deadlocking when the main loop never gets a chance to read the messages.
crossbeam_channel::unbounded()
};
/// Executor that runs all check operations on the current thread.
///
/// The executor does not schedule dependencies for checking.
/// The main motivation for scheduling dependencies
/// in a multithreaded environment is to parse and index the dependencies concurrently.
/// However, that doesn't make sense in a single threaded environment, because the dependencies then compute
/// with checking the open files. Checking dependencies in a single threaded environment is more likely
/// to hurt performance because we end up analyzing files in their entirety, even if we only need to type check parts of them.
#[derive(Debug, Default)]
struct SingleThreadedExecutor;
let context = CheckContext::new(self.cancellation_token.clone(), sender.clone());
impl CheckExecutor for SingleThreadedExecutor {
fn run(self, context: &mut CheckContext) -> QueryResult<()> {
let mut queue = context.check_open_files();
for file in files {
self.queue_file(file, context.clone())?;
}
let noop_schedule_dependency = |_| {};
self.run_impl(receiver, &context)
}
while let Some(file) = queue.pop() {
context.program().cancelled()?;
fn run_impl(
mut self,
receiver: crossbeam_channel::Receiver<CheckFileMessage>,
context: &CheckContext,
) -> Result<Vec<String>, CheckError> {
if self.cancellation_token.is_cancelled() {
return Err(CheckError::Cancelled);
}
let task_context = context.task_context(&noop_schedule_dependency);
context.push_diagnostics(&file.run(&task_context)?);
let mut result = Vec::default();
for message in receiver {
match message {
CheckFileMessage::Completed(diagnostics) => {
result.extend_from_slice(&diagnostics);
self.pending -= 1;
if self.pending == 0 {
break;
}
}
CheckFileMessage::Queue(id) => {
self.queue_file(id, context.clone())?;
}
CheckFileMessage::Cancelled => {
return Err(CheckError::Cancelled);
}
}
}
Ok(result)
}
fn queue_file(&mut self, file_id: FileId, context: CheckContext) -> Result<(), CheckError> {
if context.is_cancelled() {
return Err(CheckError::Cancelled);
}
if self.queued_files.insert(file_id) {
self.pending += 1;
self.scheduler
.check_file(CheckFileTask { file_id, context });
}
Ok(())
}
}
/// Executor that runs the check operations on a thread pool.
///
/// The executor runs each check operation as its own task using a thread pool.
///
/// Other than [`SingleThreadedExecutor`], this executor schedules dependencies for checking. It
/// even schedules dependencies for checking when the thread pool size is 1 for a better debugging experience.
#[derive(Debug, Default)]
struct ThreadPoolExecutor;
impl CheckExecutor for ThreadPoolExecutor {
fn run(self, context: &mut CheckContext) -> QueryResult<()> {
let num_threads = current_num_threads();
let single_threaded = num_threads == 1;
let span = tracing::trace_span!("ThreadPoolExecutor::run", num_threads);
let _ = span.enter();
let mut queue: Vec<_> = context
.check_open_files()
.into_iter()
.map(CheckFileTask::OpenFile)
.collect();
let (sender, receiver) = if single_threaded {
// Use an unbounded queue for single threaded execution to prevent deadlocks
// when a single file schedules multiple dependencies.
crossbeam::channel::unbounded()
} else {
// Use a bounded queue to apply backpressure when the orchestration thread isn't able to keep
// up processing messages from the worker threads.
crossbeam::channel::bounded(num_threads)
};
let schedule_sender = sender.clone();
let schedule_dependency = move |file_id| {
schedule_sender
.send(ThreadPoolMessage::ScheduleDependency(file_id))
.unwrap();
};
let result = rayon::in_place_scope(|scope| {
let mut pending = 0usize;
loop {
context.program().cancelled()?;
// 1. Try to get a queued message to ensure that we have always remaining space in the channel to prevent blocking the worker threads.
// 2. Try to process a queued file
// 3. If there's no queued file wait for the next incoming message.
// 4. Exit if there are no more messages and no senders.
let message = if let Ok(message) = receiver.try_recv() {
message
} else if let Some(task) = queue.pop() {
pending += 1;
let task_context = context.task_context(&schedule_dependency);
let sender = sender.clone();
let task_span = tracing::trace_span!(
parent: &span,
"CheckFileTask::run",
file_id = task.file_id().as_u32(),
);
scope.spawn(move |_| {
task_span.in_scope(|| match task.run(&task_context) {
Ok(result) => {
sender.send(ThreadPoolMessage::Completed(result)).unwrap();
}
Err(err) => sender.send(ThreadPoolMessage::Errored(err)).unwrap(),
});
});
// If this is a single threaded rayon thread pool, yield the current thread
// or we never start processing the work items.
if single_threaded {
yield_local();
}
continue;
} else if let Ok(message) = receiver.recv() {
message
} else {
break;
};
match message {
ThreadPoolMessage::ScheduleDependency(dependency) => {
if let Some(task) = context.check_dependency(dependency) {
queue.push(CheckFileTask::Dependency(task));
}
}
ThreadPoolMessage::Completed(diagnostics) => {
context.push_diagnostics(&diagnostics);
pending -= 1;
if pending == 0 && queue.is_empty() {
break;
}
}
ThreadPoolMessage::Errored(err) => {
return Err(err);
}
}
}
Ok(())
});
result
}
}
#[derive(Debug)]
enum ThreadPoolMessage {
ScheduleDependency(FileId),
enum CheckFileMessage {
Completed(Diagnostics),
Errored(QueryError),
Queue(FileId),
Cancelled,
}

View File

@@ -1,38 +1,45 @@
use std::collections::hash_map::Entry;
use std::path::{Path, PathBuf};
pub mod check;
use std::path::Path;
use std::sync::Arc;
use rustc_hash::FxHashMap;
use crate::db::{
Database, Db, DbRuntime, HasJar, HasJars, JarsStorage, LintDb, LintJar, ParallelDatabase,
QueryResult, SemanticDb, SemanticJar, Snapshot, SourceDb, SourceJar,
};
use crate::db::{Db, HasJar, SemanticDb, SemanticJar, SourceDb, SourceJar};
use crate::files::{FileId, Files};
use crate::lint::{lint_semantic, lint_syntax, Diagnostics};
use crate::lint::{
lint_semantic, lint_syntax, Diagnostics, LintSemanticStorage, LintSyntaxStorage,
};
use crate::module::{
add_module, file_to_module, path_to_module, resolve_module, set_module_search_paths, Module,
ModuleData, ModuleName, ModuleSearchPath,
ModuleData, ModuleName, ModuleResolver, ModuleSearchPath,
};
use crate::parse::{parse, Parsed};
use crate::source::{source_text, Source};
use crate::symbols::{symbol_table, SymbolId, SymbolTable};
use crate::types::{infer_symbol_type, Type};
use crate::parse::{parse, Parsed, ParsedStorage};
use crate::source::{source_text, Source, SourceStorage};
use crate::symbols::{symbol_table, SymbolId, SymbolTable, SymbolTablesStorage};
use crate::types::{infer_symbol_type, Type, TypeStore};
use crate::Workspace;
pub mod check;
#[derive(Debug)]
pub struct Program {
jars: JarsStorage<Program>,
files: Files,
source: SourceJar,
semantic: SemanticJar,
workspace: Workspace,
}
impl Program {
pub fn new(workspace: Workspace) -> Self {
pub fn new(workspace: Workspace, module_search_paths: Vec<ModuleSearchPath>) -> Self {
Self {
jars: JarsStorage::default(),
source: SourceJar {
sources: SourceStorage::default(),
parsed: ParsedStorage::default(),
lint_syntax: LintSyntaxStorage::default(),
},
semantic: SemanticJar {
module_resolver: ModuleResolver::new(module_search_paths),
symbol_tables: SymbolTablesStorage::default(),
type_store: TypeStore::default(),
lint_semantic: LintSemanticStorage::default(),
},
files: Files::default(),
workspace,
}
@@ -40,25 +47,19 @@ impl Program {
pub fn apply_changes<I>(&mut self, changes: I)
where
I: IntoIterator<Item = FileWatcherChange>,
I: IntoIterator<Item = FileChange>,
{
let mut aggregated_changes = AggregatedChanges::default();
aggregated_changes.extend(changes.into_iter().map(|change| FileChange {
id: self.files.intern(&change.path),
kind: change.kind,
}));
let (source, semantic, lint) = self.jars_mut();
for change in aggregated_changes.iter() {
semantic.module_resolver.remove_module(change.id);
semantic.symbol_tables.remove(&change.id);
source.sources.remove(&change.id);
source.parsed.remove(&change.id);
for change in changes {
self.semantic
.module_resolver
.remove_module(&self.file_path(change.id));
self.semantic.symbol_tables.remove(&change.id);
self.source.sources.remove(&change.id);
self.source.parsed.remove(&change.id);
self.source.lint_syntax.remove(&change.id);
// TODO: remove all dependent modules as well
semantic.type_store.remove_module(change.id);
lint.lint_syntax.remove(&change.id);
lint.lint_semantic.remove(&change.id);
self.semantic.type_store.remove_module(change.id);
self.semantic.lint_semantic.remove(&change.id);
}
}
@@ -84,36 +85,44 @@ impl SourceDb for Program {
self.files.path(file_id)
}
fn source(&self, file_id: FileId) -> QueryResult<Source> {
fn source(&self, file_id: FileId) -> Source {
source_text(self, file_id)
}
fn parse(&self, file_id: FileId) -> QueryResult<Parsed> {
fn parse(&self, file_id: FileId) -> Parsed {
parse(self, file_id)
}
fn lint_syntax(&self, file_id: FileId) -> Diagnostics {
lint_syntax(self, file_id)
}
}
impl SemanticDb for Program {
fn resolve_module(&self, name: ModuleName) -> QueryResult<Option<Module>> {
fn resolve_module(&self, name: ModuleName) -> Option<Module> {
resolve_module(self, name)
}
fn file_to_module(&self, file_id: FileId) -> QueryResult<Option<Module>> {
fn file_to_module(&self, file_id: FileId) -> Option<Module> {
file_to_module(self, file_id)
}
fn path_to_module(&self, path: &Path) -> QueryResult<Option<Module>> {
fn path_to_module(&self, path: &Path) -> Option<Module> {
path_to_module(self, path)
}
fn symbol_table(&self, file_id: FileId) -> QueryResult<Arc<SymbolTable>> {
fn symbol_table(&self, file_id: FileId) -> Arc<SymbolTable> {
symbol_table(self, file_id)
}
fn infer_symbol_type(&self, file_id: FileId, symbol_id: SymbolId) -> QueryResult<Type> {
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)
@@ -124,104 +133,44 @@ impl SemanticDb for Program {
}
}
impl LintDb for Program {
fn lint_syntax(&self, file_id: FileId) -> QueryResult<Diagnostics> {
lint_syntax(self, file_id)
}
fn lint_semantic(&self, file_id: FileId) -> QueryResult<Diagnostics> {
lint_semantic(self, file_id)
}
}
impl Db for Program {}
impl Database for Program {
fn runtime(&self) -> &DbRuntime {
self.jars.runtime()
}
fn runtime_mut(&mut self) -> &mut DbRuntime {
self.jars.runtime_mut()
}
}
impl ParallelDatabase for Program {
fn snapshot(&self) -> Snapshot<Self> {
Snapshot::new(Self {
jars: self.jars.snapshot(),
files: self.files.snapshot(),
workspace: self.workspace.clone(),
})
}
}
impl HasJars for Program {
type Jars = (SourceJar, SemanticJar, LintJar);
fn jars(&self) -> QueryResult<&Self::Jars> {
self.jars.jars()
}
fn jars_mut(&mut self) -> &mut Self::Jars {
self.jars.jars_mut()
}
}
impl HasJar<SourceJar> for Program {
fn jar(&self) -> QueryResult<&SourceJar> {
Ok(&self.jars()?.0)
fn jar(&self) -> &SourceJar {
&self.source
}
fn jar_mut(&mut self) -> &mut SourceJar {
&mut self.jars_mut().0
&mut self.source
}
}
impl HasJar<SemanticJar> for Program {
fn jar(&self) -> QueryResult<&SemanticJar> {
Ok(&self.jars()?.1)
fn jar(&self) -> &SemanticJar {
&self.semantic
}
fn jar_mut(&mut self) -> &mut SemanticJar {
&mut self.jars_mut().1
}
}
impl HasJar<LintJar> for Program {
fn jar(&self) -> QueryResult<&LintJar> {
Ok(&self.jars()?.2)
}
fn jar_mut(&mut self) -> &mut LintJar {
&mut self.jars_mut().2
}
}
#[derive(Clone, Debug)]
pub struct FileWatcherChange {
path: PathBuf,
kind: FileChangeKind,
}
impl FileWatcherChange {
pub fn new(path: PathBuf, kind: FileChangeKind) -> Self {
Self { path, kind }
&mut self.semantic
}
}
#[derive(Copy, Clone, Debug)]
struct FileChange {
pub struct FileChange {
id: FileId,
kind: FileChangeKind,
}
impl FileChange {
fn file_id(self) -> FileId {
pub fn new(file_id: FileId, kind: FileChangeKind) -> Self {
Self { id: file_id, kind }
}
pub fn file_id(&self) -> FileId {
self.id
}
fn kind(self) -> FileChangeKind {
pub fn kind(&self) -> FileChangeKind {
self.kind
}
}
@@ -232,74 +181,3 @@ pub enum FileChangeKind {
Modified,
Deleted,
}
#[derive(Default, Debug)]
struct AggregatedChanges {
changes: FxHashMap<FileId, FileChangeKind>,
}
impl AggregatedChanges {
fn add(&mut self, change: FileChange) {
match self.changes.entry(change.file_id()) {
Entry::Occupied(mut entry) => {
let merged = entry.get_mut();
match (merged, change.kind()) {
(FileChangeKind::Created, FileChangeKind::Deleted) => {
// Deletion after creations means that ruff never saw the file.
entry.remove();
}
(FileChangeKind::Created, FileChangeKind::Modified) => {
// No-op, for ruff, modifying a file that it doesn't yet know that it exists is still considered a creation.
}
(FileChangeKind::Modified, FileChangeKind::Created) => {
// Uhh, that should probably not happen. Continue considering it a modification.
}
(FileChangeKind::Modified, FileChangeKind::Deleted) => {
*entry.get_mut() = FileChangeKind::Deleted;
}
(FileChangeKind::Deleted, FileChangeKind::Created) => {
*entry.get_mut() = FileChangeKind::Modified;
}
(FileChangeKind::Deleted, FileChangeKind::Modified) => {
// That's weird, but let's consider it a modification.
*entry.get_mut() = FileChangeKind::Modified;
}
(FileChangeKind::Created, FileChangeKind::Created)
| (FileChangeKind::Modified, FileChangeKind::Modified)
| (FileChangeKind::Deleted, FileChangeKind::Deleted) => {
// No-op transitions. Some of them should be impossible but we handle them anyway.
}
}
}
Entry::Vacant(entry) => {
entry.insert(change.kind());
}
}
}
fn extend<I>(&mut self, changes: I)
where
I: IntoIterator<Item = FileChange>,
{
let iter = changes.into_iter();
let (lower, _) = iter.size_hint();
self.changes.reserve(lower);
for change in iter {
self.add(change);
}
}
fn iter(&self) -> impl Iterator<Item = FileChange> + '_ {
self.changes.iter().map(|(id, kind)| FileChange {
id: *id,
kind: *kind,
})
}
}

View File

@@ -1,5 +1,5 @@
use crate::cache::KeyValueCache;
use crate::db::{HasJar, QueryResult, SourceDb, SourceJar};
use crate::db::{HasJar, SourceDb, SourceJar};
use ruff_notebook::Notebook;
use ruff_python_ast::PySourceType;
use std::ops::{Deref, DerefMut};
@@ -8,11 +8,11 @@ use std::sync::Arc;
use crate::files::FileId;
#[tracing::instrument(level = "debug", skip(db))]
pub(crate) fn source_text<Db>(db: &Db, file_id: FileId) -> QueryResult<Source>
pub(crate) fn source_text<Db>(db: &Db, file_id: FileId) -> Source
where
Db: SourceDb + HasJar<SourceJar>,
{
let sources = &db.jar()?.sources;
let sources = &db.jar().sources;
sources.get(&file_id, |file_id| {
let path = db.file_path(*file_id);
@@ -43,7 +43,7 @@ where
}
};
Ok(Source { kind })
Source { kind }
})
}

View File

@@ -16,22 +16,22 @@ use ruff_python_ast::visitor::preorder::PreorderVisitor;
use crate::ast_ids::TypedNodeKey;
use crate::cache::KeyValueCache;
use crate::db::{HasJar, QueryResult, SemanticDb, SemanticJar};
use crate::db::{HasJar, SemanticDb, SemanticJar};
use crate::files::FileId;
use crate::module::ModuleName;
use crate::Name;
#[allow(unreachable_pub)]
#[tracing::instrument(level = "debug", skip(db))]
pub fn symbol_table<Db>(db: &Db, file_id: FileId) -> QueryResult<Arc<SymbolTable>>
pub fn symbol_table<Db>(db: &Db, file_id: FileId) -> Arc<SymbolTable>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let jar = db.jar()?;
let jar = db.jar();
jar.symbol_tables.get(&file_id, |_| {
let parsed = db.parse(file_id)?;
Ok(Arc::from(SymbolTable::from_ast(parsed.ast())))
let parsed = db.parse(file_id);
Arc::from(SymbolTable::from_ast(parsed.ast()))
})
}
@@ -68,7 +68,7 @@ pub(crate) struct Scope {
name: Name,
kind: ScopeKind,
child_scopes: Vec<ScopeId>,
/// symbol IDs, hashed by symbol name
// symbol IDs, hashed by symbol name
symbols_by_name: Map<SymbolId, ()>,
}
@@ -107,7 +107,6 @@ bitflags! {
pub(crate) struct Symbol {
name: Name,
flags: SymbolFlags,
scope_id: ScopeId,
// kind: Kind,
}
@@ -142,7 +141,7 @@ pub(crate) enum Definition {
// the small amount of information we need from the AST.
Import(ImportDefinition),
ImportFrom(ImportFromDefinition),
ClassDef(ClassDefinition),
ClassDef(TypedNodeKey<ast::StmtClassDef>),
FunctionDef(TypedNodeKey<ast::StmtFunctionDef>),
Assignment(TypedNodeKey<ast::StmtAssign>),
AnnotatedAssignment(TypedNodeKey<ast::StmtAnnAssign>),
@@ -175,12 +174,6 @@ impl ImportFromDefinition {
}
}
#[derive(Clone, Debug)]
pub(crate) struct ClassDefinition {
pub(crate) node_key: TypedNodeKey<ast::StmtClassDef>,
pub(crate) scope_id: ScopeId,
}
#[derive(Debug, Clone)]
pub enum Dependency {
Module(ModuleName),
@@ -339,11 +332,7 @@ impl SymbolTable {
*entry.key()
}
RawEntryMut::Vacant(entry) => {
let id = self.symbols_by_id.push(Symbol {
name,
flags,
scope_id,
});
let id = self.symbols_by_id.push(Symbol { name, flags });
entry.insert_with_hasher(hash, id, (), |_| hash);
id
}
@@ -470,8 +459,8 @@ impl SymbolTableBuilder {
symbol_id
}
fn push_scope(&mut self, name: &str, kind: ScopeKind) -> ScopeId {
let scope_id = self.table.add_child_scope(self.cur_scope(), name, kind);
fn push_scope(&mut self, child_of: ScopeId, name: &str, kind: ScopeKind) -> ScopeId {
let scope_id = self.table.add_child_scope(child_of, name, kind);
self.scopes.push(scope_id);
scope_id
}
@@ -493,10 +482,10 @@ impl SymbolTableBuilder {
&mut self,
name: &str,
params: &Option<Box<ast::TypeParams>>,
nested: impl FnOnce(&mut Self) -> ScopeId,
) -> ScopeId {
nested: impl FnOnce(&mut Self),
) {
if let Some(type_params) = params {
self.push_scope(name, ScopeKind::Annotation);
self.push_scope(self.cur_scope(), name, ScopeKind::Annotation);
for type_param in &type_params.type_params {
let name = match type_param {
ast::TypeParam::TypeVar(ast::TypeParamTypeVar { name, .. }) => name,
@@ -506,11 +495,10 @@ impl SymbolTableBuilder {
self.add_or_update_symbol(name, SymbolFlags::IS_DEFINED);
}
}
let scope_id = nested(self);
nested(self);
if params.is_some() {
self.pop_scope();
}
scope_id
}
}
@@ -537,26 +525,21 @@ impl PreorderVisitor<'_> for SymbolTableBuilder {
// TODO need to capture more definition statements here
match stmt {
ast::Stmt::ClassDef(node) => {
let scope_id = self.with_type_params(&node.name, &node.type_params, |builder| {
let scope_id = builder.push_scope(&node.name, ScopeKind::Class);
let def = Definition::ClassDef(TypedNodeKey::from_node(node));
self.add_or_update_symbol_with_def(&node.name, def);
self.with_type_params(&node.name, &node.type_params, |builder| {
builder.push_scope(builder.cur_scope(), &node.name, ScopeKind::Class);
ast::visitor::preorder::walk_stmt(builder, stmt);
builder.pop_scope();
scope_id
});
let def = Definition::ClassDef(ClassDefinition {
node_key: TypedNodeKey::from_node(node),
scope_id,
});
self.add_or_update_symbol_with_def(&node.name, def);
}
ast::Stmt::FunctionDef(node) => {
let def = Definition::FunctionDef(TypedNodeKey::from_node(node));
self.add_or_update_symbol_with_def(&node.name, def);
self.with_type_params(&node.name, &node.type_params, |builder| {
let scope_id = builder.push_scope(&node.name, ScopeKind::Function);
builder.push_scope(builder.cur_scope(), &node.name, ScopeKind::Function);
ast::visitor::preorder::walk_stmt(builder, stmt);
builder.pop_scope();
scope_id
});
}
ast::Stmt::Import(ast::StmtImport { names, .. }) => {

View File

@@ -1,8 +1,7 @@
#![allow(dead_code)]
use crate::ast_ids::NodeKey;
use crate::db::{HasJar, QueryResult, SemanticDb, SemanticJar};
use crate::files::FileId;
use crate::symbols::{ScopeId, SymbolId};
use crate::symbols::SymbolId;
use crate::{FxDashMap, FxIndexSet, Name};
use ruff_index::{newtype_index, IndexVec};
use rustc_hash::FxHashMap;
@@ -120,20 +119,12 @@ impl TypeStore {
self.modules.get(&file_id)
}
fn add_function(&self, file_id: FileId, name: &str, decorators: Vec<Type>) -> FunctionTypeId {
self.add_or_get_module(file_id)
.add_function(name, decorators)
fn add_function(&self, file_id: FileId, name: &str) -> FunctionTypeId {
self.add_or_get_module(file_id).add_function(name)
}
fn add_class(
&self,
file_id: FileId,
name: &str,
scope_id: ScopeId,
bases: Vec<Type>,
) -> ClassTypeId {
self.add_or_get_module(file_id)
.add_class(name, scope_id, bases)
fn add_class(&self, file_id: FileId, name: &str, bases: Vec<Type>) -> ClassTypeId {
self.add_or_get_module(file_id).add_class(name, bases)
}
fn add_union(&mut self, file_id: FileId, elems: &[Type]) -> UnionTypeId {
@@ -261,24 +252,6 @@ pub struct ClassTypeId {
class_id: ModuleClassTypeId,
}
impl ClassTypeId {
fn get_own_class_member<Db>(self, db: &Db, name: &Name) -> QueryResult<Option<Type>>
where
Db: SemanticDb + HasJar<SemanticJar>,
{
// TODO: this should distinguish instance-only members (e.g. `x: int`) and not return them
let ClassType { scope_id, .. } = *db.jar()?.type_store.get_class(self);
let table = db.symbol_table(self.file_id)?;
if let Some(symbol_id) = table.symbol_id_by_name(scope_id, name) {
Ok(Some(db.infer_symbol_type(self.file_id, symbol_id)?))
} else {
Ok(None)
}
}
// TODO: get_own_instance_member, get_class_member, get_instance_member
}
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
pub struct UnionTypeId {
file_id: FileId,
@@ -333,10 +306,9 @@ impl ModuleTypeStore {
}
}
fn add_function(&mut self, name: &str, decorators: Vec<Type>) -> FunctionTypeId {
fn add_function(&mut self, name: &str) -> FunctionTypeId {
let func_id = self.functions.push(FunctionType {
name: Name::new(name),
decorators,
});
FunctionTypeId {
file_id: self.file_id,
@@ -344,10 +316,9 @@ impl ModuleTypeStore {
}
}
fn add_class(&mut self, name: &str, scope_id: ScopeId, bases: Vec<Type>) -> ClassTypeId {
fn add_class(&mut self, name: &str, bases: Vec<Type>) -> ClassTypeId {
let class_id = self.classes.push(ClassType {
name: Name::new(name),
scope_id,
// TODO: if no bases are given, that should imply [object]
bases,
});
@@ -432,11 +403,7 @@ impl std::fmt::Display for DisplayType<'_> {
#[derive(Debug)]
pub(crate) struct ClassType {
/// Name of the class at definition
name: Name,
/// `ScopeId` of the class body
pub(crate) scope_id: ScopeId,
/// Types of all class bases
bases: Vec<Type>,
}
@@ -453,17 +420,12 @@ impl ClassType {
#[derive(Debug)]
pub(crate) struct FunctionType {
name: Name,
decorators: Vec<Type>,
}
impl FunctionType {
fn name(&self) -> &str {
self.name.as_str()
}
fn decorators(&self) -> &[Type] {
self.decorators.as_slice()
}
}
#[derive(Debug)]
@@ -527,7 +489,6 @@ impl IntersectionType {
#[cfg(test)]
mod tests {
use crate::files::Files;
use crate::symbols::SymbolTable;
use crate::types::{Type, TypeStore};
use crate::FxIndexSet;
use std::path::Path;
@@ -537,7 +498,7 @@ mod tests {
let store = TypeStore::default();
let files = Files::default();
let file_id = files.intern(Path::new("/foo"));
let id = store.add_class(file_id, "C", SymbolTable::root_scope_id(), Vec::new());
let id = store.add_class(file_id, "C", Vec::new());
assert_eq!(store.get_class(id).name(), "C");
let inst = Type::Instance(id);
assert_eq!(format!("{}", inst.display(&store)), "C");
@@ -548,9 +509,8 @@ mod tests {
let store = TypeStore::default();
let files = Files::default();
let file_id = files.intern(Path::new("/foo"));
let id = store.add_function(file_id, "func", vec![Type::Unknown]);
let id = store.add_function(file_id, "func");
assert_eq!(store.get_function(id).name(), "func");
assert_eq!(store.get_function(id).decorators(), vec![Type::Unknown]);
let func = Type::Function(id);
assert_eq!(format!("{}", func.display(&store)), "func");
}
@@ -560,8 +520,8 @@ mod tests {
let mut store = TypeStore::default();
let files = Files::default();
let file_id = files.intern(Path::new("/foo"));
let c1 = store.add_class(file_id, "C1", SymbolTable::root_scope_id(), Vec::new());
let c2 = store.add_class(file_id, "C2", SymbolTable::root_scope_id(), Vec::new());
let c1 = store.add_class(file_id, "C1", Vec::new());
let c2 = store.add_class(file_id, "C2", Vec::new());
let elems = vec![Type::Instance(c1), Type::Instance(c2)];
let id = store.add_union(file_id, &elems);
assert_eq!(
@@ -577,9 +537,9 @@ mod tests {
let mut store = TypeStore::default();
let files = Files::default();
let file_id = files.intern(Path::new("/foo"));
let c1 = store.add_class(file_id, "C1", SymbolTable::root_scope_id(), Vec::new());
let c2 = store.add_class(file_id, "C2", SymbolTable::root_scope_id(), Vec::new());
let c3 = store.add_class(file_id, "C3", SymbolTable::root_scope_id(), Vec::new());
let c1 = store.add_class(file_id, "C1", Vec::new());
let c2 = store.add_class(file_id, "C2", Vec::new());
let c3 = store.add_class(file_id, "C3", Vec::new());
let pos = vec![Type::Instance(c1), Type::Instance(c2)];
let neg = vec![Type::Instance(c3)];
let id = store.add_intersection(file_id, &pos, &neg);

View File

@@ -2,33 +2,32 @@
use ruff_python_ast::AstNode;
use crate::db::{HasJar, QueryResult, SemanticDb, SemanticJar};
use crate::db::{HasJar, SemanticDb, SemanticJar};
use crate::module::ModuleName;
use crate::symbols::{ClassDefinition, Definition, ImportFromDefinition, SymbolId};
use crate::symbols::{Definition, ImportFromDefinition, SymbolId};
use crate::types::Type;
use crate::FileId;
use ruff_python_ast as ast;
// FIXME: Figure out proper dead-lock free synchronisation now that this takes `&db` instead of `&mut db`.
#[tracing::instrument(level = "trace", skip(db))]
pub fn infer_symbol_type<Db>(db: &Db, file_id: FileId, symbol_id: SymbolId) -> QueryResult<Type>
pub fn infer_symbol_type<Db>(db: &Db, file_id: FileId, symbol_id: SymbolId) -> Type
where
Db: SemanticDb + HasJar<SemanticJar>,
{
let symbols = db.symbol_table(file_id)?;
let symbols = db.symbol_table(file_id);
let defs = symbols.definitions(symbol_id);
if let Some(ty) = db
.jar()?
.jar()
.type_store
.get_cached_symbol_type(file_id, symbol_id)
{
return Ok(ty);
return ty;
}
// TODO handle multiple defs, conditional defs...
assert_eq!(defs.len(), 1);
let type_store = &db.jar()?.type_store;
let ty = match &defs[0] {
Definition::ImportFrom(ImportFromDefinition {
@@ -39,11 +38,11 @@ where
// TODO relative imports
assert!(matches!(level, 0));
let module_name = ModuleName::new(module.as_ref().expect("TODO relative imports"));
if let Some(module) = db.resolve_module(module_name)? {
let remote_file_id = module.path(db)?.file();
let remote_symbols = db.symbol_table(remote_file_id)?;
if let Some(module) = db.resolve_module(module_name) {
let remote_file_id = module.path(db).file();
let remote_symbols = db.symbol_table(remote_file_id);
if let Some(remote_symbol_id) = remote_symbols.root_symbol_id_by_name(name) {
db.infer_symbol_type(remote_file_id, remote_symbol_id)?
db.infer_symbol_type(remote_file_id, remote_symbol_id)
} else {
Type::Unknown
}
@@ -51,77 +50,71 @@ where
Type::Unknown
}
}
Definition::ClassDef(ClassDefinition { node_key, scope_id }) => {
if let Some(ty) = type_store.get_cached_node_type(file_id, node_key.erased()) {
ty
} else {
let parsed = db.parse(file_id)?;
Definition::ClassDef(node_key) => db
.jar()
.type_store
.get_cached_node_type(file_id, node_key.erased())
.unwrap_or_else(|| {
let parsed = db.parse(file_id);
let ast = parsed.ast();
let node = node_key.resolve_unwrap(ast.as_any_node_ref());
let mut bases = Vec::with_capacity(node.bases().len());
let bases: Vec<_> = node
.bases()
.iter()
.map(|base_expr| infer_expr_type(db, file_id, base_expr))
.collect();
for base in node.bases() {
bases.push(infer_expr_type(db, file_id, base)?);
}
let ty =
Type::Class(type_store.add_class(file_id, &node.name.id, *scope_id, bases));
type_store.cache_node_type(file_id, *node_key.erased(), ty);
let store = &db.jar().type_store;
let ty = Type::Class(store.add_class(file_id, &node.name.id, bases));
store.cache_node_type(file_id, *node_key.erased(), ty);
ty
}
}
Definition::FunctionDef(node_key) => {
if let Some(ty) = type_store.get_cached_node_type(file_id, node_key.erased()) {
ty
} else {
let parsed = db.parse(file_id)?;
}),
Definition::FunctionDef(node_key) => db
.jar()
.type_store
.get_cached_node_type(file_id, node_key.erased())
.unwrap_or_else(|| {
let parsed = db.parse(file_id);
let ast = parsed.ast();
let node = node_key
.resolve(ast.as_any_node_ref())
.expect("node key should resolve");
let decorator_tys = node
.decorator_list
.iter()
.map(|decorator| infer_expr_type(db, file_id, &decorator.expression))
.collect::<QueryResult<_>>()?;
let ty = type_store
.add_function(file_id, &node.name.id, decorator_tys)
.into();
type_store.cache_node_type(file_id, *node_key.erased(), ty);
let store = &db.jar().type_store;
let ty = store.add_function(file_id, &node.name.id).into();
store.cache_node_type(file_id, *node_key.erased(), ty);
ty
}
}
}),
Definition::Assignment(node_key) => {
let parsed = db.parse(file_id)?;
let parsed = db.parse(file_id);
let ast = parsed.ast();
let node = node_key.resolve_unwrap(ast.as_any_node_ref());
// TODO handle unpacking assignment correctly
infer_expr_type(db, file_id, &node.value)?
infer_expr_type(db, file_id, &node.value)
}
_ => todo!("other kinds of definitions"),
};
type_store.cache_symbol_type(file_id, symbol_id, ty);
db.jar()
.type_store
.cache_symbol_type(file_id, symbol_id, ty);
// TODO record dependencies
Ok(ty)
ty
}
fn infer_expr_type<Db>(db: &Db, file_id: FileId, expr: &ast::Expr) -> QueryResult<Type>
fn infer_expr_type<Db>(db: &Db, file_id: FileId, expr: &ast::Expr) -> Type
where
Db: SemanticDb + HasJar<SemanticJar>,
{
// TODO cache the resolution of the type on the node
let symbols = db.symbol_table(file_id)?;
let symbols = db.symbol_table(file_id);
match expr {
ast::Expr::Name(name) => {
if let Some(symbol_id) = symbols.root_symbol_id_by_name(&name.id) {
db.infer_symbol_type(file_id, symbol_id)
} else {
Ok(Type::Unknown)
Type::Unknown
}
}
_ => todo!("full expression type resolution"),
@@ -134,7 +127,6 @@ mod tests {
use crate::db::{HasJar, SemanticDb, SemanticJar};
use crate::module::{ModuleName, ModuleSearchPath, ModuleSearchPathKind};
use crate::types::Type;
use crate::Name;
// TODO with virtual filesystem we shouldn't have to write files to disk for these
// tests
@@ -162,7 +154,7 @@ mod tests {
}
#[test]
fn follow_import_to_class() -> anyhow::Result<()> {
fn follow_import_to_class() -> std::io::Result<()> {
let case = create_test()?;
let db = &case.db;
@@ -171,18 +163,18 @@ mod tests {
std::fs::write(a_path, "from b import C as D; E = D")?;
std::fs::write(b_path, "class C: pass")?;
let a_file = db
.resolve_module(ModuleName::new("a"))?
.resolve_module(ModuleName::new("a"))
.expect("module should be found")
.path(db)?
.path(db)
.file();
let a_syms = db.symbol_table(a_file)?;
let a_syms = db.symbol_table(a_file);
let e_sym = a_syms
.root_symbol_id_by_name("E")
.expect("E symbol should be found");
let ty = db.infer_symbol_type(a_file, e_sym)?;
let ty = db.infer_symbol_type(a_file, e_sym);
let jar = HasJar::<SemanticJar>::jar(db)?;
let jar = HasJar::<SemanticJar>::jar(db);
assert!(matches!(ty, Type::Class(_)));
assert_eq!(format!("{}", ty.display(&jar.type_store)), "Literal[C]");
@@ -190,28 +182,28 @@ mod tests {
}
#[test]
fn resolve_base_class_by_name() -> anyhow::Result<()> {
fn resolve_base_class_by_name() -> std::io::Result<()> {
let case = create_test()?;
let db = &case.db;
let path = case.src.path().join("mod.py");
std::fs::write(path, "class Base: pass\nclass Sub(Base): pass")?;
let file = db
.resolve_module(ModuleName::new("mod"))?
.resolve_module(ModuleName::new("mod"))
.expect("module should be found")
.path(db)?
.path(db)
.file();
let syms = db.symbol_table(file)?;
let syms = db.symbol_table(file);
let sym = syms
.root_symbol_id_by_name("Sub")
.expect("Sub symbol should be found");
let ty = db.infer_symbol_type(file, sym)?;
let ty = db.infer_symbol_type(file, sym);
let Type::Class(class_id) = ty else {
panic!("Sub is not a Class")
};
let jar = HasJar::<SemanticJar>::jar(db)?;
let jar = HasJar::<SemanticJar>::jar(db);
let base_names: Vec<_> = jar
.type_store
.get_class(class_id)
@@ -224,42 +216,4 @@ mod tests {
Ok(())
}
#[test]
fn resolve_method() -> anyhow::Result<()> {
let case = create_test()?;
let db = &case.db;
let path = case.src.path().join("mod.py");
std::fs::write(path, "class C:\n def f(self): pass")?;
let file = db
.resolve_module(ModuleName::new("mod"))?
.expect("module should be found")
.path(db)?
.file();
let syms = db.symbol_table(file)?;
let sym = syms
.root_symbol_id_by_name("C")
.expect("C symbol should be found");
let ty = db.infer_symbol_type(file, sym)?;
let Type::Class(class_id) = ty else {
panic!("C is not a Class");
};
let member_ty = class_id
.get_own_class_member(db, &Name::new("f"))
.expect("C.f to resolve");
let Some(Type::Function(func_id)) = member_ty else {
panic!("C.f is not a Function");
};
let jar = HasJar::<SemanticJar>::jar(db)?;
let function = jar.type_store.get_function(func_id);
assert_eq!(function.name(), "f");
Ok(())
}
}

View File

@@ -1,38 +1,38 @@
use anyhow::Context;
use std::path::Path;
use anyhow::Context;
use crate::files::Files;
use crate::program::{FileChange, FileChangeKind};
use notify::event::{CreateKind, RemoveKind};
use notify::{recommended_watcher, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
use crate::program::{FileChangeKind, FileWatcherChange};
pub struct FileWatcher {
watcher: RecommendedWatcher,
}
pub trait EventHandler: Send + 'static {
fn handle(&self, changes: Vec<FileWatcherChange>);
fn handle(&self, changes: Vec<FileChange>);
}
impl<F> EventHandler for F
where
F: Fn(Vec<FileWatcherChange>) + Send + 'static,
F: Fn(Vec<FileChange>) + Send + 'static,
{
fn handle(&self, changes: Vec<FileWatcherChange>) {
fn handle(&self, changes: Vec<FileChange>) {
let f = self;
f(changes);
}
}
impl FileWatcher {
pub fn new<E>(handler: E) -> anyhow::Result<Self>
pub fn new<E>(handler: E, files: Files) -> anyhow::Result<Self>
where
E: EventHandler,
{
Self::from_handler(Box::new(handler))
Self::from_handler(Box::new(handler), files)
}
fn from_handler(handler: Box<dyn EventHandler>) -> anyhow::Result<Self> {
fn from_handler(handler: Box<dyn EventHandler>, files: Files) -> anyhow::Result<Self> {
let watcher = recommended_watcher(move |changes: notify::Result<Event>| {
match changes {
Ok(event) => {
@@ -50,7 +50,8 @@ impl FileWatcher {
for path in event.paths {
if path.is_file() {
changes.push(FileWatcherChange::new(path, change_kind));
let id = files.intern(&path);
changes.push(FileChange::new(id, change_kind));
}
}

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff"
version = "0.4.3"
version = "0.4.2"
publish = false
authors = { workspace = true }
edition = { workspace = true }
@@ -41,6 +41,7 @@ is-macro = { workspace = true }
itertools = { workspace = true }
log = { workspace = true }
notify = { workspace = true }
num_cpus = { workspace = true }
path-absolutize = { workspace = true, features = ["once_cell_cache"] }
rayon = { workspace = true }
regex = { workspace = true }

View File

@@ -338,7 +338,7 @@ pub struct CheckCommand {
/// The name of the file when passing it through stdin.
#[arg(long, help_heading = "Miscellaneous")]
pub stdin_filename: Option<PathBuf>,
/// List of mappings from file extension to language (one of `python`, `ipynb`, `pyi`). For
/// List of mappings from file extension to language (one of ["python", "ipynb", "pyi"]). For
/// example, to treat `.ipy` files as IPython notebooks, use `--extension ipy:ipynb`.
#[arg(long, value_delimiter = ',')]
pub extension: Option<Vec<ExtensionPair>>,
@@ -466,7 +466,7 @@ pub struct FormatCommand {
/// The name of the file when passing it through stdin.
#[arg(long, help_heading = "Miscellaneous")]
pub stdin_filename: Option<PathBuf>,
/// List of mappings from file extension to language (one of `python`, `ipynb`, `pyi`). For
/// List of mappings from file extension to language (one of ["python", "ipynb", "pyi"]). For
/// example, to treat `.ipy` files as IPython notebooks, use `--extension ipy:ipynb`.
#[arg(long, value_delimiter = ',')]
pub extension: Option<Vec<ExtensionPair>>,

View File

@@ -23,6 +23,7 @@ use ruff_linter::message::Message;
use ruff_linter::{warn_user, VERSION};
use ruff_macros::CacheKey;
use ruff_notebook::NotebookIndex;
use ruff_python_ast::imports::ImportMap;
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_workspace::resolver::Resolver;
@@ -347,7 +348,7 @@ impl FileCache {
} else {
FxHashMap::default()
};
Diagnostics::new(messages, notebook_indexes)
Diagnostics::new(messages, lint.imports.clone(), notebook_indexes)
})
}
}
@@ -393,7 +394,7 @@ pub(crate) fn init(path: &Path) -> Result<()> {
#[derive(Deserialize, Debug, Serialize, PartialEq)]
pub(crate) struct LintCacheData {
/// Imports made.
// pub(super) imports: ImportMap,
pub(super) imports: ImportMap,
/// Diagnostic messages.
pub(super) messages: Vec<CacheMessage>,
/// Source code of the file.
@@ -409,6 +410,7 @@ pub(crate) struct LintCacheData {
impl LintCacheData {
pub(crate) fn from_messages(
messages: &[Message],
imports: ImportMap,
notebook_index: Option<NotebookIndex>,
) -> Self {
let source = if let Some(msg) = messages.first() {
@@ -436,6 +438,7 @@ impl LintCacheData {
.collect();
Self {
imports,
messages,
source,
notebook_index,

View File

@@ -17,6 +17,7 @@ use ruff_linter::registry::Rule;
use ruff_linter::settings::types::UnsafeFixes;
use ruff_linter::settings::{flags, LinterSettings};
use ruff_linter::{fs, warn_user_once, IOError};
use ruff_python_ast::imports::ImportMap;
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_workspace::resolver::{
@@ -133,6 +134,7 @@ pub(crate) fn check(
dummy,
TextSize::default(),
)],
ImportMap::default(),
FxHashMap::default(),
)
} else {

View File

@@ -23,6 +23,7 @@ use ruff_linter::settings::{flags, LinterSettings};
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::{fs, IOError, SyntaxError};
use ruff_notebook::{Notebook, NotebookError, NotebookIndex};
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
@@ -34,17 +35,20 @@ use crate::cache::{Cache, FileCacheKey, LintCacheData};
pub(crate) struct Diagnostics {
pub(crate) messages: Vec<Message>,
pub(crate) fixed: FixMap,
pub(crate) imports: ImportMap,
pub(crate) notebook_indexes: FxHashMap<String, NotebookIndex>,
}
impl Diagnostics {
pub(crate) fn new(
messages: Vec<Message>,
imports: ImportMap,
notebook_indexes: FxHashMap<String, NotebookIndex>,
) -> Self {
Self {
messages,
fixed: FixMap::default(),
imports,
notebook_indexes,
}
}
@@ -88,6 +92,7 @@ impl Diagnostics {
dummy,
TextSize::default(),
)],
ImportMap::default(),
FxHashMap::default(),
)
} else {
@@ -122,6 +127,7 @@ impl Add for Diagnostics {
impl AddAssign for Diagnostics {
fn add_assign(&mut self, other: Self) {
self.messages.extend(other.messages);
self.imports.extend(other.imports);
self.fixed += other.fixed;
self.notebook_indexes.extend(other.notebook_indexes);
}
@@ -261,7 +267,7 @@ pub(crate) fn lint_path(
// Lint the file.
let (
LinterResult {
data: messages,
data: (messages, imports),
error: parse_error,
},
transformed,
@@ -329,6 +335,8 @@ pub(crate) fn lint_path(
(result, transformed, fixed)
};
let imports = imports.unwrap_or_default();
if let Some((cache, relative_path, key)) = caching {
// We don't cache parsing errors.
if parse_error.is_none() {
@@ -346,6 +354,7 @@ pub(crate) fn lint_path(
&key,
LintCacheData::from_messages(
&messages,
imports.clone(),
transformed.as_ipy_notebook().map(Notebook::index).cloned(),
),
);
@@ -369,6 +378,7 @@ pub(crate) fn lint_path(
Ok(Diagnostics {
messages,
fixed: FixMap::from_iter([(fs::relativize_path(path), fixed)]),
imports,
notebook_indexes,
})
}
@@ -406,7 +416,7 @@ pub(crate) fn lint_stdin(
// Lint the inputs.
let (
LinterResult {
data: messages,
data: (messages, imports),
error: parse_error,
},
transformed,
@@ -484,6 +494,8 @@ pub(crate) fn lint_stdin(
(result, transformed, fixed)
};
let imports = imports.unwrap_or_default();
if let Some(error) = parse_error {
error!(
"{}",
@@ -506,6 +518,7 @@ pub(crate) fn lint_stdin(
fs::relativize_path(path.unwrap_or_else(|| Path::new("-"))),
fixed,
)]),
imports,
notebook_indexes,
})
}

View File

@@ -214,14 +214,13 @@ fn format(args: FormatCommand, global_options: GlobalConfigArgs) -> Result<ExitS
fn server(args: ServerCommand, log_level: LogLevel) -> Result<ExitStatus> {
let ServerCommand { preview } = args;
let four = NonZeroUsize::new(4).unwrap();
// by default, we set the number of worker threads to `num_cpus`, with a maximum of 4.
let worker_threads = std::thread::available_parallelism()
.unwrap_or(four)
.max(four);
commands::server::run_server(preview, worker_threads, log_level)
let worker_threads = num_cpus::get().max(4);
commands::server::run_server(
preview,
NonZeroUsize::try_from(worker_threads).expect("a non-zero worker thread count"),
log_level,
)
}
pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<ExitStatus> {

View File

@@ -344,7 +344,7 @@ pub struct RemoveSoftLinesBuffer<'a, Context> {
/// Caches the interned elements after the soft line breaks have been removed.
///
/// The `key` is the [Interned] element as it has been passed to [`Self::write_element`] or the child of another
/// The `key` is the [Interned] element as it has been passed to [Self::write_element] or the child of another
/// [Interned] element. The `value` is the matching document of the key where all soft line breaks have been removed.
///
/// It's fine to not snapshot the cache. The worst that can happen is that it holds on interned elements

View File

@@ -18,10 +18,10 @@ pub enum FormatError {
InvalidDocument(InvalidDocumentError),
/// Formatting failed because some content encountered a situation where a layout
/// choice by an enclosing [`crate::Format`] resulted in a poor layout for a child [`crate::Format`].
/// choice by an enclosing [crate::Format] resulted in a poor layout for a child [crate::Format].
///
/// It's up to an enclosing [`crate::Format`] to handle the error and pick another layout.
/// This error should not be raised if there's no outer [`crate::Format`] handling the poor layout error,
/// It's up to an enclosing [crate::Format] to handle the error and pick another layout.
/// This error should not be raised if there's no outer [crate::Format] handling the poor layout error,
/// avoiding that formatting of the whole document fails.
PoorLayout,
}

View File

@@ -19,10 +19,10 @@ use ruff_text_size::TextSize;
/// Use the helper functions like [`crate::builders::space`], [`crate::builders::soft_line_break`] etc. defined in this file to create elements.
#[derive(Clone, Eq, PartialEq)]
pub enum FormatElement {
/// A space token, see [`crate::builders::space`] for documentation.
/// A space token, see [crate::builders::space] for documentation.
Space,
/// A new line, see [`crate::builders::soft_line_break`], [`crate::builders::hard_line_break`], and [`crate::builders::soft_line_break_or_space`] for documentation.
/// A new line, see [crate::builders::soft_line_break], [crate::builders::hard_line_break], and [crate::builders::soft_line_break_or_space] for documentation.
Line(LineMode),
/// Forces the parent group to print in expanded mode.
@@ -108,13 +108,13 @@ impl std::fmt::Debug for FormatElement {
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum LineMode {
/// See [`crate::builders::soft_line_break_or_space`] for documentation.
/// See [crate::builders::soft_line_break_or_space] for documentation.
SoftOrSpace,
/// See [`crate::builders::soft_line_break`] for documentation.
/// See [crate::builders::soft_line_break] for documentation.
Soft,
/// See [`crate::builders::hard_line_break`] for documentation.
/// See [crate::builders::hard_line_break] for documentation.
Hard,
/// See [`crate::builders::empty_line`] for documentation.
/// See [crate::builders::empty_line] for documentation.
Empty,
}

View File

@@ -9,14 +9,14 @@ use std::num::NonZeroU8;
/// will be applied to all elements in between the start/end tags.
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Tag {
/// Indents the content one level deeper, see [`crate::builders::indent`] for documentation and examples.
/// Indents the content one level deeper, see [crate::builders::indent] for documentation and examples.
StartIndent,
EndIndent,
/// Variant of [`TagKind::Indent`] that indents content by a number of spaces. For example, `Align(2)`
/// Variant of [TagKind::Indent] that indents content by a number of spaces. For example, `Align(2)`
/// indents any content following a line break by an additional two spaces.
///
/// Nesting (Aligns)[`TagKind::Align`] has the effect that all except the most inner align are handled as (Indent)[`TagKind::Indent`].
/// Nesting (Aligns)[TagKind::Align] has the effect that all except the most inner align are handled as (Indent)[TagKind::Indent].
StartAlign(Align),
EndAlign,
@@ -29,7 +29,7 @@ pub enum Tag {
/// - on a single line: Omitting `LineMode::Soft` line breaks and printing spaces for `LineMode::SoftOrSpace`
/// - on multiple lines: Printing all line breaks
///
/// See [`crate::builders::group`] for documentation and examples.
/// See [crate::builders::group] for documentation and examples.
StartGroup(Group),
EndGroup,
@@ -44,22 +44,22 @@ pub enum Tag {
EndConditionalGroup,
/// Allows to specify content that gets printed depending on whatever the enclosing group
/// is printed on a single line or multiple lines. See [`crate::builders::if_group_breaks`] for examples.
/// is printed on a single line or multiple lines. See [crate::builders::if_group_breaks] for examples.
StartConditionalContent(Condition),
EndConditionalContent,
/// Optimized version of [`Tag::StartConditionalContent`] for the case where some content
/// Optimized version of [Tag::StartConditionalContent] for the case where some content
/// should be indented if the specified group breaks.
StartIndentIfGroupBreaks(GroupId),
EndIndentIfGroupBreaks,
/// Concatenates multiple elements together with a given separator printed in either
/// flat or expanded mode to fill the print width. Expect that the content is a list of alternating
/// [element, separator] See [`crate::Formatter::fill`].
/// [element, separator] See [crate::Formatter::fill].
StartFill,
EndFill,
/// Entry inside of a [`Tag::StartFill`]
/// Entry inside of a [Tag::StartFill]
StartEntry,
EndEntry,
@@ -77,7 +77,7 @@ pub enum Tag {
/// Special semantic element marking the content with a label.
/// This does not directly influence how the content will be printed.
///
/// See [`crate::builders::labelled`] for documentation.
/// See [crate::builders::labelled] for documentation.
StartLabelled(LabelId),
EndLabelled,

View File

@@ -189,6 +189,27 @@ impl<'a, 'print> Queue<'a> for FitsQueue<'a, 'print> {
}
}
/// Iterator that calls [`Queue::pop`] until it reaches the end of the document.
///
/// The iterator traverses into the content of any [`FormatElement::Interned`].
pub(super) struct QueueIterator<'a, 'q, Q: Queue<'a>> {
queue: &'q mut Q,
lifetime: PhantomData<&'a ()>,
}
impl<'a, Q> Iterator for QueueIterator<'a, '_, Q>
where
Q: Queue<'a>,
{
type Item = &'a FormatElement;
fn next(&mut self) -> Option<Self::Item> {
self.queue.pop()
}
}
impl<'a, Q> FusedIterator for QueueIterator<'a, '_, Q> where Q: Queue<'a> {}
pub(super) struct QueueContentIterator<'a, 'q, Q: Queue<'a>> {
queue: &'q mut Q,
kind: TagKind,

View File

@@ -8,6 +8,9 @@ pub(super) trait Stack<T> {
/// Returns the last element if any
fn top(&self) -> Option<&T>;
/// Returns `true` if the stack is empty
fn is_empty(&self) -> bool;
}
impl<T> Stack<T> for Vec<T> {
@@ -22,6 +25,10 @@ impl<T> Stack<T> for Vec<T> {
fn top(&self) -> Option<&T> {
self.last()
}
fn is_empty(&self) -> bool {
self.is_empty()
}
}
/// A Stack that is stacked on top of another stack. Guarantees that the underlying stack remains unchanged.
@@ -73,6 +80,10 @@ where
.last()
.or_else(|| self.original.as_slice().last())
}
fn is_empty(&self) -> bool {
self.stack.is_empty() && self.original.len() == 0
}
}
#[cfg(test)]

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff_linter"
version = "0.4.3"
version = "0.4.2"
publish = false
authors = { workspace = true }
edition = { workspace = true }

View File

@@ -115,25 +115,25 @@ class non_keyword_abcmeta_2(abc.ABCMeta): # safe
# very invalid code, but that's up to mypy et al to check
class keyword_abc_1(metaclass=ABC): # incorrect but outside scope of this check
class keyword_abc_1(metaclass=ABC): # safe
def method(self):
foo()
class keyword_abc_2(metaclass=abc.ABC): # incorrect but outside scope of this check
class keyword_abc_2(metaclass=abc.ABC): # safe
def method(self):
foo()
class abc_set_class_variable_1(ABC): # safe (abstract attribute)
class abc_set_class_variable_1(ABC): # safe
foo: int
class abc_set_class_variable_2(ABC): # error (not an abstract attribute)
class abc_set_class_variable_2(ABC): # safe
foo = 2
class abc_set_class_variable_3(ABC): # error (not an abstract attribute)
class abc_set_class_variable_3(ABC): # safe
foo: int = 2

View File

@@ -8,6 +8,7 @@ class Class:
pass
if False:
def extra_bad_method(this):
pass
@@ -93,7 +94,6 @@ class ModelClass:
def badstatic(foo):
pass
class SelfInArgsClass:
def self_as_argument(this, self):
pass
@@ -110,7 +110,6 @@ class SelfInArgsClass:
def self_as_kwargs(this, **self):
pass
class RenamingInMethodBodyClass:
def bad_method(this):
this = this
@@ -118,8 +117,3 @@ class RenamingInMethodBodyClass:
def bad_method(this):
self = this
class RenamingWithNFKC:
def formula(household):
hºusehold(1)

View File

@@ -72,18 +72,3 @@ def f():
result = Foo()
for i in items:
result.append(i) # Ok
def f():
items = [1, 2, 3, 4]
result = []
async for i in items:
if i % 2:
result.append(i) # PERF401
def f():
items = [1, 2, 3, 4]
result = []
async for i in items:
result.append(i) # PERF401

View File

@@ -43,10 +43,3 @@ def f():
for path in ("foo", "bar"):
sys.path.append(path) # OK
def f():
items = [1, 2, 3, 4]
result = []
async for i in items:
result.append(i) # PERF402

View File

@@ -1,36 +0,0 @@
"""__init__.py without __all__
Unused stdlib and third party imports are unsafe removals
Unused first party imports get changed to redundant aliases
"""
# stdlib
import os # Ok: is used
_ = os
import argparse as argparse # Ok: is redundant alias
import sys # F401: remove unused
# first-party
from . import used # Ok: is used
_ = used
from . import aliased as aliased # Ok: is redundant alias
from . import unused # F401: change to redundant alias
from . import renamed as bees # F401: no fix

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1,42 +0,0 @@
"""__init__.py with __all__
Unused stdlib and third party imports are unsafe removals
Unused first party imports get added to __all__
"""
# stdlib
import os # Ok: is used
_ = os
import argparse # Ok: is exported in __all__
import sys # F401: remove unused
# first-party
from . import used # Ok: is used
_ = used
from . import aliased as aliased # Ok: is redundant alias
from . import exported # Ok: is exported in __all__
# from . import unused # F401: add to __all__
# from . import renamed as bees # F401: add to __all__
__all__ = ["argparse", "exported"]

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1 +0,0 @@
# empty module imported by __init__.py for test fixture

View File

@@ -1,6 +1,6 @@
"""Tests for constructs allowed in `.pyi` stub files but not at runtime"""
from typing import Optional, TypeAlias, Union, TYPE_CHECKING
from typing import Optional, TypeAlias, Union
__version__: str
__author__: str
@@ -42,7 +42,3 @@ class MyClass:
baz: MyClass
eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file
eggs = "baz" # always okay
if TYPE_CHECKING:
class Tree3(list[Tree3 | Leaf]): ... # Always okay if it's in a `TYPE_CHECKING` block
Recursive: TypeAlias = Recursive | None # Always okay if it's in a `TYPE_CHECKING` block

View File

@@ -1,7 +1,7 @@
"""Tests for forward references (some allowed, some still not) in the context of `from __future__ import annotations`"""
"""Tests for constructs allowed when `__future__` annotations are enabled but not otherwise"""
from __future__ import annotations
from typing import Optional, TypeAlias, Union, TYPE_CHECKING
from typing import Optional, TypeAlias, Union
__version__: str
__author__: str
@@ -45,8 +45,4 @@ class D: ...
# More circular references
class Leaf: ...
class Tree(list[Tree | Leaf]): ... # Still invalid even when `__future__.annotations` are enabled
class Tree2(list["Tree2 | Leaf"]): ... # always okay
if TYPE_CHECKING:
class Tree3(list[Tree3 | Leaf]): ... # Always okay if it's in a `TYPE_CHECKING` block
Recursive: TypeAlias = Recursive | None # Always okay if it's in a `TYPE_CHECKING` block
class Tree2(list["Tree | Leaf"]): ... # always okay

View File

@@ -28,13 +28,3 @@ class MyClassBase(metaclass=ABCMeta):
@abstractmethod
def example(self, value):
"""Setter."""
class VariadicParameters:
@property
def attribute_var_args(self, *args): # [property-with-parameters]
return sum(args)
@property
def attribute_var_kwargs(self, **kwargs): #[property-with-parameters]
return {key: value * 2 for key, value in kwargs.items()}

View File

@@ -73,10 +73,3 @@ def op_add4(x, y=1):
def op_add5(x, y):
print("op_add5")
return x + y
# OK
class Class:
@staticmethod
def add(x, y):
return x + y

View File

@@ -73,8 +73,3 @@ def foo():
async def test():
return [check async for check in async_func()]
async def test() -> str:
vals = [str(val) for val in await async_func(1)]
return ",".join(vals)

View File

@@ -109,7 +109,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
};
let definitions = std::mem::take(&mut checker.semantic.definitions);
let mut overloaded_name: Option<&str> = None;
let mut overloaded_name: Option<String> = None;
for ContextualizedDefinition {
definition,
visibility,
@@ -127,7 +127,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
if !overloaded_name.is_some_and(|overloaded_name| {
flake8_annotations::helpers::is_overload_impl(
definition,
overloaded_name,
&overloaded_name,
&checker.semantic,
)
}) {

View File

@@ -32,8 +32,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if let Some(operator) = typing::to_pep604_operator(value, slice, &checker.semantic)
{
if checker.enabled(Rule::FutureRewritableTypeAnnotation) {
if !checker.semantic.in_typing_only_context()
&& !checker.semantic.future_annotations()
if !checker.semantic.future_annotations_or_stub()
&& checker.settings.target_version < PythonVersion::Py310
&& checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.in_annotation()
@@ -45,10 +44,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
if checker.enabled(Rule::NonPEP604Annotation) {
if checker.semantic.in_typing_only_context()
if checker.source_type.is_stub()
|| checker.settings.target_version >= PythonVersion::Py310
|| (checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.future_annotations()
&& checker.semantic.future_annotations_or_stub()
&& checker.semantic.in_annotation()
&& !checker.settings.pyupgrade.keep_runtime_typing)
{
@@ -60,8 +59,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
// Ex) list[...]
if checker.enabled(Rule::FutureRequiredTypeAnnotation) {
if !checker.semantic.in_typing_only_context()
&& !checker.semantic.future_annotations()
if !checker.semantic.future_annotations_or_stub()
&& checker.settings.target_version < PythonVersion::Py39
&& checker.semantic.in_annotation()
&& typing::is_pep585_generic(value, &checker.semantic)
@@ -80,6 +78,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::DuplicateUnionMember,
Rule::RedundantLiteralUnion,
Rule::UnnecessaryTypeUnion,
Rule::NeverUnion,
]) {
// Avoid duplicate checks if the parent is a union, since these rules already
// traverse nested unions.
@@ -191,8 +190,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
typing::to_pep585_generic(expr, &checker.semantic)
{
if checker.enabled(Rule::FutureRewritableTypeAnnotation) {
if !checker.semantic.in_typing_only_context()
&& !checker.semantic.future_annotations()
if !checker.semantic.future_annotations_or_stub()
&& checker.settings.target_version < PythonVersion::Py39
&& checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.in_annotation()
@@ -202,10 +200,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
if checker.enabled(Rule::NonPEP585Annotation) {
if checker.semantic.in_typing_only_context()
if checker.source_type.is_stub()
|| checker.settings.target_version >= PythonVersion::Py39
|| (checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.future_annotations()
&& checker.semantic.future_annotations_or_stub()
&& checker.semantic.in_annotation()
&& !checker.settings.pyupgrade.keep_runtime_typing)
{
@@ -275,8 +273,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
]) {
if let Some(replacement) = typing::to_pep585_generic(expr, &checker.semantic) {
if checker.enabled(Rule::FutureRewritableTypeAnnotation) {
if !checker.semantic.in_typing_only_context()
&& !checker.semantic.future_annotations()
if !checker.semantic.future_annotations_or_stub()
&& checker.settings.target_version < PythonVersion::Py39
&& checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.in_annotation()
@@ -288,10 +285,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
if checker.enabled(Rule::NonPEP585Annotation) {
if checker.semantic.in_typing_only_context()
if checker.source_type.is_stub()
|| checker.settings.target_version >= PythonVersion::Py39
|| (checker.settings.target_version >= PythonVersion::Py37
&& checker.semantic.future_annotations()
&& checker.semantic.future_annotations_or_stub()
&& checker.semantic.in_annotation()
&& !checker.settings.pyupgrade.keep_runtime_typing)
{
@@ -1181,8 +1178,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}) => {
// Ex) `str | None`
if checker.enabled(Rule::FutureRequiredTypeAnnotation) {
if !checker.semantic.in_typing_only_context()
&& !checker.semantic.future_annotations()
if !checker.semantic.future_annotations_or_stub()
&& checker.settings.target_version < PythonVersion::Py310
&& checker.semantic.in_annotation()
{

View File

@@ -877,7 +877,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) {
checker.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithNestedImportStarUsage {
name: helpers::format_import_from(level, module).to_string(),
name: helpers::format_import_from(level, module),
},
stmt.range(),
));
@@ -886,7 +886,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::UndefinedLocalWithImportStar) {
checker.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithImportStar {
name: helpers::format_import_from(level, module).to_string(),
name: helpers::format_import_from(level, module),
},
stmt.range(),
));
@@ -1323,10 +1323,10 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::dict_iter_missing_items(checker, target, iter);
}
if checker.enabled(Rule::ManualListComprehension) {
perflint::rules::manual_list_comprehension(checker, for_stmt);
perflint::rules::manual_list_comprehension(checker, target, body);
}
if checker.enabled(Rule::ManualListCopy) {
perflint::rules::manual_list_copy(checker, for_stmt);
perflint::rules::manual_list_copy(checker, target, body);
}
if checker.enabled(Rule::ManualDictComprehension) {
perflint::rules::manual_dict_comprehension(checker, target, body);

View File

@@ -56,11 +56,10 @@ impl AnnotationContext {
_ => {}
}
// If `__future__` annotations are enabled or it's a stub file
// or we're in an `if TYPE_CHECKING` block,
// If `__future__` annotations are enabled or it's a stub file,
// then annotations are never evaluated at runtime,
// so we can treat them as typing-only.
if semantic.in_typing_only_context() || semantic.future_annotations() {
if semantic.future_annotations_or_stub() {
return Self::TypingOnly;
}
@@ -89,7 +88,7 @@ impl AnnotationContext {
semantic,
) {
Self::RuntimeRequired
} else if semantic.in_typing_only_context() || semantic.future_annotations() {
} else if semantic.future_annotations_or_stub() {
Self::TypingOnly
} else {
Self::RuntimeEvaluated

View File

@@ -893,7 +893,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
// Step 0: Pre-processing
if self.semantic.in_typing_only_context()
if self.source_type.is_stub()
&& self.semantic.in_class_base()
&& !self.semantic.in_deferred_class_base()
{
@@ -903,16 +903,15 @@ impl<'a> Visitor<'a> for Checker<'a> {
return;
}
// `in_deferred_type_definition()` will only be `true` if we're now visiting the deferred nodes
// after having already traversed the source tree once. If we're now visiting the deferred nodes,
// we can't defer again, or we'll infinitely recurse!
let defer_probable_type_definition = self.semantic.in_type_definition()
if !self.semantic.in_typing_literal()
// `in_deferred_type_definition()` will only be `true` if we're now visiting the deferred nodes
// after having already traversed the source tree once. If we're now visiting the deferred nodes,
// we can't defer again, or we'll infinitely recurse!
&& !self.semantic.in_deferred_type_definition()
&& !self.semantic.in_typing_literal()
&& (self.semantic.in_typing_only_context()
|| (self.semantic.future_annotations() && self.semantic.in_annotation()));
if defer_probable_type_definition {
&& self.semantic.in_type_definition()
&& self.semantic.future_annotations_or_stub()
&& (self.semantic.in_annotation() || self.source_type.is_stub())
{
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
self.visit.string_type_definitions.push((
expr.range(),
@@ -1954,10 +1953,9 @@ impl<'a> Checker<'a> {
/// After initial traversal of the AST, visit all class bases that were deferred.
///
/// This method should only be relevant in stub files or `TYPE_CHECKING` blocks,
/// where forward references are legal in class bases. In other contexts,
/// using a forward reference in a class base is never legal,
/// so `self.visit.class_bases` should always be empty.
/// This method should only be relevant in stub files, where forward references are
/// legal in class bases. For other kinds of Python files, using a forward reference
/// in a class base is never legal, so `self.visit.class_bases` should always be empty.
///
/// For example, in a stub file:
/// ```python
@@ -1967,9 +1965,12 @@ impl<'a> Checker<'a> {
fn visit_deferred_class_bases(&mut self) {
let snapshot = self.semantic.snapshot();
let deferred_bases = std::mem::take(&mut self.visit.class_bases);
debug_assert!(
self.source_type.is_stub() || deferred_bases.is_empty(),
"Class bases should never be deferred outside of stub files"
);
for (expr, snapshot) in deferred_bases {
self.semantic.restore(snapshot);
debug_assert!(self.semantic.in_typing_only_context());
// Set this flag to avoid infinite recursion, or we'll just defer it again:
self.semantic.flags |= SemanticModelFlags::DEFERRED_CLASS_BASE;
self.visit_expr(expr);
@@ -1980,9 +1981,8 @@ impl<'a> Checker<'a> {
/// After initial traversal of the AST, visit all "future type definitions".
///
/// A "future type definition" is a type definition where [PEP 563] semantics
/// apply (i.e., a definition in a module that has `from __future__ import annotations`
/// at the top of the file, a definition in a `TYPE_CHECKING` block,
/// or an annotation in a stub file). These type definitions
/// apply (i.e., an annotation in a module that has `from __future__ import annotations`
/// at the top of the file, or an annotation in a stub file). These type definitions
/// support forward references, so they are deferred on initial traversal
/// of the source tree.
///
@@ -2003,6 +2003,15 @@ impl<'a> Checker<'a> {
let type_definitions = std::mem::take(&mut self.visit.future_type_definitions);
for (expr, snapshot) in type_definitions {
self.semantic.restore(snapshot);
// Type definitions should only be considered "`__future__` type definitions"
// if they are annotations in a module where `from __future__ import
// annotations` is active, or they are type definitions in a stub file.
debug_assert!(
self.semantic.future_annotations_or_stub()
&& (self.source_type.is_stub() || self.semantic.in_annotation())
);
self.semantic.flags |= SemanticModelFlags::TYPE_DEFINITION
| SemanticModelFlags::FUTURE_TYPE_DEFINITION;
self.visit_expr(expr);
@@ -2062,10 +2071,7 @@ impl<'a> Checker<'a> {
self.semantic.restore(snapshot);
if self.semantic.in_annotation()
&& (self.semantic.in_typing_only_context()
|| self.semantic.future_annotations())
{
if self.semantic.in_annotation() && self.semantic.future_annotations_or_stub() {
if self.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, value, range);
}

View File

@@ -1,13 +1,17 @@
//! Lint rules based on import analysis.
use std::borrow::Cow;
use std::path::Path;
use ruff_diagnostics::Diagnostic;
use ruff_notebook::CellOffsets;
use ruff_python_ast::helpers::to_module_path;
use ruff_python_ast::imports::{ImportMap, ModuleImport};
use ruff_python_ast::statement_visitor::StatementVisitor;
use ruff_python_ast::{PySourceType, Suite};
use ruff_python_ast::{self as ast, PySourceType, Stmt, Suite};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
use crate::directives::IsortDirectives;
use crate::registry::Rule;
@@ -15,6 +19,57 @@ use crate::rules::isort;
use crate::rules::isort::block::{Block, BlockBuilder};
use crate::settings::LinterSettings;
fn extract_import_map(path: &Path, package: Option<&Path>, blocks: &[&Block]) -> Option<ImportMap> {
let module_path = to_module_path(package?, path)?;
let num_imports = blocks.iter().map(|block| block.imports.len()).sum();
let mut module_imports = Vec::with_capacity(num_imports);
for stmt in blocks.iter().flat_map(|block| &block.imports) {
match stmt {
Stmt::Import(ast::StmtImport { names, range: _ }) => {
module_imports.extend(
names
.iter()
.map(|name| ModuleImport::new(name.name.to_string(), stmt.range())),
);
}
Stmt::ImportFrom(ast::StmtImportFrom {
module,
names,
level,
range: _,
}) => {
let level = *level as usize;
let module = if let Some(module) = module {
let module: &String = module.as_ref();
if level == 0 {
Cow::Borrowed(module)
} else {
if module_path.len() <= level {
continue;
}
let prefix = module_path[..module_path.len() - level].join(".");
Cow::Owned(format!("{prefix}.{module}"))
}
} else {
if module_path.len() <= level {
continue;
}
Cow::Owned(module_path[..module_path.len() - level].join("."))
};
module_imports.extend(names.iter().map(|name| {
ModuleImport::new(format!("{}.{}", module, name.name), name.range())
}));
}
_ => panic!("Expected Stmt::Import | Stmt::ImportFrom"),
}
}
let mut import_map = ImportMap::default();
import_map.insert(module_path.join("."), module_imports);
Some(import_map)
}
#[allow(clippy::too_many_arguments)]
pub(crate) fn check_imports(
python_ast: &Suite,
@@ -23,10 +78,11 @@ pub(crate) fn check_imports(
directives: &IsortDirectives,
settings: &LinterSettings,
stylist: &Stylist,
path: &Path,
package: Option<&Path>,
source_type: PySourceType,
cell_offsets: Option<&CellOffsets>,
) -> Vec<Diagnostic> {
) -> (Vec<Diagnostic>, Option<ImportMap>) {
// Extract all import blocks from the AST.
let tracker = {
let mut tracker =
@@ -66,5 +122,8 @@ pub(crate) fn check_imports(
));
}
diagnostics
// Extract import map.
let imports = extract_import_map(path, package, &blocks);
(diagnostics, imports)
}

View File

@@ -971,32 +971,32 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Preview, rules::ruff::rules::RedirectedNOQA),
(Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "900") => (RuleGroup::Stable, rules::ruff::rules::StableTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "901") => (RuleGroup::Stable, rules::ruff::rules::StableTestRuleSafeFix),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "902") => (RuleGroup::Stable, rules::ruff::rules::StableTestRuleUnsafeFix),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "903") => (RuleGroup::Stable, rules::ruff::rules::StableTestRuleDisplayOnlyFix),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "911") => (RuleGroup::Preview, rules::ruff::rules::PreviewTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
#[allow(deprecated)]
(Ruff, "912") => (RuleGroup::Nursery, rules::ruff::rules::NurseryTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "920") => (RuleGroup::Deprecated, rules::ruff::rules::DeprecatedTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "921") => (RuleGroup::Deprecated, rules::ruff::rules::AnotherDeprecatedTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "930") => (RuleGroup::Removed, rules::ruff::rules::RemovedTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "931") => (RuleGroup::Removed, rules::ruff::rules::AnotherRemovedTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "940") => (RuleGroup::Removed, rules::ruff::rules::RedirectedFromTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "950") => (RuleGroup::Stable, rules::ruff::rules::RedirectedToTestRule),
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
(Ruff, "960") => (RuleGroup::Removed, rules::ruff::rules::RedirectedFromPrefixTestRule),

View File

@@ -273,7 +273,7 @@ pub(crate) struct TodoComment<'a> {
pub(crate) directive: TodoDirective<'a>,
/// The comment's actual [`TextRange`].
pub(crate) range: TextRange,
/// The comment range's position in [`Indexer::comment_ranges`]
/// The comment range's position in [`Indexer`].comment_ranges()
pub(crate) range_index: usize,
}

View File

@@ -122,28 +122,6 @@ pub(crate) fn remove_unused_imports<'a>(
}
}
/// Edits to make the specified imports explicit, e.g. change `import x` to `import x as x`.
pub(crate) fn make_redundant_alias<'a>(
member_names: impl Iterator<Item = &'a str>,
stmt: &Stmt,
) -> Vec<Edit> {
let aliases = match stmt {
Stmt::Import(ast::StmtImport { names, .. }) => names,
Stmt::ImportFrom(ast::StmtImportFrom { names, .. }) => names,
_ => {
return Vec::new();
}
};
member_names
.filter_map(|name| {
aliases
.iter()
.find(|alias| alias.asname.is_none() && name == alias.name.id)
.map(|alias| Edit::range_replacement(format!("{name} as {name}"), alias.range))
})
.collect()
}
#[derive(Debug, Copy, Clone)]
pub(crate) enum Parentheses {
/// Remove parentheses, if the removed argument is the only argument left.
@@ -479,12 +457,11 @@ fn all_lines_fit(
mod tests {
use anyhow::Result;
use ruff_diagnostics::Edit;
use ruff_python_parser::parse_suite;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_text_size::{Ranged, TextSize};
use crate::fix::edits::{make_redundant_alias, next_stmt_break, trailing_semicolon};
use crate::fix::edits::{next_stmt_break, trailing_semicolon};
#[test]
fn find_semicolon() -> Result<()> {
@@ -555,35 +532,4 @@ x = 1 \
TextSize::from(12)
);
}
#[test]
fn redundant_alias() {
let contents = "import x, y as y, z as bees";
let program = parse_suite(contents).unwrap();
let stmt = program.first().unwrap();
assert_eq!(
make_redundant_alias(["x"].into_iter(), stmt),
vec![Edit::range_replacement(
String::from("x as x"),
TextRange::new(TextSize::new(7), TextSize::new(8)),
)],
"make just one item redundant"
);
assert_eq!(
make_redundant_alias(vec!["x", "y"].into_iter(), stmt),
vec![Edit::range_replacement(
String::from("x as x"),
TextRange::new(TextSize::new(7), TextSize::new(8)),
)],
"the second item is already a redundant alias"
);
assert_eq!(
make_redundant_alias(vec!["x", "z"].into_iter(), stmt),
vec![Edit::range_replacement(
String::from("x as x"),
TextRange::new(TextSize::new(7), TextSize::new(8)),
)],
"the third item is already aliased to something else"
);
}
}

View File

@@ -56,7 +56,7 @@ impl CacheKey for LineLength {
pub enum ParseLineWidthError {
/// The string could not be parsed as a valid [u16]
ParseError(ParseIntError),
/// The [u16] value of the string is not a valid [`LineLength`]
/// The [u16] value of the string is not a valid [LineLength]
TryFromIntError(LineLengthFromIntError),
}

View File

@@ -10,6 +10,7 @@ use rustc_hash::FxHashMap;
use ruff_diagnostics::Diagnostic;
use ruff_notebook::Notebook;
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::{PySourceType, Suite};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
@@ -32,7 +33,7 @@ use crate::message::Message;
use crate::noqa::add_noqa;
use crate::registry::{AsRule, Rule, RuleSet};
use crate::rules::pycodestyle;
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
use crate::rules::ruff::rules::test_rules::{self, TestRule, TEST_RULES};
use crate::settings::types::UnsafeFixes;
use crate::settings::{flags, LinterSettings};
@@ -61,7 +62,7 @@ pub type FixTable = FxHashMap<Rule, usize>;
pub struct FixerResult<'a> {
/// The result returned by the linter, after applying any fixes.
pub result: LinterResult<Vec<Message>>,
pub result: LinterResult<(Vec<Message>, Option<ImportMap>)>,
/// The resulting source code, after applying any fixes.
pub transformed: Cow<'a, SourceKind>,
/// The number of fixes applied for each [`Rule`].
@@ -83,9 +84,10 @@ pub fn check_path(
source_kind: &SourceKind,
source_type: PySourceType,
tokens: TokenSource,
) -> LinterResult<Vec<Diagnostic>> {
) -> LinterResult<(Vec<Diagnostic>, Option<ImportMap>)> {
// Aggregate all diagnostics.
let mut diagnostics = vec![];
let mut imports = None;
let mut error = None;
// Collect doc lines. This requires a rare mix of tokens (for comments) and AST
@@ -167,18 +169,19 @@ pub fn check_path(
));
}
if use_imports {
let import_diagnostics = check_imports(
let (import_diagnostics, module_imports) = check_imports(
&python_ast,
locator,
indexer,
&directives.isort,
settings,
stylist,
path,
package,
source_type,
cell_offsets,
);
imports = module_imports;
diagnostics.extend(import_diagnostics);
}
if use_doc_lines {
@@ -215,7 +218,7 @@ pub fn check_path(
}
// Raise violations for internal test rules
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
{
for test_rule in TEST_RULES {
if !settings.rules.enabled(*test_rule) {
@@ -337,7 +340,7 @@ pub fn check_path(
}
}
LinterResult::new(diagnostics, error)
LinterResult::new((diagnostics, imports), error)
}
const MAX_ITERATIONS: usize = 100;
@@ -407,7 +410,7 @@ pub fn add_noqa_to_path(
// TODO(dhruvmanila): Add support for Jupyter Notebooks
add_noqa(
path,
&diagnostics,
&diagnostics.0,
&locator,
indexer.comment_ranges(),
&settings.external,
@@ -426,7 +429,7 @@ pub fn lint_only(
source_kind: &SourceKind,
source_type: PySourceType,
data: ParseSource,
) -> LinterResult<Vec<Message>> {
) -> LinterResult<(Vec<Message>, Option<ImportMap>)> {
// Tokenize once.
let tokens = data.into_token_source(source_kind, source_type);
@@ -462,7 +465,12 @@ pub fn lint_only(
tokens,
);
result.map(|diagnostics| diagnostics_to_messages(diagnostics, path, &locator, &directives))
result.map(|(diagnostics, imports)| {
(
diagnostics_to_messages(diagnostics, path, &locator, &directives),
imports,
)
})
}
/// Convert from diagnostics to messages.
@@ -575,7 +583,7 @@ pub fn lint_fix<'a>(
code: fixed_contents,
fixes: applied,
source_map,
}) = fix_file(&result.data, &locator, unsafe_fixes)
}) = fix_file(&result.data.0, &locator, unsafe_fixes)
{
if iterations < MAX_ITERATIONS {
// Count the number of fixed errors.
@@ -592,12 +600,15 @@ pub fn lint_fix<'a>(
continue;
}
report_failed_to_converge_error(path, transformed.source_code(), &result.data);
report_failed_to_converge_error(path, transformed.source_code(), &result.data.0);
}
return Ok(FixerResult {
result: result.map(|diagnostics| {
diagnostics_to_messages(diagnostics, path, &locator, &directives)
result: result.map(|(diagnostics, imports)| {
(
diagnostics_to_messages(diagnostics, path, &locator, &directives),
imports,
)
}),
transformed,
fixed,

View File

@@ -207,15 +207,6 @@ impl RuleSet {
*self = set.union(&RuleSet::from_rule(rule));
}
#[inline]
pub fn set(&mut self, rule: Rule, enabled: bool) {
if enabled {
self.insert(rule);
} else {
self.remove(rule);
}
}
/// Removes `rule` from the set.
///
/// ## Examples

View File

@@ -6,7 +6,7 @@ use itertools::Itertools;
use ruff_diagnostics::Edit;
use ruff_python_codegen::Stylist;
use ruff_python_semantic::{Binding, BindingKind, Scope, ScopeId, SemanticModel};
use ruff_text_size::Ranged;
use ruff_text_size::{Ranged, TextSize};
pub(crate) struct Renamer;
@@ -215,6 +215,7 @@ impl Renamer {
let quote = stylist.quote();
format!("{quote}{target}{quote}")
} else {
debug_assert_eq!(TextSize::of(name), reference.range().len());
target.to_string()
}
};

View File

@@ -104,10 +104,10 @@ static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = Lazy::new(|| {
("PGH001", "S307"),
("PGH002", "G010"),
// Test redirect by exact code
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
("RUF940", "RUF950"),
// Test redirect by prefix
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
("RUF96", "RUF95"),
// See: https://github.com/astral-sh/ruff/issues/10791
("PLW0117", "PLW0177"),

View File

@@ -323,7 +323,7 @@ mod schema {
})
.filter(|_rule| {
// Filter out all test-only rules
#[cfg(any(feature = "test-rules", test))]
#[cfg(feature = "test-rules")]
#[allow(clippy::used_underscore_binding)]
if _rule.starts_with("RUF9") {
return false;

View File

@@ -17,13 +17,10 @@ use crate::importer::{ImportRequest, Importer};
use crate::settings::types::PythonVersion;
/// Return the name of the function, if it's overloaded.
pub(crate) fn overloaded_name<'a>(
definition: &'a Definition,
semantic: &SemanticModel,
) -> Option<&'a str> {
pub(crate) fn overloaded_name(definition: &Definition, semantic: &SemanticModel) -> Option<String> {
let function = definition.as_function_def()?;
if visibility::is_overload(&function.decorator_list, semantic) {
Some(function.name.as_str())
Some(function.name.to_string())
} else {
None
}

View File

@@ -56,7 +56,6 @@ impl Violation for AbstractBaseClassWithoutAbstractMethod {
format!("`{name}` is an abstract base class, but it has no abstract methods")
}
}
/// ## What it does
/// Checks for empty methods in abstract base classes without an abstract
/// decorator.
@@ -157,13 +156,8 @@ pub(crate) fn abstract_base_class(
let mut has_abstract_method = false;
for stmt in body {
// https://github.com/PyCQA/flake8-bugbear/issues/293
// If an ABC declares an attribute by providing a type annotation
// but does not actually assign a value for that attribute,
// assume it is intended to be an "abstract attribute"
if matches!(
stmt,
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
) {
// Ignore abc's that declares a class attribute that must be set
if let Stmt::AnnAssign(_) | Stmt::Assign(_) = stmt {
has_abstract_method = true;
continue;
}

View File

@@ -41,20 +41,6 @@ B024.py:92:7: B024 `notabc_Base_1` is an abstract base class, but it has no abst
94 | foo()
|
B024.py:132:7: B024 `abc_set_class_variable_2` is an abstract base class, but it has no abstract methods
|
132 | class abc_set_class_variable_2(ABC): # error (not an abstract attribute)
| ^^^^^^^^^^^^^^^^^^^^^^^^ B024
133 | foo = 2
|
B024.py:136:7: B024 `abc_set_class_variable_3` is an abstract base class, but it has no abstract methods
|
136 | class abc_set_class_variable_3(ABC): # error (not an abstract attribute)
| ^^^^^^^^^^^^^^^^^^^^^^^^ B024
137 | foo: int = 2
|
B024.py:141:7: B024 `abc_set_class_variable_4` is an abstract base class, but it has no abstract methods
|
140 | # this doesn't actually declare a class variable, it's just an expression
@@ -62,3 +48,5 @@ B024.py:141:7: B024 `abc_set_class_variable_4` is an abstract base class, but it
| ^^^^^^^^^^^^^^^^^^^^^^^^ B024
142 | foo
|

View File

@@ -531,7 +531,7 @@ pub(crate) fn fix_unnecessary_double_cast_or_process(
.find(|argument| argument.keyword.is_none())
{
let mut arg = arg.clone();
arg.comma.clone_from(&first.comma);
arg.comma = first.comma.clone();
arg.whitespace_after_arg = first.whitespace_after_arg.clone();
iter::once(arg)
.chain(rest.iter().cloned())

View File

@@ -8,8 +8,8 @@ use ruff_text_size::Ranged;
/// Checks for incorrect import of pytest.
///
/// ## Why is this bad?
/// For consistency, `pytest` should be imported as `import pytest` and its members should be
/// accessed in the form of `pytest.xxx.yyy` for consistency
/// `pytest` should be imported as `import pytest` and its members should be accessed in the form of
/// `pytest.xxx.yyy` for consistency and to make it easier for linting tools to analyze the code.
///
/// ## Example
/// ```python
@@ -27,7 +27,7 @@ pub struct PytestIncorrectPytestImport;
impl Violation for PytestIncorrectPytestImport {
#[derive_message_formats]
fn message(&self) -> String {
format!("Incorrect import of `pytest`; use `import pytest` instead")
format!("Found incorrect import of pytest, use simple `import pytest` instead")
}
}

View File

@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
---
PT013.py:11:1: PT013 Incorrect import of `pytest`; use `import pytest` instead
PT013.py:11:1: PT013 Found incorrect import of pytest, use simple `import pytest` instead
|
9 | # Error
10 |
@@ -11,7 +11,7 @@ PT013.py:11:1: PT013 Incorrect import of `pytest`; use `import pytest` instead
13 | from pytest import fixture as other_name
|
PT013.py:12:1: PT013 Incorrect import of `pytest`; use `import pytest` instead
PT013.py:12:1: PT013 Found incorrect import of pytest, use simple `import pytest` instead
|
11 | import pytest as other_name
12 | from pytest import fixture
@@ -19,10 +19,12 @@ PT013.py:12:1: PT013 Incorrect import of `pytest`; use `import pytest` instead
13 | from pytest import fixture as other_name
|
PT013.py:13:1: PT013 Incorrect import of `pytest`; use `import pytest` instead
PT013.py:13:1: PT013 Found incorrect import of pytest, use simple `import pytest` instead
|
11 | import pytest as other_name
12 | from pytest import fixture
13 | from pytest import fixture as other_name
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT013
|

View File

@@ -67,8 +67,8 @@ pub(crate) fn fix_multiple_with_statements(
outer_with.items.append(&mut inner_with.items);
if outer_with.lpar.is_none() {
outer_with.lpar.clone_from(&inner_with.lpar);
outer_with.rpar.clone_from(&inner_with.rpar);
outer_with.lpar = inner_with.lpar.clone();
outer_with.rpar = inner_with.rpar.clone();
}
outer_with.body = inner_with.body.clone();

View File

@@ -322,7 +322,7 @@ pub(crate) fn unused_arguments(
return;
}
let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else {
let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
return;
};

View File

@@ -29,38 +29,29 @@ pub(crate) struct CommentSet<'a> {
pub(crate) inline: Vec<Cow<'a, str>>,
}
pub(crate) trait Importable<'a> {
fn module_name(&self) -> Cow<'a, str>;
pub(crate) trait Importable {
fn module_name(&self) -> String;
fn module_base(&self) -> String;
}
fn module_base(&self) -> Cow<'a, str> {
match self.module_name() {
Cow::Borrowed(module_name) => Cow::Borrowed(
module_name
.split('.')
.next()
.expect("module to include at least one segment"),
),
Cow::Owned(module_name) => Cow::Owned(
module_name
.split('.')
.next()
.expect("module to include at least one segment")
.to_owned(),
),
}
impl Importable for AliasData<'_> {
fn module_name(&self) -> String {
self.name.to_string()
}
fn module_base(&self) -> String {
self.module_name().split('.').next().unwrap().to_string()
}
}
impl<'a> Importable<'a> for AliasData<'a> {
fn module_name(&self) -> Cow<'a, str> {
Cow::Borrowed(self.name)
}
}
impl<'a> Importable<'a> for ImportFromData<'a> {
fn module_name(&self) -> Cow<'a, str> {
impl Importable for ImportFromData<'_> {
fn module_name(&self) -> String {
format_import_from(self.level, self.module)
}
fn module_base(&self) -> String {
self.module_name().split('.').next().unwrap().to_string()
}
}
#[derive(Debug, Default)]

View File

@@ -190,7 +190,7 @@ pub(crate) fn invalid_first_argument_name(
panic!("Expected ScopeKind::Function")
};
let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else {
let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
return;
};

View File

@@ -20,159 +20,160 @@ N805.py:7:20: N805 [*] First argument of a method should be named `self`
9 9 |
10 10 | if False:
N805.py:11:30: N805 [*] First argument of a method should be named `self`
N805.py:12:30: N805 [*] First argument of a method should be named `self`
|
10 | if False:
11 | def extra_bad_method(this):
11 |
12 | def extra_bad_method(this):
| ^^^^ N805
12 | pass
13 | pass
|
= help: Rename `this` to `self`
Unsafe fix
8 8 | pass
9 9 |
10 10 | if False:
11 |- def extra_bad_method(this):
11 |+ def extra_bad_method(self):
12 12 | pass
13 13 |
14 14 | def good_method(self):
11 11 |
12 |- def extra_bad_method(this):
12 |+ def extra_bad_method(self):
13 13 | pass
14 14 |
15 15 | def good_method(self):
N805.py:30:15: N805 [*] First argument of a method should be named `self`
N805.py:31:15: N805 [*] First argument of a method should be named `self`
|
29 | @pydantic.validator
30 | def lower(cls, my_field: str) -> str:
30 | @pydantic.validator
31 | def lower(cls, my_field: str) -> str:
| ^^^ N805
31 | pass
32 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
27 27 | return x
28 28 |
29 29 | @pydantic.validator
30 |- def lower(cls, my_field: str) -> str:
30 |+ def lower(self, my_field: str) -> str:
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")
28 28 | return x
29 29 |
30 30 | @pydantic.validator
31 |- def lower(cls, my_field: str) -> str:
31 |+ def lower(self, my_field: str) -> str:
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
N805.py:34:15: N805 [*] First argument of a method should be named `self`
N805.py:35:15: N805 [*] First argument of a method should be named `self`
|
33 | @pydantic.validator("my_field")
34 | def lower(cls, my_field: str) -> str:
34 | @pydantic.validator("my_field")
35 | def lower(cls, my_field: str) -> str:
| ^^^ N805
35 | pass
36 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")
34 |- def lower(cls, my_field: str) -> str:
34 |+ def lower(self, my_field: str) -> str:
35 35 | pass
36 36 |
37 37 | def __init__(self):
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
35 |- def lower(cls, my_field: str) -> str:
35 |+ def lower(self, my_field: str) -> str:
36 36 | pass
37 37 |
38 38 | def __init__(self):
N805.py:63:29: N805 [*] First argument of a method should be named `self`
N805.py:64:29: N805 [*] First argument of a method should be named `self`
|
61 | pass
62 |
63 | def bad_method_pos_only(this, blah, /, something: str):
62 | pass
63 |
64 | def bad_method_pos_only(this, blah, /, something: str):
| ^^^^ N805
64 | pass
65 | pass
|
= help: Rename `this` to `self`
Unsafe fix
60 60 | def good_method_pos_only(self, blah, /, something: str):
61 61 | pass
62 62 |
63 |- def bad_method_pos_only(this, blah, /, something: str):
63 |+ def bad_method_pos_only(self, blah, /, something: str):
64 64 | pass
65 65 |
61 61 | def good_method_pos_only(self, blah, /, something: str):
62 62 | pass
63 63 |
64 |- def bad_method_pos_only(this, blah, /, something: str):
64 |+ def bad_method_pos_only(self, blah, /, something: str):
65 65 | pass
66 66 |
67 67 |
N805.py:69:13: N805 [*] First argument of a method should be named `self`
N805.py:70:13: N805 [*] First argument of a method should be named `self`
|
67 | class ModelClass:
68 | @hybrid_property
69 | def bad(cls):
68 | class ModelClass:
69 | @hybrid_property
70 | def bad(cls):
| ^^^ N805
70 | pass
71 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
66 66 |
67 67 | class ModelClass:
68 68 | @hybrid_property
69 |- def bad(cls):
69 |+ def bad(self):
70 70 | pass
71 71 |
72 72 | @bad.expression
67 67 |
68 68 | class ModelClass:
69 69 | @hybrid_property
70 |- def bad(cls):
70 |+ def bad(self):
71 71 | pass
72 72 |
73 73 | @bad.expression
N805.py:77:13: N805 [*] First argument of a method should be named `self`
N805.py:78:13: N805 [*] First argument of a method should be named `self`
|
76 | @bad.wtf
77 | def bad(cls):
77 | @bad.wtf
78 | def bad(cls):
| ^^^ N805
78 | pass
79 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
74 74 | pass
75 75 |
76 76 | @bad.wtf
77 |- def bad(cls):
77 |+ def bad(self):
78 78 | pass
79 79 |
80 80 | @hybrid_property
75 75 | pass
76 76 |
77 77 | @bad.wtf
78 |- def bad(cls):
78 |+ def bad(self):
79 79 | pass
80 80 |
81 81 | @hybrid_property
N805.py:85:14: N805 [*] First argument of a method should be named `self`
N805.py:86:14: N805 [*] First argument of a method should be named `self`
|
84 | @good.expression
85 | def good(cls):
85 | @good.expression
86 | def good(cls):
| ^^^ N805
86 | pass
87 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
82 82 | pass
83 83 |
84 84 | @good.expression
85 |- def good(cls):
85 |+ def good(self):
86 86 | pass
87 87 |
88 88 | @good.wtf
83 83 | pass
84 84 |
85 85 | @good.expression
86 |- def good(cls):
86 |+ def good(self):
87 87 | pass
88 88 |
89 89 | @good.wtf
N805.py:93:19: N805 [*] First argument of a method should be named `self`
N805.py:94:19: N805 [*] First argument of a method should be named `self`
|
92 | @foobar.thisisstatic
93 | def badstatic(foo):
93 | @foobar.thisisstatic
94 | def badstatic(foo):
| ^^^ N805
94 | pass
95 | pass
|
= help: Rename `foo` to `self`
Unsafe fix
90 90 | pass
91 91 |
92 92 | @foobar.thisisstatic
93 |- def badstatic(foo):
93 |+ def badstatic(self):
94 94 | pass
95 95 |
91 91 | pass
92 92 |
93 93 | @foobar.thisisstatic
94 |- def badstatic(foo):
94 |+ def badstatic(self):
95 95 | pass
96 96 |
97 97 | class SelfInArgsClass:
N805.py:98:26: N805 First argument of a method should be named `self`
|
@@ -223,66 +224,45 @@ N805.py:110:24: N805 First argument of a method should be named `self`
|
= help: Rename `this` to `self`
N805.py:115:20: N805 [*] First argument of a method should be named `self`
N805.py:114:20: N805 [*] First argument of a method should be named `self`
|
114 | class RenamingInMethodBodyClass:
115 | def bad_method(this):
113 | class RenamingInMethodBodyClass:
114 | def bad_method(this):
| ^^^^ N805
116 | this = this
117 | this
115 | this = this
116 | this
|
= help: Rename `this` to `self`
Unsafe fix
111 111 | pass
112 112 |
113 113 |
114 114 | class RenamingInMethodBodyClass:
115 |- def bad_method(this):
116 |- this = this
117 |- this
115 |+ def bad_method(self):
116 |+ self = self
117 |+ self
118 118 |
119 119 | def bad_method(this):
120 120 | self = this
113 113 | class RenamingInMethodBodyClass:
114 |- def bad_method(this):
115 |- this = this
116 |- this
114 |+ def bad_method(self):
115 |+ self = self
116 |+ self
117 117 |
118 118 | def bad_method(this):
119 119 | self = this
N805.py:119:20: N805 [*] First argument of a method should be named `self`
N805.py:118:20: N805 [*] First argument of a method should be named `self`
|
117 | this
118 |
119 | def bad_method(this):
116 | this
117 |
118 | def bad_method(this):
| ^^^^ N805
120 | self = this
119 | self = this
|
= help: Rename `this` to `self`
Unsafe fix
116 116 | this = this
117 117 | this
118 118 |
119 |- def bad_method(this):
120 |- self = this
119 |+ def bad_method(self):
120 |+ self = self
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
N805.py:124:17: N805 [*] First argument of a method should be named `self`
|
123 | class RenamingWithNFKC:
124 | def formula(household):
| ^^^^^^^^^ N805
125 | hºusehold(1)
|
= help: Rename `household` to `self`
Unsafe fix
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
124 |- def formula(household):
125 |- hºusehold(1)
124 |+ def formula(self):
125 |+ self(1)
115 115 | this = this
116 116 | this
117 117 |
118 |- def bad_method(this):
119 |- self = this
118 |+ def bad_method(self):
119 |+ self = self

View File

@@ -20,102 +20,103 @@ N805.py:7:20: N805 [*] First argument of a method should be named `self`
9 9 |
10 10 | if False:
N805.py:11:30: N805 [*] First argument of a method should be named `self`
N805.py:12:30: N805 [*] First argument of a method should be named `self`
|
10 | if False:
11 | def extra_bad_method(this):
11 |
12 | def extra_bad_method(this):
| ^^^^ N805
12 | pass
13 | pass
|
= help: Rename `this` to `self`
Unsafe fix
8 8 | pass
9 9 |
10 10 | if False:
11 |- def extra_bad_method(this):
11 |+ def extra_bad_method(self):
12 12 | pass
13 13 |
14 14 | def good_method(self):
11 11 |
12 |- def extra_bad_method(this):
12 |+ def extra_bad_method(self):
13 13 | pass
14 14 |
15 15 | def good_method(self):
N805.py:63:29: N805 [*] First argument of a method should be named `self`
N805.py:64:29: N805 [*] First argument of a method should be named `self`
|
61 | pass
62 |
63 | def bad_method_pos_only(this, blah, /, something: str):
62 | pass
63 |
64 | def bad_method_pos_only(this, blah, /, something: str):
| ^^^^ N805
64 | pass
65 | pass
|
= help: Rename `this` to `self`
Unsafe fix
60 60 | def good_method_pos_only(self, blah, /, something: str):
61 61 | pass
62 62 |
63 |- def bad_method_pos_only(this, blah, /, something: str):
63 |+ def bad_method_pos_only(self, blah, /, something: str):
64 64 | pass
65 65 |
61 61 | def good_method_pos_only(self, blah, /, something: str):
62 62 | pass
63 63 |
64 |- def bad_method_pos_only(this, blah, /, something: str):
64 |+ def bad_method_pos_only(self, blah, /, something: str):
65 65 | pass
66 66 |
67 67 |
N805.py:69:13: N805 [*] First argument of a method should be named `self`
N805.py:70:13: N805 [*] First argument of a method should be named `self`
|
67 | class ModelClass:
68 | @hybrid_property
69 | def bad(cls):
68 | class ModelClass:
69 | @hybrid_property
70 | def bad(cls):
| ^^^ N805
70 | pass
71 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
66 66 |
67 67 | class ModelClass:
68 68 | @hybrid_property
69 |- def bad(cls):
69 |+ def bad(self):
70 70 | pass
71 71 |
72 72 | @bad.expression
67 67 |
68 68 | class ModelClass:
69 69 | @hybrid_property
70 |- def bad(cls):
70 |+ def bad(self):
71 71 | pass
72 72 |
73 73 | @bad.expression
N805.py:77:13: N805 [*] First argument of a method should be named `self`
N805.py:78:13: N805 [*] First argument of a method should be named `self`
|
76 | @bad.wtf
77 | def bad(cls):
77 | @bad.wtf
78 | def bad(cls):
| ^^^ N805
78 | pass
79 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
74 74 | pass
75 75 |
76 76 | @bad.wtf
77 |- def bad(cls):
77 |+ def bad(self):
78 78 | pass
79 79 |
80 80 | @hybrid_property
75 75 | pass
76 76 |
77 77 | @bad.wtf
78 |- def bad(cls):
78 |+ def bad(self):
79 79 | pass
80 80 |
81 81 | @hybrid_property
N805.py:93:19: N805 [*] First argument of a method should be named `self`
N805.py:94:19: N805 [*] First argument of a method should be named `self`
|
92 | @foobar.thisisstatic
93 | def badstatic(foo):
93 | @foobar.thisisstatic
94 | def badstatic(foo):
| ^^^ N805
94 | pass
95 | pass
|
= help: Rename `foo` to `self`
Unsafe fix
90 90 | pass
91 91 |
92 92 | @foobar.thisisstatic
93 |- def badstatic(foo):
93 |+ def badstatic(self):
94 94 | pass
95 95 |
91 91 | pass
92 92 |
93 93 | @foobar.thisisstatic
94 |- def badstatic(foo):
94 |+ def badstatic(self):
95 95 | pass
96 96 |
97 97 | class SelfInArgsClass:
N805.py:98:26: N805 First argument of a method should be named `self`
|
@@ -166,66 +167,45 @@ N805.py:110:24: N805 First argument of a method should be named `self`
|
= help: Rename `this` to `self`
N805.py:115:20: N805 [*] First argument of a method should be named `self`
N805.py:114:20: N805 [*] First argument of a method should be named `self`
|
114 | class RenamingInMethodBodyClass:
115 | def bad_method(this):
113 | class RenamingInMethodBodyClass:
114 | def bad_method(this):
| ^^^^ N805
116 | this = this
117 | this
115 | this = this
116 | this
|
= help: Rename `this` to `self`
Unsafe fix
111 111 | pass
112 112 |
113 113 |
114 114 | class RenamingInMethodBodyClass:
115 |- def bad_method(this):
116 |- this = this
117 |- this
115 |+ def bad_method(self):
116 |+ self = self
117 |+ self
118 118 |
119 119 | def bad_method(this):
120 120 | self = this
113 113 | class RenamingInMethodBodyClass:
114 |- def bad_method(this):
115 |- this = this
116 |- this
114 |+ def bad_method(self):
115 |+ self = self
116 |+ self
117 117 |
118 118 | def bad_method(this):
119 119 | self = this
N805.py:119:20: N805 [*] First argument of a method should be named `self`
N805.py:118:20: N805 [*] First argument of a method should be named `self`
|
117 | this
118 |
119 | def bad_method(this):
116 | this
117 |
118 | def bad_method(this):
| ^^^^ N805
120 | self = this
119 | self = this
|
= help: Rename `this` to `self`
Unsafe fix
116 116 | this = this
117 117 | this
118 118 |
119 |- def bad_method(this):
120 |- self = this
119 |+ def bad_method(self):
120 |+ self = self
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
N805.py:124:17: N805 [*] First argument of a method should be named `self`
|
123 | class RenamingWithNFKC:
124 | def formula(household):
| ^^^^^^^^^ N805
125 | hºusehold(1)
|
= help: Rename `household` to `self`
Unsafe fix
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
124 |- def formula(household):
125 |- hºusehold(1)
124 |+ def formula(self):
125 |+ self(1)
115 115 | this = this
116 116 | this
117 117 |
118 |- def bad_method(this):
119 |- self = this
118 |+ def bad_method(self):
119 |+ self = self

View File

@@ -20,140 +20,141 @@ N805.py:7:20: N805 [*] First argument of a method should be named `self`
9 9 |
10 10 | if False:
N805.py:11:30: N805 [*] First argument of a method should be named `self`
N805.py:12:30: N805 [*] First argument of a method should be named `self`
|
10 | if False:
11 | def extra_bad_method(this):
11 |
12 | def extra_bad_method(this):
| ^^^^ N805
12 | pass
13 | pass
|
= help: Rename `this` to `self`
Unsafe fix
8 8 | pass
9 9 |
10 10 | if False:
11 |- def extra_bad_method(this):
11 |+ def extra_bad_method(self):
12 12 | pass
13 13 |
14 14 | def good_method(self):
11 11 |
12 |- def extra_bad_method(this):
12 |+ def extra_bad_method(self):
13 13 | pass
14 14 |
15 15 | def good_method(self):
N805.py:30:15: N805 [*] First argument of a method should be named `self`
N805.py:31:15: N805 [*] First argument of a method should be named `self`
|
29 | @pydantic.validator
30 | def lower(cls, my_field: str) -> str:
30 | @pydantic.validator
31 | def lower(cls, my_field: str) -> str:
| ^^^ N805
31 | pass
32 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
27 27 | return x
28 28 |
29 29 | @pydantic.validator
30 |- def lower(cls, my_field: str) -> str:
30 |+ def lower(self, my_field: str) -> str:
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")
28 28 | return x
29 29 |
30 30 | @pydantic.validator
31 |- def lower(cls, my_field: str) -> str:
31 |+ def lower(self, my_field: str) -> str:
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
N805.py:34:15: N805 [*] First argument of a method should be named `self`
N805.py:35:15: N805 [*] First argument of a method should be named `self`
|
33 | @pydantic.validator("my_field")
34 | def lower(cls, my_field: str) -> str:
34 | @pydantic.validator("my_field")
35 | def lower(cls, my_field: str) -> str:
| ^^^ N805
35 | pass
36 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")
34 |- def lower(cls, my_field: str) -> str:
34 |+ def lower(self, my_field: str) -> str:
35 35 | pass
36 36 |
37 37 | def __init__(self):
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
35 |- def lower(cls, my_field: str) -> str:
35 |+ def lower(self, my_field: str) -> str:
36 36 | pass
37 37 |
38 38 | def __init__(self):
N805.py:63:29: N805 [*] First argument of a method should be named `self`
N805.py:64:29: N805 [*] First argument of a method should be named `self`
|
61 | pass
62 |
63 | def bad_method_pos_only(this, blah, /, something: str):
62 | pass
63 |
64 | def bad_method_pos_only(this, blah, /, something: str):
| ^^^^ N805
64 | pass
65 | pass
|
= help: Rename `this` to `self`
Unsafe fix
60 60 | def good_method_pos_only(self, blah, /, something: str):
61 61 | pass
62 62 |
63 |- def bad_method_pos_only(this, blah, /, something: str):
63 |+ def bad_method_pos_only(self, blah, /, something: str):
64 64 | pass
65 65 |
61 61 | def good_method_pos_only(self, blah, /, something: str):
62 62 | pass
63 63 |
64 |- def bad_method_pos_only(this, blah, /, something: str):
64 |+ def bad_method_pos_only(self, blah, /, something: str):
65 65 | pass
66 66 |
67 67 |
N805.py:69:13: N805 [*] First argument of a method should be named `self`
N805.py:70:13: N805 [*] First argument of a method should be named `self`
|
67 | class ModelClass:
68 | @hybrid_property
69 | def bad(cls):
68 | class ModelClass:
69 | @hybrid_property
70 | def bad(cls):
| ^^^ N805
70 | pass
71 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
66 66 |
67 67 | class ModelClass:
68 68 | @hybrid_property
69 |- def bad(cls):
69 |+ def bad(self):
70 70 | pass
71 71 |
72 72 | @bad.expression
67 67 |
68 68 | class ModelClass:
69 69 | @hybrid_property
70 |- def bad(cls):
70 |+ def bad(self):
71 71 | pass
72 72 |
73 73 | @bad.expression
N805.py:77:13: N805 [*] First argument of a method should be named `self`
N805.py:78:13: N805 [*] First argument of a method should be named `self`
|
76 | @bad.wtf
77 | def bad(cls):
77 | @bad.wtf
78 | def bad(cls):
| ^^^ N805
78 | pass
79 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
74 74 | pass
75 75 |
76 76 | @bad.wtf
77 |- def bad(cls):
77 |+ def bad(self):
78 78 | pass
79 79 |
80 80 | @hybrid_property
75 75 | pass
76 76 |
77 77 | @bad.wtf
78 |- def bad(cls):
78 |+ def bad(self):
79 79 | pass
80 80 |
81 81 | @hybrid_property
N805.py:85:14: N805 [*] First argument of a method should be named `self`
N805.py:86:14: N805 [*] First argument of a method should be named `self`
|
84 | @good.expression
85 | def good(cls):
85 | @good.expression
86 | def good(cls):
| ^^^ N805
86 | pass
87 | pass
|
= help: Rename `cls` to `self`
Unsafe fix
82 82 | pass
83 83 |
84 84 | @good.expression
85 |- def good(cls):
85 |+ def good(self):
86 86 | pass
87 87 |
88 88 | @good.wtf
83 83 | pass
84 84 |
85 85 | @good.expression
86 |- def good(cls):
86 |+ def good(self):
87 87 | pass
88 88 |
89 89 | @good.wtf
N805.py:98:26: N805 First argument of a method should be named `self`
|
@@ -204,66 +205,45 @@ N805.py:110:24: N805 First argument of a method should be named `self`
|
= help: Rename `this` to `self`
N805.py:115:20: N805 [*] First argument of a method should be named `self`
N805.py:114:20: N805 [*] First argument of a method should be named `self`
|
114 | class RenamingInMethodBodyClass:
115 | def bad_method(this):
113 | class RenamingInMethodBodyClass:
114 | def bad_method(this):
| ^^^^ N805
116 | this = this
117 | this
115 | this = this
116 | this
|
= help: Rename `this` to `self`
Unsafe fix
111 111 | pass
112 112 |
113 113 |
114 114 | class RenamingInMethodBodyClass:
115 |- def bad_method(this):
116 |- this = this
117 |- this
115 |+ def bad_method(self):
116 |+ self = self
117 |+ self
118 118 |
119 119 | def bad_method(this):
120 120 | self = this
113 113 | class RenamingInMethodBodyClass:
114 |- def bad_method(this):
115 |- this = this
116 |- this
114 |+ def bad_method(self):
115 |+ self = self
116 |+ self
117 117 |
118 118 | def bad_method(this):
119 119 | self = this
N805.py:119:20: N805 [*] First argument of a method should be named `self`
N805.py:118:20: N805 [*] First argument of a method should be named `self`
|
117 | this
118 |
119 | def bad_method(this):
116 | this
117 |
118 | def bad_method(this):
| ^^^^ N805
120 | self = this
119 | self = this
|
= help: Rename `this` to `self`
Unsafe fix
116 116 | this = this
117 117 | this
118 118 |
119 |- def bad_method(this):
120 |- self = this
119 |+ def bad_method(self):
120 |+ self = self
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
N805.py:124:17: N805 [*] First argument of a method should be named `self`
|
123 | class RenamingWithNFKC:
124 | def formula(household):
| ^^^^^^^^^ N805
125 | hºusehold(1)
|
= help: Rename `household` to `self`
Unsafe fix
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
124 |- def formula(household):
125 |- hºusehold(1)
124 |+ def formula(self):
125 |+ self(1)
115 115 | this = this
116 116 | this
117 117 |
118 |- def bad_method(this):
119 |- self = this
118 |+ def bad_method(self):
119 |+ self = self

View File

@@ -44,28 +44,22 @@ use crate::checkers::ast::Checker;
/// filtered.extend(x for x in original if x % 2)
/// ```
#[violation]
pub struct ManualListComprehension {
is_async: bool,
}
pub struct ManualListComprehension;
impl Violation for ManualListComprehension {
#[derive_message_formats]
fn message(&self) -> String {
let ManualListComprehension { is_async } = self;
match is_async {
false => format!("Use a list comprehension to create a transformed list"),
true => format!("Use an async list comprehension to create a transformed list"),
}
format!("Use a list comprehension to create a transformed list")
}
}
/// PERF401
pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::StmtFor) {
let Expr::Name(ast::ExprName { id, .. }) = &*for_stmt.target else {
pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, body: &[Stmt]) {
let Expr::Name(ast::ExprName { id, .. }) = target else {
return;
};
let (stmt, if_test) = match &*for_stmt.body {
let (stmt, if_test) = match body {
// ```python
// for x in y:
// if z:
@@ -127,13 +121,10 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S
return;
}
// Ignore direct list copies (e.g., `for x in y: filtered.append(x)`), unless it's async, which
// `manual-list-copy` doesn't cover.
if !for_stmt.is_async {
if if_test.is_none() {
if arg.as_name_expr().is_some_and(|arg| arg.id == *id) {
return;
}
// Ignore direct list copies (e.g., `for x in y: filtered.append(x)`).
if if_test.is_none() {
if arg.as_name_expr().is_some_and(|arg| arg.id == *id) {
return;
}
}
@@ -188,10 +179,7 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S
return;
}
checker.diagnostics.push(Diagnostic::new(
ManualListComprehension {
is_async: for_stmt.is_async,
},
*range,
));
checker
.diagnostics
.push(Diagnostic::new(ManualListComprehension, *range));
}

View File

@@ -45,16 +45,12 @@ impl Violation for ManualListCopy {
}
/// PERF402
pub(crate) fn manual_list_copy(checker: &mut Checker, for_stmt: &ast::StmtFor) {
if for_stmt.is_async {
return;
}
let Expr::Name(ast::ExprName { id, .. }) = &*for_stmt.target else {
pub(crate) fn manual_list_copy(checker: &mut Checker, target: &Expr, body: &[Stmt]) {
let Expr::Name(ast::ExprName { id, .. }) = target else {
return;
};
let [stmt] = &*for_stmt.body else {
let [stmt] = body else {
return;
};

View File

@@ -17,18 +17,4 @@ PERF401.py:13:9: PERF401 Use a list comprehension to create a transformed list
| ^^^^^^^^^^^^^^^^^^^^ PERF401
|
PERF401.py:82:13: PERF401 Use an async list comprehension to create a transformed list
|
80 | async for i in items:
81 | if i % 2:
82 | result.append(i) # PERF401
| ^^^^^^^^^^^^^^^^ PERF401
|
PERF401.py:89:9: PERF401 Use an async list comprehension to create a transformed list
|
87 | result = []
88 | async for i in items:
89 | result.append(i) # PERF401
| ^^^^^^^^^^^^^^^^ PERF401
|

View File

@@ -31,21 +31,18 @@ use super::LogicalLine;
/// The rule is also incompatible with the [formatter] when using
/// `indent-width` with a value other than `4`.
///
/// ## Options
/// - `indent-width`
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
/// [formatter]:https://docs.astral.sh/ruff/formatter/
#[violation]
pub struct IndentationWithInvalidMultiple {
indent_width: usize,
indent_size: usize,
}
impl Violation for IndentationWithInvalidMultiple {
#[derive_message_formats]
fn message(&self) -> String {
let Self { indent_width } = self;
format!("Indentation is not a multiple of {indent_width}")
let Self { indent_size } = self;
format!("Indentation is not a multiple of {indent_size}")
}
}
@@ -74,21 +71,18 @@ impl Violation for IndentationWithInvalidMultiple {
/// The rule is also incompatible with the [formatter] when using
/// `indent-width` with a value other than `4`.
///
/// ## Options
/// - `indent-width`
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
/// [formatter]:https://docs.astral.sh/ruff/formatter/
#[violation]
pub struct IndentationWithInvalidMultipleComment {
indent_width: usize,
indent_size: usize,
}
impl Violation for IndentationWithInvalidMultipleComment {
#[derive_message_formats]
fn message(&self) -> String {
let Self { indent_width } = self;
format!("Indentation is not a multiple of {indent_width} (comment)")
let Self { indent_size } = self;
format!("Indentation is not a multiple of {indent_size} (comment)")
}
}
@@ -263,13 +257,9 @@ pub(crate) fn indentation(
if indent_level % indent_size != 0 {
diagnostics.push(if logical_line.is_comment_only() {
DiagnosticKind::from(IndentationWithInvalidMultipleComment {
indent_width: indent_size,
})
DiagnosticKind::from(IndentationWithInvalidMultipleComment { indent_size })
} else {
DiagnosticKind::from(IndentationWithInvalidMultiple {
indent_width: indent_size,
})
DiagnosticKind::from(IndentationWithInvalidMultiple { indent_size })
});
}
let indent_expect = prev_logical_line

View File

@@ -60,7 +60,7 @@ pub(crate) fn redundant_backslash(
let start = locator.line_start(token.start());
start_index = continuation_lines
.binary_search(&start)
.unwrap_or_else(|err_index| err_index);
.map_or_else(|err_index| err_index, |ok_index| ok_index);
}
parens += 1;
}
@@ -70,7 +70,7 @@ pub(crate) fn redundant_backslash(
let end = locator.line_start(token.start());
let end_index = continuation_lines
.binary_search(&end)
.unwrap_or_else(|err_index| err_index);
.map_or_else(|err_index| err_index, |ok_index| ok_index);
for continuation_line in &continuation_lines[start_index..end_index] {
let backslash_end = locator.line_end(*continuation_line);
let backslash_start = backslash_end - TextSize::new(1);

View File

@@ -205,9 +205,6 @@ mod tests {
}
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
#[test_case(Rule::UnusedImport, Path::new("__init__.py"))]
#[test_case(Rule::UnusedImport, Path::new("F401_24/__init__.py"))]
#[test_case(Rule::UnusedImport, Path::new("F401_25__all/__init__.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
@@ -252,6 +249,19 @@ mod tests {
Ok(())
}
#[test]
fn init_unused_import_opt_in_to_fix() -> Result<()> {
let diagnostics = test_path(
Path::new("pyflakes/__init__.py"),
&LinterSettings {
ignore_init_module_imports: false,
..LinterSettings::for_rules(vec![Rule::UnusedImport])
},
)?;
assert_messages!(diagnostics);
Ok(())
}
#[test]
fn default_builtins() -> Result<()> {
let diagnostics = test_path(
@@ -601,7 +611,7 @@ mod tests {
&indexer,
);
let LinterResult {
data: mut diagnostics,
data: (mut diagnostics, ..),
..
} = check_path(
Path::new("<filename>"),

View File

@@ -1,24 +1,21 @@
use std::borrow::Cow;
use std::iter;
use anyhow::{anyhow, bail, Result};
use anyhow::Result;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Stmt, StmtImportFrom};
use ruff_python_semantic::{AnyImport, Exceptions, Imported, NodeId, Scope};
use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::fix;
use crate::registry::Rule;
use crate::rules::{isort, isort::ImportSection, isort::ImportType};
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
enum UnusedImportContext {
ExceptHandler,
Init { first_party: bool },
Init,
}
/// ## What it does
@@ -96,7 +93,7 @@ impl Violation for UnusedImport {
"`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability"
)
}
Some(UnusedImportContext::Init { .. }) => {
Some(UnusedImportContext::Init) => {
format!(
"`{name}` imported but unused; consider removing, adding to `__all__`, or using a redundant alias"
)
@@ -107,47 +104,14 @@ impl Violation for UnusedImport {
fn fix_title(&self) -> Option<String> {
let UnusedImport { name, multiple, .. } = self;
let resolution = match self.context {
Some(UnusedImportContext::Init { first_party: true }) => "Use a redundant alias",
_ => "Remove unused import",
};
Some(if *multiple {
resolution.to_string()
"Remove unused import".to_string()
} else {
format!("{resolution}: `{name}`")
format!("Remove unused import: `{name}`")
})
}
}
fn is_first_party(qualified_name: &str, level: u32, checker: &Checker) -> bool {
let category = isort::categorize(
qualified_name,
level,
&checker.settings.src,
checker.package(),
checker.settings.isort.detect_same_package,
&checker.settings.isort.known_modules,
checker.settings.target_version,
checker.settings.isort.no_sections,
&checker.settings.isort.section_order,
&checker.settings.isort.default_section,
);
matches! {
category,
ImportSection::Known(ImportType::FirstParty | ImportType::LocalFolder)
}
}
/// For some unused binding in an import statement...
///
/// __init__.py ∧ 1stpty → safe, convert to redundant-alias
/// __init__.py ∧ stdlib → unsafe, remove
/// __init__.py ∧ 3rdpty → unsafe, remove
///
/// ¬__init__.py ∧ 1stpty → safe, remove
/// ¬__init__.py ∧ stdlib → safe, remove
/// ¬__init__.py ∧ 3rdpty → safe, remove
///
pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
// Collect all unused imports by statement.
let mut unused: FxHashMap<(NodeId, Exceptions), Vec<ImportBinding>> = FxHashMap::default();
@@ -196,82 +160,42 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
}
let in_init = checker.path().ends_with("__init__.py");
let fix_init = checker.settings.preview.is_enabled();
let fix_init = !checker.settings.ignore_init_module_imports;
// Generate a diagnostic for every import, but share fixes across all imports within the same
// Generate a diagnostic for every import, but share a fix across all imports within the same
// statement (excluding those that are ignored).
for ((import_statement, exceptions), bindings) in unused {
for ((node_id, exceptions), imports) in unused {
let in_except_handler =
exceptions.intersects(Exceptions::MODULE_NOT_FOUND_ERROR | Exceptions::IMPORT_ERROR);
let multiple = bindings.len() > 1;
let level = match checker.semantic().statement(import_statement) {
Stmt::Import(_) => 0,
Stmt::ImportFrom(StmtImportFrom { level, .. }) => *level,
_ => {
continue;
}
};
let multiple = imports.len() > 1;
// pair each binding with context; divide them by how we want to fix them
let (to_reexport, to_remove): (Vec<_>, Vec<_>) = bindings
.into_iter()
.map(|binding| {
let context = if in_except_handler {
Some(UnusedImportContext::ExceptHandler)
} else if in_init {
Some(UnusedImportContext::Init {
first_party: is_first_party(
&binding.import.qualified_name().to_string(),
level,
checker,
),
})
} else {
None
};
(binding, context)
})
.partition(|(_, context)| {
matches!(
context,
Some(UnusedImportContext::Init { first_party: true })
)
});
// generate fixes that are shared across bindings in the statement
let (fix_remove, fix_reexport) = if (!in_init || fix_init) && !in_except_handler {
(
fix_by_removing_imports(
checker,
import_statement,
to_remove.iter().map(|(binding, _)| binding),
in_init,
)
.ok(),
fix_by_reexporting(
checker,
import_statement,
to_reexport.iter().map(|(binding, _)| binding),
)
.ok(),
)
let fix = if (!in_init || fix_init) && !in_except_handler {
fix_imports(checker, node_id, &imports, in_init).ok()
} else {
(None, None)
None
};
for ((binding, context), fix) in iter::Iterator::chain(
iter::zip(to_remove, iter::repeat(fix_remove)),
iter::zip(to_reexport, iter::repeat(fix_reexport)),
) {
for ImportBinding {
import,
range,
parent_range,
} in imports
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: binding.import.qualified_name().to_string(),
context,
name: import.qualified_name().to_string(),
context: if in_except_handler {
Some(UnusedImportContext::ExceptHandler)
} else if in_init {
Some(UnusedImportContext::Init)
} else {
None
},
multiple,
},
binding.range,
range,
);
if let Some(range) = binding.parent_range {
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
if !in_except_handler {
@@ -324,22 +248,20 @@ impl Ranged for ImportBinding<'_> {
}
/// Generate a [`Fix`] to remove unused imports from a statement.
fn fix_by_removing_imports<'a>(
fn fix_imports(
checker: &Checker,
node_id: NodeId,
imports: impl Iterator<Item = &'a ImportBinding<'a>>,
imports: &[ImportBinding],
in_init: bool,
) -> Result<Fix> {
let statement = checker.semantic().statement(node_id);
let parent = checker.semantic().parent_statement(node_id);
let member_names: Vec<Cow<'_, str>> = imports
.iter()
.map(|ImportBinding { import, .. }| import)
.map(Imported::member_name)
.collect();
if member_names.is_empty() {
bail!("Expected import bindings");
}
let edit = fix::edits::remove_unused_imports(
member_names.iter().map(AsRef::as_ref),
@@ -349,43 +271,15 @@ fn fix_by_removing_imports<'a>(
checker.stylist(),
checker.indexer(),
)?;
// It's unsafe to remove things from `__init__.py` because it can break public interfaces
let applicability = if in_init {
Applicability::Unsafe
} else {
Applicability::Safe
};
Ok(
Fix::applicable_edit(edit, applicability).isolate(Checker::isolation(
checker.semantic().parent_statement_id(node_id),
)),
)
}
/// Generate a [`Fix`] to make bindings in a statement explicit, by changing from `import a` to
/// `import a as a`.
fn fix_by_reexporting<'a>(
checker: &Checker,
node_id: NodeId,
imports: impl Iterator<Item = &'a ImportBinding<'a>>,
) -> Result<Fix> {
let statement = checker.semantic().statement(node_id);
let member_names = imports
.map(|binding| binding.import.member_name())
.collect::<Vec<_>>();
if member_names.is_empty() {
bail!("Expected import bindings");
}
let edits = fix::edits::make_redundant_alias(member_names.iter().map(AsRef::as_ref), statement);
// Only emit a fix if there are edits
let mut tail = edits.into_iter();
let head = tail.next().ok_or(anyhow!("No edits to make"))?;
let isolation = Checker::isolation(checker.semantic().parent_statement_id(node_id));
Ok(Fix::safe_edits(head, tail).isolate(isolation))
}

View File

@@ -42,5 +42,5 @@ F821_27.py:47:17: F821 Undefined name `Tree`
46 | class Leaf: ...
47 | class Tree(list[Tree | Leaf]): ... # Still invalid even when `__future__.annotations` are enabled
| ^^^^ F821
48 | class Tree2(list["Tree2 | Leaf"]): ... # always okay
48 | class Tree2(list["Tree | Leaf"]): ... # always okay
|

View File

@@ -1,42 +0,0 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
__init__.py:19:8: F401 [*] `sys` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
|
19 | import sys # F401: remove unused
| ^^^ F401
|
= help: Remove unused import: `sys`
Unsafe fix
16 16 | import argparse as argparse # Ok: is redundant alias
17 17 |
18 18 |
19 |-import sys # F401: remove unused
20 19 |
21 20 |
22 21 | # first-party
__init__.py:33:15: F401 [*] `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
|
33 | from . import unused # F401: change to redundant alias
| ^^^^^^ F401
|
= help: Use a redundant alias: `.unused`
Safe fix
30 30 | from . import aliased as aliased # Ok: is redundant alias
31 31 |
32 32 |
33 |-from . import unused # F401: change to redundant alias
33 |+from . import unused as unused # F401: change to redundant alias
34 34 |
35 35 |
36 36 | from . import renamed as bees # F401: no fix
__init__.py:36:26: F401 `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
|
36 | from . import renamed as bees # F401: no fix
| ^^^^ F401
|
= help: Use a redundant alias: `.renamed`

View File

@@ -1,18 +0,0 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
__init__.py:19:8: F401 [*] `sys` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
|
19 | import sys # F401: remove unused
| ^^^ F401
|
= help: Remove unused import: `sys`
Unsafe fix
16 16 | import argparse # Ok: is exported in __all__
17 17 |
18 18 |
19 |-import sys # F401: remove unused
20 19 |
21 20 |
22 21 | # first-party

View File

@@ -64,7 +64,7 @@ pub(crate) fn bad_staticmethod_argument(
..
} = func;
let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else {
let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
return;
};

Some files were not shown because too many files have changed in this diff Show More