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

Mark redefined-but-unused imports as unused regardless of scope (#1173)

This commit is contained in:
Charlie Marsh 2022-12-09 23:17:33 -05:00 committed by GitHub
parent 305326f7d7
commit 6739602806
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 62 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/ast/types.rs
View File

@ -73,7 +73,11 @@ pub struct Scope<'a> {
pub kind: ScopeKind<'a>,
pub import_starred: bool,
pub uses_locals: bool,
/// A map from bound name to binding index.
pub values: FxHashMap<&'a str, usize>,
/// A list of (name, index) pairs for bindings that were overridden in the
/// scope.
pub overridden: Vec<(&'a str, usize)>,
}
impl<'a> Scope<'a> {
@ -84,6 +88,7 @@ impl<'a> Scope<'a> {
import_starred: false,
uses_locals: false,
values: FxHashMap::default(),
overridden: Vec::new(),
}
}
}
@ -117,8 +122,6 @@ pub struct Binding<'a> {
/// Tuple of (scope index, range) indicating the scope and range at which
/// the binding was last used.
pub used: Option<(usize, Range)>,
/// A list of pointers to `Binding` instances that redefined this binding.
pub redefined: Vec<usize>,
}
// Pyflakes defines the following binding hierarchy (via inheritance):

67
src/check_ast.rs
View File

@ -4,6 +4,7 @@ use std::path::Path;
use itertools::Itertools;
use log::error;
use nohash_hasher::IntMap;
use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::Location;
use rustpython_parser::ast::{
@ -68,6 +69,7 @@ pub struct Checker<'a> {
pub(crate) depths: FxHashMap<RefEquality<'a, Stmt>, usize>,
pub(crate) child_to_parent: FxHashMap<RefEquality<'a, Stmt>, RefEquality<'a, Stmt>>,
pub(crate) bindings: Vec<Binding<'a>>,
pub(crate) redefinitions: IntMap<usize, Vec<usize>>,
scopes: Vec<Scope<'a>>,
scope_stack: Vec<usize>,
dead_scopes: Vec<usize>,
@ -114,6 +116,7 @@ impl<'a> Checker<'a> {
depths: FxHashMap::default(),
child_to_parent: FxHashMap::default(),
bindings: vec![],
redefinitions: IntMap::default(),
scopes: vec![],
scope_stack: vec![],
dead_scopes: vec![],
@ -250,7 +253,6 @@ where
used: None,
range: Range::from_located(stmt),
source: None,
redefined: vec![],
});
self.scopes[GLOBAL_SCOPE_INDEX].values.insert(name, index);
}
@ -266,7 +268,6 @@ where
used: usage,
range: Range::from_located(stmt),
source: None,
redefined: vec![],
});
scope.values.insert(name, index);
}
@ -297,7 +298,6 @@ where
used: usage,
range: Range::from_located(stmt),
source: None,
redefined: vec![],
});
scope.values.insert(name, index);
}
@ -513,7 +513,6 @@ where
used: None,
range: Range::from_located(stmt),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
}
@ -623,7 +622,6 @@ where
used: None,
range: Range::from_located(alias),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
} else {
@ -664,7 +662,6 @@ where
},
range: Range::from_located(alias),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
}
@ -809,7 +806,6 @@ where
)),
range: Range::from_located(alias),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
@ -841,7 +837,6 @@ where
used: None,
range: Range::from_located(stmt),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
@ -911,7 +906,6 @@ where
},
range,
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
}
@ -1237,7 +1231,6 @@ where
used: None,
range: Range::from_located(stmt),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
};
@ -2498,7 +2491,6 @@ where
used: None,
range: Range::from_located(arg),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
@ -2564,7 +2556,6 @@ impl<'a> Checker<'a> {
range: Range::default(),
used: None,
source: None,
redefined: vec![],
});
scope.values.insert(builtin, index);
}
@ -2593,8 +2584,9 @@ impl<'a> Checker<'a> {
where
'b: 'a,
{
let index = self.bindings.len();
let binding_index = self.bindings.len();
let mut overridden = None;
if let Some((stack_index, scope_index)) = self
.scope_stack
.iter()
@ -2602,8 +2594,8 @@ impl<'a> Checker<'a> {
.enumerate()
.find(|(_, scope_index)| self.scopes[**scope_index].values.contains_key(&name))
{
let existing_index = self.scopes[*scope_index].values.get(&name).unwrap();
let existing = &self.bindings[*existing_index];
let existing_binding_index = self.scopes[*scope_index].values.get(&name).unwrap();
let existing = &self.bindings[*existing_binding_index];
let in_current_scope = stack_index == 0;
if !matches!(existing.kind, BindingKind::Builtin)
&& existing.source.as_ref().map_or(true, |left| {
@ -2626,6 +2618,7 @@ impl<'a> Checker<'a> {
| BindingKind::FutureImportation
);
if matches!(binding.kind, BindingKind::LoopVar) && existing_is_import {
overridden = Some((*scope_index, *existing_binding_index));
if self.settings.enabled.contains(&CheckCode::F402) {
self.add_check(Check::new(
CheckKind::ImportShadowedByLoopVar(
@ -2645,6 +2638,7 @@ impl<'a> Checker<'a> {
cast::decorator_list(existing.source.as_ref().unwrap().0),
))
{
overridden = Some((*scope_index, *existing_binding_index));
if self.settings.enabled.contains(&CheckCode::F811) {
self.add_check(Check::new(
CheckKind::RedefinedWhileUnused(
@ -2656,11 +2650,21 @@ impl<'a> Checker<'a> {
}
}
} else if existing_is_import && binding.redefines(existing) {
self.bindings[*existing_index].redefined.push(index);
self.redefinitions
.entry(*existing_binding_index)
.or_insert_with(Vec::new)
.push(binding_index);
}
}
}
// If we're about to lose the binding, store it as overriden.
if let Some((scope_index, binding_index)) = overridden {
self.scopes[scope_index]
.overridden
.push((name, binding_index));
}
// Assume the rebound name is used as a global or within a loop.
let scope = self.current_scope();
let binding = match scope.values.get(&name) {
@ -2675,7 +2679,7 @@ impl<'a> Checker<'a> {
// in scope.
let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))];
if !(matches!(binding.kind, BindingKind::Annotation) && scope.values.contains_key(name)) {
scope.values.insert(name, index);
scope.values.insert(name, binding_index);
}
self.bindings.push(binding);
@ -2850,7 +2854,6 @@ impl<'a> Checker<'a> {
used: None,
range: Range::from_located(expr),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
return;
@ -2868,7 +2871,6 @@ impl<'a> Checker<'a> {
used: None,
range: Range::from_located(expr),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
return;
@ -2882,7 +2884,6 @@ impl<'a> Checker<'a> {
used: None,
range: Range::from_located(expr),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
return;
@ -2933,7 +2934,6 @@ impl<'a> Checker<'a> {
used: None,
range: Range::from_located(expr),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
return;
@ -2947,7 +2947,6 @@ impl<'a> Checker<'a> {
used: None,
range: Range::from_located(expr),
source: Some(self.current_parent().clone()),
redefined: vec![],
},
);
}
@ -3225,14 +3224,16 @@ impl<'a> Checker<'a> {
continue;
}
for index in &binding.redefined {
checks.push(Check::new(
CheckKind::RedefinedWhileUnused(
(*name).to_string(),
binding.range.location.row(),
),
self.bindings[*index].range,
));
if let Some(indices) = self.redefinitions.get(index) {
for index in indices {
checks.push(Check::new(
CheckKind::RedefinedWhileUnused(
(*name).to_string(),
binding.range.location.row(),
),
self.bindings[*index].range,
));
}
}
}
}
@ -3279,7 +3280,11 @@ impl<'a> Checker<'a> {
let mut unused: FxHashMap<BindingContext, Vec<UnusedImport>> = FxHashMap::default();
for (name, index) in &scope.values {
for (name, index) in scope
.values
.iter()
.chain(scope.overridden.iter().map(|(a, b)| (a, b)))
{
let binding = &self.bindings[*index];
let (BindingKind::Importation(_, full_name)

37
src/pyflakes/mod.rs
View File

@ -1105,11 +1105,11 @@ mod tests {
fn aliased_import() -> Result<()> {
flakes(
"import fu as FU, bar as FU",
&[CheckCode::F811, CheckCode::F401],
&[CheckCode::F401, CheckCode::F811, CheckCode::F401],
)?;
flakes(
"from moo import fu as FU, bar as FU",
&[CheckCode::F811, CheckCode::F401],
&[CheckCode::F401, CheckCode::F811, CheckCode::F401],
)?;
Ok(())
@ -1146,9 +1146,15 @@ mod tests {
#[test]
fn redefined_while_unused() -> Result<()> {
flakes("import fu; fu = 3", &[CheckCode::F811])?;
flakes("import fu; fu, bar = 3", &[CheckCode::F811])?;
flakes("import fu; [fu, bar] = 3", &[CheckCode::F811])?;
flakes("import fu; fu = 3", &[CheckCode::F401, CheckCode::F811])?;
flakes(
"import fu; fu, bar = 3",
&[CheckCode::F401, CheckCode::F811],
)?;
flakes(
"import fu; [fu, bar] = 3",
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
}
@ -1165,7 +1171,7 @@ mod tests {
import os
os.path
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -1203,7 +1209,7 @@ mod tests {
pass
os.path
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -1279,7 +1285,7 @@ mod tests {
from bb import mixer
mixer(123)
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -1345,14 +1351,13 @@ mod tests {
#[test]
fn redefined_by_function() -> Result<()> {
// TODO(charlie): Why does this differ from the next test assertion?
flakes(
r#"
import fu
def fu():
pass
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -1430,7 +1435,7 @@ mod tests {
class fu:
pass
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -1694,7 +1699,7 @@ mod tests {
for fu in range(2):
pass
"#,
&[CheckCode::F402],
&[CheckCode::F401, CheckCode::F402],
)?;
Ok(())
@ -1851,7 +1856,7 @@ mod tests {
try: pass
except Exception as fu: pass
"#,
&[CheckCode::F811, CheckCode::F841],
&[CheckCode::F401, CheckCode::F811, CheckCode::F841],
)?;
Ok(())
@ -2161,7 +2166,7 @@ mod tests {
import fu.bar, fu.bar
fu.bar
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
flakes(
r#"
@ -2169,7 +2174,7 @@ mod tests {
import fu.bar
fu.bar
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())
@ -2325,7 +2330,7 @@ mod tests {
fu
fu
"#,
&[CheckCode::F811],
&[CheckCode::F401, CheckCode::F811],
)?;
Ok(())