Skip to content

fix Feature: inheritance checks for typed dictionaries #1346

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix Feature: inheritance checks for typed dictionaries #1346

wants to merge 2 commits into from
+216 −32

Conversation

asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Oct 21, 2025 •
edited
Loading

part of #52

@meta-cla meta-cla bot added the cla signed label Oct 21, 2025
@yangdanny97
Copy link
Contributor

Running test.py should generate some changes, could you please include them in the commit?

@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 21, 2025 03:43
yangdanny97
yangdanny97 reviewed Oct 22, 2025
View reviewed changes
yangdanny97
yangdanny97 reviewed Oct 22, 2025
View reviewed changes
yangdanny97
yangdanny97 reviewed Oct 22, 2025
View reviewed changes
conformance/third_party/conformance.result
"typeddicts_readonly_consistency.py": [],
"typeddicts_readonly_inheritance.py": [
"Line 98: Expected 1 errors",
"Line 106: Expected 1 errors",
Copy link
Contributor

Choose a reason for hiding this comment

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

also, I don't think this fully fixes #52, we also have to apply these rules in multiple inheritance.

The code pointer for that is

pyrefly/pyrefly/lib/alt/class/class_field.rs

Line 2007 in 41db847

pub fn check_consistent_multiple_inheritance(&self, cls: &Class, errors: &ErrorCollector) {

yangdanny97
yangdanny97 requested changes Oct 22, 2025
View reviewed changes
Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

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

back to you with a few comments

@asukaminato0721 asukaminato0721 marked this pull request as draft October 23, 2025 01:45
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 23, 2025 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yangdanny97 yangdanny97 Awaiting requested review from yangdanny97

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

cla signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@asukaminato0721 @yangdanny97