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

Fix B023 shadowed variables in nested functions #4111

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Apr 26, 2023

Fixes #4090

The issue was that we didn't return early for FunctionDefs and Lambda expressions and, therefore, visited the body twice. This becomes a problem if there are nested functions because we then no-longer exclude the outer function's argument names from the suspicious variables.

This should also be good for performance because we no longer visit the body of every function twice (O(l^2) where l is the nesting level)

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

.filter(|(id, ..)| !arg_names.contains(id)),
);
return;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the real fix. We don't want to walk the function node again because that results in visiting the argument initializers (and a lot of unnecessary visiting)

@MichaReiser MichaReiser marked this pull request as ready for review April 26, 2023 04:50
@github-actions
Copy link
Contributor

github-actions bot commented Apr 26, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     15.3±0.05ms     2.7 MB/sec    1.00     15.4±0.07ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.9±0.00ms     4.3 MB/sec    1.00      3.9±0.00ms     4.3 MB/sec
linter/all-rules/numpy/globals.py          1.00    424.7±1.59µs     6.9 MB/sec    1.00    425.6±2.04µs     6.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.5±0.02ms     3.9 MB/sec    1.01      6.6±0.02ms     3.9 MB/sec
linter/default-rules/large/dataset.py      1.00      8.0±0.01ms     5.1 MB/sec    1.01      8.1±0.02ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1763.4±2.99µs     9.4 MB/sec    1.00   1767.9±4.27µs     9.4 MB/sec
linter/default-rules/numpy/globals.py      1.00    182.7±0.29µs    16.1 MB/sec    1.02    185.7±1.14µs    15.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.01ms     7.0 MB/sec    1.02      3.7±0.01ms     6.9 MB/sec
parser/large/dataset.py                    1.02      6.4±0.00ms     6.3 MB/sec    1.00      6.3±0.00ms     6.4 MB/sec
parser/numpy/ctypeslib.py                  1.00   1259.0±2.33µs    13.2 MB/sec    1.00   1258.6±1.59µs    13.2 MB/sec
parser/numpy/globals.py                    1.00    124.9±1.09µs    23.6 MB/sec    1.01    126.6±0.51µs    23.3 MB/sec
parser/pydantic/types.py                   1.00      2.7±0.00ms     9.3 MB/sec    1.00      2.7±0.00ms     9.3 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
linter/all-rules/large/dataset.py          1.01     22.0±0.56ms  1896.6 KB/sec     1.00     21.7±0.67ms  1919.2 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      5.6±0.18ms     3.0 MB/sec     1.00      5.6±0.17ms     3.0 MB/sec
linter/all-rules/numpy/globals.py          1.00   678.4±23.49µs     4.3 MB/sec     1.00   680.1±34.65µs     4.3 MB/sec
linter/all-rules/pydantic/types.py         1.00      9.3±0.40ms     2.7 MB/sec     1.01      9.3±0.54ms     2.7 MB/sec
linter/default-rules/large/dataset.py      1.04     11.3±0.57ms     3.6 MB/sec     1.00     10.9±0.28ms     3.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.4±0.10ms     6.9 MB/sec     1.01      2.4±0.14ms     6.8 MB/sec
linter/default-rules/numpy/globals.py      1.00   294.0±19.95µs    10.0 MB/sec     1.00   292.7±18.53µs    10.1 MB/sec
linter/default-rules/pydantic/types.py     1.02      5.2±0.29ms     4.9 MB/sec     1.00      5.1±0.28ms     5.0 MB/sec
parser/large/dataset.py                    1.01      8.9±0.20ms     4.6 MB/sec     1.00      8.8±0.25ms     4.6 MB/sec
parser/numpy/ctypeslib.py                  1.03  1744.6±174.00µs     9.5 MB/sec    1.00  1687.4±52.22µs     9.9 MB/sec
parser/numpy/globals.py                    1.00   179.2±14.87µs    16.5 MB/sec     1.00    178.7±7.16µs    16.5 MB/sec
parser/pydantic/types.py                   1.00      3.8±0.16ms     6.6 MB/sec     1.01      3.9±0.52ms     6.6 MB/sec

@MichaReiser MichaReiser merged commit 17db2e2 into main Apr 26, 2023
14 checks passed
@MichaReiser MichaReiser deleted the fix-b023-shadow branch April 26, 2023 21:01
renovate bot added a commit to ixm-one/pytest-cmake-presets that referenced this pull request May 2, 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.263` ->
`^0.0.264` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/compatibility-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/confidence-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

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

#### What's Changed

##### Rules

- Autofix `EM101`, `EM102`, `EM103` if possible by
[@&#8203;dhruvmanila](https://togithub.com/dhruvmanila) in
[astral-sh/ruff#4123
- Add bugbear immutable functions as allowed in dataclasses by
[@&#8203;mosauter](https://togithub.com/mosauter) in
[astral-sh/ruff#4122

##### Settings

- Add support for providing command-line arguments via `argfile` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4087

##### Bug Fixes

- Make D410/D411 autofixes mutually exclusive by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4110
- Remove `pyright` comment prefix from PYI033 checks by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4152
- Fix F811 false positive with match by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4161
- Fix `E713` and `E714` false positives for multiple comparisons by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4083
- Fix B023 shadowed variables in nested functions by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4111
- Preserve star-handling special-casing for force-single-line by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4129
- Respect parent-scoping rules for `NamedExpr` assignments by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4145
- Fix UP032 auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4165
- Allow boolean parameters for `pytest.param` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4176

##### Internal

- Replace row/column based `Location` with byte-offsets. by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3931
- perf(logical-lines): Various small perf improvements by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4022
- Use `memchr` to speedup newline search on x86 by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3985
- Remove `ScopeStack` in favor of child-parent `ScopeId` pointers by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4138

**Full Changelog**:
astral-sh/ruff@v0.0.263...v0.0.264

</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:eyJjcmVhdGVkSW5WZXIiOiIzNS42Ni4zIiwidXBkYXRlZEluVmVyIjoiMzUuNjYuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

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 May 3, 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.263` ->
`==0.0.264` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/compatibility-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/confidence-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

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

#### What's Changed

##### Rules

- Autofix `EM101`, `EM102`, `EM103` if possible by
[@&#8203;dhruvmanila](https://togithub.com/dhruvmanila) in
[astral-sh/ruff#4123
- Add bugbear immutable functions as allowed in dataclasses by
[@&#8203;mosauter](https://togithub.com/mosauter) in
[astral-sh/ruff#4122

##### Settings

- Add support for providing command-line arguments via `argfile` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4087

##### Bug Fixes

- Make D410/D411 autofixes mutually exclusive by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4110
- Remove `pyright` comment prefix from PYI033 checks by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4152
- Fix F811 false positive with match by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4161
- Fix `E713` and `E714` false positives for multiple comparisons by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4083
- Fix B023 shadowed variables in nested functions by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4111
- Preserve star-handling special-casing for force-single-line by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4129
- Respect parent-scoping rules for `NamedExpr` assignments by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4145
- Fix UP032 auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4165
- Allow boolean parameters for `pytest.param` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4176

##### Internal

- Replace row/column based `Location` with byte-offsets. by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3931
- perf(logical-lines): Various small perf improvements by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4022
- Use `memchr` to speedup newline search on x86 by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3985
- Remove `ScopeStack` in favor of child-parent `ScopeId` pointers by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4138

**Full Changelog**:
astral-sh/ruff@v0.0.263...v0.0.264

</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:eyJjcmVhdGVkSW5WZXIiOiIzNS42OS4zIiwidXBkYXRlZEluVmVyIjoiMzUuNjkuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to allenporter/pyrainbird that referenced this pull request May 3, 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.263` ->
`==0.0.264` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/compatibility-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.264/confidence-slim/0.0.263)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

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

#### What's Changed

##### Rules

- Autofix `EM101`, `EM102`, `EM103` if possible by
[@&#8203;dhruvmanila](https://togithub.com/dhruvmanila) in
[astral-sh/ruff#4123
- Add bugbear immutable functions as allowed in dataclasses by
[@&#8203;mosauter](https://togithub.com/mosauter) in
[astral-sh/ruff#4122

##### Settings

- Add support for providing command-line arguments via `argfile` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4087

##### Bug Fixes

- Make D410/D411 autofixes mutually exclusive by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4110
- Remove `pyright` comment prefix from PYI033 checks by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#4152
- Fix F811 false positive with match by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4161
- Fix `E713` and `E714` false positives for multiple comparisons by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4083
- Fix B023 shadowed variables in nested functions by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4111
- Preserve star-handling special-casing for force-single-line by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4129
- Respect parent-scoping rules for `NamedExpr` assignments by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4145
- Fix UP032 auto-fix by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#4165
- Allow boolean parameters for `pytest.param` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4176

##### Internal

- Replace row/column based `Location` with byte-offsets. by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3931
- perf(logical-lines): Various small perf improvements by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#4022
- Use `memchr` to speedup newline search on x86 by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#3985
- Remove `ScopeStack` in favor of child-parent `ScopeId` pointers by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#4138

**Full Changelog**:
astral-sh/ruff@v0.0.263...v0.0.264

</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/pyrainbird).

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

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

B023 potential false positive (with shadowed variable names)
2 participants