Implement FURB136

[siiptuo]

Summary

Implements FURB136 that checks for if expressions that can be replaced with min() or max() calls. See issue #1348 for more information.

This implementation diverges from Refurb's original implementation by retaining the order of equal values. For example, Refurb suggest that the following expressions:

highest_score1 = score1 if score1 > score2 else score2
highest_score2 = score1 if score1 >= score2 else score2

should be to rewritten as:

highest_score1 = max(score1, score2)
highest_score2 = max(score1, score2)

whereas this implementation provides more correct alternatives:

highest_score1 = max(score2, score1)
highest_score2 = max(score1, score2)

Test Plan

Unit test checks all eight possibilities.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 41 projects)

apache/airflow (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --select ALL --preview

+ airflow/providers/apache/kafka/operators/consume.py:158:30: FURB136 [*] Replace `if` expression with `min(messages_left, self.max_batch_size)`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FURB136	1	1	0	0	0

[charliermarsh]

Nice, thank you, this looks great! I may make some small tweaks to better conform to some of our idioms but shouldn't require any major changes.

[charliermarsh]

Can you instead pass in the ast::ExprIfExp here? That's what we prefer for new rules. So e.g. on line 1284, you'd change to:

Expr::IfExp(if_exp @ ast::ExprIfExp {
    test,
    body,
    orelse,
    range: _,
}) => {
    ...
}

And then here, you'd pass in if_exp instead of the destructured fields. That way, callers can't pass in the "wrong" values.

[siiptuo]

Good point!

[charliermarsh]

Thanks!