pylint icon indicating copy to clipboard operation
pylint copied to clipboard

Published 20 hours ago • verified publisher icon pylint-dev

Resolve `used-before-assignment` false negative when a function is defined under `TYPE_CHECKING`

Open zenlyj opened this issue 1 year ago • 3 comments

Type of Changes

Type
✓ :bug: Bug fix
:sparkles: New feature
:hammer: Refactoring
:scroll: Docs

Description

Raise used-before-assignment when a function is defined under TYPE_CHECKING and used later.

Refs #10028

zenlyj avatar Oct 19 '24 07:10 zenlyj

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@15a5ac0). Learn more about missing BASE report. Report is 67 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10034   +/-   ##
=======================================
  Coverage        ?   95.80%           
=======================================
  Files           ?      174           
  Lines           ?    18956           
  Branches        ?        0           
=======================================
  Hits            ?    18160           
  Misses          ?      796           
  Partials        ?        0
Files with missing lines Coverage Δ
pylint/checkers/variables.py 97.24% <100.00%> (ø)

codecov[bot] avatar Oct 19 '24 07:10 codecov[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/nanoleaf/init.py#L77

The following messages are no longer emitted:

  1. possibly-used-before-assignment: Possibly using variable 'start' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/loader.py#L1203
  2. possibly-used-before-assignment: Possibly using variable 'service' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/cover/reproduce_state.py#L90
  3. possibly-used-before-assignment: Possibly using variable 'state' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/cover/device_condition.py#L137
  4. possibly-used-before-assignment: Possibly using variable 'to_state' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/cover/device_trigger.py#L157
  5. possibly-used-before-assignment: Possibly using variable 'name' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/steam_online/config_flow.py#L80
  6. possibly-used-before-assignment: Possibly using variable 'child_item_type' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/squeezebox/browse_media.py#L152
  7. possibly-used-before-assignment: Possibly using variable 'can_play' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/squeezebox/browse_media.py#L153
  8. possibly-used-before-assignment: Possibly using variable 'can_expand' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/squeezebox/browse_media.py#L154
  9. possibly-used-before-assignment: Possibly using variable 'year' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/homeassistant/triggers/time.py#L144
  10. possibly-used-before-assignment: Possibly using variable 'month' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/homeassistant/triggers/time.py#L145
  11. possibly-used-before-assignment: Possibly using variable 'day' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/homeassistant/triggers/time.py#L146
  12. possibly-used-before-assignment: Possibly using variable 'offset_value' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/google/calendar.py#L416
  13. possibly-used-before-assignment: Possibly using variable 'service' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/vacuum/reproduce_state.py#L88
  14. used-before-assignment: Using variable 'value_type' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/mysensors/climate.py#L179
  15. possibly-used-before-assignment: Possibly using variable 'device' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/alarmdecoder/config_flow.py#L118
  16. possibly-used-before-assignment: Possibly using variable 'title' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/alarmdecoder/config_flow.py#L127
  17. possibly-used-before-assignment: Possibly using variable 'unique_id' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/islamic_prayer_times/init.py#L69

Effect on django: The following messages are no longer emitted:

  1. possibly-used-before-assignment: Possibly using variable 'session_auth_hash' before assignment https://github.com/django/django/blob/99dcc59237f384d7ade98acfd1cae8d90e6d60ab/django/contrib/auth/init.py#L351
  2. possibly-used-before-assignment: Possibly using variable 'confirm' before assignment https://github.com/django/django/blob/99dcc59237f384d7ade98acfd1cae8d90e6d60ab/django/db/backends/oracle/creation.py#L48
  3. possibly-used-before-assignment: Possibly using variable 'confirm' before assignment https://github.com/django/django/blob/99dcc59237f384d7ade98acfd1cae8d90e6d60ab/django/db/backends/oracle/creation.py#L174
  4. possibly-used-before-assignment: Possibly using variable 'confirm' before assignment https://github.com/django/django/blob/99dcc59237f384d7ade98acfd1cae8d90e6d60ab/django/db/backends/sqlite3/creation.py#L43

Effect on pandas: The following messages are no longer emitted:

  1. redefined-variable-type: Redefinition of inferred_dtype type from str to pandas.core.dtypes.dtypes.ArrowDtype https://github.com/pandas-dev/pandas/blob/2a10e04a099d5f1633abcdfbb2dd9fdf09142f8d/pandas/core/dtypes/cast.py#L1153
  2. used-before-assignment: Using variable 'result' before assignment https://github.com/pandas-dev/pandas/blob/2a10e04a099d5f1633abcdfbb2dd9fdf09142f8d/pandas/core/arrays/arrow/array.py#L776

Effect on sentry: The following messages are no longer emitted:

  1. possibly-used-before-assignment: Possibly using variable 'state' before assignment https://github.com/getsentry/sentry/blob/bca4b1dc8055e082e826a7dde2cc9ebc9e44f542/src/sentry/integrations/slack/webhooks/action.py#L373
  2. possibly-used-before-assignment: Possibly using variable 'values' before assignment https://github.com/getsentry/sentry/blob/bca4b1dc8055e082e826a7dde2cc9ebc9e44f542/src/sentry/integrations/slack/webhooks/action.py#L375

This comment was generated for commit 33495488e1f8cefe9592804db8cf8cb82b84733b

github-actions[bot] avatar Oct 19 '24 07:10 github-actions[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit bd636411dfa85da90948f4950a1fda5ab9c1ee3c

github-actions[bot] avatar Oct 20 '24 10:10 github-actions[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/9d0701a62b63d95abac9c5da36039f18ed0bd8c7/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit a20eb069815839dd206807dd5cc3523082d26650

github-actions[bot] avatar Oct 22 '24 11:10 github-actions[bot]

Thanks so much @zenlyj, is this ready for review?

jacobtylerwalls avatar Oct 31 '24 11:10 jacobtylerwalls

i don't think the current approach is correct, it silences the check in many cases and something like this is not flagged even though it would blow up in runtime:

def outer(callback):
    def inner():
        nonlocal callback
        def inner2():
            callback()  # this should raise used-before-assignment
            def callback():
                pass
        inner2()
    inner()

def foo():
    print('foo')

outer(foo)

zenlyj avatar Oct 31 '24 14:10 zenlyj

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b234b5937af158e206905b8f0cf479386ab38153/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit 06555684defd55ddd25d54d84cbe10fdcf8cf356

github-actions[bot] avatar Oct 31 '24 15:10 github-actions[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b234b5937af158e206905b8f0cf479386ab38153/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit ac4f03a1c0d61ae33eacb702bc719f393ed3f041

github-actions[bot] avatar Oct 31 '24 16:10 github-actions[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b09e54c961db279785b75b5c3d192624b3d65664/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit ce768c2f3ae669f91a4350c00c3a98d22339cbf8

github-actions[bot] avatar Nov 01 '24 12:11 github-actions[bot]

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b09e54c961db279785b75b5c3d192624b3d65664/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit b6d7fc9e4ffaa93b7fe5d18a79c581207f49828b

github-actions[bot] avatar Nov 02 '24 13:11 github-actions[bot]

@jacobtylerwalls this is ready for review. let me know if there is anything i missed here, thank you!

zenlyj avatar Nov 02 '24 14:11 zenlyj

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b09e54c961db279785b75b5c3d192624b3d65664/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit d7ddc8967c105b9a115daa7768896953ace8c2d5

github-actions[bot] avatar Nov 03 '24 12:11 github-actions[bot]

I'll handle the Home Assistant false positive in another PR.

jacobtylerwalls avatar Nov 03 '24 12:11 jacobtylerwalls

hey @jacobtylerwalls, i was just thinking about this case:

def nonlocal_in_outer_frame_ok(callback, condition_a, condition_b):
    def outer():
        nonlocal callback
        def inner():
            if condition_a:
                def inner2():
                    callback()  # possibly-used-before-assignment?
                inner2()
            else:
                if condition_b:
                    def callback():
                        pass
        inner()
    outer()

we should probably raise a message here?

zenlyj avatar Nov 03 '24 13:11 zenlyj

we should probably raise a message here?

Good catch, I'm fine with opening a new issue for it. I think it should be real UBA, not "possibly".

jacobtylerwalls avatar Nov 03 '24 13:11 jacobtylerwalls

it is worth noting that prior to my changes in this PR, we're emitting possibly-used-before-assignment for this snippet. but i feel like this will be tough to crack as it requires some form of control flow analysis?

zenlyj avatar Nov 03 '24 13:11 zenlyj

🤖 Effect of this PR on checked open source code: 🤖

Effect on home-assistant: The following messages are now emitted:

  1. possibly-used-before-assignment: Possibly using variable 'touch_event_callback' before assignment https://github.com/home-assistant/core/blob/b09e54c961db279785b75b5c3d192624b3d65664/homeassistant/components/nanoleaf/init.py#L77

This comment was generated for commit 3808c4ecbf55b1d568475385869ac642f83a36be

github-actions[bot] avatar Nov 03 '24 13:11 github-actions[bot]

Hello @zenlyj. You've been doing a lot of great work for pylint lately, including this one, would you be interested in becoming a triager ? (I would normally have asked you privately but your commit mail is a no reply :) )

Pierre-Sassoulas avatar Nov 03 '24 14:11 Pierre-Sassoulas

Hello @zenlyj. You've been doing a lot of great work for pylint lately, including this one, would you be interested in becoming a triager ? (I would normally have asked you privately but your commit mail is a no reply :) )

hello, i just dropped you an email, let's talk there

zenlyj avatar Nov 03 '24 15:11 zenlyj