pylint-dev / astroid

A common base representation of python source code for pylint and other projects
https://pylint.readthedocs.io/projects/astroid/en/latest/
GNU Lesser General Public License v2.1
528 stars 273 forks source link

Module's locals and globals are the same? #712

Open queengooborg opened 4 years ago

queengooborg commented 4 years ago

I am working on a Pylint transform plugin I forked, and have been running into some unfortunate issues that took me a while to understand (sadly, it seems that documentation about writing transform plugins is quite sparse). I've been able solve most of them, except for one fatal issue.

The plugin is designed to add the imports and definitions that web2py adds to its controllers and models to remove all the "variable undefined" errors. Additionally, it combines all the code from the models, mimicking web2py's behavior. As a part of the process, it removes any unused imports from the module node (astroid.scoped_node.Module) globals as a way to remove those pesky "unused import" messages. However, if there's a local in the file that is also an import or a global, the plugin crashes with a KeyError inside of Astroid.

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.4_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.4_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/usr/local/lib/python3.7/site-packages/pylint/__main__.py", line 7, in <module>
    pylint.run_pylint()
  File "/usr/local/lib/python3.7/site-packages/pylint/__init__.py", line 20, in run_pylint
    Run(sys.argv[1:])
  File "/usr/local/lib/python3.7/site-packages/pylint/lint.py", line 1731, in __init__
    linter.check(args)
  File "/usr/local/lib/python3.7/site-packages/pylint/lint.py", line 1004, in check
    self._do_check(files_or_modules)
  File "/usr/local/lib/python3.7/site-packages/pylint/lint.py", line 1165, in _do_check
    self.check_astroid_module(ast_node, walker, rawcheckers, tokencheckers)
  File "/usr/local/lib/python3.7/site-packages/pylint/lint.py", line 1252, in check_astroid_module
    walker.walk(ast_node)
  File "/usr/local/lib/python3.7/site-packages/pylint/utils/ast_walker.py", line 77, in walk
    self.walk(child)
  File "/usr/local/lib/python3.7/site-packages/pylint/utils/ast_walker.py", line 74, in walk
    callback(astroid)
  File "/usr/local/lib/python3.7/site-packages/pylint/checkers/base.py", line 584, in visit_functiondef
    self._check_redefinition(node.is_method() and "method" or "function", node)
  File "/usr/local/lib/python3.7/site-packages/pylint/checkers/base.py", line 826, in _check_redefinition
    redefinitions = parent_frame.locals[node.name]
KeyError: 'languages'

I found that this is because locals and globals are the same variable (not just the same value; setting a key in one sets it in both) inside of the Module class in Astroid. I found an old commit that sets this behavior, but I feel that this is not the right way to solve whatever bug it was trying to fix. Otherwise, why have two separate variables in the first place?

I'm running PyLint 2.4.2 and Astroid 2.3.1.

PCManticore commented 4 years ago

Hey @vinyldarkscratch Thank you for opening an issue. I agree this doesn't make that much sense and the linked commit doesn't have any information pertaining to this change. We should investigate what would take to separate globals from locals in both astroid and pylint but seems we only have a handful of occurrences.