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.

Sign up for GitHub

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

Jump to bottom

Preserve parent CallContext when inferring nested functions #1982

Merged
merged 1 commit into from
Jan 29, 2023
Merged

Preserve parent CallContext when inferring nested functions #1982

merged 1 commit into from
Jan 29, 2023

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Jan 28, 2023

Description

Alternative to #1981
At the moment infer_call creates a new CallContext for each infer_call_result call overwriting the current one. Result was that nested functions with dynamic values couldn't be inferred.

Instead of overwriting the CallContext, keep it in parent_call_context so it can be reused later.
With that nested functions can now be inferred. The custom inference tip for typing.cast is thus no longer necessary.

https://github.com/PyCQA/astroid/blob/dfd88f5edc636df80c8cabd61a6b8d6bc8746ca9/astroid/inference.py#L274-L278

def f(x):
    return g(x)

def g(y):
    return y + 2

f(1)  #@

Closes pylint-dev/pylint#8074

@cdce8p cdce8p added this to the 2.13.4 milestone Jan 28, 2023
@cdce8p cdce8p mentioned this pull request Jan 28, 2023
Closed
@cdce8p cdce8p added the pylint-tested PRs that don't cause major regressions with pylint label Jan 28, 2023
@cdce8p cdce8p changed the title Preserve parent CallContext when inferring nested functions Preserve parent CallContext when inferring nested functions Jan 28, 2023
@codecov
Copy link

codecov bot commented Jan 28, 2023

Codecov Report

Merging #1982 (deb6573) into main (dfd88f5) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1982      +/-   ##
==========================================
- Coverage   92.65%   92.64%   -0.01%     
==========================================
  Files          94       94              
  Lines       10903    10891      -12     
==========================================
- Hits        10102    10090      -12     
  Misses        801      801
Flag Coverage Δ
linux 92.44% <100.00%> (+0.02%) ⬆️
pypy 80.09% <100.00%> (-8.45%) ⬇️
windows 92.36% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
astroid/brain/brain_typing.py 90.37% <ø> (+1.85%) ⬆️
astroid/inference.py 94.54% <ø> (ø)
astroid/context.py 97.36% <100.00%> (+0.03%) ⬆️
astroid/protocols.py 90.66% <100.00%> (ø)
astroid/interpreter/objectmodel.py 92.79% <0.00%> (-0.42%) ⬇️
astroid/rebuilder.py 97.94% <0.00%> (-0.35%) ⬇️

@cdce8p cdce8p mentioned this pull request Jan 28, 2023
Closed
Pierre-Sassoulas
Pierre-Sassoulas approved these changes Jan 29, 2023
View reviewed changes
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing code and fixing bug at the same time ! You're the bane of bad productivity metrics, Marc ;)

@cdce8p
Copy link
Member Author

cdce8p commented Jan 29, 2023

Removing code and fixing bug at the same time ! You're the bane of bad productivity metrics, Marc ;)

Thanks 😄 Took me literally the whole day yesterday for such an easy fix. I find the constant recursion in astroid quite difficult to debug but maybe that's just me.

@cdce8p cdce8p merged commit a0d219c into pylint-dev:main Jan 29, 2023
@cdce8p cdce8p deleted the dynamic-call-inference-2 branch January 29, 2023 08:23
github-actions bot pushed a commit that referenced this pull request Jan 29, 2023
@cdce8p @github-actions
Preserve parent CallContext when inferring nested functions (#1982)
0c58266 
(cherry picked from commit a0d219c)
@github-actions github-actions bot mentioned this pull request Jan 29, 2023
Merged
@cdce8p
Copy link
Member Author

cdce8p commented Jan 29, 2023

@Pierre-Sassoulas What do you think about the 2.13.4 release? It could be a good time after the backport is merged. Shortly after that, we could do 2.14.0 with just #1977, it's basically ready. That way could update the astroid version in pylint for the next release.

cdce8p added a commit that referenced this pull request Jan 29, 2023
@github-actions @cdce8p
Preserve parent CallContext when inferring nested functions (#1982) (#…
c8f8316 
…1983)

(cherry picked from commit a0d219c)

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
@DanielNoord
Copy link
Collaborator

Wonderful fix @cdce8p! Glad to see you opening some PRs again 😄

@cdce8p
Copy link
Member Author

cdce8p commented Jan 29, 2023

Glad to see you opening some PRs again 😄

Only for a short while, unfortunately. Will be quite busy the next month.

@cdce8p cdce8p mentioned this pull request Jan 29, 2023
Merged
@Pierre-Sassoulas
Copy link
Member

I'm definitely going to release 2.13.4 tomorrow.

@cdce8p cdce8p added the backported Assigned once the backport is done label Jan 31, 2023
@cdce8p cdce8p modified the milestone: 2.13.4 Jan 31, 2023
@cdce8p cdce8p mentioned this pull request Jan 31, 2023
Merged
cdce8p added a commit to cdce8p/astroid that referenced this pull request Jan 31, 2023
@cdce8p
Revert CallContext change since it caused a RecursionError regression
ffd9e37 
This reverts commit a0d219c (pylint-dev#1982).
cdce8p added a commit to cdce8p/astroid that referenced this pull request Jan 31, 2023
@cdce8p
Revert CallContext change since it caused a RecursionError regression
56fc74b 
This reverts commit a0d219c (pylint-dev#1982).
@cdce8p cdce8p mentioned this pull request Jan 31, 2023
Merged
cdce8p added a commit that referenced this pull request Jan 31, 2023
@cdce8p
Revert CallContext change since it caused a RecursionError regression (…
72f5afb 
…#2000)

This reverts commit a0d219c (#1982).
github-actions bot pushed a commit that referenced this pull request Jan 31, 2023
@cdce8p @github-actions
Revert CallContext change since it caused a RecursionError regression (…
549dbac 
…#2000)

This reverts commit a0d219c (#1982).

(cherry picked from commit 72f5afb)
cdce8p added a commit to cdce8p/astroid that referenced this pull request Jan 31, 2023
@cdce8p
[Backport 2.13.x] Revert CallContext change since it caused a Recursi…
3ee7a77 
…onError regression (pylint-dev#2000)

This reverts commit a0d219c (pylint-dev#1982).

(cherry picked from commit 72f5afb)
@cdce8p cdce8p mentioned this pull request Jan 31, 2023
Merged
cdce8p added a commit that referenced this pull request Jan 31, 2023
@cdce8p
[Backport 2.13.x] Revert CallContext change since it caused a Recursi…
af92f23 
…onError regression (#2000) (#2002)

This reverts commit a0d219c (#1982).

(cherry picked from commit 72f5afb)
cdce8p added a commit that referenced this pull request Jan 31, 2023
@github-actions @cdce8p
Revert CallContext change since it caused a RecursionError regression (…
56582d5 
…#2000) (#2001)

This reverts commit a0d219c (#1982).

(cherry picked from commit 72f5afb)

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
@jacobtylerwalls jacobtylerwalls mentioned this pull request Apr 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas approved these changes

Assignees
No one assigned
Labels
backported Assigned once the backport is done inference pylint-tested PRs that don't cause major regressions with pylint
Projects
None yet
Milestone
2.13.4
Development

Successfully merging this pull request may close these issues.

False positive not-a-mapping with cast and TypeVar
3 participants
@cdce8p @DanielNoord @Pierre-Sassoulas