astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.64k stars 1.09k forks source link

Disallow single letter variables outside of comprehensions #4866

Open gaborbernat opened 1 year ago

gaborbernat commented 1 year ago

Single letter variables are not descriptive enough, so disallow them outside of list comprehensions; because explicit is better than implicit.

zanieb commented 1 year ago

In your mind, would this include:

for i in range(...):
   ...
gaborbernat commented 1 year ago

Most definitely 😊

for index/at/position in range(...):
   ...

is preferable to a magic i. Hopefully we moved out of learning to code where i/j is a good practice, but not IMHO in production level coding.

charliermarsh commented 1 year ago

This would sort of be solved by implementing flake8-variable-names (https://github.com/charliermarsh/ruff/issues/3463), though that doesn't special-case comprehensions.

Skylion007 commented 1 year ago

"index" is a commonly used and override it would trigger a complaint from the flake8-builtin rules.

eli-schwartz commented 1 year ago

Most definitely :blush:

for index/at/position in range(...):
   ...

is preferable to a magic i. Hopefully we moved out of learning to code where i/j is a good practice, but not IMHO in production level coding.

I'm interested in learning more about the perspective that sed'ing all instances of i/a/p to index/at/position while still reusing the variable name in multiple incompatible contexts is better practice due to being longer.

My understanding was that i is a magic programming word that means the English prose "index" in the same way that index is a magic programming word that means the English prose "index". Indexes are not exactly uncommon, so there will be many such variables.

A reasonable argument could be made that people should be linted away from using for index in foobars: and told to instead use for foobar in foobars:. If you're going to concede and allow genericised terms like "index" I see no reason whatsoever to then go and mark genericised terms like "i" as violations.

Maybe instead of designing a lint rule that allows people to mandate a minimum number of characters, a lint rule should be designed that allows people to mandate a minimum cognitive uniqueness? The referenced flake8-variable-names does this via VNE002: variable name should be clarified, it prevents using names like data/result/item/value/content/info but happens to be missing "index". It also has "val" but is missing "key"... probably both should be triggering that lint violation. :)

gaborbernat commented 1 year ago

I'd be ok to disallow index 😂 but at least single letter variables is my MVP.

mwesthelle commented 1 year ago

I'm a fan of the pylint approach, which forbids single letter variable names by default, but allows custom exceptions e.g. for loop variables (i, j ,k) via a good-names setting: https://pylint.pycqa.org/en/latest/user_guide/configuration/all-options.html#good-names.