From fc50d28fcf4b9dcba49bb5490d0db9e9d57e69a5 Mon Sep 17 00:00:00 2001
From: AreamanM <aryaman.marathe@gmail.com>
Date: Wed, 8 Mar 2023 22:46:48 +0000
Subject: [PATCH] cleanup impl for plc1901

---
 .../pylint/rules/compare_to_empty_string.rs   | 46 ++++++++++++++-----
 1 file changed, 35 insertions(+), 11 deletions(-)

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 864e4b422c..2a0e835361 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,20 +1,36 @@
 use itertools::Itertools;
 use ruff_macros::{derive_message_formats, violation};
-use rustpython_parser::ast::{Cmpop, Expr, ExprKind, Located};
-
 use ruff_python_ast::helpers::unparse_constant;
+use ruff_python_ast::helpers::unparse_expr;
 use ruff_python_ast::types::Range;
+use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Located};
+
 use crate::checkers::ast::Checker;
 use crate::registry::Diagnostic;
 use crate::violation::Violation;
 
+use super::comparison_of_constant::ViolationsCmpop;
+
 #[violation]
-pub struct CompareToEmptyString;
+pub struct CompareToEmptyString {
+    pub lhs: String,
+    pub op: ViolationsCmpop,
+    pub rhs: String,
+}
 
 impl Violation for CompareToEmptyString {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("todo")
+        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",
+        };
+        format!(
+            "{} {} {} can be simplified to {} {} as strings are falsey",
+            self.lhs, self.op, self.rhs, cond, self.lhs
+        )
     }
 }
 
@@ -24,17 +40,25 @@ pub fn compare_to_empty_string(
     ops: &[Cmpop],
     comparators: &[Expr],
 ) {
-    for ((left, rhs), op) in std::iter::once(left)
+    for ((lhs, rhs), op) in std::iter::once(left)
         .chain(comparators.iter())
         .tuple_windows::<(&Located<_>, &Located<_>)>()
         .zip(ops)
     {
-        if matches!(op, Cmpop::Eq | Cmpop::NotEq) {
-            if let ExprKind::Constant { value: v, .. } = &rhs.node {
-                let k = unparse_constant(v, checker.stylist);
-                if k == "\"\"" || k == "''" {
-                    let diag = Diagnostic::new(CompareToEmptyString {}, Range::from_located(left));
-                    checker.diagnostics.push(diag);
+        if matches!(op, Cmpop::Eq | Cmpop::NotEq | Cmpop::Is | Cmpop::IsNot) {
+            if let ExprKind::Constant { value, .. } = &rhs.node {
+                if let Constant::Str(s) = value {
+                    if s.is_empty() {
+                        let diag = Diagnostic::new(
+                            CompareToEmptyString {
+                                lhs: unparse_expr(lhs, checker.stylist),
+                                op: op.into(),
+                                rhs: unparse_constant(value, checker.stylist),
+                            },
+                            Range::from_located(lhs)
+                        );
+                        checker.diagnostics.push(diag);
+                    }
                 }
             }
         }