[flake8-simplify] Stabilize detection of Yoda conditions for "constant" collections (SIM300)
#12050
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A bit more controversial than the others because the scope is larger (hopefully ecosystem reflects that). |
AlexWaygood
reviewed
Jun 26, 2024
crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs
Outdated
Show resolved
Hide resolved
|
I pushed to your branch to fix some trivial clippy issues, but looks like some snapshots are out of date as well |
charliermarsh
force-pushed
the
charlie/sim300
branch
from
a79596a
to
8f31343
Compare
charliermarsh
force-pushed
the
charlie/sim300
branch
from
8f31343
to
5410662
Compare
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| SIM300 | 4308 | 2403 | 1905 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+1990 -1990 violations, +0 -0 fixes in 3 projects; 1 project error; 46 projects unchanged)
apache/airflow (+1827 -1827 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
+ airflow/providers/arangodb/sensors/arangodb.py:54:16: SIM300 [*] Yoda condition detected - airflow/providers/arangodb/sensors/arangodb.py:54:16: SIM300 [*] Yoda conditions are discouraged, use `records != 0` instead + airflow/providers/google/cloud/sensors/dataproc.py:118:14: SIM300 [*] Yoda condition detected - airflow/providers/google/cloud/sensors/dataproc.py:118:14: SIM300 [*] Yoda conditions are discouraged, use `state == JobStatus.State.DONE` instead + airflow/providers/google/cloud/sensors/dataproc.py:121:14: SIM300 [*] Yoda condition detected - airflow/providers/google/cloud/sensors/dataproc.py:121:14: SIM300 [*] Yoda conditions are discouraged, use `state == JobStatus.State.ATTEMPT_FAILURE` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1169:17: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1169:17: SIM300 [*] Yoda conditions are discouraged, use `label.lower() == "amd"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1170:20: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1170:20: SIM300 [*] Yoda conditions are discouraged, use `label.lower() == "amd64"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1171:20: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1171:20: SIM300 [*] Yoda conditions are discouraged, use `label.lower() == "x64"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1172:20: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1172:20: SIM300 [*] Yoda conditions are discouraged, use `label == "asf-runner"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:17: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:17: SIM300 [*] Yoda conditions are discouraged, use `label.lower() == "arm"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:43: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:43: SIM300 [*] Yoda conditions are discouraged, use `label.lower() == "arm64"` instead + dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:71: SIM300 [*] Yoda condition detected - dev/breeze/src/airflow_breeze/utils/selective_checks.py:1189:71: SIM300 [*] Yoda conditions are discouraged, use `label == "asf-arm"` instead + dev/breeze/tests/test_packages.py:112:12: SIM300 [*] Yoda condition detected - dev/breeze/tests/test_packages.py:112:12: SIM300 [*] Yoda conditions are discouraged, use `get_removed_provider_ids() == []` instead + dev/breeze/tests/test_packages.py:117:12: SIM300 [*] Yoda condition detected - dev/breeze/tests/test_packages.py:117:12: SIM300 [*] Yoda conditions are discouraged, use `get_suspended_provider_ids() == []` instead + dev/breeze/tests/test_packages.py:122:12: SIM300 [*] Yoda condition detected - dev/breeze/tests/test_packages.py:122:12: SIM300 [*] Yoda conditions are discouraged, use `get_suspended_provider_folders() == []` instead + docker_tests/test_prod_image.py:60:16: SIM300 [*] Yoda condition detected - docker_tests/test_prod_image.py:60:16: SIM300 [*] Yoda conditions are discouraged, use `ctx.value.return_code == 2` instead + docker_tests/test_prod_image.py:66:16: SIM300 [*] Yoda condition detected - docker_tests/test_prod_image.py:66:16: SIM300 [*] Yoda conditions are discouraged, use `ctx.value.return_code == 2` instead + helm_tests/airflow_aux/test_airflow_common.py:102:16: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:102:16: SIM300 [*] Yoda conditions are discouraged, use `len(docs) == 3` instead + helm_tests/airflow_aux/test_airflow_common.py:167:16: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:167:16: SIM300 [*] Yoda conditions are discouraged, use `len(k8s_objects) == 7` instead + helm_tests/airflow_aux/test_airflow_common.py:228:16: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:228:16: SIM300 [*] Yoda conditions are discouraged, use `len(k8s_objects) == 12` instead + helm_tests/airflow_aux/test_airflow_common.py:236:20: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:236:20: SIM300 [*] Yoda conditions are discouraged + helm_tests/airflow_aux/test_airflow_common.py:244:20: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:244:20: SIM300 [*] Yoda conditions are discouraged + helm_tests/airflow_aux/test_airflow_common.py:245:20: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:245:20: SIM300 [*] Yoda conditions are discouraged + helm_tests/airflow_aux/test_airflow_common.py:246:20: SIM300 [*] Yoda condition detected - helm_tests/airflow_aux/test_airflow_common.py:246:20: SIM300 [*] Yoda conditions are discouraged + helm_tests/airflow_aux/test_airflow_common.py:387:16: SIM300 [*] Yoda condition detected ... 3609 additional changes omitted for project
bokeh/bokeh (+162 -162 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
+ examples/topics/stats/sinaplot.py:42:19: SIM300 [*] Yoda condition detected - examples/topics/stats/sinaplot.py:42:19: SIM300 [*] Yoda conditions are discouraged, use `month == df.MONTH` instead + src/bokeh/core/property/numeric.py:118:17: SIM300 [*] Yoda condition detected - src/bokeh/core/property/numeric.py:118:17: SIM300 [*] Yoda conditions are discouraged, use `value > 0` instead + src/bokeh/core/property/numeric.py:68:17: SIM300 [*] Yoda condition detected - src/bokeh/core/property/numeric.py:68:17: SIM300 [*] Yoda conditions are discouraged, use `value >= 0` instead + src/bokeh/core/property/numeric.py:80:17: SIM300 [*] Yoda condition detected - src/bokeh/core/property/numeric.py:80:17: SIM300 [*] Yoda conditions are discouraged, use `value > 0` instead + src/bokeh/core/property/numeric.py:99:17: SIM300 [*] Yoda condition detected - src/bokeh/core/property/numeric.py:99:17: SIM300 [*] Yoda conditions are discouraged, use `value >= 0` instead ... 314 additional changes omitted for project
model-bakers/model_bakery (+1 -1 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
+ tests/test_baker.py:817:16: SIM300 [*] Yoda condition detected - tests/test_baker.py:817:16: SIM300 [*] Yoda conditions are discouraged, use `instance.fk == ["foo"]` instead
demisto/content (error)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
- 'ignore' -> 'lint.ignore'
- 'select' -> 'lint.select'
- 'unfixable' -> 'lint.unfixable'
- 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| SIM300 | 3980 | 1990 | 1990 | 0 | 0 |
AlexWaygood
approved these changes
Jun 27, 2024
|
It seems that the vast majority of new violations is for usages in tests, specifically inside of I'm also not sure if I would count this as a yoga condition: The right isn't a variable, it's a function call. |
|
I would always put the thing I was testing (the result of the function call) on the left, and the thing I expected it to be on the right. |
AlexWaygood
force-pushed
the
charlie/sim300
branch
from
46fde36
to
7fd2ab6
Compare
|
I'm pretty strongly of the opinion that this is a clear improvement for this rule. In all of the Python codebases I've worked on, whether the test suite is using Pytest doesn't appear to display it any better or worse depending on the order of the assertion:
|
Merged
MichaReiser
pushed a commit
that referenced
this pull request
Jun 27, 2024
…ant" collections (`SIM300`) (#12050) Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
MichaReiser
pushed a commit
that referenced
this pull request
Jun 27, 2024
…ant" collections (`SIM300`) (#12050) Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
See: #9164.
We made a bunch of improvements to the rule, to handle "constant" collections (e.g., lists that consist of constants). This will lead to new violations, but I do think it's a better behavior and it's clearly stable.