Skip to content

Commit

Permalink
Avoid attempting infinite open fix with re-bound builtin (#3650)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 21, 2023
1 parent 33394e4 commit 3b1709b
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 11 deletions.
8 changes: 4 additions & 4 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP020.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from io import open
import io

with open("f.txt") as f:
with io.open("f.txt", mode="r", buffering=-1, **kwargs) as f:
print(f.read())

import io
from io import open

with io.open("f.txt", mode="r", buffering=-1, **kwargs) as f:
with open("f.txt") as f:
print(f.read())
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pyupgrade/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ mod tests {
#[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"); "UP015")]
#[test_case(Rule::NativeLiterals, Path::new("UP018.py"); "UP018")]
#[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"); "UP019")]
#[test_case(Rule::OpenAlias, Path::new("UP020.py"); "UP020")]
#[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"); "UP021")]
#[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"); "UP022")]
#[test_case(Rule::DeprecatedCElementTree, Path::new("UP023.py"); "UP023")]
Expand Down
23 changes: 16 additions & 7 deletions crates/ruff/src/rules/pyupgrade/rules/open_alias.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
use rustpython_parser::ast::Expr;

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

#[violation]
pub struct OpenAlias;
pub struct OpenAlias {
pub fixable: bool,
}

impl Violation for OpenAlias {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;

impl AlwaysAutofixableViolation for OpenAlias {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use builtin `open`")
}

fn autofix_title(&self) -> String {
"Replace with builtin `open`".to_string()
fn autofix_title_formatter(&self) -> Option<fn(&Self) -> String> {
self.fixable
.then_some(|_| format!("Replace with builtin `open`"))
}
}

Expand All @@ -28,8 +33,12 @@ pub fn open_alias(checker: &mut Checker, expr: &Expr, func: &Expr) {
.resolve_call_path(func)
.map_or(false, |call_path| call_path.as_slice() == ["io", "open"])
{
let mut diagnostic = Diagnostic::new(OpenAlias, Range::from(expr));
if checker.patch(diagnostic.kind.rule()) {
let fixable = checker
.ctx
.find_binding("open")
.map_or(true, |binding| binding.kind.is_builtin());
let mut diagnostic = Diagnostic::new(OpenAlias { fixable }, Range::from(expr));
if fixable && checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
"open".to_string(),
func.location,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
expression: diagnostics
---
- kind:
name: OpenAlias
body: "Use builtin `open`"
suggestion: "Replace with builtin `open`"
fixable: true
location:
row: 3
column: 5
end_location:
row: 3
column: 55
fix:
content: open
location:
row: 3
column: 5
end_location:
row: 3
column: 12
parent: ~
- kind:
name: OpenAlias
body: "Use builtin `open`"
suggestion: ~
fixable: false
location:
row: 8
column: 5
end_location:
row: 8
column: 18
fix: ~
parent: ~

0 comments on commit 3b1709b

Please sign in to comment.