fix(schema): move SchemaExpr &&/|| to companion implicit class to en…#1463
fix(schema): move SchemaExpr &&/|| to companion implicit class to en…#1463googley42 wants to merge 10 commits into
Conversation
…able downstream bridges
In Scala 2, a direct method found by name on a type suppresses implicit
class receiver views even when the argument type does not match (spec
§6.26 — the view fires only when the method is "not found"). With &&
and || as direct methods on SchemaExpr, downstream libraries cannot
provide an implicit class bridge such as
implicit class Bridge[S](self: SchemaExpr[S, Boolean]) {
def &&(other: DomainExpr[S]): DomainExpr[S] = ...
}
because Scala commits to SchemaExpr.&&(SchemaExpr) by name and
reports a type mismatch on the argument rather than trying the bridge.
Moving &&/|| to BooleanOps, an implicit class in the SchemaExpr
companion, removes the direct-method lock-in. The companion implicit
always provides &&(SchemaExpr) for the existing use case; a downstream
bridge providing &&(OtherType) is unambiguous because only one
candidate applies per call site based on the argument type.
Source compatibility is fully maintained: se1 && se2 continues to work
identically. Binary compatibility is intentionally broken (direct
virtual method replaced by implicit class dispatch) as expected for
this kind of extensibility improvement.
Scala 3 is unaffected by the receiver-view limitation but benefits from
the same structural change; implicit class is retained over extension
syntax for cross-version consistency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Moves boolean && / || combinators off SchemaExpr methods and into a companion-scope implicit class to avoid Scala 2 “member method suppresses implicit receiver views” behavior and enable downstream extensions.
Changes:
- Removed
&&/||fromSchemaExprinstance methods - Added
SchemaExpr.BooleanOpsimplicit value class providing&&/||forSchemaExpr[A, Boolean]
| /** | ||
| * Logical combinators for boolean SchemaExprs. | ||
| * | ||
| * Provided as an implicit class rather than direct methods on SchemaExpr so that | ||
| * downstream libraries can supply their own implicit class with &&/|| overloads for | ||
| * a different result type (e.g. a DDB-specific or SQL-specific expression ADT). | ||
| * In Scala 2, a direct method found by name suppresses implicit receiver views even | ||
| * when the argument type does not match; an implicit class in the companion avoids | ||
| * this and allows downstream bridges to fire without ambiguity. | ||
| */ | ||
| implicit final class BooleanOps[A](private val self: SchemaExpr[A, Boolean]) extends AnyVal { |
There was a problem hiding this comment.
Good catch on the Scaladoc — "without ambiguity" was too broad and has been updated to be more precise (see latest commit).
On the underlying design: the ambiguity concern holds when two implicits compete at the same priority level (e.g. both companion-scope). That case is called out in the updated doc.
However, the motivating use case — a downstream library like zio-dynamodb adding &&(CE): CE overloads — avoids this entirely: those ops are introduced via an explicit import
DdbBlocksApi._, which in Scala 2 places them in import scope, which has higher priority than companion scope (BooleanOps). So for se && ce the downstream implicit wins cleanly, and for se &&
se2 there is no applicable downstream overload (wrong arg type), so BooleanOps is the only candidate.
The opt-in object logical approach would eliminate the companion-scope collision risk entirely, but at the cost of requiring an extra import for users of SchemaExpr directly. The
non-symbolic and/or approach is the cleanest separation but trades ergonomics for purity. For a general-purpose expression algebra where &&/|| are the natural operators, companion scope
with the priority caveat documented seems the right balance — but happy to discuss if you feel differently.
The B2 type parameter with =:= evidence was equivalent to writing Boolean directly; remove the indirection for cleaner API surface and better type-error messages at call sites. Also clarify Scaladoc on implicit priority guarantees for downstream ops classes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per symbolic-ops policy: named method is the primary API, symbolic operator is sugar that delegates to it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🚀 Preview deployed to Netlify: https://zio-blocks-pr-1463--zio-dev.netlify.app |
| def and(that: SchemaExpr[A, Boolean]): SchemaExpr[A, Boolean] = | ||
| SchemaExpr( | ||
| DynamicSchemaExpr.Logical(self.dynamic, that.dynamic, DynamicSchemaExpr.LogicalOperator.And), | ||
| self.inputSchema, | ||
| Schema[Boolean] | ||
| ) | ||
|
|
||
| def or(that: SchemaExpr[A, Boolean]): SchemaExpr[A, Boolean] = | ||
| SchemaExpr( | ||
| DynamicSchemaExpr.Logical(self.dynamic, that.dynamic, DynamicSchemaExpr.LogicalOperator.Or), | ||
| self.inputSchema, | ||
| Schema[Boolean] | ||
| ) |
There was a problem hiding this comment.
The concern is noted but I don't think self.outputSchema is the right choice here. and/or combine two operands — self and that — each with their own outputSchema. Picking the LHS schema
is arbitrary and asymmetric. More fundamentally, the output schema for a boolean logical combinator carries no meaningful content to preserve: Schema[Boolean] is a fixed primitive schema,
and any schema annotations in a SchemaExpr attach to the input type (inputSchema), not to the boolean result type. Using the summoned canonical Schema[Boolean] is correct here. If the
caller needs a specific annotated boolean schema for the result, the right API would be an explicit Schema[Boolean] parameter on and/or — not silent inheritance from one side.
…ility Boolean is final so this makes no practical difference, but using a type bound rather than fixing B = Boolean is accurate to the original intent and avoids a source-incompatible narrowing for abstract-bounded types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
googley42
changed the title
|
@plokhotnyuk if you have some time to review this PR please that would great |
…able downstream bridges
In Scala 2, a direct method found by name on a type suppresses implicit class receiver views even when the argument type does not match (spec §6.26 — the view fires only when the method is "not found"). With && and || as direct methods on SchemaExpr, downstream libraries cannot provide an implicit class bridge such as
because Scala commits to SchemaExpr.&&(SchemaExpr) by name and reports a type mismatch on the argument rather than trying the bridge.
Moving &&/|| to BooleanOps, an implicit class in the SchemaExpr companion, removes the direct-method lock-in. The companion implicit always provides &&(SchemaExpr) for the existing use case; a downstream bridge providing &&(OtherType) is unambiguous because only one candidate applies per call site based on the argument type.
Source compatibility is fully maintained: se1 && se2 continues to work identically. Binary compatibility is intentionally broken (direct virtual method replaced by implicit class dispatch) as expected for this kind of extensibility improvement.
Scala 3 is unaffected by the receiver-view limitation but benefits from the same structural change; implicit class is retained over extension syntax for cross-version consistency.
Closes #1470