Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type-hinted additional-builtins raise error since 2.12 #6388

Closed
jgraichen opened this issue Apr 19, 2022 · 2 comments · Fixed by #6392
Closed

Type-hinted additional-builtins raise error since 2.12 #6388

jgraichen opened this issue Apr 19, 2022 · 2 comments · Fixed by #6392
Assignees
Labels
Bug 🪲 C: undefined-variable Issues related to 'undefined-variable' check False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@jgraichen
Copy link

jgraichen commented Apr 19, 2022

Bug description

# pylint: disable=missing-docstring

from typing import TYPE_CHECKING, Any, Dict

if TYPE_CHECKING:
    __additional_builtin__: Dict[str, Any]


def run():
    return __additional_builtin__["test"]

This example will warn about __additional_builtin__ being an undefined-variable since 2.12.0 despite __additional_builtin__ being listed in .pylintrc.

Without the type hints, no warning is issued.

I looked at the source code, and tested it locally, and the new warning is added here: https://github.com/PyCQA/pylint/blob/main/pylint/checkers/variables.py#L1683-L1685=

The __additional_builtin__ variables is correctly identified as only previously assigned as a type hint, but the branch does not check for VARIABLES.additional-builtins as, for example, here: https://github.com/PyCQA/pylint/blob/main/pylint/checkers/variables.py#L1438=


I am developing a few plugins, e.g. for saltstack/salt, which are invoked with additional built ins by salt. Pylint is used in CI and the version maintained by renovate-bot. That highlighted the first failures here: jgraichen/salt-tower#25

Configuration

[VARIABLES]
additional-builtins=__additional_builtin__

Command used

pylint test.py

Pylint output

************* Module test
test.py:10:11: E0602: Undefined variable '__additional_builtin__' (undefined-variable)

------------------------------------------------------------------
Your code has been rated at 0.00/10 (previous run: 0.00/10, +0.00)

Expected behavior


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Pylint version

pylint 2.13.5
astroid 2.11.2
Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110]

OS / Environment

Debian 11

Additional dependencies

No response

@jgraichen jgraichen added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 19, 2022
@jgraichen jgraichen changed the title Type-hinted additional-builtins raise error since 1.12 Type-hinted additional-builtins raise error since 2.12 Apr 19, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Regression False Positive 🦟 A message is emitted but nothing is wrong with the code Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Regression False Positive 🦟 A message is emitted but nothing is wrong with the code labels Apr 19, 2022
@Pierre-Sassoulas
Copy link
Member

Why is __additional_builtin__ defined in TYPE_CHECKING guard and then used in run as something that is not typing ?

@jgraichen
Copy link
Author

jgraichen commented Apr 19, 2022

Some additional built ins are injected when salt is running plugins, for example, __salt__ or __grains__, or __opts__.

IDEs and type checkers do not know anything of these "built ins". Adding type hints assists e.g. IDEs to hint e.g. when calling wrong method, or accessing a dict with invalid keys.

In practice, this looks like:

if TYPE_CHECKING:
    __grains__: salt.utils.context.NamespacedDictWrapper
    __opts__: Dict[str, Any]
    __salt__: salt.loader.LazyLoader

image

@jacobtylerwalls jacobtylerwalls added Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code C: undefined-variable Issues related to 'undefined-variable' check and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 19, 2022
@jacobtylerwalls jacobtylerwalls self-assigned this Apr 19, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.13.6 milestone Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 C: undefined-variable Issues related to 'undefined-variable' check False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
3 participants