DEV: Remove ignore Ruff rule RET503 #3209
Conversation
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3209 +/- ##
==========================================
- Coverage 96.53% 96.52% -0.02%
==========================================
Files 53 53
Lines 8930 8933 +3
Branches 1642 1642
==========================================
+ Hits 8621 8623 +2
- Misses 186 187 +1
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pypdf/_page.py
Outdated
| @@ -1248,6 +1248,7 @@ def _merge_page( | |||
| self.replace_contents(ContentStream(new_content_array, self.pdf)) | |||
| self[NameObject(PG.RESOURCES)] = new_resources | |||
| self[NameObject(PG.ANNOTS)] = new_annots | |||
| return | |||
There was a problem hiding this comment.
Does this not make sense as this method should always return None.
There was a problem hiding this comment.
Without the return:
RET503 Missing explicit return at the end of function able to return non-None value.
I think you are right, that this function only returns None. I have added _page.py to [tool.ruff.lint.per-file-ignores].
There was a problem hiding this comment.
I disagree with this. We should never exclude a whole file from a rule if it is only relevant for one line. Apart from this, it seems like this might be a bug in the detection logic - this method will never return something different than None given its type hints and implementation.
| @@ -2187,6 +2187,7 @@ def clean_forms( | |||
|
|
|||
| clean(content, images, forms) | |||
| page.replace_contents(content) | |||
| return | |||
There was a problem hiding this comment.
How is this a valid return value for the given type hints?
| @@ -3105,6 +3106,7 @@ def find_outline_item( | |||
| o = cast(TreeObject, o["/Next"]) | |||
| else: | |||
| return None | |||
| return | |||
There was a problem hiding this comment.
There are two issues here:
- We always return
None- why does this check trigger at all? - If this is a valid case, we should have a test for it as well.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.
RET503: Missing explicit "return" at the end of function able to return non-"None" value.