Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint]: bad-str-strip-call #2570

Merged
merged 10 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Got working MVP
  • Loading branch information
colin99d committed Feb 4, 2023
commit 0e74a5657aa7251d663e6a1dd4ff8fb680c290ee
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/) on PyPI.
| PLE0604 | invalid-all-object | Invalid object in `__all__`, must contain only strings | |
| PLE0605 | invalid-all-format | Invalid format for `__all__`, must be `tuple` or `list` | |
| PLE1142 | await-outside-async | `await` should be used within an async function | |
| PLE1310 | bad-str-strip-call | Strip string contains duplicate characters | 🛠 |

#### Refactor (PLR)

Expand Down
1 change: 1 addition & 0 deletions resources/test/fixtures/pylint/bad_str_strip_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"Hello World".strip("Hello")
3 changes: 3 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,9 @@
"PLE11",
"PLE114",
"PLE1142",
"PLE13",
"PLE131",
"PLE1310",
"PLR",
"PLR0",
"PLR01",
Expand Down
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2614,6 +2614,9 @@ where
if self.settings.rules.enabled(&Rule::ConsiderUsingSysExit) {
pylint::rules::consider_using_sys_exit(self, func);
}
if self.settings.rules.enabled(&Rule::BadStrStripCall) {
pylint::rules::bad_str_strip_call(self, func, args);
}

// flake8-pytest-style
if self.settings.rules.enabled(&Rule::PatchWithLambda) {
Expand Down
1 change: 1 addition & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ ruff_macros::define_rule_mapping!(
// pylint
PLE0604 => rules::pylint::rules::InvalidAllObject,
PLE0605 => rules::pylint::rules::InvalidAllFormat,
PLE1310 => rules::pylint::rules::BadStrStripCall,
PLC0414 => rules::pylint::rules::UselessImportAlias,
PLC3002 => rules::pylint::rules::UnnecessaryDirectLambdaCall,
PLE0117 => rules::pylint::rules::NonlocalWithoutBinding,
Expand Down
79 changes: 79 additions & 0 deletions src/rules/pylint/rules/bad_str_strip_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
use crate::define_violation;
use crate::violation::AlwaysAutofixableViolation;
use ruff_macros::derive_message_formats;
use rustpython_ast::{Constant, Expr, ExprKind};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::rules::pydocstyle::helpers::leading_quote;

const STRIPS: &[&str] = &["strip", "lstrip", "rstrip"];

define_violation!(
pub struct BadStrStripCall;
);
impl AlwaysAutofixableViolation for BadStrStripCall {
#[derive_message_formats]
fn message(&self) -> String {
format!("Strip string contains duplicate characters")
}

fn autofix_title(&self) -> String {
"Removed duplicated characters".to_string()
}
}

fn remove_duplicates(s: &str) -> String {
let mut set = std::collections::HashSet::new();
let mut result = String::new();
for c in s.chars() {
if set.insert(c) {
result.push(c);
}
}
result
}

/// PLE1310
pub fn bad_str_strip_call(checker: &mut Checker, func: &Expr, args: &[Expr]) {
if let ExprKind::Attribute { value, attr, .. } = &func.node {
if let ExprKind::Constant {
value: Constant::Str(_),
..
} = &value.node
{
if STRIPS.contains(&attr.as_str()) {
if let Some(arg) = args.get(0) {
if let ExprKind::Constant {
value: Constant::Str(item),
..
} = &arg.node
{
let cleaned = remove_duplicates(item);
let module_text = checker
.locator
.slice_source_code_range(&Range::from_located(arg));
let quote = match leading_quote(module_text) {
Some(item) => item,
None => return,
};
if &cleaned != item {
let mut diagnostic =
Diagnostic::new(BadStrStripCall, Range::from_located(arg));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
format!("{quote}{cleaned}{quote}"),
arg.location,
arg.end_location.unwrap(),
));
};
checker.diagnostics.push(diagnostic);
}
}
}
}
}
}
}
2 changes: 2 additions & 0 deletions src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub use await_outside_async::{await_outside_async, AwaitOutsideAsync};
pub use bad_str_strip_call::{bad_str_strip_call, BadStrStripCall};
pub use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant};
pub use consider_using_sys_exit::{consider_using_sys_exit, ConsiderUsingSysExit};
pub use global_variable_not_assigned::GlobalVariableNotAssigned;
Expand All @@ -21,6 +22,7 @@ pub use useless_else_on_loop::{useless_else_on_loop, UselessElseOnLoop};
pub use useless_import_alias::{useless_import_alias, UselessImportAlias};

mod await_outside_async;
mod bad_str_strip_call;
mod comparison_of_constant;
mod consider_using_sys_exit;
mod global_variable_not_assigned;
Expand Down