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

[pylint] Implement useless-return (R1711) #3116

Merged
merged 16 commits into from Mar 17, 2023

Conversation

tomecki
Copy link
Contributor

@tomecki tomecki commented Feb 22, 2023

This PR implements https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/useless-return.html , including autofixing.

Impl change from original approach: the check locates the actual useless return statements, in contrast to pylint's implemnentation which flags the functions which have useless return statements.

@tomecki
Copy link
Contributor Author

tomecki commented Feb 22, 2023

Root ticket for pylint coverage: #970

Co-authored-by: Jeong YunWon <69878+youknowone@users.noreply.github.com>
@charliermarsh
Copy link
Member

I think the one issue with this rule is that it may conflict with some of the flake8-return rules, e.g. those that require an explicit return None at the end of the function, like RET502 and RET503. I'm hesitant to introduce rules that are in direct conflict. An alternative would be to make the flake8-return rules parameterizable such that they can enforce this behavior if desired, in lieu of enforcing the opposite.

@r3m0t
Copy link
Contributor

r3m0t commented Mar 16, 2023

Looks like in pylint the rule ignores functions which can return values. https://github.com/PyCQA/pylint/blob/1eef2273aee4c5a2e287f7470d6da3a3ae7d0760/pylint/checkers/refactoring/refactoring_checker.py#L2022

Which makes it the same as/very similar to R501 - https://pypi.org/project/flake8-return/

@charliermarsh
Copy link
Member

Oh, in that case, it may actually be different than any existing checks? Since none of those tell you to remove a return entirely, only that the return should be implicit or explicit in various cases. (R501 only tells you to change return None to return if all returns are None, not to drop a return / return None at the end of a method if it's the only return.)

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Mar 17, 2023
@charliermarsh charliermarsh requested review from charliermarsh and removed request for youknowone March 17, 2023 03:51
@charliermarsh
Copy link
Member

But we need to adjust this such that it only warns if this is the only return in the function.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2023

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+18, -0, 0 error(s))

zulip (+2, -0)

+ zerver/lib/streams.py:243:5: PLR1711 [*] Useless `return` statement at end of function
+ zerver/lib/test_helpers.py:122:9: PLR1711 [*] Useless `return` statement at end of function

bokeh (+10, -0)

+ src/bokeh/application/application.py:236:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/application/application.py:249:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/client/connection.py:281:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/contexts.py:301:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/contexts.py:321:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/tornado.py:715:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/views/ws.py:223:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/views/ws.py:262:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/views/ws.py:288:9: PLR1711 [*] Useless `return` statement at end of function
+ src/bokeh/server/views/ws.py:335:9: PLR1711 [*] Useless `return` statement at end of function

airflow (+6, -0)

+ airflow/providers/google/cloud/operators/dataflow.py:1378:9: PLR1711 [*] Useless `return` statement at end of function
+ airflow/providers/microsoft/azure/sensors/data_factory.py:134:9: PLR1711 [*] Useless `return` statement at end of function
+ airflow/utils/db.py:1475:5: PLR1711 [*] Useless `return` statement at end of function
+ dev/check_files.py:223:5: PLR1711 [*] Useless `return` statement at end of function
+ dev/check_files.py:237:5: PLR1711 [*] Useless `return` statement at end of function
+ tests/jobs/test_scheduler_job.py:1924:17: PLR1711 [*] Useless `return` statement at end of function

### Benchmark #### Linux ``` group main pr ----- ---- -- linter/all-rules/large/dataset.py 1.00 18.0±1.03ms 2.3 MB/sec 1.03 18.6±0.89ms 2.2 MB/sec linter/all-rules/numpy/ctypeslib.py 1.00 4.5±0.16ms 3.7 MB/sec 1.05 4.7±0.17ms 3.5 MB/sec linter/all-rules/numpy/globals.py 1.00 637.2±21.11µs 4.6 MB/sec 1.02 652.7±24.48µs 4.5 MB/sec linter/all-rules/pydantic/types.py 1.00 7.9±0.39ms 3.2 MB/sec 1.05 8.3±0.26ms 3.1 MB/sec linter/default-rules/large/dataset.py 1.00 9.9±0.44ms 4.1 MB/sec 1.01 10.0±0.50ms 4.1 MB/sec linter/default-rules/numpy/ctypeslib.py 1.03 2.2±0.11ms 7.6 MB/sec 1.00 2.1±0.10ms 7.8 MB/sec linter/default-rules/numpy/globals.py 1.00 260.4±7.24µs 11.3 MB/sec 1.02 265.4±22.20µs 11.1 MB/sec linter/default-rules/pydantic/types.py 1.00 4.5±0.13ms 5.7 MB/sec 1.04 4.7±0.17ms 5.5 MB/sec ```

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     18.3±0.33ms     2.2 MB/sec    1.02     18.6±0.40ms     2.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      5.0±0.17ms     3.3 MB/sec    1.00      5.0±0.12ms     3.4 MB/sec
linter/all-rules/numpy/globals.py          1.00   637.8±21.98µs     4.6 MB/sec    1.01   647.3±33.93µs     4.6 MB/sec
linter/all-rules/pydantic/types.py         1.00      8.3±0.28ms     3.1 MB/sec    1.00      8.3±0.19ms     3.1 MB/sec
linter/default-rules/large/dataset.py      1.00     10.2±0.19ms     4.0 MB/sec    1.04     10.6±0.22ms     3.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.2±0.08ms     7.5 MB/sec    1.04      2.3±0.08ms     7.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    248.4±9.31µs    11.9 MB/sec    1.04   257.7±11.80µs    11.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.7±0.17ms     5.4 MB/sec    1.01      4.8±0.12ms     5.3 MB/sec

@charliermarsh charliermarsh merged commit 61653b9 into astral-sh:main Mar 17, 2023
12 checks passed
@charliermarsh charliermarsh changed the title [pylint] R1711: useless-return [pylint] Implement useless-return (R1711) Mar 17, 2023
renovate bot added a commit to ixm-one/pytest-cmake-presets that referenced this pull request Mar 18, 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.256` ->
`^0.0.257` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/compatibility-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/confidence-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

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

#### What's Changed

##### Rules

- \[`ruff`] Prefer `itertools.pairwise()` over `zip()` for successive
pairs (`RUF007`) by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#3501
- \[`flake8-bugbear`] Add `no-explicit-stacklevel` (`B028`) by
[@&#8203;johnor](https://togithub.com/johnor) in
[astral-sh/ruff#3550
- \[`pylint`] invalid-characters-\* by
[@&#8203;r3m0t](https://togithub.com/r3m0t) in
[astral-sh/ruff#3552
- \[`pylint`] Implement `useless-return` (`R1711`) by
[@&#8203;tomecki](https://togithub.com/tomecki) in
[astral-sh/ruff#3116
- \[`pylint`]: Implement `continue-in-finally` (`E0116`) by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3541

##### Bug Fixes

- Rewrite mock import with starred imports by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3566
- Respect `type` overrides in E721 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3582
- Use `value > max` style in pylint and mccabe messages by
[@&#8203;edgarrmondragon](https://togithub.com/edgarrmondragon) in
[astral-sh/ruff#3553
- Fix autofix conflict between `D209` and `D400` by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3564
- Avoid C1901 violations within subscripts by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3517
- Avoid adding dashed line outside of docstring by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3581
- Enable ANSI colors on Windows 10 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3583

#### New Contributors

- [@&#8203;johnor](https://togithub.com/johnor) made their first
contribution in
[astral-sh/ruff#3550

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

</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:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMC4yIiwidXBkYXRlZEluVmVyIjoiMzUuMTAuMiJ9-->

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 21, 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.256` ->
`==0.0.257` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/compatibility-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/confidence-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

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

#### What's Changed

##### Rules

- \[`ruff`] Prefer `itertools.pairwise()` over `zip()` for successive
pairs (`RUF007`) by
[@&#8203;evanrittenhouse](https://togithub.com/evanrittenhouse) in
[astral-sh/ruff#3501
- \[`flake8-bugbear`] Add `no-explicit-stacklevel` (`B028`) by
[@&#8203;johnor](https://togithub.com/johnor) in
[astral-sh/ruff#3550
- \[`pylint`] invalid-characters-\* by
[@&#8203;r3m0t](https://togithub.com/r3m0t) in
[astral-sh/ruff#3552
- \[`pylint`] Implement `useless-return` (`R1711`) by
[@&#8203;tomecki](https://togithub.com/tomecki) in
[astral-sh/ruff#3116
- \[`pylint`]: Implement `continue-in-finally` (`E0116`) by
[@&#8203;latonis](https://togithub.com/latonis) in
[astral-sh/ruff#3541

##### Bug Fixes

- Rewrite mock import with starred imports by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3566
- Respect `type` overrides in E721 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3582
- Use `value > max` style in pylint and mccabe messages by
[@&#8203;edgarrmondragon](https://togithub.com/edgarrmondragon) in
[astral-sh/ruff#3553
- Fix autofix conflict between `D209` and `D400` by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#3564
- Avoid C1901 violations within subscripts by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3517
- Avoid adding dashed line outside of docstring by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3581
- Enable ANSI colors on Windows 10 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#3583

#### New Contributors

- [@&#8203;johnor](https://togithub.com/johnor) made their first
contribution in
[astral-sh/ruff#3550

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

</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:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNC4yIiwidXBkYXRlZEluVmVyIjoiMzUuMTQuMiJ9-->

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
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants