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

try and use named arguments from caller for matching name #2294

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

asottile
Copy link
Contributor

test failing prior to change:

__________________ nullable_foreign_key_with_init_overridden ___________________
/home/asottile/workspace/django-stubs/tests/typecheck/fields/test_related.yml:806: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:2: note: Revealed type is "myapp.models.B" (diff)
E   Expected:
E     main:2: note: Revealed type is "Union[myapp.models.B, None]" (diff)
E   Alignment of first line difference:
E     E: ...te: Revealed type is "Union[myapp.models.B, None]"
E     A: ...te: Revealed type is "myapp.models.B"
E                                 ^

the FK example matches a similar one in sentry

@asottile asottile force-pushed the named-args-from-caller branch from 298983c to 43ac4a8 Compare July 30, 2024 15:51
for kinds, argnames, args in zip(ctx.arg_kinds, ctx.arg_names, ctx.args):
for kind, argname, arg in zip(kinds, argnames, args):
if kind == ArgKind.ARG_NAMED and argname == name:
return arg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

one could imagine a perverse class that does something like:

class FK(ForeignKey[_ST, _GT]):
   def __init__(self, *args, **kwargs):
       kwargs['null'] = not kwargs.get('null', False)
       super().__init__(*args, **kwargs)

but I'm thinking that's probably not going to happen and possibly ok to ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess they could even do that today and trick the current code with:

class FK(ForeignKey[_ST, _GT]):
    def __init__(self, *args, null: bool = False, **kwargs):
        null = not null
        super().__init__(*args, null=null, **kwargs)

so definitely not a problem imo :)

Copy link
Member

Choose a reason for hiding this comment

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

What do you think about reusing _get_argument from mypy.plugins.common? Would it solve this problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the current code is equivalent to that so it wouldn't help (_get_argument only looks at callee args)

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks!

@sobolevn sobolevn merged commit 7b7ddae into typeddjango:master Jul 30, 2024
36 checks passed
@asottile asottile deleted the named-args-from-caller branch August 4, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants