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

[red-knot] Handle possibly-unbound instance members #16363

Merged
merged 4 commits into from
Feb 25, 2025

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Feb 25, 2025

Summary

Adds support for possibly-unbound/undeclared instance members.

Test Plan

New MD tests.

@sharkdp sharkdp added the red-knot Multi-file analysis & type inference label Feb 25, 2025
Comment on lines +879 to +880
# Emitting a diagnostic in a case like this is not something we support, and it's unclear
# if we ever will (or want to)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We came to the same conclusion in an earlier PR. This is also mentioned higher up in this test suite. And mypy/pyright don't emit a diagnostic for this either (which is also mentioned higher up). This test is only added here for easier reference / completeness.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Looks good! A couple of minor questions

@carljm
Copy link
Contributor

carljm commented Feb 25, 2025

The handling of inherited attributes here reminds me that we will also need to add eager Liskov checking for inherited classes (once we have callable types and can do callable subtype checks). For a mutable attribute (which all are by default) a strict Liskov interpretation would require invariance, which would make the union handling here irrelevant.

@sharkdp
Copy link
Contributor Author

sharkdp commented Feb 25, 2025

which would make the union handling here irrelevant

Mainly curious: is this true even in the presence of gradual types? Could a subclass refine a base-class Any attribute to a more static type T? And would it then make sense to treat a possibly-unbound-on-child-class attribute as being of type Any | T?

@carljm
Copy link
Contributor

carljm commented Feb 25, 2025

Mainly curious: is this true even in the presence of gradual types? Could a subclass refine a base-class Any attribute to a more static type T? And would it then make sense to treat a possibly-unbound-on-child-class attribute as being of type Any | T?

That's a good point: I think that would make sense.

@sharkdp sharkdp force-pushed the david/possibly-unbound-instance-members branch from ad83ea5 to 8916f25 Compare February 25, 2025 18:39
@sharkdp sharkdp merged commit f88328e into main Feb 25, 2025
21 checks passed
@sharkdp sharkdp deleted the david/possibly-unbound-instance-members branch February 25, 2025 19:00
@sharkdp sharkdp mentioned this pull request Feb 25, 2025
18 tasks
dcreager added a commit that referenced this pull request Feb 25, 2025
* main:
  [red-knot] Rename constraint to predicate (#16382)
  [red-knot] Correct modeling of dunder calls (#16368)
  [red-knot] Handle possibly-unbound instance members (#16363)
class.own_instance_member(db, name)
{
return member;
// TODO: We could raise a diagnostic here if there are conflicting type qualifiers
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably the diagnostic would be raised by an eager Liskov check at the inheritance site, rather than here? But it doesn't hurt to have a TODO here until we do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants