[flake8-simplify] Stabilize detection of Yoda conditions for "constant" collections (SIM300) (#12050)

Co-authored-by: Alex Waygood <[email protected]>

Author: 2 people

crates/ruff_linter/src/rules/flake8_simplify/mod.rs

@@ -9,7 +9,6 @@ mod tests {
     use test_case::test_case;
 
     use crate::registry::Rule;
-    use crate::settings::types::PreviewMode;
     use crate::test::test_path;
     use crate::{assert_messages, settings};
 
@@ -55,22 +54,4 @@ mod tests {
         assert_messages!(snapshot, diagnostics);
         Ok(())
     }
-
-    #[test_case(Rule::YodaConditions, Path::new("SIM300.py"))]
-    fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!(
-            "preview__{}_{}",
-            rule_code.noqa_code(),
-            path.to_string_lossy()
-        );
-        let diagnostics = test_path(
-            Path::new("flake8_simplify").join(path).as_path(),
-            &settings::LinterSettings {
-                preview: PreviewMode::Enabled,
-                ..settings::LinterSettings::for_rule(rule_code)
-            },
-        )?;
-        assert_messages!(snapshot, diagnostics);
-        Ok(())
-    }
 }

crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs

@@ -16,7 +16,6 @@ use crate::cst::helpers::or_space;
 use crate::cst::matchers::{match_comparison, transform_expression};
 use crate::fix::edits::pad;
 use crate::fix::snippet::SourceCodeSnippet;
-use crate::settings::types::PreviewMode;
 
 /// ## What it does
 /// Checks for conditions that position a constant on the left-hand side of the
@@ -58,26 +57,15 @@ impl Violation for YodaConditions {
 
     #[derive_message_formats]
     fn message(&self) -> String {
-        let YodaConditions { suggestion } = self;
-        if let Some(suggestion) = suggestion
-            .as_ref()
-            .and_then(SourceCodeSnippet::full_display)
-        {
-            format!("Yoda conditions are discouraged, use `{suggestion}` instead")
-        } else {
-            format!("Yoda conditions are discouraged")
-        }
+        format!("Yoda condition detected")
     }
 
     fn fix_title(&self) -> Option<String> {
         let YodaConditions { suggestion } = self;
-        suggestion.as_ref().map(|suggestion| {
-            if let Some(suggestion) = suggestion.full_display() {
-                format!("Replace Yoda condition with `{suggestion}`")
-            } else {
-                format!("Replace Yoda condition")
-            }
-        })
+        suggestion
+            .as_ref()
+            .and_then(|suggestion| suggestion.full_display())
+            .map(|suggestion| format!("Rewrite as `{suggestion}`"))
     }
 }
 
@@ -94,9 +82,9 @@ enum ConstantLikelihood {
     Definitely = 2,
 }
 
-impl ConstantLikelihood {
+impl From<&Expr> for ConstantLikelihood {
     /// Determine the [`ConstantLikelihood`] of an expression.
-    fn from_expression(expr: &Expr, preview: PreviewMode) -> Self {
+    fn from(expr: &Expr) -> Self {
         match expr {
             _ if expr.is_literal_expr() => ConstantLikelihood::Definitely,
             Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
@@ -105,34 +93,36 @@ impl ConstantLikelihood {
             Expr::Name(ast::ExprName { id, .. }) => ConstantLikelihood::from_identifier(id),
             Expr::Tuple(ast::ExprTuple { elts, .. }) => elts
                 .iter()
-                .map(|expr| ConstantLikelihood::from_expression(expr, preview))
+                .map(ConstantLikelihood::from)
                 .min()
                 .unwrap_or(ConstantLikelihood::Definitely),
-            Expr::List(ast::ExprList { elts, .. }) if preview.is_enabled() => elts
+            Expr::List(ast::ExprList { elts, .. }) => elts
                 .iter()
-                .map(|expr| ConstantLikelihood::from_expression(expr, preview))
+                .map(ConstantLikelihood::from)
                 .min()
                 .unwrap_or(ConstantLikelihood::Definitely),
-            Expr::Dict(ast::ExprDict { items, .. }) if preview.is_enabled() => {
+            Expr::Dict(ast::ExprDict { items, .. }) => {
                 if items.is_empty() {
                     ConstantLikelihood::Definitely
                 } else {
                     ConstantLikelihood::Probably
                 }
             }
             Expr::BinOp(ast::ExprBinOp { left, right, .. }) => cmp::min(
-                ConstantLikelihood::from_expression(left, preview),
-                ConstantLikelihood::from_expression(right, preview),
+                ConstantLikelihood::from(&**left),
+                ConstantLikelihood::from(&**right),
             ),
             Expr::UnaryOp(ast::ExprUnaryOp {
                 op: UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert,
                 operand,
                 range: _,
-            }) => ConstantLikelihood::from_expression(operand, preview),
+            }) => ConstantLikelihood::from(&**operand),
             _ => ConstantLikelihood::Unlikely,
         }
     }
+}
 
+impl ConstantLikelihood {
     /// Determine the [`ConstantLikelihood`] of an identifier.
     fn from_identifier(identifier: &str) -> Self {
         if str::is_cased_uppercase(identifier) {
@@ -230,9 +220,7 @@ pub(crate) fn yoda_conditions(
         return;
     }
 
-    if ConstantLikelihood::from_expression(left, checker.settings.preview)
-        <= ConstantLikelihood::from_expression(right, checker.settings.preview)
-    {
+    if ConstantLikelihood::from(left) <= ConstantLikelihood::from(right) {
         return;
     }