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

Check for raise Exception or BaseException #7494

Closed
avylove opened this issue Sep 19, 2022 · 9 comments · Fixed by #7709
Closed

Check for raise Exception or BaseException #7494

avylove opened this issue Sep 19, 2022 · 9 comments · Fixed by #7709
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@avylove
Copy link
Contributor

avylove commented Sep 19, 2022

Current problem

Linting a codebase with a lot of raise Exception() statements. This is a bad practice as it makes it impossible to write narrow exception clauses.

Desired solution

Forgive me if I missed it, but as far as I can tell, a check doesn't currently exist. The check should fail when an instance of Exception or BaseException is raised directly.

@avylove avylove added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Sep 19, 2022
@Pierre-Sassoulas
Copy link
Member

That would be broad-except, can you try to enable it ?

@Pierre-Sassoulas Pierre-Sassoulas added Question and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 19, 2022
@avylove
Copy link
Contributor Author

avylove commented Sep 19, 2022

Isn't broad-except on the catch side, not the raise side?

@Pierre-Sassoulas
Copy link
Member

Ha, right !

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Help wanted 🙏 Outside help would be appreciated, good for new contributors Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Question labels Sep 19, 2022
@clavedeluna
Copy link
Collaborator

What should the linter name be here, broad-raise?

@Pierre-Sassoulas
Copy link
Member

I'm thinking that we could rename broad-except to broad-exception-caught and name this one broad-exception-raised. We need to use 'old-names' for that.

@clavedeluna
Copy link
Collaborator

'old-names' for that.

That's a way to handle backward compat, right? Or since this is a breaking change would this only be released in pylint 3.x? Trying to understand where to put this PR to

@Pierre-Sassoulas
Copy link
Member

Yes, old names handle the backward compatibility so that it's not a breaking change. See C0111 /missing-docstring for example.

@clavedeluna
Copy link
Collaborator

See C0111 /missing-docstring for example.

Just curious since I've used this msg as an example. Should code using module-docstrong such as @utils.only_required_for_messages("missing-docstring", "empty-docstring") be updated at some point to use the new name(s)? That way the old name only ever appears in old_name. As it stands right now the code / tests are confusing.

@Pierre-Sassoulas
Copy link
Member

We could replace it by the three new names yes (not in tests because sometime we're testing that the old_names work for multiple new name with this message).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants