From ca26f664ec2b110b8d84315331a1c76b080aa3bc Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Fri, 27 Jan 2023 17:20:21 +0100 Subject: [PATCH] Fix SIM300 to take Python constants into account (#2255) SIM300 currently doesn't take Python constants into account when looking for Yoda conditions, this PR fixes that behavior. ```python # Errors YODA == age # SIM300 YODA > age # SIM300 YODA >= age # SIM300 # OK age == YODA age < YODA age <= YODA ``` Ref: --- .../test/fixtures/flake8_simplify/SIM300.py | 7 +++ .../flake8_simplify/rules/yoda_conditions.rs | 16 ++++-- ...ke8_simplify__tests__SIM300_SIM300.py.snap | 57 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/resources/test/fixtures/flake8_simplify/SIM300.py b/resources/test/fixtures/flake8_simplify/SIM300.py index 6aac1c20f4..a573cc5fc2 100644 --- a/resources/test/fixtures/flake8_simplify/SIM300.py +++ b/resources/test/fixtures/flake8_simplify/SIM300.py @@ -5,6 +5,9 @@ "yoda" <= compare # SIM300 'yoda' < compare # SIM300 42 > age # SIM300 +YODA == age # SIM300 +YODA > age # SIM300 +YODA >= age # SIM300 # OK compare == "yoda" @@ -13,3 +16,7 @@ x == y "yoda" == compare == 1 "yoda" == compare == someothervar "yoda" == "yoda" +age == YODA +age < YODA +age <= YODA +YODA == YODA diff --git a/src/rules/flake8_simplify/rules/yoda_conditions.rs b/src/rules/flake8_simplify/rules/yoda_conditions.rs index bab8795843..3c5f7871a3 100644 --- a/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -3,9 +3,19 @@ use rustpython_ast::{Cmpop, Expr, ExprKind}; use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::fix::Fix; +use crate::python::string::{self}; use crate::registry::Diagnostic; use crate::violations; +/// Return `true` if an [`Expr`] is a constant or a constant-like name. +fn is_constant(expr: &Expr) -> bool { + match &expr.node { + ExprKind::Constant { .. } => true, + ExprKind::Name { id, .. } => string::is_upper(id), + _ => false, + } +} + /// SIM300 pub fn yoda_conditions( checker: &mut Checker, @@ -24,10 +34,8 @@ pub fn yoda_conditions( ) { return; } - if !matches!(&left.node, &ExprKind::Constant { .. }) { - return; - } - if matches!(&right.node, &ExprKind::Constant { .. }) { + + if !is_constant(left) || is_constant(right) { return; } diff --git a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap index 578f0dc772..3fc55c11c3 100644 --- a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap +++ b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap @@ -116,4 +116,61 @@ expression: diagnostics row: 7 column: 8 parent: ~ +- kind: + YodaConditions: + suggestion: age == YODA + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 11 + fix: + content: + - age == YODA + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 11 + parent: ~ +- kind: + YodaConditions: + suggestion: age < YODA + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 10 + fix: + content: + - age < YODA + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 10 + parent: ~ +- kind: + YodaConditions: + suggestion: age <= YODA + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 11 + fix: + content: + - age <= YODA + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 11 + parent: ~