fix __bool__ check should look at Never return #1021

[asukaminato0721]

Summary

Fixes #1021

tightening the __bool__ guard.

The check still allows a raw Never value in boolean position, but it now rejects a callable __bool__ whose return type is Never

Test Plan

add test

[Copilot]

Pull request overview

Tightens Pyrefly’s truthiness checking so __bool__ implementations that never return (i.e., return type Never) are rejected in boolean contexts, aligning behavior with the expectation in issue #1021.

Changes:

• Update check_dunder_bool_is_callable to additionally error when a callable __bool__ has return type Never.
• Add a regression test covering def __bool__(...) -> Never used in an if condition.

Reviewed changes

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

File	Description
pyrefly/lib/alt/attr.rs	Adds a Never-return check for callable __bool__ attributes during truthiness validation.
pyrefly/lib/test/simple.rs	Adds a regression testcase asserting an error when __bool__ returns Never.

---

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

[Copilot]

callable_return_type only works for top-level callable types (Function/BoundMethod/Callable/Overload). Since callability is checked via as_call_target, dunder_bool_ty can still be callable via __call__ (e.g., an instance/class/protocol with __call__) but then callable_return_type returns None and this Never-return guard won’t fire. If the intent is to reject any callable __bool__ whose call result is Never, consider extracting the return type from the CallTarget (or doing a no-arg call_infer with swallowed errors) rather than relying on Type::callable_return_type.

[stroxler]

I think this makes sense, I'll try to get a second review and merge. Thanks!

[meta-codesync]

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

[fangyi-zhou]

It's interesting why isna is triggering new errors for pandas.

• ERROR pandas/core/arrays/base.py:592:12-42: The __bool__ method of NDFrame returns Never, so it cannot be used as a boolean [invalid-argument]

https://github.com/pandas-dev/pandas/blob/1bba48f900275af485e457b056f3e4fc82b38f94/pandas/core/arrays/base.py#L592

• ERROR pandas/core/arrays/interval.py:1054:12-28: The __bool__ method of NDFrame returns Never, so it cannot be used as a boolean [invalid-argument]

https://github.com/pandas-dev/pandas/blob/1bba48f900275af485e457b056f3e4fc82b38f94/pandas/core/arrays/interval.py#L1054

Looking at the definition of isna https://github.com/pandas-dev/pandas/blob/main/pandas/core/dtypes/missing.py
It looks like we're getting back an union type with one of the return types with invalid __bool__ check.

[fangyi-zhou]

class Foo:
    def __bool__(self):
        raise TypeError
class Bar(Foo):
    def __bool__(self) -> bool:
        return True
bar: Foo = Bar()
if bar: ...   # <-- We're reporting a false positive here

I managed to produce a false positive error here, maybe we shall tighten the check?

[fangyi-zhou]

Review automatically exported from Phabricator review in Meta.

[fangyi-zhou]

Thanks for addressing the issue with the false positive! I'd defer to members of the team to see if we need to do anything with the union issue.

[github-actions]

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

pandas (https://github.com/pandas-dev/pandas)
+ ERROR pandas/core/arrays/base.py:598:12-42: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/base.py:1540:12-28: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/interval.py:1053:12-28: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/masked.py:1155:13-80: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/masked.py:1173:12-44: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/sparse/array.py:868:12-28: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/arrays/sparse/array.py:2041:8-24: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/generic.py:10058:12-19: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/groupby/generic.py:2851:20-38: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/indexes/multi.py:3596:12-40: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/nanops.py:358:20-40: The `__bool__` method of `NDFrame` returns `Never`, so it cannot be used as a boolean [invalid-argument]
+ ERROR pandas/core/reshape/merge.py:1606:28-55: The `__bool__` method of `Series` returns `Never`, so it cannot be used as a boolean [invalid-argument]

[github-actions]

Primer Diff Classification

❌ 1 regression(s) | 1 project(s) total | +12 errors

1 regression(s) across pandas. error kinds: invalid-argument.

Project	Verdict	Changes	Error Kinds	Root Cause
pandas	❌ Regression	+12	invalid-argument	pyrefly/lib/alt/attr.rs

Detailed analysis

❌ Regression (1)

pandas (+12)

These 12 errors are false positives (regression). The error message says 'The __bool__ method of NDFrame returns Never, so it cannot be used as a boolean.' The pandas NDFrame.__bool__ method does NOT have an explicit -> Never return annotation — it simply raises a ValueError. The PR's intent was to only flag __bool__ methods with an explicit -> Never annotation, but the callable_has_explicit_return_annotation check appears to be incorrectly returning true for pandas's NDFrame.__bool__, which has no return annotation at all (its return type is merely inferred as Never because the body unconditionally raises). The PR's own test case test_dunder_bool_returning_never_subclass_override demonstrates the intended behavior: a __bool__ without annotation that raises should NOT trigger this error. Neither mypy nor pyright flags any of these 12 locations. The def_index lookup in function_def_has_return_annotation may be resolving to the wrong function definition or failing to correctly determine that no annotation exists, causing the false positives.
Attribution: The change in pyrefly/lib/alt/attr.rs adds a new check: when __bool__'s return type is Never AND the function has an explicit return annotation, pyrefly now reports an error. The callable_has_explicit_return_annotation function checks whether the function definition has a return annotation. The second test case (test_dunder_bool_returning_never_subclass_override) shows the intent: if a base class's __bool__ has no annotation but raises (inferred Never), and a subclass overrides it with -> bool, the base class usage should NOT be flagged. However, the issue is that function_def_has_return_annotation in pyrefly/lib/binding/bindings.rs is being called via def_index, and the check callable_has_explicit_return_annotation is supposed to filter out cases where the return type is inferred (not explicitly annotated). The pandas NDFrame.__bool__ does NOT have an explicit -> Never annotation — it just raises. Looking at the test case test_dunder_bool_returning_never_subclass_override, the base class Foo.__bool__ has no return annotation and just raises, and the test expects NO error for if bar: .... But the errors in pandas suggest that pyrefly IS flagging classes whose __bool__ has no explicit return annotation but whose body is inferred as Never. This means the callable_has_explicit_return_annotation check is not working correctly for the pandas case — it's returning true when it should return false.

Suggested fixes

Summary: The function_def_has_return_annotation method in bindings.rs fails to correctly determine that pandas' NDFrame.__bool__ lacks an explicit return annotation, likely because the KeyUndecoratedFunctionRange lookup or the short_identifier extraction returns incorrect results for inherited/cross-module method definitions, causing 12 false positive errors in pandas.

1. In function_def_has_return_annotation() in pyrefly/lib/binding/bindings.rs, the method returns false when the key_to_idx_hashed_opt lookup fails (i.e., None), but the issue is that when the lookup succeeds, self.get(idx).0 extracts a ShortIdentifier from the binding at that index. However, the binding retrieved via KeyUndecoratedFunctionRange(def_index) may not yield the correct ShortIdentifier — the .0 field access on the binding likely doesn't reliably return the function's ShortIdentifier needed for the ReturnType key lookup. When the extraction goes wrong, function_has_return_annotation_at_short_identifier may match a different binding or hit the panic/fallback path. The fix: in callable_has_explicit_return_annotation() in pyrefly/lib/alt/attr.rs, when the FunctionKind::Def case has def_index as None, return false (already handled). But also, when the function's def_index points to a definition in a different module than the current bindings, the lookup will either fail silently or return wrong data. Add a guard to check that the function's module matches self.bindings().module() before calling function_def_has_return_annotation. If the modules don't match, resolve the bindings for the function's own module. Alternatively, a simpler and safer fix: in function_def_has_return_annotation(), when self.get(idx) returns a binding, verify it is actually a function-related binding before extracting .0 as a ShortIdentifier. If it's not the expected type, return false instead of proceeding with potentially garbage data. The most robust fix is: in callable_has_explicit_return_annotation in attr.rs, for the FunctionKind::Def case, resolve the bindings from the function's own module (using func_id's module info) rather than always using self.bindings(), which refers to the current module being checked — not necessarily the module where __bool__ is defined (e.g., pandas' NDFrame is in a different file than where if ndframe: is used).

Files: pyrefly/lib/alt/attr.rs, pyrefly/lib/binding/bindings.rs
Confidence: high
Affected projects: pandas
Fixes: invalid-argument
The pandas NDFrame.__bool__ method has no return annotation — it simply raises ValueError. Its return type is inferred as Never. The callable_has_explicit_return_annotation check is supposed to filter this out, but it's returning true. The most likely cause: self.bindings() in callable_has_explicit_return_annotation returns the bindings for the current module being analyzed (where if ndframe: ... appears), not the module where NDFrame.__bool__ is defined. When function_def_has_return_annotation(def_index) looks up KeyUndecoratedFunctionRange(def_index) in the wrong module's bindings, the lookup either finds an unrelated binding whose .0 happens to produce a ShortIdentifier that matches some annotated function, or the subsequent function_has_return_annotation_at_short_identifier call matches a wrong ReturnType key. The test cases pass because they define everything in a single module. This would eliminate all 12 false positive errors in pandas.

---

Was this helpful? React with 👍 or 👎

_{Classification by primer-classifier (1 LLM)}