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

False negative: no-member in conditional branch #9405

Open
hemberger opened this issue Jan 31, 2024 · 2 comments
Open

False negative: no-member in conditional branch #9405

hemberger opened this issue Jan 31, 2024 · 2 comments
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@hemberger
Copy link
Contributor

hemberger commented Jan 31, 2024

Bug description

There is a difference in behavior inside conditional branches depending on if the branch is reachable or not. If it is unreachable, then undefined-variable warnings are emitted, but no-member warnings are not (whereas if it is reachable, both undefined-variable and no-member warnings are emitted).

For example:

#!/usr/bin/env python3
"""False negative no-member"""
import datetime

SOME_BOOL = False
if SOME_BOOL:
    print(bar)
    print(datetime.does_not_exist)

This does not report no-member for datetime.does_not_exist. However, if we change SOME_BOOL = False to SOME_BOOL = True, then the no-member warning is emitted.

This can also happen for more complex conditionals, where it's not clear whether or not the branch is reachable. If pylint is performing fewer checks in conditional branches that it determines are unreachable as a performance optimization, then it probably should at least report that the branch as unreachable.

Thank you!

Configuration

No response

Command used

pylint test.py

Pylint output

************* Module test
test.py:7:10: E0602: Undefined variable 'bar' (undefined-variable)

Expected behavior

************* Module test
test.py:7:10: E0602: Undefined variable 'bar' (undefined-variable)
test.py:8:10: E1101: Module 'datetime' has no 'does_not_exist' member (no-member)

Pylint version

pylint 3.0.3
astroid 3.0.1
Python 3.10.13 (main, Sep 11 2023, 13:44:35) [GCC 11.2.0]

OS / Environment

RHEL8

Additional dependencies

No response

@hemberger hemberger added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 31, 2024
@jacobtylerwalls
Copy link
Member

This change was made in #4764 to address false positives where the if guards are performing a type-narrowing function.

There are two follow-ups I see immediately from your report, though.

  1. there's a better way of doing this now that we have constraints in astroid. See Support "is None" constraints from if statements during inference astroid#1189. I think your report should stand as the feature request for that.

  2. Regarding:

This can also happen for more complex conditionals, where it's not clear whether or not the branch is reachable.

We might be able to solve that by leveraging a recent feature:

diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index 9c73fee82..e559a8bdd 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -513,7 +513,7 @@ def _emit_no_member(
     parent: nodes.NodeNG = node.parent
     while parent != scope:
         if isinstance(parent, (nodes.If, nodes.IfExp)):
-            inferred = safe_infer(parent.test)
+            inferred = safe_infer(parent.test, compare_constants=True)
             if (  # pylint: disable=too-many-boolean-expressions
                 isinstance(inferred, nodes.Const)
                 and inferred.bool_value() is False

Would you be willing to see if that solves your use case? And if so, open a modest PR attaching a functional test? Many thanks!

@jacobtylerwalls jacobtylerwalls added False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 3, 2024
@hemberger
Copy link
Contributor Author

Thanks for the response! Your suggested patch does indeed fix the more complex conditional case, though the simple case still produces a false negative. I'll see if I can create a test from the more complex case, and if so then I'll submit a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants