Skip to content

Commit

Permalink
Avoid re-using imports beyond current edit size
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 11, 2023
1 parent 51278dd commit 410e489
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 21 deletions.
16 changes: 16 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
"""Case: `contextlib` is imported after the call site."""


def foo():
pass


def bar():
# SIM105
try:
foo()
except ValueError:
pass


import contextlib
23 changes: 14 additions & 9 deletions crates/ruff/src/autofix/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,18 @@ pub fn get_or_import_symbol(
) -> Result<(Edit, String)> {
if let Some((source, binding)) = context.resolve_qualified_import_name(module, member) {
// If the symbol is already available in the current scope, use it.
//

// The exception: the symbol source (i.e., the import statement) comes after the current
// location. For example, we could be generating an edit within a function, and the import
// could be defined in the module scope, but after the function definition. In this case,
// it's unclear whether we can use the symbol (the function could be called between the
// import and the current location, and thus the symbol would not be available). It's also
// unclear whether should add an import statement at the top of the file, since it could
// be shadowed between the import and the current location.
if source.start() > at {
bail!("Unable to use existing symbol `{binding}` due to late-import");
}

// We also add a no-op edit to force conflicts with any other fixes that might try to
// remove the import. Consider:
//
Expand Down Expand Up @@ -476,10 +487,7 @@ pub fn get_or_import_symbol(
let import_edit = importer.add_member(stmt, member)?;
Ok((import_edit, member.to_string()))
} else {
bail!(
"Unable to insert `{}` into scope due to name conflict",
member
)
bail!("Unable to insert `{member}` into scope due to name conflict")
}
} else {
// Case 2: No `functools` import is in scope; thus, we add `import functools`, and
Expand All @@ -492,10 +500,7 @@ pub fn get_or_import_symbol(
importer.add_import(&AnyImport::Import(Import::module(module)), at);
Ok((import_edit, format!("{module}.{member}")))
} else {
bail!(
"Unable to insert `{}` into scope due to name conflict",
module
)
bail!("Unable to insert `{module}` into scope due to name conflict")
}
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_simplify/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod tests {
#[test_case(Rule::SuppressibleException, Path::new("SIM105_0.py"); "SIM105_0")]
#[test_case(Rule::SuppressibleException, Path::new("SIM105_1.py"); "SIM105_1")]
#[test_case(Rule::SuppressibleException, Path::new("SIM105_2.py"); "SIM105_2")]
#[test_case(Rule::SuppressibleException, Path::new("SIM105_3.py"); "SIM105_3")]
#[test_case(Rule::ReturnInTryExceptFinally, Path::new("SIM107.py"); "SIM107")]
#[test_case(Rule::IfElseBlockInsteadOfIfExp, Path::new("SIM108.py"); "SIM108")]
#[test_case(Rule::CompareWithTuple, Path::new("SIM109.py"); "SIM109")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ pub fn suppressible_exception(
stmt.range(),
);

println!("stmt: {:?}", stmt);
println!("handler: {:?}", handler);

if fixable && checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
let (import_edit, binding) = get_or_import_symbol(
Expand All @@ -106,7 +103,7 @@ pub fn suppressible_exception(
&checker.importer,
checker.locator,
)?;
println!("binding: {:?}", binding);

let replace_try = Edit::range_replacement(
format!("with {binding}({exception})"),
TextRange::at(stmt.start(), "try".text_len()),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/ruff/src/rules/flake8_simplify/mod.rs
---
SIM105_3.py:10:5: SIM105 Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass`
|
10 | def bar():
11 | # SIM105
12 | try:
| _____^
13 | | foo()
14 | | except ValueError:
15 | | pass
| |____________^ SIM105
|
= help: Replace with `contextlib.suppress(ValueError)`


8 changes: 0 additions & 8 deletions foo.py

This file was deleted.

0 comments on commit 410e489

Please sign in to comment.