Error detection feature request for new language server about python-language-server HOT 6 OPEN

This could be a warning since raising an exception isn't an error. @MikhailArkhipov how doable is this? Wouldn't decorators and metaclasses make this hard to get right? My worry would be the false-positives would just be too much if we can't make it reasonably robust.

from python-language-server.

@brettcannon
Raising an exception isn't an error, but not handing it is. That's when you need to show error.

About false positives.
This can be said almost about anything in python since it's dynamic. Consider this for example.

import conf

locals()['a'] = 1
print(a)

both pylint and pyflakes will complain

$ pylint delme.py
delme.py:1:0: E0401: Unable to import 'conf' (import-error)
delme.py:4:6: E0602: Undefined variable 'a' (undefined-variable)

$ pyflakes delme.py
delme.py:1: 'conf' imported but unused
delme.py:4: undefined name 'a'

But this successfully executes

$ python delme.py
1

Because conf is a virtual module created via .pth file and because of locals manipulation. I don't know about everyone but I expect python linters to not be smart enough to know this (except locals and globals manipulation) and I would prefer to see error and not a warning in this case.

I think warning should be issued when code could possibly work and an error should be issued when there's no chance the code would work. But that's not how all popular python linters work. They all show errors on code that would work.

from python-language-server.

The problem is if you're developing a library then not catching an exception may be on purpose as you want the application to catch it and handle it, e.g. StopIteration or something. That means we can't universally consider unhandled exceptions are an error.

And you're right that nearly everything could be considered a potential false-positive that isn't syntactically determined. Because of this we have so far not done any linter-style checks in the language server until we determine what we consider absolutely critical to the vast majority of users and what is better left up to third-party linters like flake8 and Pylint and not to duplicate their work if it isn't going to be useful to almost all of our users.

from python-language-server.

@brettcannon Somehow my post got lost, so I will have to repeat it briefly. My opinion on this is it doesn't really matter whether to show potential error as error or as a warning. What matters is to implement ability to ignore errors in the right way. And inserting special comments as many linters and PyCharm do is the wrong way to do it (it's probably okay to do that if linting is part of CI). Inserting comments for ignoring certain types of errors can prevent detecting new error when code is changed. The reason for the same type of error would be different. It should be reported to user, but it wouldn't. That's the area where you can be better than any available solution.

from python-language-server.

FYI pylint complains about this, but flake8 does not. But then pylint gives false-positives for the check.

def deco(func):
    def wrapper(*args, **kwargs):
        return do_some_stuff(42)
    return wrapper

@deco
def do_some_stuff(real_arg):
    pass


do_some_stuff(bad_arg='some')

Sample Pylint output:

************* Module other_example
other_example.py:1:0: C0111: Missing module docstring (missing-docstring)
other_example.py:1:0: C0111: Missing function docstring (missing-docstring)
other_example.py:2:0: W0613: Unused argument 'args' (unused-argument)
other_example.py:2:0: W0613: Unused argument 'kwargs' (unused-argument)
other_example.py:1:9: W0613: Unused argument 'func' (unused-argument)
other_example.py:7:0: C0111: Missing function docstring (missing-docstring)
other_example.py:7:18: W0613: Unused argument 'real_arg' (unused-argument)
other_example.py:11:0: E1123: Unexpected keyword argument 'bad_arg' in function call (unexpected-keyword-arg)
other_example.py:11:0: E1120: No value for argument 'real_arg' in function call (no-value-for-parameter)

from python-language-server.

@brettcannon
Using your example to show what I mean.

def deco(func):
    def wrapper(*args, **kwargs):
        return do_some_stuff(42)
    return wrapper

@deco
def do_some_stuff(real_arg):
    pass


#pylint: disable=E1120
do_some_stuff(bad_arg='some')

Now I change the code

def deco(func):
    def wrapper(*args, **kwargs):
        return do_some_stuff() # remove argument
    return wrapper

@deco
def do_some_stuff(real_arg):
    pass


#pylint: disable=E1120
do_some_stuff(bad_arg='some')

And pylint hides real problem now.

from python-language-server.