WPS111 too strict inside comprehensions

[webknjaz]

Bug report

I think that WPS111 is too restrictive with vars that are only used in comprehension-like expressions.

What's wrong

It says that s is too short here:

[s.encode() for s in cythonize_args]

How is that should be

it doesn't make sense for throwaway vars in list/dict/set comprehensions or generator expressions to be long.

[sobolevn]

Thanks a lot for the report, @webknjaz! I get your point.

But. Sometimes these comprehensions can grow. [s.encode() for s in cythonize_args] can become [s.encode() for s in cythonize_args if s is not None] and it can become [normalize(s.encode()) for s in cythonize_args if s is not None].

And its final form can be [normalize(s.encode()) for key, s in cythonize_args if s is not None and key in allowed_keys] Boom!

Moreover, I still don't know what s is (from your example). Let me guess, that it is something like [byte_arg.encode() for byte_arg in cythonize_args]. Which is better in my opinion.

Sadly, I cannot draw a good line to separate really simple cases from not so simple ones. And that's why I don't think that we will be adding a support for comprehensions in this rule.

Do you agree with me on this one? 🙂

[webknjaz]

I agree that comprehensions can grow but in such cases, Jones Complexity rule hits me instead. So I'd say this case is covered.
Also, here it's a comprehension of style "i don't care what the items are, i just want them converted".

I can easily rewrite this as list(map(str.encode, cythonize_args)) and it will pass the linter but will be ultimately less readable. I mean, I'd be okay if there was an option to prefix the var with an underscore but the rule doesn't seem to care if I declare it unimportant (e.g. _s).

[webknjaz]

Also, this rule forces me to multiline the expression just because naming vars cause the line to not fit within limits while the cognitive complexity of the original line doesn't really require it.

[sobolevn]

I agree that comprehensions can grow but in such cases, Jones Complexity rule hits me instead.

Not really, if you use \n to separate for and if on new lines (like a lot of people do)

list(map(str.encode, cythonize_args)) and it will pass the linter but will be ultimately less readable

Yes, I agree. map is controversial. Let's ignore it for now.

Anyway, does two letter names work for you in this case? Like xy? Or ss?

[webknjaz]

Well, I think that most of the time unimportant var should be one-char. Sometimes it's _ (I actually forgot to check it against this rule).

xy/ss seem to add some confusion visually because if after the first char there's the second one, it creates an impression that this var is a part of series of similarly-named vars (su, st, ss / xx, xy, xz) when its name doesn't resemble words.

[orsinium]

Related:

Short variable names work well when the distance between their declaration and last use is short.

• https://dave.cheney.net/practical-go/presentations/qcon-china.html#_identifier_length

[sobolevn]

I am going to decline this feature. I really don't like short variable names. Sorry 😓

[FyZzyss]

Hello! You have a very good plugin! I fell in love with this at first sight, but your uncooperative approach is frustrating. I think it would be possible to make a parameter whether to check a variable in a loop that defaults to True