Compare commits

...

8 Commits
0.8.0 ... C1901

Author SHA1 Message Date
Charlie Marsh
7460ca28dd Fixup 2023-03-09 16:25:38 -05:00
AreamanM
56e8a4fd14 cleanup plc1901 impl 2023-03-09 15:49:09 -05:00
AreamanM
67444143b5 merge with main 2023-03-09 15:49:09 -05:00
AreamanM
daeb3ff37e fix cargo fmt warning 2023-03-09 15:49:09 -05:00
AreamanM
2f3734dd22 update test for plc1901 2023-03-09 15:49:09 -05:00
AreamanM
fc50d28fcf cleanup impl for plc1901 2023-03-09 15:49:09 -05:00
AreamanM
158dc5e7d4 update crate::ast:: to ruff_python_ast:: 2023-03-09 15:49:09 -05:00
AreamanM
9a42be8a90 rough implementation of c1901 2023-03-09 15:49:09 -05:00
9 changed files with 238 additions and 0 deletions

View File

@@ -0,0 +1,19 @@
x = "a string"
y = "another string"
z = ""
def errors():
if x is "" or x == "":
print("x is an empty string")
if y is not "" or y != "":
print("y is not an empty string")
if "" != z:
print("z is an empty string")
def ok():
if x and not y:
print("x is not an empty string, but y is an empty string")

View File

@@ -3299,6 +3299,10 @@ where
pylint::rules::comparison_of_constant(self, left, ops, comparators);
}
if self.settings.rules.enabled(&Rule::CompareToEmptyString) {
pylint::rules::compare_to_empty_string(self, left, ops, comparators);
}
if self.settings.rules.enabled(&Rule::MagicValueComparison) {
pylint::rules::magic_value_comparison(self, left, comparators);
}

View File

@@ -177,6 +177,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Pylint, "E1142") => Rule::AwaitOutsideAsync,
(Pylint, "R0206") => Rule::PropertyWithParameters,
(Pylint, "R0402") => Rule::ConsiderUsingFromImport,
(Pylint, "C1901") => Rule::CompareToEmptyString,
(Pylint, "R0133") => Rule::ComparisonOfConstant,
(Pylint, "R1701") => Rule::ConsiderMergingIsinstance,
(Pylint, "R1722") => Rule::ConsiderUsingSysExit,

View File

@@ -158,6 +158,7 @@ ruff_macros::register_rules!(
rules::pylint::rules::PropertyWithParameters,
rules::pylint::rules::ReturnInInit,
rules::pylint::rules::ConsiderUsingFromImport,
rules::pylint::rules::CompareToEmptyString,
rules::pylint::rules::ComparisonOfConstant,
rules::pylint::rules::ConsiderMergingIsinstance,
rules::pylint::rules::ConsiderUsingSysExit,

View File

@@ -25,6 +25,7 @@ mod tests {
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"); "PLE0117")]
#[test_case(Rule::UsedPriorGlobalDeclaration, Path::new("used_prior_global_declaration.py"); "PLE0118")]
#[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async.py"); "PLE1142")]
#[test_case(Rule::CompareToEmptyString, Path::new("compare_to_empty_string.py"); "PLC1901")]
#[test_case(Rule::ComparisonOfConstant, Path::new("comparison_of_constant.py"); "PLR0133")]
#[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"); "PLR0206")]
#[test_case(Rule::ConsiderUsingFromImport, Path::new("import_aliasing.py"); "PLR0402")]

View File

@@ -0,0 +1,136 @@
use anyhow::bail;
use itertools::Itertools;
use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{unparse_constant, unparse_expr};
use ruff_python_ast::types::Range;
use crate::checkers::ast::Checker;
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum EmptyStringCmpop {
Is,
IsNot,
Eq,
NotEq,
}
impl TryFrom<&Cmpop> for EmptyStringCmpop {
type Error = anyhow::Error;
fn try_from(value: &Cmpop) -> Result<Self, Self::Error> {
match value {
Cmpop::Is => Ok(Self::Is),
Cmpop::IsNot => Ok(Self::IsNot),
Cmpop::Eq => Ok(Self::Eq),
Cmpop::NotEq => Ok(Self::NotEq),
_ => bail!("{value:?} cannot be converted to EmptyStringCmpop"),
}
}
}
impl EmptyStringCmpop {
pub fn into_unary(self) -> &'static str {
match self {
Self::Is | Self::Eq => "",
Self::IsNot | Self::NotEq => "not ",
}
}
}
impl std::fmt::Display for EmptyStringCmpop {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let repr = match self {
Self::Is => "is",
Self::IsNot => "is not",
Self::Eq => "==",
Self::NotEq => "!=",
};
write!(f, "{repr}")
}
}
#[violation]
pub struct CompareToEmptyString {
pub existing: String,
pub replacement: String,
}
impl Violation for CompareToEmptyString {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"`{}` can be simplified to `{}` as an empty string is falsey",
self.existing, self.replacement,
)
}
}
pub fn compare_to_empty_string(
checker: &mut Checker,
left: &Expr,
ops: &[Cmpop],
comparators: &[Expr],
) {
let mut first = true;
for ((lhs, rhs), op) in std::iter::once(left)
.chain(comparators.iter())
.tuple_windows::<(&Expr<_>, &Expr<_>)>()
.zip(ops)
{
if let Ok(op) = EmptyStringCmpop::try_from(op) {
if std::mem::take(&mut first) {
// Check the left-most expression.
if let ExprKind::Constant { value, .. } = &lhs.node {
if let Constant::Str(s) = value {
if s.is_empty() {
let existing = format!(
"{} {} {}",
unparse_constant(value, checker.stylist),
op,
unparse_expr(rhs, checker.stylist)
);
let replacement = format!(
"{}{}",
op.into_unary(),
unparse_expr(rhs, checker.stylist)
);
checker.diagnostics.push(Diagnostic::new(
CompareToEmptyString {
existing,
replacement,
},
Range::from(lhs),
));
}
}
}
}
// Check all right-hand expressions.
if let ExprKind::Constant { value, .. } = &rhs.node {
if let Constant::Str(s) = value {
if s.is_empty() {
let existing = format!(
"{} {} {}",
unparse_expr(lhs, checker.stylist),
op,
unparse_constant(value, checker.stylist),
);
let replacement =
format!("{}{}", op.into_unary(), unparse_expr(lhs, checker.stylist));
checker.diagnostics.push(Diagnostic::new(
CompareToEmptyString {
existing,
replacement,
},
Range::from(rhs),
));
}
}
}
}
}
}

View File

@@ -3,6 +3,7 @@ pub use bad_str_strip_call::{bad_str_strip_call, BadStrStripCall};
pub use bad_string_format_type::{bad_string_format_type, BadStringFormatType};
pub use bidirectional_unicode::{bidirectional_unicode, BidirectionalUnicode};
pub use collapsible_else_if::{collapsible_else_if, CollapsibleElseIf};
pub use compare_to_empty_string::{compare_to_empty_string, CompareToEmptyString};
pub use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant};
pub use consider_using_sys_exit::{consider_using_sys_exit, ConsiderUsingSysExit};
pub use global_statement::{global_statement, GlobalStatement};
@@ -36,6 +37,7 @@ mod bad_str_strip_call;
mod bad_string_format_type;
mod bidirectional_unicode;
mod collapsible_else_if;
mod compare_to_empty_string;
mod comparison_of_constant;
mod consider_using_sys_exit;
mod global_statement;

View File

@@ -0,0 +1,70 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
name: CompareToEmptyString
body: "`x is \"\"` can be simplified to `x` as an empty string is falsey"
suggestion: ~
fixable: false
location:
row: 7
column: 12
end_location:
row: 7
column: 14
fix: ~
parent: ~
- kind:
name: CompareToEmptyString
body: "`x == \"\"` can be simplified to `x` as an empty string is falsey"
suggestion: ~
fixable: false
location:
row: 7
column: 23
end_location:
row: 7
column: 25
fix: ~
parent: ~
- kind:
name: CompareToEmptyString
body: "`y is not \"\"` can be simplified to `not y` as an empty string is falsey"
suggestion: ~
fixable: false
location:
row: 10
column: 16
end_location:
row: 10
column: 18
fix: ~
parent: ~
- kind:
name: CompareToEmptyString
body: "`y != \"\"` can be simplified to `not y` as an empty string is falsey"
suggestion: ~
fixable: false
location:
row: 10
column: 27
end_location:
row: 10
column: 29
fix: ~
parent: ~
- kind:
name: CompareToEmptyString
body: "`\"\" != z` can be simplified to `not z` as an empty string is falsey"
suggestion: ~
fixable: false
location:
row: 13
column: 7
end_location:
row: 13
column: 9
fix: ~
parent: ~

4
ruff.schema.json generated
View File

@@ -1817,6 +1817,10 @@
"PLC04",
"PLC041",
"PLC0414",
"PLC1",
"PLC19",
"PLC190",
"PLC1901",
"PLC3",
"PLC30",
"PLC300",