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

[flake8-return] Only add return None at end of function (RET503) #11074

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JonathanPlasse
Copy link
Contributor

@JonathanPlasse JonathanPlasse commented Apr 21, 2024

Summary

In RET503 documentation, it is said that it only adds return None at the end of functions.
The current behavior can add return None in multiple place in the function, but not at the end of the function.
This behavior does not match the documentation and goes against other flake8-return rules like RET505 and caused people to open #5765.
The new behavior only adds return None at the end of functions like in the documentation and results in more readable code.
The new behavior is only available in preview mode.
The range of RET503 is changed to be the entire function, as it will be more clear it concerns the entire function.

Before

def main(value: str) -> int | None:
    if value == "one":
        return 1

    if value.startswith("do_stuff"):
        print("doing stuff")

        if value.endswith("haha"):
            print("haha")
            return None
        else:
            print("Instruction unclear.")
            return None
    else:
        print("Doing nothing.")
        return None

After

def main(value: str) -> int | None:
    if value == "one":
        return 1

    if value.startswith("do_stuff"):
        print("doing stuff")

        if value.endswith("haha"):
            print("haha")
        else:
            print("Instruction unclear.")
    else:
        print("Doing nothing.")
    return None

Test Plan

I added a preview snapshot for RET503.
I compared the diff between the RET503 stable and preview snapshot.
I checked the ecosystem reports and fixed one regression.

Copy link

github-actions bot commented Apr 21, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+364 -400 violations, +0 -0 fixes in 6 projects; 38 projects unchanged)

apache/airflow (+68 -76 violations, +0 -0 fixes)

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

+ airflow/dag_processing/processor.py:654:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- airflow/dag_processing/processor.py:657:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ airflow/io/path.py:325:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- airflow/io/path.py:342:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ airflow/jobs/triggerer_job_runner.py:127:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- airflow/jobs/triggerer_job_runner.py:128:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ airflow/metrics/otel_logger.py:181:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- airflow/metrics/otel_logger.py:202:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ airflow/metrics/otel_logger.py:207:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- airflow/metrics/otel_logger.py:228:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
... 134 additional changes omitted for project

bokeh/bokeh (+43 -48 violations, +0 -0 fixes)

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

+ examples/server/app/clustering/main.py:82:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- examples/server/app/clustering/main.py:83:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ src/bokeh/core/has_props.py:314:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- src/bokeh/core/has_props.py:340:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ src/bokeh/core/has_props.py:342:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- src/bokeh/core/has_props.py:367:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ src/bokeh/core/serialization.py:386:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- src/bokeh/core/serialization.py:409:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ src/bokeh/core/serialization.py:438:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- src/bokeh/core/serialization.py:469:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
... 81 additional changes omitted for project

demisto/content (+245 -271 violations, +0 -0 fixes)

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

- Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:100:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:167:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:168:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:17:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:18:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:193:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:194:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AWS-Enrichment-Remediation/Scripts/AWSAccountHierarchy/AWSAccountHierarchy_test.py:99:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AgariPhishingDefense/Integrations/AgariPhishingDefense/AgariPhishingDefense.py:116:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AgariPhishingDefense/Integrations/AgariPhishingDefense/AgariPhishingDefense.py:189:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:214:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:222:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:246:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:248:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:251:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:253:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:85:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/ApiModules/Scripts/DemistoClassApiModule/DemistoClassApiModule.py:87:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/ApiModules/Scripts/MicrosoftAzureStorageApiModule/MicrosoftAzureStorageApiModule.py:106:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/ApiModules/Scripts/MicrosoftAzureStorageApiModule/MicrosoftAzureStorageApiModule.py:130:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AppNovi/Integrations/appNovi/appNovi_test.py:17:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AppNovi/Integrations/appNovi/appNovi_test.py:22:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AzureStorageFileShare/Integrations/AzureStorageFileShare/AzureStorageFileShare.py:725:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AzureStorageFileShare/Integrations/AzureStorageFileShare/AzureStorageFileShare.py:745:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AzureStorageQueue/Integrations/AzureStorageQueue/AzureStorageQueue.py:792:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AzureStorageQueue/Integrations/AzureStorageQueue/AzureStorageQueue.py:822:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/AzureStorageTable/Integrations/AzureStorageTable/AzureStorageTable.py:567:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/AzureStorageTable/Integrations/AzureStorageTable/AzureStorageTable.py:588:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/Base/Scripts/DBotPredictPhishingWords/DBotPredictPhishingWords.py:17:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/Base/Scripts/DBotPredictPhishingWords/DBotPredictPhishingWords.py:30:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/Base/Scripts/DBotPredictPhishingWords/DBotPredictPhishingWords.py:37:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ Packs/Base/Scripts/DBotPredictPhishingWords/DBotPredictPhishingWords.py:48:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- Packs/Base/Scripts/DBotPredictPhishingWords/DBotPredictPhishingWords.py:79:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
... 483 additional changes omitted for project

ibis-project/ibis (+4 -0 violations, +0 -0 fixes)

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

+ ibis/backends/duckdb/__init__.py:518:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ ibis/backends/duckdb/__init__.py:572:39: RUF100 Unused `noqa` directive (unused: `RET503`)
+ ibis/backends/pyspark/__init__.py:698:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ ibis/backends/pyspark/__init__.py:740:39: RUF100 Unused `noqa` directive (unused: `RET503`)

milvus-io/pymilvus (+1 -1 violations, +0 -0 fixes)

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

+ _version_helper.py:41:1: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- _version_helper.py:42:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value

zulip/zulip (+3 -4 violations, +0 -0 fixes)

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

- zerver/lib/addressee.py:136:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ zerver/lib/addressee.py:95:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ zerver/tests/test_import_export.py:1413:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- zerver/tests/test_import_export.py:1416:17: RET503 Missing explicit `return` at the end of function able to return non-`None` value
+ zerver/tests/test_openapi.py:483:5: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- zerver/tests/test_openapi.py:562:9: RET503 Missing explicit `return` at the end of function able to return non-`None` value
- zerver/tests/test_openapi.py:563:13: RET503 Missing explicit `return` at the end of function able to return non-`None` value

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
RET503 762 362 400 0 0
RUF100 2 2 0 0 0

@charliermarsh charliermarsh self-assigned this Apr 22, 2024
Comment on lines +469 to +471
if checker.settings.preview.is_enabled() {
return result;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Early return if in preview mode because we only need to know if the function has at least one implicit return to add a single return None at the end of the function.
The stable version can add multiple return so it cannot return early.
The code should be simplified once the preview version become stable.

@JonathanPlasse
Copy link
Contributor Author

A comment should be added that when the preview version becomes stable the code can be simplified with early return everywhere.

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.

Autofix for RET503 breaks RET505 (in one case)
2 participants