Skip to content

Use inference to determine if **kwargs is missing a named parameter #8785

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

Open
jacobtylerwalls opened this issue Jun 18, 2023 · 3 comments
Open
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Jun 18, 2023

I would expect these two lines to raise the same set of messages. We don't use inference to analyze whether **kwargs contains the missing named parameter x. We could.

import copy
import os

copy.copy(**{"y": os.environ})  # [unexpected-keyword-arg]
copy.copy(y=os.environ)  # [no-value-for-parameter, unexpected-keyword-arg]

False negative for no-value-for-parameter.

@jacobtylerwalls jacobtylerwalls added Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling False Negative 🦋 No message is emitted but something is wrong with the code Good first issue Friendly and approachable by new contributors labels Jun 18, 2023
@Pierre-Sassoulas
Copy link
Member

We did that recently for #8719, we could design a generic solution for checker that analyses args / kwargs,

@Pierre-Sassoulas Pierre-Sassoulas added Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 18, 2023
@jacobtylerwalls
Copy link
Member Author

Yep, the aftermath of that PR is what led me to #8775 and then #8774 and then this :-)

@jacobtylerwalls
Copy link
Member Author

I think a broader refactor is a separate issue, though. This issue could be as simple as just removing this assumption:

# 3. Match the **kwargs, if any.
if node.kwargs:
for i, [(name, _defval), _assigned] in enumerate(parameters):
# Assume that *kwargs provides values for all remaining
# unassigned named parameters.
if name is not None:
parameters[i] = (parameters[i][0], True)

@jacobtylerwalls jacobtylerwalls added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. labels Jul 17, 2023
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 Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants