Scoped for the 0.3.0 breaking release.
Problem
The fluent builder on Expression is our most-used public surface, but nine of its fifteen combinators declare a class-level @internal type as their return type, while the other six return self. So the surface both leaks internal symbols and contradicts itself:
returns self (intended) |
returns an @internal concrete type (leak) |
neq (:21), lt (:62), gte (:67), lte (:72), and_ (:82), not (:92) |
eq (:16→Eq), subtract (:26→Subtract), add (:31→Add), multiply (:36→Multiply), divide (:44→Divide), modulo (:52→Modulo), gt (:57→Gt), or_ (:77→Or_), call (:106→Call) |
eq returns Eq but its sibling neq returns self; gt returns Gt but lt/gte/lte return self. The not() docblock already states the intended rule outright: returning self is the shape every builder is headed for.
Separately, Eq and Gt are BC shims: both are @internal final subclasses of Comparison that add nothing but a fixed constructor, and their own docblocks ask to be folded into Comparison in the next breaking release. They only still exist because Expression::eq()/gt() (and Expr::eq()/gt()) declared them as return types before Comparison existed.
These two fixes compose: once every builder returns self, Eq and Gt are no longer named in any public signature, so collapsing them becomes a purely internal change.
Proposed change
- Make all fifteen
Expression builder methods return self.
- Collapse
Eq and Gt into Comparison; drop the two classes. Comparison::equals() already matches on self, so an Eq/Gt value and a plain Comparison compare equal — behavior is preserved.
BC impact
Structural break (return-type narrowing on public methods + class removals) — the Roave checker will flag it. Correct for a 0.MINOR bump; frozen after 1.0.0, at which point we'd be stuck publishing Eq/Add/… as permanent public types.
Scoped for the 0.3.0 breaking release.
Problem
The fluent builder on
Expressionis our most-used public surface, but nine of its fifteen combinators declare a class-level@internaltype as their return type, while the other six returnself. So the surface both leaks internal symbols and contradicts itself:self(intended)@internalconcrete type (leak)neq(:21),lt(:62),gte(:67),lte(:72),and_(:82),not(:92)eq(:16→Eq),subtract(:26→Subtract),add(:31→Add),multiply(:36→Multiply),divide(:44→Divide),modulo(:52→Modulo),gt(:57→Gt),or_(:77→Or_),call(:106→Call)eqreturnsEqbut its siblingneqreturnsself;gtreturnsGtbutlt/gte/ltereturnself. Thenot()docblock already states the intended rule outright: returningselfis the shape every builder is headed for.Separately,
EqandGtare BC shims: both are@internal finalsubclasses ofComparisonthat add nothing but a fixed constructor, and their own docblocks ask to be folded intoComparisonin the next breaking release. They only still exist becauseExpression::eq()/gt()(andExpr::eq()/gt()) declared them as return types beforeComparisonexisted.These two fixes compose: once every builder returns
self,EqandGtare no longer named in any public signature, so collapsing them becomes a purely internal change.Proposed change
Expressionbuilder methods returnself.EqandGtintoComparison; drop the two classes.Comparison::equals()already matches onself, so anEq/Gtvalue and a plainComparisoncompare equal — behavior is preserved.BC impact
Structural break (return-type narrowing on public methods + class removals) — the Roave checker will flag it. Correct for a
0.MINORbump; frozen after 1.0.0, at which point we'd be stuck publishingEq/Add/… as permanent public types.