diff --git a/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py b/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py index fbf3abd1e9..59f2bbccef 100644 --- a/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py +++ b/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py @@ -1,11 +1,13 @@ x = "a string" y = "another string" -if x is "" or x == "": - print("x is an empty string") +def should_raise_lint_warning(): + 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 y is not "" or y != "": + print("y is not an empty string") -if x and not y: - print("x is not an empty string, but y is an empty string") +def should_not_raise_lint_warning(): + if x and not y: + print("x is not an empty string, but y is an empty string") diff --git a/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs b/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs index f05431230e..13cbb68a36 100644 --- a/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs +++ b/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs @@ -1,3 +1,4 @@ +use anyhow::anyhow; use itertools::Itertools; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::{unparse_constant, unparse_expr}; @@ -5,27 +6,59 @@ use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Located}; use crate::{checkers::ast::Checker, registry::Diagnostic, violation::Violation}; -use super::comparison_of_constant::ViolationsCmpop; +#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub enum EmptyStringViolationsCmpop { + Is, + IsNot, + Eq, + NotEq, +} + +impl TryFrom<&Cmpop> for EmptyStringViolationsCmpop { + type Error = anyhow::Error; + + fn try_from(value: &Cmpop) -> Result { + match value { + Cmpop::Is => Ok(Self::Is), + Cmpop::IsNot => Ok(Self::IsNot), + Cmpop::Eq => Ok(Self::Eq), + Cmpop::NotEq => Ok(Self::NotEq), + _ => Err(anyhow!( + "{value:?} cannot be converted to EmptyStringViolationsCmpop" + )), + } + } +} + +impl std::fmt::Display for EmptyStringViolationsCmpop { + 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 lhs: String, - pub op: ViolationsCmpop, + pub op: EmptyStringViolationsCmpop, pub rhs: String, } impl Violation for CompareToEmptyString { #[derive_message_formats] fn message(&self) -> String { - let cond = match self.op { - ViolationsCmpop::Is | ViolationsCmpop::Eq => "", - // the assumption is that this message will only ever be shown - // if op is `in`, `=`, `not in`, `!=` - _ => "not", + let prefix = match self.op { + EmptyStringViolationsCmpop::Is | EmptyStringViolationsCmpop::Eq => "", + EmptyStringViolationsCmpop::IsNot | EmptyStringViolationsCmpop::NotEq => "not ", }; format!( - "{} {} {} can be simplified to {} {} as strings are falsey", - self.lhs, self.op, self.rhs, cond, self.lhs + "`{} {} {}` can be simplified to `{}{}` as an empty string is falsey", + self.lhs, self.op, self.rhs, prefix, self.lhs ) } } @@ -48,7 +81,10 @@ pub fn compare_to_empty_string( let diag = Diagnostic::new( CompareToEmptyString { lhs: unparse_expr(lhs, checker.stylist), - op: op.into(), + // we know `op` can be safely converted into a + // EmptyStringViolationCmpop due to the first if statement being + // true in this branch + op: op.try_into().unwrap(), rhs: unparse_constant(value, checker.stylist), }, lhs.into(), diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC1901_compare_to_empty_string.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC1901_compare_to_empty_string.py.snap index 7a7db84792..7cb120be29 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC1901_compare_to_empty_string.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC1901_compare_to_empty_string.py.snap @@ -3,55 +3,55 @@ source: crates/ruff/src/rules/pylint/mod.rs expression: diagnostics --- - kind: - CompareToEmptyString: - lhs: x - op: Is - rhs: "\"\"" + name: CompareToEmptyString + body: "`x is \"\"` can be simplified to `x` as an empty string is falsey" + suggestion: ~ + fixable: false location: - row: 4 - column: 3 + row: 5 + column: 7 end_location: - row: 4 - column: 4 + row: 5 + column: 8 fix: ~ parent: ~ - kind: - CompareToEmptyString: - lhs: x - op: Eq - rhs: "\"\"" + name: CompareToEmptyString + body: "`x == \"\"` can be simplified to `x` as an empty string is falsey" + suggestion: ~ + fixable: false location: - row: 4 - column: 14 - end_location: - row: 4 - column: 15 - fix: ~ - parent: ~ -- kind: - CompareToEmptyString: - lhs: y - op: IsNot - rhs: "\"\"" - location: - row: 7 - column: 3 - end_location: - row: 7 - column: 4 - fix: ~ - parent: ~ -- kind: - CompareToEmptyString: - lhs: y - op: NotEq - rhs: "\"\"" - location: - row: 7 + row: 5 column: 18 end_location: - row: 7 + row: 5 column: 19 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: 8 + column: 7 + end_location: + row: 8 + column: 8 + fix: ~ + parent: ~ +- kind: + name: CompareToEmptyString + body: "`y != \"\"` can be simplified to `not y` as an empty string is falsey" + suggestion: ~ + fixable: false + location: + row: 8 + column: 22 + end_location: + row: 8 + column: 23 + fix: ~ + parent: ~