Skip to content

Commit

Permalink
Add generic add_argument util and replace fix with call to it
Browse files Browse the repository at this point in the history
  • Loading branch information
qdegraaf committed Dec 1, 2023
1 parent fec762c commit 5e7be98
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ def func(*args, **kwargs):
tempfile.SpooledTemporaryFile(0, "w", -1, "utf-8")
tempfile.SpooledTemporaryFile(0, "wb")
tempfile.SpooledTemporaryFile(0, )

open("test.txt",)
66 changes: 65 additions & 1 deletion crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! Interface for generating fix edits from higher-level actions (e.g., "remove an argument").
use anyhow::{Context, Result};
use itertools::{Either, Itertools};

use ruff_diagnostics::Edit;
use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt};
use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_trivia::{
Expand Down Expand Up @@ -138,6 +139,69 @@ pub(crate) fn remove_argument<T: Ranged>(
}
}

#[derive(Debug, Copy, Clone)]
pub(crate) enum ArgumentType {
Keyword,
}

/// Generic function to add arguments or keyword arguments to function
/// calls and class definitions. (For classes `args` should be considered
/// `bases`)
pub(crate) fn add_argument(
argument: &str,
arguments: &Arguments,
arg_type: ArgumentType,
source: &str,
) -> Result<Edit> {
// Partition into arguments before and after the argument to remove.
let (positional, keyword): (Vec<_>, Vec<_>) =
arguments
.arguments_source_order()
.partition_map(|arg| match arg {
ArgOrKeyword::Arg(_) => Either::Left(arg),
ArgOrKeyword::Keyword(_) => Either::Right(arg),
});

if positional.is_empty() && keyword.is_empty() {
// Case 1: no arguments. Add argument to call without comma
// TODO: Check no parentheses case
Ok(Edit::insertion(
argument.to_string(),
arguments.start() + TextSize::from(1),
))
} else {
match arg_type {
ArgumentType::Keyword => {
// Case 3: Keyword arg passed. Can be added at the end of call
// Look ahead back from last argument for a trailing comma
let Some(last_arg) = arguments.args.last() else {
panic!("No last argument found")
};
let mut tokenizer = SimpleTokenizer::starts_at(last_arg.range().end(), source);

// Find the next non-whitespace token.
let next = tokenizer
.find(|token| {
token.kind != SimpleTokenKind::Whitespace
&& token.kind != SimpleTokenKind::Newline
})
.context("Unable to find next token")?;

match next.kind {
SimpleTokenKind::Comma => Ok(Edit::insertion(
argument.to_string(),
arguments.end() - TextSize::from(1),
)),
_ => Ok(Edit::insertion(
format!(", {argument}"),
arguments.end() - TextSize::from(1),
)),
}
}
}
}
}

/// Determine if a vector contains only one, specific element.
fn is_only<T: PartialEq>(vec: &[T], value: &T) -> bool {
vec.len() == 1 && vec[0] == *value
Expand Down
44 changes: 30 additions & 14 deletions crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use anyhow::Result;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::call_path::{format_call_path, CallPath};
use ruff_python_ast::imports::{AnyImport, Import};
use ruff_text_size::{Ranged, TextSize};

use crate::checkers::ast::Checker;
use crate::fix::edits::add_argument;
use crate::fix::edits::ArgumentType::Keyword;
use crate::settings::types::PythonVersion;

/// ## What it does
Expand Down Expand Up @@ -85,25 +88,38 @@ pub(crate) fn unspecified_encoding(checker: &mut Checker, call: &ast::ExprCall)
);

if checker.settings.target_version >= PythonVersion::Py310 {
diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
", encoding=\"locale\"".to_string(),
call.range().end() - TextSize::from(1),
)));
diagnostic.try_set_fix(|| {
add_argument(
"encoding=\"locale\"",
&call.arguments,
Keyword,
checker.locator().contents(),
)
.map(Fix::unsafe_edit)
});
} else {
let import_edit = checker.importer().add_import(
&AnyImport::Import(Import::module("locale")),
TextSize::default(),
);
let call_edit = Edit::insertion(
", encoding=locale.getpreferredencoding(False)".to_string(),
call.range().end() - TextSize::from(1),
);
diagnostic.set_fix(Fix::unsafe_edits(import_edit, [call_edit]));
diagnostic.try_set_fix(|| generate_fix(checker, call));
}

checker.diagnostics.push(diagnostic);
}

/// Generate a [`Edit`] for Python39 and older.
fn generate_fix(checker: &Checker, call: &ast::ExprCall) -> Result<Fix> {
Ok(Fix::unsafe_edits(
checker.importer().add_import(
&AnyImport::Import(Import::module("locale")),
TextSize::default(),
),
[add_argument(
"encoding=locale.getpreferredencoding(False)",
&call.arguments,
Keyword,
checker.locator().contents(),
)?],
))
}

/// Returns `true` if the given expression is a string literal containing a `b` character.
fn is_binary_mode(expr: &ast::Expr) -> Option<bool> {
Some(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,20 @@ unspecified_encoding.py:14:1: PLW1514 [*] `tempfile.SpooledTemporaryFile` in tex
16 16 | # Non-errors.
17 17 | open("test.txt", encoding="utf-8")

unspecified_encoding.py:46:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
|
44 | tempfile.SpooledTemporaryFile(0, )
45 |
46 | open("test.txt",)
| ^^^^ PLW1514
|
= help: Add explicit `encoding` argument

Unsafe fix
43 43 | tempfile.SpooledTemporaryFile(0, "wb")
44 44 | tempfile.SpooledTemporaryFile(0, )
45 45 |
46 |-open("test.txt",)
46 |+open("test.txt",encoding="locale")


Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,25 @@ unspecified_encoding.py:14:1: PLW1514 [*] `tempfile.SpooledTemporaryFile` in tex
16 17 | # Non-errors.
17 18 | open("test.txt", encoding="utf-8")

unspecified_encoding.py:46:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
|
44 | tempfile.SpooledTemporaryFile(0, )
45 |
46 | open("test.txt",)
| ^^^^ PLW1514
|
= help: Add explicit `encoding` argument

Unsafe fix
1 |+import locale
1 2 | import io
2 3 | import sys
3 4 | import tempfile
--------------------------------------------------------------------------------
43 44 | tempfile.SpooledTemporaryFile(0, "wb")
44 45 | tempfile.SpooledTemporaryFile(0, )
45 46 |
46 |-open("test.txt",)
47 |+open("test.txt",encoding=locale.getpreferredencoding(False))


0 comments on commit 5e7be98

Please sign in to comment.