[pylint] Implement `boolean-chained-comparison` (`R1716`) #13435

vincevannoort · 2024-09-21T10:25:28Z

Summary

This pull request implements the R1716 pylint rule: pylint documentation.

It checks boolean and operations that can can be chained

a = int(input())
b = int(input())
c = int(input())
if a < b and b < c:
    pass

And suggest to combine them:

a = int(input())
b = int(input())
c = int(input())
if a < b < c:
    pass

Test Plan

I have added test cases, and checked the ruff ecosystem results.

github-actions · 2024-09-21T11:03:36Z

`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 1 projects; 53 projects unchanged)

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

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/utils/usage_data_collection.py:120:12: PLR1716 [*] Contains chained boolean comparison that can be simplified

Changes by rule (1 rules affected)

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

MichaReiser

This is great. I've a few nits that should help improve the performance of this rule.

crates/ruff_linter/src/codes.rs

crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs

vincevannoort · 2024-09-24T05:32:18Z

This is great. I've a few nits that should help improve the performance of this rule.

@MichaReiser thanks so much for thorough feedback! 👌

I have fixed most of the review comments, but two are still open where I need some extra help from you.

vincevannoort · 2024-09-24T05:49:03Z

crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py

+a = int(input())
+b = int(input())
+c = int(input())
+if a > b and b < c: 


This to me does seem like a case where it could be written as a single expression a > b < c, but I am not entirely sure.

Possibly, but I'm not convinced that it improves readability.

MichaReiser

Thanks, this is great

## 0.6.9 ### Preview features - Fix codeblock dynamic line length calculation for indented docstring examples ([#13523](astral-sh/ruff#13523)) - \[`refurb`\] Mark `FURB118` fix as unsafe ([#13613](astral-sh/ruff#13613)) ### Rule changes - \[`pydocstyle`\] Don't raise `D208` when last line is non-empty ([#13372](astral-sh/ruff#13372)) - \[`pylint`\] Preserve trivia (i.e. comments) in `PLR5501` autofix ([#13573](astral-sh/ruff#13573)) ### Configuration - \[`pyflakes`\] Add `allow-unused-imports` setting for `unused-import` rule (`F401`) ([#13601](astral-sh/ruff#13601)) ### Bug fixes - Support ruff discovery in pip build environments ([#13591](astral-sh/ruff#13591)) - \[`flake8-bugbear`\] Avoid short circuiting `B017` for multiple context managers ([#13609](astral-sh/ruff#13609)) - \[`pylint`\] Do not offer an invalid fix for `PLR1716` when the comparisons contain parenthesis ([#13527](astral-sh/ruff#13527)) - \[`pyupgrade`\] Fix `UP043` to apply to `collections.abc.Generator` and `collections.abc.AsyncGenerator` ([#13611](astral-sh/ruff#13611)) - \[`refurb`\] Fix handling of slices in tuples for `FURB118`, e.g., `x[:, 1]` ([#13518](astral-sh/ruff#13518)) ### Documentation - Update GitHub Action link to `astral-sh/ruff-action` ([#13551](astral-sh/ruff#13551)) ## 0.6.8 ### Preview features - Remove unnecessary parentheses around `match case` clauses ([#13510](astral-sh/ruff#13510)) - Parenthesize overlong `if` guards in `match..case` clauses ([#13513](astral-sh/ruff#13513)) - Detect basic wildcard imports in `ruff analyze graph` ([#13486](astral-sh/ruff#13486)) - \[`pylint`\] Implement `boolean-chained-comparison` (`R1716`) ([#13435](astral-sh/ruff#13435)) ### Rule changes - \[`lake8-simplify`\] Detect `SIM910` when using variadic keyword arguments, i.e., `**kwargs` ([#13503](astral-sh/ruff#13503)) - \[`pyupgrade`\] Avoid false negatives with non-reference shadowed bindings of loop variables (`UP028`) ([#13504](astral-sh/ruff#13504)) ### Bug fixes - Detect tuples bound to variadic positional arguments i.e. `*args` ([#13512](astral-sh/ruff#13512)) - Exit gracefully on broken pipe errors ([#13485](astral-sh/ruff#13485)) - Avoid panic when analyze graph hits broken pipe ([#13484](astral-sh/ruff#13484)) ### Performance - Reuse `BTreeSets` in module resolver ([#13440](astral-sh/ruff#13440)) - Skip traversal for non-compound statements ([#13441](astral-sh/ruff#13441)) ## 0.6.7 ### Preview features - Add Python version support to ruff analyze CLI ([#13426](astral-sh/ruff#13426)) - Add `exclude` support to `ruff analyze` ([#13425](astral-sh/ruff#13425)) - Fix parentheses around return type annotations ([#13381](astral-sh/ruff#13381)) ### Rule changes - \[`pycodestyle`\] Fix: Don't autofix if the first line ends in a question mark? (D400) ([#13399](astral-sh/ruff#13399)) ### Bug fixes - Respect `lint.exclude` in ruff check `--add-noqa` ([#13427](astral-sh/ruff#13427)) ### Performance - Avoid tracking module resolver files in Salsa ([#13437](astral-sh/ruff#13437)) - Use `forget` for module resolver database ([#13438](astral-sh/ruff#13438)) ## 0.6.6 ### Preview features - \[`refurb`\] Skip `slice-to-remove-prefix-or-suffix` (`FURB188`) when non-trivial slice steps are present ([#13405](astral-sh/ruff#13405)) - Add a subcommand to generate dependency graphs ([#13402](astral-sh/ruff#13402)) ### Formatter - Fix placement of inline parameter comments ([#13379](astral-sh/ruff#13379)) ### Server - Fix off-by one error in the `LineIndex::offset` calculation ([#13407](astral-sh/ruff#13407)) ### Bug fixes - \[`fastapi`\] Respect FastAPI aliases in route definitions ([#13394](astral-sh/ruff#13394)) - \[`pydocstyle`\] Respect word boundaries when detecting function signature in docs ([#13388](astral-sh/ruff#13388)) ### Documentation - Add backlinks to rule overview linter ([#13368](astral-sh/ruff#13368)) - Fix documentation for editor vim plugin ALE ([#13348](astral-sh/ruff#13348)) - Fix rendering of `FURB188` docs ([#13406](astral-sh/ruff#13406))

vincevannoort changed the title ~~[pylint] Implement boolean-chained_comparison (R1716)~~ [pylint] Implement boolean-chained-comparison (R1716) Sep 21, 2024

This comment has been minimized.

Sign in to view

vincevannoort added 8 commits September 22, 2024 09:13

feat(pylint): implement PLR1716

317a182

fix(pylint): update schema

583a6d4

fix(pylint): set as safe edit

cf59b3f

fix(pylint): false positives

86a599b

fix(pylint): update tests

fab0996

feat(pylint): update message and fix_title

dce9440

fix(pylint): update typo

faaf682

fix(pylint): remove space from example

e596c10

vincevannoort force-pushed the pylint/chained-comparison branch from c4e39ae to e596c10 Compare September 22, 2024 07:16

fix(pylint): update documentation

7a973a5

vincevannoort marked this pull request as ready for review September 22, 2024 07:58

MichaReiser requested changes Sep 23, 2024

View reviewed changes

MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Sep 23, 2024

vincevannoort added 8 commits September 24, 2024 06:55

fix(pylint): convert rule from stable to preview

483a305

fix(pylint): remove comment

a453653

fix(pylint): remove early exit check

cbdfc41

fix(pylint): import range trait

757f6fd

fix(pylint): remove collect

ffeaf68

fix(pylint): use optionals instead of results

d6eaac6

fix(pylint): put values in edit above diagnostic

6dfa935

fix(pylint): update fix_title

a476783

vincevannoort requested a review from MichaReiser September 24, 2024 05:32

fix(pylint): update tests

8dd1ea6

vincevannoort commented Sep 24, 2024

View reviewed changes

fix(pylint): match ops using let pattern

d05191d

MichaReiser added 4 commits September 25, 2024 10:54

Remove unnecessary collects

c9b34c2

Remove fix-only fields from violation

4090f6c

Set correct fixability

203e364

Remove TODO from tests, use and instead of or in one case

fa64950

MichaReiser approved these changes Sep 25, 2024

View reviewed changes

MichaReiser enabled auto-merge (squash) September 25, 2024 09:09

MichaReiser merged commit ca0ae0a into astral-sh:main Sep 25, 2024
17 checks passed

BrewTestBot mentioned this pull request Sep 26, 2024

ruff 0.6.8 Homebrew/homebrew-core#191970

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pylint] Implement `boolean-chained-comparison` (`R1716`) #13435

[pylint] Implement `boolean-chained-comparison` (`R1716`) #13435

vincevannoort commented Sep 21, 2024 •

edited

Loading

github-actions bot commented Sep 21, 2024 •

edited

Loading

This comment has been minimized.

MichaReiser left a comment

vincevannoort commented Sep 24, 2024

vincevannoort Sep 24, 2024

MichaReiser Sep 25, 2024

MichaReiser left a comment

[pylint] Implement boolean-chained-comparison (R1716) #13435

[pylint] Implement boolean-chained-comparison (R1716) #13435

Conversation

vincevannoort commented Sep 21, 2024 • edited Loading

Summary

Test Plan

github-actions bot commented Sep 21, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

This comment has been minimized.

MichaReiser left a comment

Choose a reason for hiding this comment

vincevannoort commented Sep 24, 2024

vincevannoort Sep 24, 2024

Choose a reason for hiding this comment

MichaReiser Sep 25, 2024

Choose a reason for hiding this comment

MichaReiser left a comment

Choose a reason for hiding this comment

[pylint] Implement `boolean-chained-comparison` (`R1716`) #13435

[pylint] Implement `boolean-chained-comparison` (`R1716`) #13435

vincevannoort commented Sep 21, 2024 •

edited

Loading

github-actions bot commented Sep 21, 2024 •

edited

Loading

`ruff-ecosystem` results