Skip to content

Conversation

@fangyi-zhou
Copy link
Contributor

@fangyi-zhou fangyi-zhou commented Nov 26, 2025

Summary

This diff does two minor fixes.
(1)
The test test_runtime_checkable_unsafe_overlap_with_inheritance was not a bug. @runtime_checkable requires a direct inheritance of Protocol, so the error message is valid.
Otherwise, a runtime error would be thrown.
This diff adds the direct inheritance and the expected error for unsafe overlap is emitted.

(2)
When there are multiple unsafe overlaps exists for a runtime checkable protocol, multiple errors were emitted.
This treatment was suboptimal, so this diff groups the errors into a single error, and explains all the overlapping fields in a multi-line error message.

Test Plan

test.py

… test case

Summary:
This diff does two minor fixes.
(1)
The test `test_runtime_checkable_unsafe_overlap_with_inheritance` was
not a bug. `@runtime_checkable` requires a direct inheritance of
`Protocol`, so the error message is valid.
Otherwise, a runtime error would be thrown.
This diff adds the direct inheritance and the expected
error for unsafe overlap is emitted.

(2)
When there are multiple unsafe overlaps exists for a runtime checkable
protocol, multiple errors were emitted.
This treatment was suboptimal, so this diff groups the errors into a
single error, and explains all the overlapping fields in a multi-line
error message.

Test Plan:
`test.py`
@meta-cla meta-cla bot added the cla signed label Nov 26, 2025
Copy link
Contributor

@stroxler stroxler left a comment

Choose a reason for hiding this comment

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

LGTM, I'll try to get a second review

@meta-codesync
Copy link

meta-codesync bot commented Nov 28, 2025

@stroxler has imported this pull request. If you are a Meta employee, you can view this in D88008446.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants