Skip to content

New rule returns_implicit_none: a function that falls off the end never satisfies its declared return type #401

Description

@MelbourneDeveloper

Summary

Add a new default-on checker rule, returns_implicit_none, that flags a function whose declared return type cannot be satisfied because control can reach the end of the body without returning a value. Python then implicitly returns None.

Basilisk currently misses this entirely. It is not an inference gap — it is a control-flow gap — which makes it independent of, and much cheaper than, the type-inference integration work it sits next to.

Surfaced while investigating Refs #397, which is the same silent-None failure seen from the other end: this rule catches the None where it is produced, #397's unfixed half catches it where it is consumed. Details in Relationship to the type-inference cluster below.

Motivating report (paraphrased from a user post): "Every so often my code blows up trying to process a None because I forget that Python functions don't automatically return the results of their last computation."

def double(x: int) -> int:
    x * 2          # no `return` — implicitly returns None

Inspired by this Bluesky post

Current behaviour — measured

Against a release build of bidirectionaltype-inference (c009ca5):

Case Basilisk
def f(x: int) -> int: / x * 2 (falls off the end) silent
def f(x: int) -> int: / if x: / return 1 (falls off one path) silent
def f(x: int) -> None: / print(x) (correct code) silent ✓

returns_compatibility only inspects the value of an explicit return statement, so a function that never reaches one is invisible to it. No rule in crates/basilisk-checker/src/rules/ models a function lacking a return.

Every comparator flags both cases

Measured locally on the same two files, latest released versions:

Checker Falls off the end Falls off one path
mypy 1.19.1 error: Missing return statement [return] error: Missing return statement [return]
pyright 1.1.408 error: Function with declared return type "int" must return value on all code paths — "None" is not assignable to "int" (reportReturnType) error: Function with declared return type "int" must return value on all code paths
ty 0.0.19 error[invalid-return-type]: Function always implicitly returns None, which is not assignable to return type int error[invalid-return-type]: Function can implicitly return None, which is not assignable to return type int
pyrefly 0.54.0 ERROR Function declared to return int but is missing an explicit return [bad-return] ERROR Function declared to return int, but one or more paths are missing an explicit return [bad-return]

Basilisk is the only one of the five that is silent.

Spec basis

This is spec-mandated, not a house-style preference: a function annotated -> int that completes without returning yields None, and None is not assignable to int. So the rule belongs in the default-on PEP set, not the opt-in set.

It is not exercised by the python/typing conformance suite — a grep of conformance/tests/ finds no fall-off-the-end fixture. So this changes no conformance number in either direction; it must simply not introduce a false positive on any of the 147 fixtures.

Proposed structure

Naming follows the descriptive convention (no BSK-NNNN numbering): returns_implicit_none, alongside the existing returns_compatibility / returns_compatibility_2.

Two pieces, matching the established resolver-collects-facts / rule-consumes-facts split:

1. Resolver — crates/basilisk-resolver/src/visitor/terminates.rs (new)

Adds one field to FunctionInfo:

/// `true` when control can reach the end of the body without returning.
pub body_falls_through: bool,

The existing body_last_stmt_terminates is not sufficient — it inspects only the last top-level statement, so if x: return 1 else: return 2 would look like a fall-through and false-positive. This needs a real all-paths walk.

block_terminates(stmts) is true if any statement terminates (statements after it are unreachable):

Construct Terminates when
return, raise always
bare call expression always — conservatively assumes a possible NoReturn callee (sys.exit())
if / elif / else an else clause is present and every branch terminates
while True: no break in the body
with its body terminates
try finally terminates, or body and every handler terminate
match every case body terminates
assert False always
everything else never

The bias is deliberately toward "terminates" — anything not modelled reads as terminating and stays silent. False negatives are acceptable here; a false positive on valid code is not.

2. Checker — crates/basilisk-checker/src/rules/returns_implicit_none.rs (new)

Fires only when all hold:

  • body_falls_through
  • return annotation is ReturnAnnotationKind::Other — excludes Missing, Any, None, and the invalid-literal kind
  • the annotation does not admit None: not Optional[...], not a ... | None union, not object, not Never/NoReturn (owned by specialtypes_never), not a TypeVar or PEP 695 type parameter that could bind None
  • not a generator (is_generator) — a -> Iterator[int] function never returns a value; Generator[Y, S, R] fall-through is already annotations_generators' job
  • not is_stub_context(...)@overload, @abstractmethod, Protocol methods, and .../pass/docstring-only bodies
  • not is_no_type_check(...)
  • not a .pyi file

async def is in scope: async def f() -> int that falls through resolves to None too.

Diagnostic wording should teach the actual misconception, per the repo's diagnostic standard — that Python does not return the last expression implicitly.

Relationship to the type-inference cluster

Same user-visible symptom — a None shows up somewhere far from where it was created — reached by two different mechanisms:

  • Definition site (this issue). Pure control-flow analysis. Needs no type engine. Catches the None where it is produced.
  • Use site (the unfixed half of Refs Clarification or help needed for a basic check #397). a: int = compute(3) where compute returns None is silent, because assignment_compatibility only reads syntactic right-hand sides and nothing routes BidirEngine::synth_call (crates/basilisk-checker/src/bidir/engine.rs:266) into any rule. Catches the None where it is consumed.

Also adjacent, sharing the "declared return types are not really verified" theme: Refs #378 (return/assignment checks drop every nominal annotation), Refs #379, Refs #317, Refs #290.

No parent/child relationship is proposed. These are siblings sharing a root theme, not a decomposition of one another — this rule ships independently of all of them and blocks none of them. Linking them as sub-issues would assert a hierarchy that does not exist. (A genuine umbrella epic for "wire the inference engine into live rules" could be worth creating separately, covering #317/#290/#378 and the use-site half of #397 — but that is a different piece of work from this rule.)

Acceptance criteria

  • Both measured cases above produce a diagnostic; the -> None control case stays silent
  • if/else with a return in every branch, while True without break, try/finally, exhaustive match, and always-raising bodies stay silent
  • Generators, @overload, @abstractmethod, Protocol methods, stub/docstring-only bodies, and .pyi files stay silent
  • Optional[int], int | None, Any, object, and TypeVar returns stay silent
  • 0 false positives across all 147 conformance fixtures; conformance stays 100% / 0 FP
  • Torture golden gate stays green; make bench shows no regression (a resolver visitor is a hot path)
  • Rule doc-comment added and python3 scripts/gen_rules_reference.py --data re-run so website/src/_data/rules.json and the /errors/returns_implicit_none page stay in sync
  • Registered in all_rules() and listed in docs/specs/CHECKER-ARCHITECTURE-SPEC.md under [CHKARCH-DIAG-TYPESAFETY]

Risk

This is a default-on rule, so it is inside the prime-directive blast radius. The mitigation is the terminator analysis' deliberate bias toward silence plus the 147-fixture false-positive sweep before it lands. The known cost of that bias is missed detections — notably a body whose only return is inside a for loop, where the loop may iterate zero times — which can be tightened later once the rule has proven quiet.

Metadata

Metadata

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions