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

Implement --allow-global-unused-variables from PyLint #11426

Open
Mellowdv opened this issue May 14, 2024 · 1 comment
Open

Implement --allow-global-unused-variables from PyLint #11426

Mellowdv opened this issue May 14, 2024 · 1 comment
Labels
multifile-analysis Requires analysis across multiple files

Comments

@Mellowdv
Copy link

Mellowdv commented May 14, 2024

PyLint allows users to pass --allow-global-unused-variables=n(o) option to catch not only function scope unused variables, but also module scope variables. This would modify rule F841: Unused Variable.

I would like to implement this.
Now there is a question that I would like to have answered before committing and perhaps open a discussion on it, if no clear answer is available.

Is 1-to-1 (or let's say bug-for-bug) parity with PyLint desirable?
PyLint has some undesirable behaviour, for example reporting class definitions not used in the same file as the definition as "unused variables" (in our code base and probably most code bases, it is fairly normal to have classes defined in one file and using them in another). I would prefer to define a "global-unused-variable" as not within function scope, not within class scope, not within generator/lambda scope and unused.

As mentioned above, PyLint reports the classes as unused (at least in our code base), and so it seems that it does not check this between files.

As such, I think Ruff is capable of checking whether a Module scoped variable that is not in a Function, Class, Generator or a Lambda, is unused and reporting it (given 'allow-global-unused-variables = false' is set in ruff.toml or other config file). In fact, I've got it working more or less on our codebase, just need to check whether the behaviour is in line with my expectations.

Please let me know if I'm wrong and what the opinion on PyLint parity is.

@MichaReiser MichaReiser added the multifile-analysis Requires analysis across multiple files label May 27, 2024
@njharman
Copy link

Your implementation is what I would expect. I would be annoyed by the pylint behavior.

Recently an unused global var caused bug for me. Found this issue while searching for "why is this not reported?" So, I'm +1 on this feature being implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multifile-analysis Requires analysis across multiple files
Projects
None yet
Development

No branches or pull requests

3 participants