[DOC201] Permit explicit None in functions that only return None#13064
Merged
AlexWaygood merged 5 commits intoastral-sh:mainfrom Aug 27, 2024
Merged
[DOC201] Permit explicit None in functions that only return None#13064AlexWaygood merged 5 commits intoastral-sh:mainfrom
DOC201] Permit explicit None in functions that only return None#13064AlexWaygood merged 5 commits intoastral-sh:mainfrom
Conversation
871f8ac to
dfee156
Compare
DOC201] Permit explicit None early returnsDOC201] Permit explicit None in functions that only return None
DOC201] Permit explicit None in functions that only return NoneDOC201] Permit explicit None in functions that only return None
Contributor
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| DOC201 | 67 | 0 | 67 | 0 | 0 |
Member
|
There's some situations in the ecosystem check here where the diagnostic is going away and it looks like a false negative is introduced, such as https://github.com/apache/airflow/blob/6c878866dedc182c55e18e490bff3f3a9e9c5747/airflow/providers/airbyte/operators/airbyte.py#L126. The reason there for the error going away is that the function is typed as always returning |
CodSpeed Performance ReportMerging #13064 will degrade performances by 9.33%Comparing Summary
Benchmarks breakdown
|
Member
Looks spurious; it's been very unreliable recently sadly |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
No longer reports docstring-missing-returns (DOC201) on explicit returns in functions that only return
None.For example,
no longer reports DOC201.
Closes #13062
Methodology
When visiting, count the number of
return Nones visited in the body. There are then two scenarios under which the diagnostic is skipped:return Noneto the total number of returns (that is to say, all the paths returnNone), skip the diagnostic.None, skip the diagnostic.Return annotations
I saw in the ecosystem check that functions that only return
Nonebut annotated otherwise were now being ignored. For example,would no longer report a diagnostic. I changed this back by always checking the rule if the return annotation is not
None: if the return annotation is anything other thanNone, run the diagnostic.I think this is more likely to be the expected behaviour, especially due to the functional overlap between docstrings and type annotations. You could argue that checking against the return type annotation should be all this patch does, but that would make the rule quite unhelpful in untyped contexts. For example,
would still report DOC201.
Test Plan
cargo nextest run