Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Handle submodule imports for F401

This commit is contained in:
Charlie Marsh 2022-08-31 11:15:15 -04:00
parent 0ebed13e67
commit 20d88fc53e
3 changed files with 132 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
resources/test/src/F401.py
View File

@ -5,10 +5,14 @@ from collections import (
OrderedDict, OrderedDict,
namedtuple, namedtuple,
) )
import multiprocessing.pool
import multiprocessing.process
import logging.config
import logging.handlers
class X: class X:
def a(self) -> "namedtuple": def a(self) -> "namedtuple":
x = os.environ["1"] x = os.environ["1"]
y = Counter() y = Counter()
return X z = multiprocessing.pool.ThreadPool()

216
src/check_ast.rs
View File

@ -50,12 +50,11 @@ enum BindingKind {
FutureImportation, FutureImportation,
Importation(String), Importation(String),
StarImportation, StarImportation,
SubmoduleImportation, SubmoduleImportation(String),
} }
struct Binding { struct Binding {
kind: BindingKind, kind: BindingKind,
name: String,
location: Location, location: Location,
used: Option<usize>, used: Option<usize>,
} }
@ -99,7 +98,6 @@ impl Visitor for Checker<'_> {
name.to_string(), name.to_string(),
Binding { Binding {
kind: BindingKind::Assignment, kind: BindingKind::Assignment,
name: name.clone(),
used: Some(global_scope_id), used: Some(global_scope_id),
location: stmt.location, location: stmt.location,
}, },
@ -109,21 +107,27 @@ impl Visitor for Checker<'_> {
} }
} }
StmtKind::FunctionDef { name, .. } => { StmtKind::FunctionDef { name, .. } => {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Definition, name.to_string(),
name: name.clone(), Binding {
used: None, kind: BindingKind::Definition,
location: stmt.location,
}); used: None,
location: stmt.location,
},
);
self.push_scope(Scope::new(Function)); self.push_scope(Scope::new(Function));
} }
StmtKind::AsyncFunctionDef { name, .. } => { StmtKind::AsyncFunctionDef { name, .. } => {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Definition, name.to_string(),
name: name.clone(), Binding {
used: None, kind: BindingKind::Definition,
location: stmt.location,
}); used: None,
location: stmt.location,
},
);
self.push_scope(Scope::new(Function)); self.push_scope(Scope::new(Function));
} }
StmtKind::Return { .. } => { StmtKind::Return { .. } => {
@ -149,29 +153,35 @@ impl Visitor for Checker<'_> {
StmtKind::Import { names } => { StmtKind::Import { names } => {
for alias in names { for alias in names {
if alias.node.name.contains('.') && alias.node.asname.is_none() { if alias.node.name.contains('.') && alias.node.asname.is_none() {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::SubmoduleImportation, alias.node.name.split('.').next().unwrap().to_string(),
name: alias.node.name.clone(), Binding {
used: None, kind: BindingKind::SubmoduleImportation(
location: stmt.location, alias.node.name.to_string(),
}) ),
used: None,
location: stmt.location,
},
)
} else { } else {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Importation( alias
alias
.node
.asname
.clone()
.unwrap_or_else(|| alias.node.name.clone()),
),
name: alias
.node .node
.asname .asname
.clone() .clone()
.unwrap_or_else(|| alias.node.name.clone()), .unwrap_or_else(|| alias.node.name.clone()),
used: None, Binding {
location: stmt.location, kind: BindingKind::Importation(
}) alias
.node
.asname
.clone()
.unwrap_or_else(|| alias.node.name.clone()),
),
used: None,
location: stmt.location,
},
)
} }
} }
} }
@ -183,19 +193,25 @@ impl Visitor for Checker<'_> {
.clone() .clone()
.unwrap_or_else(|| alias.node.name.clone()); .unwrap_or_else(|| alias.node.name.clone());
if let Some("future") = module.as_deref() { if let Some("future") = module.as_deref() {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::FutureImportation,
name, name,
used: Some(self.scopes.last().expect("No current scope found.").id), Binding {
location: stmt.location, kind: BindingKind::FutureImportation,
});
used: Some(self.scopes.last().expect("No current scope found.").id),
location: stmt.location,
},
);
} else if alias.node.name == "*" { } else if alias.node.name == "*" {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::StarImportation,
name, name,
used: None, Binding {
location: stmt.location, kind: BindingKind::StarImportation,
});
used: None,
location: stmt.location,
},
);
if self if self
.settings .settings
@ -208,15 +224,15 @@ impl Visitor for Checker<'_> {
}); });
} }
} else { } else {
self.add_binding(Binding { let binding = Binding {
kind: BindingKind::Importation(match module { kind: BindingKind::Importation(match module {
None => name.clone(), None => name.clone(),
Some(parent) => format!("{}.{}", parent, name), Some(parent) => format!("{}.{}", parent, name.clone()),
}), }),
name,
used: None, used: None,
location: stmt.location, location: stmt.location,
}) };
self.add_binding(name, binding)
} }
} }
} }
@ -279,12 +295,15 @@ impl Visitor for Checker<'_> {
}; };
if let StmtKind::ClassDef { name, .. } = &stmt.node { if let StmtKind::ClassDef { name, .. } = &stmt.node {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::ClassDefinition, name.to_string(),
name: name.clone(), Binding {
used: None, kind: BindingKind::ClassDefinition,
location: stmt.location,
}); used: None,
location: stmt.location,
},
);
} }
} }
@ -390,12 +409,14 @@ impl Visitor for Checker<'_> {
} }
fn visit_arg(&mut self, arg: &Arg) { fn visit_arg(&mut self, arg: &Arg) {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Argument, arg.node.arg.to_string(),
name: arg.node.arg.clone(), Binding {
used: None, kind: BindingKind::Argument,
location: arg.location, used: None,
}); location: arg.location,
},
);
visitor::walk_arg(self, arg); visitor::walk_arg(self, arg);
} }
} }
@ -412,39 +433,40 @@ impl Checker<'_> {
fn bind_builtins(&mut self) { fn bind_builtins(&mut self) {
for builtin in BUILTINS { for builtin in BUILTINS {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Builtin, builtin.to_string(),
name: builtin.to_string(), Binding {
location: Default::default(), kind: BindingKind::Builtin,
used: None, location: Default::default(),
}) used: None,
},
)
} }
for builtin in MAGIC_GLOBALS { for builtin in MAGIC_GLOBALS {
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Builtin, builtin.to_string(),
name: builtin.to_string(), Binding {
location: Default::default(), kind: BindingKind::Builtin,
used: None, location: Default::default(),
}) used: None,
},
)
} }
} }
fn add_binding(&mut self, binding: Binding) { fn add_binding(&mut self, name: String, binding: Binding) {
let scope = self.scopes.last_mut().expect("No current scope found."); let scope = self.scopes.last_mut().expect("No current scope found.");
// TODO(charlie): Don't treat annotations as assignments if there is an existing value. // TODO(charlie): Don't treat annotations as assignments if there is an existing value.
scope.values.insert( let binding = match scope.values.get(&name) {
binding.name.clone(), None => binding,
match scope.values.get(&binding.name) { Some(existing) => Binding {
None => binding, kind: binding.kind,
Some(existing) => Binding { location: binding.location,
kind: binding.kind, used: existing.used,
name: binding.name,
location: binding.location,
used: existing.used,
},
}, },
); };
scope.values.insert(name, binding);
} }
fn handle_node_load(&mut self, expr: &Expr) { fn handle_node_load(&mut self, expr: &Expr) {
@ -505,12 +527,14 @@ impl Checker<'_> {
} }
// TODO(charlie): Handle alternate binding types (like `Annotation`). // TODO(charlie): Handle alternate binding types (like `Annotation`).
self.add_binding(Binding { self.add_binding(
kind: BindingKind::Assignment, id.to_string(),
name: id.to_string(), Binding {
used: None, kind: BindingKind::Assignment,
location: expr.location, used: None,
}); location: expr.location,
},
);
} }
} }
@ -542,11 +566,15 @@ impl Checker<'_> {
for scope in &self.dead_scopes { for scope in &self.dead_scopes {
for (_, binding) in scope.values.iter().rev() { for (_, binding) in scope.values.iter().rev() {
if binding.used.is_none() { if binding.used.is_none() {
if let BindingKind::Importation(name) = &binding.kind { match &binding.kind {
self.checks.push(Check { BindingKind::Importation(full_name)
kind: CheckKind::UnusedImport(name.clone()), | BindingKind::SubmoduleImportation(full_name) => {
location: binding.location, self.checks.push(Check {
}); kind: CheckKind::UnusedImport(full_name.to_string()),
location: binding.location,
});
}
_ => {}
} }
} }
} }

5
src/linter.rs
View File

@ -101,6 +101,11 @@ mod tests {
&cache::Mode::None, &cache::Mode::None,
)?; )?;
let expected = vec![ let expected = vec![
Message {
kind: CheckKind::UnusedImport("logging.handlers".to_string()),
location: Location::new(11, 1),
filename: "./resources/test/src/F401.py".to_string(),
},
Message { Message {
kind: CheckKind::UnusedImport("functools".to_string()), kind: CheckKind::UnusedImport("functools".to_string()),
location: Location::new(2, 1), location: Location::new(2, 1),