[asukaminato0721]

Summary

Fixes #1129

fixed the binop resolution path so NotImplementedType no longer leaks as a possible operator result.

successful dunder calls now strip only the exact NotImplementedType branch, keep searching reflected dunders when needed, and union any concrete results that can actually occur at runtime.

Test Plan

a regression test covering both pure fallback and mixed int | NotImplementedType behavior.

[Copilot]

Pull request overview

This PR fixes issue #1129, where binary operations involving dunder methods that return NotImplementedType would incorrectly leak the NotImplementedType into the inferred result type instead of following Python's runtime semantics (trying the reflected dunder when the forward one signals NotImplemented).

Changes:

• The try_binop_calls logic in operators.rs is updated to strip NotImplementedType from successful dunder return types, accumulate non-NotImplementedType results, and continue searching for reflected dunders when needed.
• NotImplementedType is added as a new stdlib entry in Stdlib, gated to Python ≥ 3.10 (consistent with how EllipsisType is handled).
• A regression test is added covering both the pure-NotImplementedType fallback and the mixed int | NotImplementedType scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pyrefly/lib/alt/operators.rs	Core fix: strips NotImplementedType from successful dunder results, accumulates partial results, and continues to reflected dunders when needed
crates/pyrefly_types/src/stdlib.rs	Adds NotImplementedType as an Option<StdlibResult<ClassType>> stdlib entry, guarded by version >= 3.10
pyrefly/lib/test/operators.rs	Regression test covering the bug from issue #1129 for both pure and mixed NotImplementedType return types

---

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

You can also share your feedback on Copilot code review. Take the survey.

[stroxler]

I don't think we can merge this as is - the mypy primer noise is a hint that something is wrong here.

I think the issue is probably tied to NotImplementedType vs Never / NoReturn.

Some AI review suggestions:

Blocking finding

Preserve genuinely non-returning dunders — [operators.rs:265–274](

pyrefly/pyrefly/lib/alt/operators.rs

Lines 265 to 274 in a987c5b

	let ret_without_not_implemented = self.distribute_over_union(&ret, |ret| {
	if matches!(ret, Type::ClassType(cls) if cls == not_implemented_type) {
	self.heap.mk_never()
	} else {
	ret.clone()
	}
	});
	if ret_without_not_implemented.is_never() {
	continue;
	}

).

The is_never() check conflates two cases:

• The method returns only NotImplementedType, so dispatch should continue.
• The method returns Never, so dispatch cannot continue.

With A.__add__(object) -> Never and B.__radd__(A) -> str, the patched checker infers str for A() + B(). At runtime, A.__add__ raises and the reflected method never executes. Without a reflected method, this instead produces a false unsupported-operation error. The same issue affects augmented assignment.

Fix: detect whether NotImplementedType was actually present; leave an original Never unchanged. This also matches the Never/NoReturn failures visible in the PR’s primer differential.

Other review concerns

• The mixed-return test does not match runtime semantics. The added test asserts C() + D() is int, but executing that exact example returns str. For a general int | NotImplementedType return, reflected results remain possible. If strip-only behavior is intentional, describe it as an approximation rather than complete runtime modeling.
• Keep the sentinel-absent fast path unchanged. Every successful dunder now allocates a vector, clones its result, and rebuilds unions—even without NotImplementedType. Selective stripping would avoid this cost.

[stroxler]

Looks like a good improvement! This is big change though - fresh PR review and analysis of mypy_primer results flagged a few more issues we should address before we merge

1. Inferred placeholder returns falsely reject inherited operators.
   [operators.rs:268–283](

   pyrefly/pyrefly/lib/alt/operators.rs

   Lines 268 to 283 in ba97ed5

   	let ret = match ret {
   	Type::ClassType(cls) if cls == *not_implemented_type => continue,
   	Type::Union(mut union) => {
   	if let Some(index) = union.members.iter().position(
   	|member| matches!(member, Type::ClassType(cls) if cls == not_implemented_type),
   	) {
   	union.members.remove(index);
   	let ret_without_not_implemented = self.unions(union.members);
   	errors.extend(callee_errors);
   	successful_ret = self.union(successful_ret, ret_without_not_implemented);
   	continue;
   	}
   	Type::Union(union)
   	}
   	ret => ret,
   	};

   )

   This explains the remaining xarray Day + Day primer error. The inherited __add__ calls self.__apply__; Pyrefly infers its return using the base implementation, which returns NotImplemented. Day overrides __apply__, so the operation actually succeeds—but the patch treats the inferred placeholder as definitive rejection.

   I reproduced this with a minimal base/subclass example: no errors before the patch, unsupported-operation afterward, and CPython returns Day. The fix should account for inferred placeholder returns before treating them like explicitly annotated sentinel-only returns.

2. Equality needs identity fallback when both methods decline.
   [operators.rs:294–310](

   pyrefly/pyrefly/lib/alt/operators.rs

   Lines 294 to 310 in ba97ed5

   	if !successful_ret.is_never() {
   	successful_ret
   	} else if let Some((callee_errors, call_errors, ret)) = first_call {
   	errors.extend(callee_errors);
   	errors.extend(call_errors);
   	ret
   	} else {
   	let dunders = calls
   	.iter()
   	.map(|(dunder, _, _)| format!("`{dunder}`"))
   	.collect::<Vec<_>>()
   	.join(" or ");
   	self.error_with_context(
   	errors,
   	range,
   	ErrorKind::UnsupportedOperation,
   	format!("Cannot find {dunders}"),

   )

   When both equality methods return NotImplemented, Python falls back to identity comparison. The patch instead reports unsupported-operation. Mixed returns are also affected: methods annotated Literal[True] | NotImplementedType produce inferred Literal[True], although comparing distinct instances can return False.

   Preserve a bool identity-fallback branch for ==/!=, and cover both pure and mixed sentinel returns.

3. The pure-sentinel arm drops deprecation diagnostics. [Line 269](

   pyrefly/pyrefly/lib/alt/operators.rs

   Line 269 in ba97ed5

   Type::ClassType(cls) if cls == *not_implemented_type => continue,

   ) continues before propagating callee_errors. I verified a deprecated forward method warns before the patch but not afterward. Preserve those diagnostics before continuing.

[github-actions]

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

============================================================
SUMMARY
============================================================
Total: +7 new errors, -2 fixed errors
By preset: +0/-1 (default), +0/-1 (strict)

Projects with changes (3):
  openlibrary: +4 -1
  manticore: +0 -1
  xarray: +3 -0
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

openlibrary (https://github.com/internetarchive/openlibrary)
- ERROR openlibrary/book_providers.py:596:92-155: Implicit conversion of `Acquisition | NotImplementedType | Unknown | None` to `bool` is not allowed [implicit-bool]
+ ERROR openlibrary/book_providers.py:596:92-155: Implicit conversion of `Acquisition | Unknown | None` to `bool` is not allowed [implicit-bool]
+ ERROR openlibrary/solr/updater/edition.py:353:16-63: Returning implicit Any from function declared to return "bool" [no-any-return-implicit]
+ ERROR openlibrary/solr/updater/edition.py:526:16-75: Returning implicit Any from function declared to return "bool" [no-any-return-implicit]
+ ERROR openlibrary/solr/updater/edition.py:530:16-78: Returning implicit Any from function declared to return "bool" [no-any-return-implicit]

manticore (https://github.com/trailofbits/manticore)
- ERROR manticore/native/manticore.py:82:24-43: `!=` is not supported between `Module[manticore.platforms]` and `Module[manticore.platforms]` [unsupported-operation]

xarray (https://github.com/pydata/xarray)
+ ERROR xarray/core/resample_cftime.py:459:23-34: The type of `lresult` is unknown because it comes from an error [unknown-from-error]
+ ERROR xarray/tests/test_cftime_offsets.py:558:14-29: The type of `result` is unknown; it is inferred as an implicit `Any` [unknown-variable-type]
+ ERROR xarray/tests/test_cftime_offsets.py:565:14-29: The type of `result` is partially unknown; it contains an implicit `Any` [partially-unknown-type]

[stroxler]

I'm actually going to close this. I think it's a good issue to fix, I'm closing it for two reasons:

• first, I realized the PR is too old, it's attached to a stale diff so auto-import doesn't work
• in addition, I actually think it might be better to make a fresh attempt off of trunk anyway due to structural isssues

Structural issues

Based on some AI review and looking at what happened when I tried to fix forward some loose ends, I think this approach is not optimal for maintainability. Here are some notes on what we might be able to do better

---

Suggested direction for a fresh NotImplemented PR

Keep the PR focused

1. Give call outcomes an explicit contract. Distinguish an absent method, a hard signature failure, and a successful call. For a successful call, describe its concrete result and whether it can decline. A genuine Never return is successful non-returning execution, not absence and not a decline. Use Option/enum state, not Never as a bookkeeping sentinel.
2. Keep dispatch policy outside return interpretation. Arithmetic, rich comparisons, and inplace operations have different completion rules. Equality adds identity fallback only when dispatch remains unresolved; arithmetic errors when no result is possible. An inplace decline begins ordinary binary fallback. Do not communicate fallback eligibility with one cumulative 'some earlier method declined' flag: it must reflect the relevant dispatch stage and preserve existing hard-signature-error recovery.
3. State the approximation for unannotated methods. An explicitly annotated sentinel-only result can definitively decline. A nontrivial inferred sentinel-only wrapper can reflect the base implementation rather than a subclass helper's result; handle it conservatively for concrete receivers, Self, and bounded TypeVars alike. Preserve overload signatures and direct sentinel bodies. Do not widen every mixed inferred union to Any: ordinary guarded operators regress. Keep the existing mixed delegated-return precision limitation explicit unless the new PR intentionally implements per-return provenance.
4. Preserve compatibility and diagnostics. Operand unions are already distributed by callers; retain and test that invariant. Preserve existing call-error fallback, deprecation diagnostics, operand labels, and missing-method messages. Keep the sentinel-absent fast path unchanged. Reuse name-based sentinel recognition rather than introducing an eager stdlib primitive dependency.

A compact return-interpretation helper plus operator-specific completion is sufficient. There is no need for a general dispatch framework or new binding-layer callable-identity metadata in this PR.

Separate work suitable for separate PRs

Correcting reflected subclass priority, identical-runtime-type arithmetic, literal/TypeVar class normalization, and inplace ordering is a second concern. Those changes can improve behavior, but combining them with sentinel handling expands both the compatibility surface and the review burden.

In particular, do not require exact function-object alias analysis to close #1129. The abandoned stack grew source-definition stability metadata, loop/function scope tracking, classmethod binding comparisons, and decorator exceptions to improve reflected-method identity. That is a distinct design problem: equal function-definition IDs do not identify equal runtime closures, and type-preserving decorators can manufacture fresh callables without changing their types. Leave existing ordering intact in the focused PR, or isolate an explicitly approximate ordering correction in a separate PR with its own runtime tests and corpus differential. Do not silently claim complete CPython dispatch fidelity.

[stroxler]

Whoops, meant to close this PR - the stale attached diff means it will be easier to turn around review for a fresh PR (with a simplified approach)