moved lists and dicts to preview

astral-sh · Dec 17, 2023 · 7879f9e · 7879f9e
1 parent 935b77e
commit 7879f9e
Show file tree

Hide file tree

Showing 5 changed files with 652 additions and 306 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py
@@ -1,6 +1,5 @@
 # Errors
 "yoda" == compare  # SIM300
-"yoda" == compare  # SIM300
 42 == age  # SIM300
 ("a", "b") == compare  # SIM300
 "yoda" <= compare  # SIM300
@@ -15,8 +14,14 @@
 0 < (number - 100)  # SIM300
 B<A[0][0]or B
 B or(B)<A[0][0]
-['upper'] == UPPER_LIST
-{} == DummyHandler.CONFIG
+
+# Errors in preview
+['upper'] == UPPER_LIST 
+{} == DummyHandler.CONFIG 
+
+# Errors in stable
+UPPER_LIST == ['upper']
+DummyHandler.CONFIG == {}
 
 # OK
 compare == "yoda"
@@ -32,8 +37,7 @@
 YODA == YODA
 age == JediOrder.YODA
 (number - 100) > 0
-UPPER_LIST == ['upper']
-DummyHandler.CONFIG == {}
-{"thats": "acceptable"} == DummyHandler.CONFIG
-SECONDS_IN_DAY == 60 * 60 * 24
-SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)
+SECONDS_IN_DAY == 60 * 60 * 24 # Error in 0.1.8
+SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # Error in 0.1.8
+{"non-empty-dict": "is-ok"} == DummyHandler.CONFIG
+
diff --git a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs
@@ -56,6 +56,7 @@ mod tests {
     }
 
     #[test_case(Rule::InDictKeys, Path::new("SIM118.py"))]
+    #[test_case(Rule::YodaConditions, Path::new("SIM300.py"))]
     #[test_case(Rule::IfElseBlockInsteadOfDictGet, Path::new("SIM401.py"))]
     #[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {

diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs
@@ -92,43 +92,51 @@ enum ConstantLikelihood {
     /// The expression is a constant for certain (e.g., `42` or `"foo"`).
     Definitely = 2,
 }
-fn how_likely_constant_str(s: &str) -> ConstantLikelihood {
-    if str::is_cased_uppercase(s) {
-        ConstantLikelihood::Probably
-    } else {
-        ConstantLikelihood::Unlikely
-    }
-}
 
-/// Return [`Expr`] [`ConstantLikelihood`] level depending on simple heuristics.
-fn how_likely_constant(expr: &Expr) -> ConstantLikelihood {
-    if expr.is_literal_expr() {
-        return ConstantLikelihood::Definitely;
-    }
-    match expr {
-        Expr::Attribute(ast::ExprAttribute { attr, .. }) => how_likely_constant_str(attr),
-        Expr::Name(ast::ExprName { id, .. }) => how_likely_constant_str(id),
-        Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) => elts
-            .iter()
-            .map(how_likely_constant)
-            .min()
-            .unwrap_or(ConstantLikelihood::Definitely),
-        Expr::Dict(ast::ExprDict { values: vs, .. }) => {
-            if vs.is_empty() {
-                ConstantLikelihood::Definitely
-            } else {
-                ConstantLikelihood::Probably
+impl ConstantLikelihood {
+    fn from_expression(expr: &Expr, is_preview_enabled: bool) -> Self {
+        match expr {
+            _ if expr.is_literal_expr() => ConstantLikelihood::Definitely,
+            Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
+                ConstantLikelihood::from_identifier(attr)
             }
+            Expr::Name(ast::ExprName { id, .. }) => ConstantLikelihood::from_identifier(id),
+            Expr::Tuple(ast::ExprTuple { elts, .. }) => elts
+                .iter()
+                .map(|x| ConstantLikelihood::from_expression(x, is_preview_enabled))
+                .min()
+                .unwrap_or(ConstantLikelihood::Definitely),
+            Expr::List(ast::ExprList { elts, .. }) if is_preview_enabled => elts
+                .iter()
+                .map(|x| ConstantLikelihood::from_expression(x, is_preview_enabled))
+                .min()
+                .unwrap_or(ConstantLikelihood::Definitely),
+            Expr::Dict(ast::ExprDict { values: vs, .. }) if is_preview_enabled => {
+                if vs.is_empty() {
+                    ConstantLikelihood::Definitely
+                } else {
+                    ConstantLikelihood::Probably
+                }
+            }
+            Expr::BinOp(ast::ExprBinOp { left, right, .. }) => cmp::min(
+                ConstantLikelihood::from_expression(left, is_preview_enabled),
+                ConstantLikelihood::from_expression(right, is_preview_enabled),
+            ),
+            Expr::UnaryOp(ast::ExprUnaryOp {
+                op: UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert,
+                operand,
+                range: _,
+            }) => ConstantLikelihood::from_expression(operand, is_preview_enabled),
+            _ => ConstantLikelihood::Unlikely,
         }
-        Expr::BinOp(ast::ExprBinOp { left, right, .. }) => {
-            cmp::min(how_likely_constant(left), how_likely_constant(right))
+    }
+
+    fn from_identifier(identifier: &str) -> Self {
+        if str::is_cased_uppercase(identifier) {
+            ConstantLikelihood::Probably
+        } else {
+            ConstantLikelihood::Unlikely
         }
-        Expr::UnaryOp(ast::ExprUnaryOp {
-            op: UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert,
-            operand,
-            range: _,
-        }) => how_likely_constant(operand),
-        _ => ConstantLikelihood::Unlikely,
     }
 }
 
@@ -219,7 +227,9 @@ pub(crate) fn yoda_conditions(
         return;
     }
 
-    if how_likely_constant(left) <= how_likely_constant(right) {
+    if ConstantLikelihood::from_expression(left, checker.settings.preview.is_enabled())
+        <= ConstantLikelihood::from_expression(right, checker.settings.preview.is_enabled())
+    {
         return;
     }