Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

Summary

Fixes part of #650

Added a sentinel literal type and wired is/is not narrowing to use final-name initializers, so Final sentinels now narrow even when the declared type isn’t literal. Updated literal display/output handling.

Test Plan

Added narrowing tests for Final bool and object sentinels.

@meta-cla meta-cla bot added the cla signed label Jan 29, 2026
@github-actions
Copy link

Diff from mypy_primer, showing the effect of this PR on open source code:

urllib3 (https://github.com/urllib3/urllib3)
- ERROR src/urllib3/util/timeout.py:128:16-79: Returned type `_TYPE_DEFAULT | float | None` is not assignable to declared return type `float | None` [bad-return]
- ERROR src/urllib3/util/timeout.py:150:19-24: Argument `_TYPE_DEFAULT | float` is not assignable to parameter `x` with type `Buffer | SupportsFloat | SupportsIndex | str` in function `float.__new__` [bad-argument-type]
- ERROR src/urllib3/util/timeout.py:158:16-26: `<=` is not supported between `_TYPE_DEFAULT` and `Literal[0]` [unsupported-operation]
- ERROR src/urllib3/util/timeout.py:270:24-34: Returned type `_TYPE_DEFAULT | float` is not assignable to declared return type `float | None` [bad-return]
- ERROR src/urllib3/util/timeout.py:271:30-84: No matching overload found for function `min` called with arguments: (float, _TYPE_DEFAULT | float) [no-matching-overload]
+ ERROR src/urllib3/util/timeout.py:271:23-85: No matching overload found for function `max` called with arguments: (Literal[0], float) [no-matching-overload]
- ERROR src/urllib3/util/timeout.py:271:31-71: `-` is not supported between `_TYPE_DEFAULT` and `float` [unsupported-operation]
- ERROR src/urllib3/util/timeout.py:273:27-67: `-` is not supported between `_TYPE_DEFAULT` and `float` [unsupported-operation]
- ::error file=src/urllib3/util/timeout.py,line=128,col=16,endLine=128,endColumn=79,title=Pyrefly bad-return::Returned type `_TYPE_DEFAULT | float | None` is not assignable to declared return type `float | None`
- ::error file=src/urllib3/util/timeout.py,line=150,col=19,endLine=150,endColumn=24,title=Pyrefly bad-argument-type::Argument `_TYPE_DEFAULT | float` is not assignable to parameter `x` with type `Buffer | SupportsFloat | SupportsIndex | str` in function `float.__new__`
- ::error file=src/urllib3/util/timeout.py,line=158,col=16,endLine=158,endColumn=26,title=Pyrefly unsupported-operation::`<=` is not supported between `_TYPE_DEFAULT` and `Literal[0]`%0A  Argument `_TYPE_DEFAULT` is not assignable to parameter `value` with type `int` in function `int.__ge__`
- ::error file=src/urllib3/util/timeout.py,line=270,col=24,endLine=270,endColumn=34,title=Pyrefly bad-return::Returned type `_TYPE_DEFAULT | float` is not assignable to declared return type `float | None`
- ::error file=src/urllib3/util/timeout.py,line=271,col=30,endLine=271,endColumn=84,title=Pyrefly no-matching-overload::No matching overload found for function `min` called with arguments: (float, _TYPE_DEFAULT | float)%0A  Possible overloads:%0A  (arg1: SupportsRichComparisonT, arg2: SupportsRichComparisonT, /, *_args: SupportsRichComparisonT, *, key: None = None) -> SupportsRichComparisonT [closest match]%0A  (arg1: _T, arg2: _T, /, *_args: _T, *, key: (_T) -> SupportsRichComparison) -> _T%0A  (iterable: Iterable[SupportsRichComparisonT], /, *, key: None = None) -> SupportsRichComparisonT%0A  (iterable: Iterable[_T], /, *, key: (_T) -> SupportsRichComparison) -> _T%0A  (iterable: Iterable[SupportsRichComparisonT], /, *, key: None = None, default: _T) -> SupportsRichComparisonT | _T%0A  (iterable: Iterable[_T1], /, *, key: (_T1) -> SupportsRichComparison, default: _T2) -> _T1 | _T2
+ ::error file=src/urllib3/util/timeout.py,line=271,col=23,endLine=271,endColumn=85,title=Pyrefly no-matching-overload::No matching overload found for function `max` called with arguments: (Literal[0], float)%0A  Possible overloads:%0A  (arg1: SupportsRichComparisonT, arg2: SupportsRichComparisonT, /, *_args: SupportsRichComparisonT, *, key: None = None) -> SupportsRichComparisonT [closest match]%0A  (arg1: _T, arg2: _T, /, *_args: _T, *, key: (_T) -> SupportsRichComparison) -> _T%0A  (iterable: Iterable[SupportsRichComparisonT], /, *, key: None = None) -> SupportsRichComparisonT%0A  (iterable: Iterable[_T], /, *, key: (_T) -> SupportsRichComparison) -> _T%0A  (iterable: Iterable[SupportsRichComparisonT], /, *, key: None = None, default: _T) -> SupportsRichComparisonT | _T%0A  (iterable: Iterable[_T1], /, *, key: (_T1) -> SupportsRichComparison, default: _T2) -> _T1 | _T2
- ::error file=src/urllib3/util/timeout.py,line=271,col=31,endLine=271,endColumn=71,title=Pyrefly unsupported-operation::`-` is not supported between `_TYPE_DEFAULT` and `float`%0A  Argument `_TYPE_DEFAULT` is not assignable to parameter `value` with type `float` in function `float.__rsub__`
- ::error file=src/urllib3/util/timeout.py,line=273,col=27,endLine=273,endColumn=67,title=Pyrefly unsupported-operation::`-` is not supported between `_TYPE_DEFAULT` and `float`%0A  Argument `_TYPE_DEFAULT` is not assignable to parameter `value` with type `float` in function `float.__rsub__`

@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 29, 2026 04:48
Copilot AI review requested due to automatic review settings January 29, 2026 04:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements sentinel literal types to support type narrowing with is/is not operations on Final variables, as part of addressing issue #650 for comprehensive type guard support.

Changes:

  • Added LitSentinel variant to the Lit enum for representing sentinel values
  • Implemented final_sentinel_literal_for_expr to detect Final sentinel values and convert them to literal types
  • Integrated sentinel narrowing into is/is not narrowing operations
  • Updated type display and output handling for sentinel literals

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pyrefly/lib/test/narrow.rs Added test cases for Final bool and object sentinel narrowing
pyrefly/lib/alt/narrow.rs Implemented sentinel detection and integrated into narrowing logic
crates/pyrefly_types/src/literal.rs Added LitSentinel struct and related trait implementations
crates/pyrefly_types/src/display.rs Added display formatting for sentinel literals
crates/pyrefly_types/src/type_output.rs Added output handling for sentinel literals
Cargo.lock Automatic checksum update for backtrace dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +71 to +90
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Visit, VisitMut, TypeEq)]
pub struct LitSentinel {
pub module: ModuleName,
pub name: Name,
pub range: Option<TextRange>,
pub class: ClassType,
}

impl Ord for LitSentinel {
fn cmp(&self, other: &Self) -> Ordering {
(&self.module, &self.name, &self.class).cmp(&(&other.module, &other.name, &other.class))
}
}

impl PartialOrd for LitSentinel {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The derived PartialEq, Eq, and Hash traits on line 71 compare/hash all fields including range, but the manual Ord and PartialOrd implementations exclude range from comparison. This is inconsistent and violates Rust's requirements:

  1. Ord must be consistent with PartialEq: if two values are equal by cmp, they must be equal by PartialEq
  2. Hash must be consistent with PartialEq: if two values are equal by PartialEq, they must have the same hash

Currently, two LitSentinel values that differ only in their range field would be:

  • Unequal by PartialEq (includes range)
  • Equal by Ord (excludes range)
  • Potentially different hashes (includes range)

This violates constraint 1. The fix is to manually implement PartialEq, Eq, and Hash to exclude range, matching the Ord behavior. See how similar types in the codebase handle this pattern.

Copilot uses AI. Check for mistakes.
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.

1 participant