Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid removing partially-unused imports (#1259)

Browse Source
This commit is contained in:
Charlie Marsh
2022-12-15 23:13:58 -05:00
committed by GitHub
parent b9f42bf5e5
commit b4bfa87104
8 changed files with 179 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
resources/test/fixtures/pyflakes/F401_7.py vendored Normal file
View File

@@ -0,0 +1,16 @@
"""Test: noqa directives."""
from module import (
A, # noqa: F401
B,
)
from module import (
A, # noqa: F401
B, # noqa: F401
)
from module import (
A,
B,
)

71
src/check_ast.rs
View File

@@ -25,6 +25,7 @@ use crate::ast::visitor::{walk_excepthandler, Visitor};
use crate::ast::{branch_detection, cast, helpers, operations, visitor};
use crate::checks::{Check, CheckCode, CheckKind, DeferralKeyword};
use crate::docstrings::definition::{Definition, DefinitionKind, Docstring, Documentable};
use crate::noqa::Directive;
use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS};
use crate::python::future::ALL_FEATURE_NAMES;
use crate::python::typing;
@@ -38,8 +39,8 @@ use crate::{
docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except,
flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_debugger,
flake8_errmsg, flake8_import_conventions, flake8_print, flake8_return, flake8_simplify,
flake8_tidy_imports, flake8_unused_arguments, mccabe, pandas_vet, pep8_naming, pycodestyle,
pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, visibility,
flake8_tidy_imports, flake8_unused_arguments, mccabe, noqa, pandas_vet, pep8_naming,
pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, visibility,
};
const GLOBAL_SCOPE_INDEX: usize = 0;
@@ -51,7 +52,9 @@ pub struct Checker<'a> {
// Input data.
path: &'a Path,
autofix: bool,
ignore_noqa: bool,
pub(crate) settings: &'a Settings,
pub(crate) noqa_line_for: &'a IntMap<usize, usize>,
pub(crate) locator: &'a SourceCodeLocator<'a>,
// Computed checks.
checks: Vec<Check>,
@@ -98,13 +101,17 @@ pub struct Checker<'a> {
impl<'a> Checker<'a> {
pub fn new(
settings: &'a Settings,
noqa_line_for: &'a IntMap<usize, usize>,
autofix: bool,
ignore_noqa: bool,
path: &'a Path,
locator: &'a SourceCodeLocator,
) -> Checker<'a> {
Checker {
settings,
noqa_line_for,
autofix,
ignore_noqa,
path,
locator,
checks: vec![],
@@ -199,6 +206,30 @@ impl<'a> Checker<'a> {
matches!(self.bindings[*index].kind, BindingKind::Builtin)
})
}
/// Return `true` if a `CheckCode` is disabled by a `noqa` directive.
pub fn is_ignored(&self, code: &CheckCode, lineno: usize) -> bool {
// TODO(charlie): `noqa` directives are mostly enforced in `check_lines.rs`.
// However, in rare cases, we need to check them here. For example, when
// removing unused imports, we create a single fix that's applied to all
// unused members on a single import. We need to pre-emptively omit any
// members from the fix that will eventually be excluded by a `noqa`.
// Unfortunately, we _do_ want to register a `Check` for each eventually-ignored
// import, so that our `noqa` counts are accurate.
if self.ignore_noqa {
return false;
}
let noqa_lineno = self.noqa_line_for.get(&lineno).unwrap_or(&lineno);
let line = self.locator.slice_source_code_range(&Range {
location: Location::new(*noqa_lineno, 0),
end_location: Location::new(noqa_lineno + 1, 0),
});
match noqa::extract_noqa_directive(&line) {
Directive::None => false,
Directive::All(..) => true,
Directive::Codes(.., codes) => noqa::includes(code, &codes),
}
}
}
impl<'a, 'b> Visitor<'b> for Checker<'a>
@@ -3396,6 +3427,8 @@ impl<'a> Checker<'a> {
(&'a RefEquality<'b, Stmt>, Option<&'a RefEquality<'b, Stmt>>);
let mut unused: FxHashMap<BindingContext, Vec<UnusedImport>> = FxHashMap::default();
let mut ignored: FxHashMap<BindingContext, Vec<UnusedImport>> =
FxHashMap::default();
for (name, index) in scope
.values
@@ -3420,12 +3453,21 @@ impl<'a> Checker<'a> {
let defined_by = binding.source.as_ref().unwrap();
let defined_in = self.child_to_parent.get(defined_by);
unused
.entry((defined_by, defined_in))
.or_default()
.push((full_name, &binding.range));
if self.is_ignored(&CheckCode::F401, binding.range.location.row()) {
ignored
.entry((defined_by, defined_in))
.or_default()
.push((full_name, &binding.range));
} else {
unused
.entry((defined_by, defined_in))
.or_default()
.push((full_name, &binding.range));
}
}
let ignore_init =
self.settings.ignore_init_module_imports && self.path.ends_with("__init__.py");
for ((defined_by, defined_in), unused_imports) in unused
.into_iter()
.sorted_by_key(|((defined_by, _), _)| defined_by.0.location)
@@ -3433,8 +3475,6 @@ impl<'a> Checker<'a> {
let child = defined_by.0;
let parent = defined_in.map(|defined_in| defined_in.0);
let ignore_init = self.settings.ignore_init_module_imports
&& self.path.ends_with("__init__.py");
let fix = if !ignore_init && self.patch(&CheckCode::F401) {
let deleted: Vec<&Stmt> =
self.deletions.iter().map(|node| node.0).collect();
@@ -3471,6 +3511,17 @@ impl<'a> Checker<'a> {
checks.push(check);
}
}
for (_, unused_imports) in ignored
.into_iter()
.sorted_by_key(|((defined_by, _), _)| defined_by.0.location)
{
for (full_name, range) in unused_imports {
checks.push(Check::new(
CheckKind::UnusedImport(full_name.clone(), ignore_init),
*range,
));
}
}
}
}
self.add_checks(checks.into_iter());
@@ -3705,11 +3756,13 @@ impl<'a> Checker<'a> {
pub fn check_ast(
python_ast: &Suite,
locator: &SourceCodeLocator,
noqa_line_for: &IntMap<usize, usize>,
settings: &Settings,
autofix: bool,
ignore_noqa: bool,
path: &Path,
) -> Vec<Check> {
let mut checker = Checker::new(settings, autofix, path, locator);
let mut checker = Checker::new(settings, noqa_line_for, autofix, ignore_noqa, path, locator);
checker.push_scope(Scope::new(ScopeKind::Module));
checker.bind_builtins();

10
src/linter.rs
View File

@@ -83,7 +83,15 @@ pub(crate) fn check_path(
match rustpython_helpers::parse_program_tokens(tokens, "<filename>") {
Ok(python_ast) => {
if use_ast {
checks.extend(check_ast(&python_ast, locator, settings, autofix, path));
checks.extend(check_ast(
&python_ast,
locator,
&directives.noqa_line_for,
settings,
autofix,
ignore_noqa,
path,
));
}
if use_imports {
checks.extend(check_imports(

2
src/pycodestyle/plugins.rs
View File

@@ -184,6 +184,8 @@ pub fn literal_comparisons(
comparator = next;
}
// TODO(charlie): Respect `noqa` directives. If one of the operators has a
// `noqa`, but another doesn't, both will be removed here.
if !bad_ops.is_empty() {
// Replace the entire comparison expression.
let ops = ops

19
src/pyflakes/fixes.rs
View File

@@ -1,7 +1,7 @@
use anyhow::{bail, Result};
use libcst_native::{
Call, Codegen, CodegenState, Dict, DictElement, Expression, ImportNames, SmallStatement,
Statement,
Call, Codegen, CodegenState, Dict, DictElement, Expression, ImportNames,
ParenthesizableWhitespace, SmallStatement, Statement,
};
use rustpython_ast::{Expr, Stmt};
@@ -60,8 +60,21 @@ pub fn remove_unused_imports(
}
}
// But avoid destroying any trailing comments.
if let Some(alias) = aliases.last_mut() {
alias.comma = trailing_comma;
let has_comment = if let Some(comma) = &alias.comma {
match &comma.whitespace_after {
ParenthesizableWhitespace::SimpleWhitespace(_) => false,
ParenthesizableWhitespace::ParenthesizedWhitespace(whitespace) => {
whitespace.first_line.comment.is_some()
}
}
} else {
false
};
if !has_comment {
alias.comma = trailing_comma;
}
}
if aliases.is_empty() {

1
src/pyflakes/mod.rs
View File

@@ -28,6 +28,7 @@ mod tests {
#[test_case(CheckCode::F401, Path::new("F401_4.py"); "F401_4")]
#[test_case(CheckCode::F401, Path::new("F401_5.py"); "F401_5")]
#[test_case(CheckCode::F401, Path::new("F401_6.py"); "F401_6")]
#[test_case(CheckCode::F401, Path::new("F401_7.py"); "F401_7")]
#[test_case(CheckCode::F402, Path::new("F402.py"); "F402")]
#[test_case(CheckCode::F403, Path::new("F403.py"); "F403")]
#[test_case(CheckCode::F404, Path::new("F404.py"); "F404")]

59
src/pyflakes/snapshots/ruff__pyflakes__tests__F401_F401_7.py.snap Normal file
View File

@@ -0,0 +1,59 @@
---
source: src/pyflakes/mod.rs
expression: checks
---
- kind:
UnusedImport:
- module.B
- false
location:
row: 5
column: 4
end_location:
row: 5
column: 5
fix:
content: "from module import (\n A, # noqa: F401\n )"
location:
row: 3
column: 0
end_location:
row: 6
column: 1
- kind:
UnusedImport:
- module.A
- false
location:
row: 14
column: 4
end_location:
row: 14
column: 5
fix:
content: ""
location:
row: 13
column: 0
end_location:
row: 17
column: 0
- kind:
UnusedImport:
- module.B
- false
location:
row: 15
column: 4
end_location:
row: 15
column: 5
fix:
content: ""
location:
row: 13
column: 0
end_location:
row: 17
column: 0

15
src/pyupgrade/fixes.rs
View File

@@ -170,8 +170,21 @@ pub fn remove_unnecessary_future_import(
aliases.remove(*index);
}
// But avoid destroying any trailing comments.
if let Some(alias) = aliases.last_mut() {
alias.comma = trailing_comma;
let has_comment = if let Some(comma) = &alias.comma {
match &comma.whitespace_after {
ParenthesizableWhitespace::SimpleWhitespace(_) => false,
ParenthesizableWhitespace::ParenthesizedWhitespace(whitespace) => {
whitespace.first_line.comment.is_some()
}
}
} else {
false
};
if !has_comment {
alias.comma = trailing_comma;
}
}
if aliases.is_empty() {