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 invalid-bytes-returned (E0308) #10959

Merged

Conversation

tibor-reiss
Copy link
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
Copy link

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

Copy link
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
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
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
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
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
17 checks passed
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.

None yet

3 participants