pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.32k stars 1.14k forks source link

False negatives ``superfluous-parens`` with superfluous parenthesis on function call #5334

Open KotlinIsland opened 2 years ago

KotlinIsland commented 2 years ago

Current problem

print((((((((((((((((((((((((((((((1))))))))))))))))))))))))))))))

This looks cringe to me. I want a pylint warning.

DanielNoord commented 2 years ago

We actually had a similar checker (contributed by me), which I removed in https://github.com/PyCQA/pylint/pull/4948

It is actually (almost) impossible to determine when a tuple with inner tuple is superfluous or when it is intended. We risk throwing a lot of false positives for valid use cases of inner tuples. We could make a check that checks whether the tuples are in a non-sensical place, such as the print call in the example, but I think that might also be unmaintainable. We would need a USELESS_INNER_TUPLE_FUNCTIONS variable or something and I'm not sure if we won't still throw many false positives. If anybody thinks they have a good idea for this while also limiting false positives we could go ahead (and you can probably copy code from https://github.com/PyCQA/pylint/pull/4948). However, I would argue to close this issue as it might be difficult to implement this satisfactorily.

Pierre-Sassoulas commented 2 years ago

Please read https://github.com/PyCQA/pylint/issues/4907 before participating to the discussion in this issue.

KotlinIsland commented 2 years ago

image PyCharm correctly handles all of these scenarios.

KotlinIsland commented 2 years ago

@DanielNoord is 'superfluous-parens' removed entirely? I can't get it to trigger under any circumstances. Your comment suggests that it is removed, but that's not in the changelog and 'superfluous-parens' is listed in the enabled messages.

KotlinIsland commented 2 years ago

OHHHHHHHHHHHHH, it only detects single unnecessary parens

for (i) in ([1]):  # superfluous-parens
    print(i)

for ((i)) in (([1])):  # no warning
    print(i)
DanielNoord commented 2 years ago

Oh, I think you might have found a nice first step towards this: Raise superfluous-parens on nested parens if the message would be triggered on single parens.

Looking at your example that is currently not the case!

KotlinIsland commented 2 years ago

This seems like something that would be absurdly simple if a CST was used instead of analyzing the raw tokens.