Skip to content

Commit

Permalink
[refurb] - add or-oper (FURB110)
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Apr 1, 2024
1 parent eb884c8 commit c67ba2a
Show file tree
Hide file tree
Showing 8 changed files with 180 additions and 0 deletions.
8 changes: 8 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
z = x if x else y # FURB110

z = x \
if x else y # FURB110

z = x if x \
else \
y # FURB110
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::IfExprMinMax) {
refurb::rules::if_expr_min_max(checker, if_exp);
}
if checker.enabled(Rule::OrOper) {
refurb::rules::or_oper(checker, if_exp);
}
}
Expr::ListComp(
comp @ ast::ExprListComp {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
// refurb
(Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
(Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
(Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::OrOper),
#[allow(deprecated)]
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
(Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod tests {

#[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))]
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
#[test_case(Rule::OrOper, Path::new("FURB110.py"))]
#[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))]
#[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))]
#[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub(crate) use isinstance_type_none::*;
pub(crate) use list_reverse_copy::*;
pub(crate) use math_constant::*;
pub(crate) use metaclass_abcmeta::*;
pub(crate) use or_oper::*;
pub(crate) use print_empty_string::*;
pub(crate) use read_whole_file::*;
pub(crate) use readlines_in_for::*;
Expand All @@ -35,6 +36,7 @@ mod isinstance_type_none;
mod list_reverse_copy;
mod math_constant;
mod metaclass_abcmeta;
mod or_oper;
mod print_empty_string;
mod read_whole_file;
mod readlines_in_for;
Expand Down
101 changes: 101 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/or_oper.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;

/// ## What it does
/// Checks for ternary `if` expressions that can be replaced with
/// `or` expressions.
///
/// ## Why is this bad?
/// Ternary if statements are more verbose than `or` expressions, and
/// generally have the same functionality.
///
/// ## Example
/// ```python
/// z = x if x else y
/// ```
///
/// Use instead:
/// ```python
/// z = x or y
/// ```
///
/// Note: if `x` depends on side-effects, then this check should be ignored.
#[violation]
pub struct OrOper {
if_true: SourceCodeSnippet,
if_false: SourceCodeSnippet,
}

impl Violation for OrOper {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let OrOper { if_true, if_false } = self;

match (if_true.full_display(), if_false.full_display()) {
(_, None) | (None, _) => {
format!("Replace ternary `if` expression with `or` expression")
}
(Some(if_true), Some(if_false)) => {
format!(
"Replace `{if_true} if {if_true} or {if_false}` with `{if_true} or {if_false}`"
)
}
}
}

fn fix_title(&self) -> Option<String> {
Some(format!("Use `or` expression"))
}
}

/// FURB110
pub(crate) fn or_oper(checker: &mut Checker, if_expr: &ast::ExprIf) {
let ast::ExprIf {
test,
body,
orelse,
range,
} = if_expr;

let if_true = body.as_ref();
let if_true_code = checker.locator().slice(if_true);
let test_expr_code = checker.locator().slice(test.as_ref());

if test_expr_code != if_true_code {
return;
}

let if_false = orelse.as_ref();

let mut diagnostic = Diagnostic::new(
OrOper {
if_true: SourceCodeSnippet::from_str(if_true_code),
if_false: SourceCodeSnippet::from_str(checker.locator().slice(if_false)),
},
*range,
);

let mut tokenizer = SimpleTokenizer::starts_at(if_true.end(), checker.locator().contents());

// find the `else` token to replace with `or`
let else_token = tokenizer
.find(|tok| tok.kind() == SimpleTokenKind::Else)
.expect("else token to exist");

let fix = Fix::unsafe_edits(
Edit::range_replacement("or".to_string(), else_token.range()),
[Edit::deletion(if_true.start(), test.start())],
);

diagnostic.set_fix(fix);

checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB110.py:1:5: FURB110 [*] Replace `x if x or y` with `x or y`
|
1 | z = x if x else y # FURB110
| ^^^^^^^^^^^^^ FURB110
2 |
3 | z = x \
|
= help: Use `or` expression

Unsafe fix
1 |-z = x if x else y # FURB110
1 |+z = x or y # FURB110
2 2 |
3 3 | z = x \
4 4 | if x else y # FURB110

FURB110.py:3:5: FURB110 [*] Replace `x if x or y` with `x or y`
|
1 | z = x if x else y # FURB110
2 |
3 | z = x \
| _____^
4 | | if x else y # FURB110
| |_______________^ FURB110
5 |
6 | z = x if x \
|
= help: Use `or` expression

Unsafe fix
1 1 | z = x if x else y # FURB110
2 2 |
3 |-z = x \
4 |- if x else y # FURB110
3 |+z = x or y # FURB110
5 4 |
6 5 | z = x if x \
7 6 | else \

FURB110.py:6:5: FURB110 [*] Replace `x if x or y` with `x or y`
|
4 | if x else y # FURB110
5 |
6 | z = x if x \
| _____^
7 | | else \
8 | | y # FURB110
| |_________^ FURB110
|
= help: Use `or` expression

Unsafe fix
3 3 | z = x \
4 4 | if x else y # FURB110
5 5 |
6 |-z = x if x \
7 |- else \
6 |+z = x \
7 |+ or \
8 8 | y # FURB110
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c67ba2a

Please sign in to comment.