Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore multiply-assigned variables in RET504 #3393

Merged
merged 1 commit into from
Mar 9, 2023

Conversation

charliermarsh
Copy link
Member

This PR acts on the suggestion from #2950 to avoid RET504 errors when a variable is assigned to multiple times. As-is, we have too many false positives like:

def get_queryset(option_1, option_2):
    queryset = Model.filter(a=1)
    if option_1:
        queryset = queryset.filter(b=2)
    if option_2:
        queryset = queryset.filter(c=3)
    return queryset

Instead, this rule will now only flag more trivial cases of "assign-then-return", like:

def get_queryset():
    queryset = Model.filter(a=1)
    return queryset

It's possible that this is too conservative, since we no longer flag cases like:

def get_queryset():
    queryset = Model.filter(a=1)
    queryset = queryset.filter(c=3)
    return queryset

But, I actually think that's reasonable -- if you're multiply-assigning, you're often doing something like a fluent interface or a conditional assignment, and in those cases, it actually seems totally fine (preferable, even) to separate the operation chain from the return.

Closes #2950.

@charliermarsh charliermarsh merged commit a7f3532 into main Mar 9, 2023
@charliermarsh charliermarsh deleted the charlie/multi-assign branch March 9, 2023 00:11
@henryiii
Copy link
Contributor

henryiii commented Mar 9, 2023

Note: the "fix" for case one was to write:

def get_queryset(option_1, option_2):
    queryset = Model.filter(a=1)
    if option_1:
        queryset = queryset.filter(b=2)
    if option_2:
        return queryset.filter(c=3)
    return queryset

Which wasn't obvious at first, and made it seem like it was misbehaving. After seeing it in place (usually option1 and option2 are not as symmetric as they seem above), I actually like it a bit better - it makes parsing the logic a bit simpler, as there's less overwriting of the same variable.

And

def get_queryset():
    queryset = Model.filter(a=1)
    return queryset.filter(c=3)

is way better, IMO, as it completely skips overwriting this variable. This "fluent" style won't work with static typing checking most of the time either, so I think it's fine to disable this check if you are using it and really want the extra line. I mean, if you don't like a check you don't have to use it? ¯\_(ツ)_/¯

renovate bot added a commit to ixm-one/pytest-cmake-presets that referenced this pull request Mar 14, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [ruff](https://togithub.com/charliermarsh/ruff) | `^0.0.254` ->
`^0.0.255` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.255/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.255/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.255/compatibility-slim/0.0.254)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.255/confidence-slim/0.0.254)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>charliermarsh/ruff</summary>

###
[`v0.0.255`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.255)

[Compare
Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.254...v0.0.255)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### Rules

- \[`flake8-pie`] Fix PIE802 broken auto-fix with trailing comma by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3402
- \[`flake8-pie`] Implement autofix for PIE810 by
[@&#8203;kyoto7250](https://togithub.com/kyoto7250) in
[astral-sh/ruff#3411
- \[`flake8-bugbear`] Add `flake8-bugbear`'s B030 rule by
[@&#8203;aacunningham](https://togithub.com/aacunningham) in
[astral-sh/ruff#3400
- \[`pycodestyle`] Add E231 by
[@&#8203;carlosmiei](https://togithub.com/carlosmiei) in
[astral-sh/ruff#3344
- \[`pyupgrade`] Flag deprecated (but renamed) imports in UP035 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3448
- \[`pyupgrade`] Remap ChainMap, Counter, and OrderedDict imports to
collections by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3392
- \[`pylint`] C1901: compare-to-empty-string by
[@&#8203;AreamanM](https://togithub.com/AreamanM) in
[astral-sh/ruff#3405
- \[`pylint`] Implement W1508 invalid-envvar-default by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3449
- \[`pylint`] Implement E1507 invalid-envvar-value by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3467

##### Settings

- Infer `target-version` from project metadata by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3470
- Implement configuration options `runtime-evaluated-decorators` and
`runtime-evaluated-base-classes` for `flake8-type-checking` by
[@&#8203;sasanjac](https://togithub.com/sasanjac) in
[astral-sh/ruff#3292
- Add Azure DevOps as a `--format` option. by
[@&#8203;StefanBRas](https://togithub.com/StefanBRas) in
[astral-sh/ruff#3335

##### Bug Fixes

- Re-enable the T and C linter prefix selectors by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3452
- Treat unary operations on constants as constant-like by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3348
- Skip byte-order-mark at start of file by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3343
- Don't enforce typing-import rules in .pyi files by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3362
- Include entire prefix when reporting rule selector errors by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3375
- Handle multi-line fixes for byte-string prefixing by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3391
- Catch RET504 usages via decorators by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3395
- Ignore multiply-assigned variables in RET504 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3393
- \[FIX] PYI011: recognize `Bool` / `Float` / `Complex` numbers as
simple defaults by [@&#8203;XuehaiPan](https://togithub.com/XuehaiPan)
in
[astral-sh/ruff#3459
- Respect ignores for runtime-import-in-type-checking-block (TCH004) by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3474
- Avoid panicking in invalid_escape_sequence by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3338
- fix: Emit a more useful error if an `extend` points at a non-existent
ruff.toml file. by [@&#8203;DanCardin](https://togithub.com/DanCardin)
in
[astral-sh/ruff#3417
- Respect `--show-fixes` with `--fix-only` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3426
- When "Args" and "Parameters" are present, prefer NumPy style by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3430
- Remove empty line after RUF100 auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3414
- Avoid removing un-aliased exceptions in `OSError`-aliased handlers by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3451
- Use a hash to fingerprint GitLab CI output by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3456
- Avoid respecting noqa directives when RUF100 is enabled by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3469
- Output GitLab paths relative to `CI_PROJECT_DIR` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3475
- Implement an iterator for universal newlines by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3454

#### New Contributors

- [@&#8203;sasanjac](https://togithub.com/sasanjac) made their first
contribution in
[astral-sh/ruff#3292
- [@&#8203;aacunningham](https://togithub.com/aacunningham) made their
first contribution in
[astral-sh/ruff#3380
- [@&#8203;orf](https://togithub.com/orf) made their first contribution
in
[astral-sh/ruff#3389
- [@&#8203;DanCardin](https://togithub.com/DanCardin) made their first
contribution in
[astral-sh/ruff#3417
- [@&#8203;AreamanM](https://togithub.com/AreamanM) made their first
contribution in
[astral-sh/ruff#3405
- [@&#8203;kyoto7250](https://togithub.com/kyoto7250) made their first
contribution in
[astral-sh/ruff#3411
- [@&#8203;latonis](https://togithub.com/latonis) made their first
contribution in
[astral-sh/ruff#3449
- [@&#8203;XuehaiPan](https://togithub.com/XuehaiPan) made their first
contribution in
[astral-sh/ruff#3459
- [@&#8203;CalumY](https://togithub.com/CalumY) made their first
contribution in
[astral-sh/ruff#3461
- [@&#8203;YDX-2147483647](https://togithub.com/YDX-2147483647) made
their first contribution in
[astral-sh/ruff#3473

**Full Changelog**:
astral-sh/ruff@v0.0.254...v0.0.255

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNjAuMCIsInVwZGF0ZWRJblZlciI6IjM0LjE2MC4wIn0=-->

Signed-off-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to allenporter/flux-local that referenced this pull request Mar 15, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [ruff](https://togithub.com/charliermarsh/ruff) | `==0.0.254` ->
`==0.0.256` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.256/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.256/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.256/compatibility-slim/0.0.254)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.256/confidence-slim/0.0.254)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>charliermarsh/ruff</summary>

###
[`v0.0.256`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.256)

[Compare
Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.255...v0.0.256)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### Bug Fixes

- PYI011: allow `math` constants in defaults by
[@&#8203;XuehaiPan](https://togithub.com/XuehaiPan) in
[astral-sh/ruff#3484
- Remove erroneous C4-to-C40 redirect by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3488
- fix lack of `not` in error message by
[@&#8203;Czaki](https://togithub.com/Czaki) in
[astral-sh/ruff#3497
- Ensure that redirect warnings appear exactly once per code by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3500
- Allow f-strings and concatenations in os.getenv by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3516
- Allow string percent formatting in os.getenv by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3518
- Refine complexity rules for try-except-else-finally by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3519
- Replicate inline comments when splitting single-line imports by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3521
- Avoid PEP 604 isinstance errors for starred tuples by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3527
- Avoid tracking as-imports separately with force-single-line by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3530
- Fix PYI011 and add auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3492
- Avoid PEP 604 panic with empty tuple by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3526
- Add last remaining deprecated typing imports by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3529
- Avoid unused argument violations in .pyi files by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3533

##### Other Changes

- Include individual path checks in --verbose logging by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3489
- Allow `# ruff:` prefix for isort action comments by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3493

**Full Changelog**:
astral-sh/ruff@v0.0.255...v0.0.256

###
[`v0.0.255`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.255)

[Compare
Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.254...v0.0.255)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### Rules

- \[`flake8-pie`] Fix PIE802 broken auto-fix with trailing comma by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3402
- \[`flake8-pie`] Implement autofix for PIE810 by
[@&#8203;kyoto7250](https://togithub.com/kyoto7250) in
[astral-sh/ruff#3411
- \[`flake8-bugbear`] Add `flake8-bugbear`'s B030 rule by
[@&#8203;aacunningham](https://togithub.com/aacunningham) in
[astral-sh/ruff#3400
- \[`pycodestyle`] Add E231 by
[@&#8203;carlosmiei](https://togithub.com/carlosmiei) in
[astral-sh/ruff#3344
- \[`pyupgrade`] Flag deprecated (but renamed) imports in UP035 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3448
- \[`pyupgrade`] Remap ChainMap, Counter, and OrderedDict imports to
collections by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3392
- \[`pylint`] C1901: compare-to-empty-string by
[@&#8203;AreamanM](https://togithub.com/AreamanM) in
[astral-sh/ruff#3405
- \[`pylint`] Implement W1508 invalid-envvar-default by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3449
- \[`pylint`] Implement E1507 invalid-envvar-value by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3467

##### Settings

- Infer `target-version` from project metadata by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3470
- Implement configuration options `runtime-evaluated-decorators` and
`runtime-evaluated-base-classes` for `flake8-type-checking` by
[@&#8203;sasanjac](https://togithub.com/sasanjac) in
[astral-sh/ruff#3292
- Add Azure DevOps as a `--format` option. by
[@&#8203;StefanBRas](https://togithub.com/StefanBRas) in
[astral-sh/ruff#3335

##### Bug Fixes

- Re-enable the T and C linter prefix selectors by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3452
- Treat unary operations on constants as constant-like by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3348
- Skip byte-order-mark at start of file by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3343
- Don't enforce typing-import rules in .pyi files by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3362
- Include entire prefix when reporting rule selector errors by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3375
- Handle multi-line fixes for byte-string prefixing by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3391
- Catch RET504 usages via decorators by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3395
- Ignore multiply-assigned variables in RET504 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3393
- \[FIX] PYI011: recognize `Bool` / `Float` / `Complex` numbers as
simple defaults by [@&#8203;XuehaiPan](https://togithub.com/XuehaiPan)
in
[astral-sh/ruff#3459
- Respect ignores for runtime-import-in-type-checking-block (TCH004) by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3474
- Avoid panicking in invalid_escape_sequence by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3338
- fix: Emit a more useful error if an `extend` points at a non-existent
ruff.toml file. by [@&#8203;DanCardin](https://togithub.com/DanCardin)
in
[astral-sh/ruff#3417
- Respect `--show-fixes` with `--fix-only` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3426
- When "Args" and "Parameters" are present, prefer NumPy style by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3430
- Remove empty line after RUF100 auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3414
- Avoid removing un-aliased exceptions in `OSError`-aliased handlers by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3451
- Use a hash to fingerprint GitLab CI output by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3456
- Avoid respecting noqa directives when RUF100 is enabled by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3469
- Output GitLab paths relative to `CI_PROJECT_DIR` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3475
- Implement an iterator for universal newlines by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3454

#### New Contributors

- [@&#8203;sasanjac](https://togithub.com/sasanjac) made their first
contribution in
[astral-sh/ruff#3292
- [@&#8203;aacunningham](https://togithub.com/aacunningham) made their
first contribution in
[astral-sh/ruff#3380
- [@&#8203;orf](https://togithub.com/orf) made their first contribution
in
[astral-sh/ruff#3389
- [@&#8203;DanCardin](https://togithub.com/DanCardin) made their first
contribution in
[astral-sh/ruff#3417
- [@&#8203;AreamanM](https://togithub.com/AreamanM) made their first
contribution in
[astral-sh/ruff#3405
- [@&#8203;kyoto7250](https://togithub.com/kyoto7250) made their first
contribution in
[astral-sh/ruff#3411
- [@&#8203;latonis](https://togithub.com/latonis) made their first
contribution in
[astral-sh/ruff#3449
- [@&#8203;XuehaiPan](https://togithub.com/XuehaiPan) made their first
contribution in
[astral-sh/ruff#3459
- [@&#8203;CalumY](https://togithub.com/CalumY) made their first
contribution in
[astral-sh/ruff#3461
- [@&#8203;YDX-2147483647](https://togithub.com/YDX-2147483647) made
their first contribution in
[astral-sh/ruff#3473

**Full Changelog**:
astral-sh/ruff@v0.0.254...v0.0.255

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNjAuMCIsInVwZGF0ZWRJblZlciI6IjM0LjE2MC4wIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RET504 false-positive when variable built up with assignments and returned
2 participants