Skip to content

[pylint] Implement invalid-bytes-returned (E0308)#10959

Merged
charliermarsh merged 7 commits intoastral-sh:mainfrom
tibor-reiss:add-E0308-invalid-bytes-returned
Apr 18, 2024
Merged

[pylint] Implement invalid-bytes-returned (E0308)#10959
charliermarsh merged 7 commits intoastral-sh:mainfrom
tibor-reiss:add-E0308-invalid-bytes-returned

Conversation

@tibor-reiss
Copy link
Copy Markdown
Contributor

Add pylint rule invalid-bytes-returned (PLE0308)

See #970 for rules

Test Plan: cargo test

@tibor-reiss tibor-reiss force-pushed the add-E0308-invalid-bytes-returned branch 2 times, most recently from 0317d41 to f7cb7d4 Compare April 15, 2024 20:25
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 15, 2024

CodSpeed Performance Report

Merging #10959 will not alter performance

Comparing tibor-reiss:add-E0308-invalid-bytes-returned (ced76b9) with main (06b3e37)

Summary

✅ 30 untouched benchmarks

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

visitor.returns
};

for stmt in returns {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this rule and the other invalid-*-returned rules also flag the case where the method has no return statement

class Test:
	def __bytes__(self):
		print("nope")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added this case as well. If this becomes the consensus, I can also add it to the already included bool (E0304) and str (E0307) cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MichaReiser As it turned out, the no-return case is not that simple: if there is just a pass/ellipsis/raise statement, then this rule should not raise. However, if there is e.g. a print statement and a raise statement, then it should raise - see the tests for examples.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I changed this to use is_stub for detecting the other cases.

@tibor-reiss tibor-reiss force-pushed the add-E0308-invalid-bytes-returned branch from a84894a to 197b5fc Compare April 16, 2024 17:58
@charliermarsh charliermarsh changed the title [pylint] Implement invalid-bytes-returned (PLE0308) [pylint] Implement invalid-bytes-returned (E0308) Apr 18, 2024
@charliermarsh charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels Apr 18, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) April 18, 2024 01:26
@charliermarsh charliermarsh force-pushed the add-E0308-invalid-bytes-returned branch from 6eb9545 to ced76b9 Compare April 18, 2024 01:30
@charliermarsh charliermarsh merged commit 1480d72 into astral-sh:main Apr 18, 2024
charliermarsh added a commit that referenced this pull request Apr 18, 2024
…e` (#11008)

## Summary

Reflecting some improvements that were made in
#10959.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants