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

Feature Request: Naming Convention Enforcement for Boolean Values & Functions #9409

Closed
nh916 opened this issue Feb 2, 2024 · 8 comments
Closed

Comments

@nh916
Copy link

nh916 commented Feb 2, 2024

Current problem

In Python, it's a common convention to name boolean variables and functions with prefixes that indicate their boolean nature, such as is, has, or have. Currently, Pylint does not offer a way to enforce this naming convention, which can lead to less readable and less self-documenting code.

Desired solution

I would like Pylint to provide an option to enforce naming conventions for boolean variables and functions. Ideally, Pylint should be able to warn or error when a boolean variable or function (one that returns a boolean value) does not start with the prefixes is, has, or have. This feature would help maintain consistent code quality and improve readability.

Additional context

This feature would be particularly useful for large projects where consistent naming conventions are crucial for maintaining code quality. Enforcing such conventions can aid in understanding code at a glance and prevent potential confusion about the nature of variables and functions.

@nh916 nh916 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 2, 2024
@Pierre-Sassoulas Pierre-Sassoulas added Proposal 📨 Needs decision 🔒 Needs a decision before implemention or rejection and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 2, 2024
@jacobtylerwalls
Copy link
Member

Sounds like a nice idea for a third-party plugin. The plugin author can decide how to define "boolean function" -- analyze the return statements, the type annotation, what happens if two different things are returned, or there's an implicit None return...? etc.

@Pierre-Sassoulas
Copy link
Member

I agree with Jacob, it rather complicated to specify and maintain and more suited to an (opinionated) plugin (or for a linter targeting a strongly typed language). If we add this to pylint we'll have to handle a lot of use case with a myriad of options because we want pylint to be one size fit all after configuration.

@jacobtylerwalls jacobtylerwalls added Won't fix/not planned and removed Needs decision 🔒 Needs a decision before implemention or rejection labels Feb 8, 2024
@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2024
@rhyn0
Copy link
Contributor

rhyn0 commented Feb 11, 2024

for anyone who does want something like this, I took a crack at it and built it here https://github.com/rhyn0/pylint-boolean-naming

Comments appreciated

@Pierre-Sassoulas
Copy link
Member

The code does not look too complicated, great plugin. You could add it to your fork of pylint as and extension and open a merge request to your main branch to see what's happening in the primers (numpy, pandas, pytest, etc. ). To likely find a bunch of false positives easily. We can add this to pylint-dev if you wish when it's ready.

@DanielNoord
Copy link
Collaborator

@Pierre-Sassoulas I think you meant to suggest to add the repository to pylint-dev right? Not in a PR like #9438

@Pierre-Sassoulas
Copy link
Member

Yes, the PR to see the primer could be in @rhyn0 's fork. I approve the pipeline in pylint-dev/pylint from time to time if it can help @rhyn0 to see the primer result faster/easier, though it does not mean we're going to add an extension, it should stay a plugin.

@rhyn0
Copy link
Contributor

rhyn0 commented Feb 15, 2024

@Pierre-Sassoulas sorry about that, thanks for explaining your original message. And another thanks for the primer workflow stuff.

I'm unsure what the "merge into pylint-dev" looks like (organization forks my repo and then that becomes the source?) but I'll try to get the primer run to work first

@Pierre-Sassoulas
Copy link
Member

No problems :) You can create a branch on your fork (rhyn0/fork-branch) and open a merge request to your own main branch from your fork (rhyn0/main) then the pipeline should work (provided the primer run once on the main branch). Happy to keep approving the pipeline on #9438 if you have issue with your fork setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants