Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Linter for undefined variables #693

Merged
merged 41 commits into from
Mar 14, 2019
Merged

Linter for undefined variables #693

merged 41 commits into from
Mar 14, 2019

Conversation

MikhailArkhipov
Copy link

@MikhailArkhipov MikhailArkhipov commented Mar 4, 2019

Fixes #639
Fixes #395
Fixes #461
Fixes #475
Fixes #476
Fixes #513
Fixes #682
Fixes #734

  • Walk AST post analysis and use analysis to find out declared variables. Special filtering and handling of comprehensions. Includes Implement storing of variable definition locations #711
  • Adds support for analysis of comprehensions. Fixes issue when comprehension variables were not declared in scope. Adds handling of tuple expressions in for in loops.
  • Adds support for lambdas. Enables related tests.

@MikhailArkhipov MikhailArkhipov changed the title [WIP|: Linter for undefined variables Linter for undefined variables Mar 5, 2019
@MikhailArkhipov
Copy link
Author

Added settings plumbing to turn linter on/off via existing extension setting.

@jakebailey
Copy link
Member

There are a few false positives, which can be seen in least_angle.py in the scikit-learn repo

  • Builtin types (str, DeprecationWarning)
  • Uses of function parameters.
  • Some builtin functions like print, isinstance, abs.

@MikhailArkhipov
Copy link
Author

Any specific for function arguments?

@jakebailey
Copy link
Member

lars_path:

image

@jakebailey
Copy link
Member

image

But:

def lars_path(X, y, Xy=None, Gram=None, max_iter=500,
              alpha_min=0, method='lar', copy_X=True,
              eps=np.finfo(np.float).eps,
              copy_Gram=True, verbose=0, return_path=True,
              return_n_iter=False, positive=False):

@jakebailey
Copy link
Member

That seemed to fix many of the cases, but some still seem to be reported, like:

image

method is still reported (it's also a param), but X and y aren't anymore with that commit. alpha_min, copy_Gram are reported, and drop is too though it's a local variable and not a parameter, defined as drop = False at the topmost level of that function.

@MikhailArkhipov MikhailArkhipov changed the title Linter for undefined variables [WIP] Linter for undefined variables Mar 6, 2019
@MikhailArkhipov MikhailArkhipov changed the title Linter for undefined variables [WIP] Linter for undefined variables Mar 11, 2019
@MikhailArkhipov
Copy link
Author

Yes, analysis was missing handling of comprehension (list) statements, handling of tuple expressions in for loops as well as handling of lambdas. Basically analysis issues, less of that of the linter. Related #734 which also requires the above.

@jakebailey
Copy link
Member

Working a lot better now, one more thing in scikit-learn's k_means_.py:

image

Which is caused by a pyx import:

image

However if I try to make a minimal example where I do from idontexist import foo and use foo, it doesn't warn.

@jakebailey
Copy link
Member

This one seems to be tuple/iterator related as well (hierarchical.py next to k_means_.py):

image

@MikhailArkhipov
Copy link
Author

Speaking of imports @AlexanderSher can comment. I think there are different cases when imported variable was declared as unknown (and hence IS defined technically) vs code didn't even reach state of declarations as, I guess, happens with pyx.

I'll look into the iterator

@AlexanderSher
Copy link
Contributor

Which is caused by a pyx import:

Yes, we don't support CPython.

@jakebailey
Copy link
Member

Right, but I think what is weird is that in one case, if the imported module doesn't exist like from idontexist import foo, foo is declared, but when it's idontexist.pyx or similar, then doing the same thing causes foo to not be declared.

@MikhailArkhipov MikhailArkhipov changed the title [WIP] Linter for undefined variables Linter for undefined variables Mar 14, 2019
@MikhailArkhipov MikhailArkhipov merged commit 390fcbb into microsoft:master Mar 14, 2019
@MikhailArkhipov MikhailArkhipov deleted the linter branch March 14, 2019 20:39
jakebailey pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
* Fix microsoft#668 (partial)

* Undef variables, first cut

* Reorg

* Tests

* Tests

* Tests

* Revert "Tests"

This reverts commit 7ffc9db.

* Options and tests

* Don't squiggle builtin-ins

* Test for function arguments

* Fix tuple assignment in analysis

* Don't false positive on functions and classes forward references

* Disable tracking assignment location
Fix declaration of variables in for loop

* Track multiple locations

* Tests

* using

* Properly look at locations

* Comprehensions etc

* Test update

* Add support for lambdas

* Add lambda linting

* Handle tuple assignment better

* Test update

* Correct comprehension iterator handlint

* Fix race condition at builtins load

* Merge master

* Restore linter
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants