Schema filter predicate silently accepts a truthy string as a rejection message (easy footgun with && / ||)

[jjhiggz]

What happened

Schema.makeFilter / schema check predicates return a FilterOutput, where a returned
string is interpreted as a rejection with that string as the error message. That string
arm is a nice, deliberate ergonomic — but because the return type is widened to include
string, a predicate that accidentally produces a string (very easy with && / ||)
type-checks fine and silently inverts its meaning.

Minimal repro

import { Schema } from "effect"

type Card = { readonly done: boolean; readonly title?: string }

// Intent: keep cards that are unfinished AND have a title.
const openTitledCard = Schema.makeFilter<Card>((c) => !c.done && c.title)
//                                                     ^^^^^^^^^^^^^^^^^^
// When done=false and title="foo", `&&` returns the *string* "foo".
// makeFilter reads that string as a REJECTION MESSAGE, so the card that
// SHOULD pass is silently filtered out — and there is no compile error.

The intended predicate was (c) => !c.done (boolean). The && c.title was a mistake, but
nothing at the type level flagged it — the string return is a legal FilterOutput.

Why it's surprising

• Everywhere else in Effect, filter predicates are typed Predicate = (a: A) => boolean
  (e.g. Array.filter). There, (c) => !c.done && c.title is a compile error — string
  isn't assignable to boolean. Here, because FilterOutput includes a string arm, the
  identical mistake type-checks and silently inverts. The footgun is specifically that the
  return type is widened to accept strings.
• && / || returning their operands (not coerced booleans) is idiomatic JS/TS, so
  "return a bare boolean-ish expression" is a natural thing to write — and it's exactly what
  breaks here.

Question / idea

The suggested Filter module already models pass/fail explicitly with a tagged Result:

  interface Filter<in Input, out Pass = Input, out Fail = Input> {
    (input: Input): Result.Result<Pass, Fail>
  }

  const positiveFilter: Filter.Filter<number> =
    (n) => n > 0 ? Result.succeed(n) : Result.fail(n)

Because you must explicitly call Result.succeed / Result.fail, it's impossible to
accidentally "return a truthy string and have it silently mean reject" — the exact footgun
above can't happen.

Would it make sense for schema checks to accept (or offer) this same Result-returning
shape as an alternative to the overloaded FilterOutput? Sketch:

  // today — overloaded return, string arm is a footgun:
  Schema.makeFilter<Card>((c) => !c.done && c.title)   // compiles, silently wrong

  // proposed alternative — explicit, mirrors the Filter module:
  Schema.makeFilter<Card>((c) =>
    !c.done ? Result.succeed(c) : Result.fail("expected an unfinished card")
  )
  // a bare `string`/`&&` return would no longer type-check here

Not asking to change the default — just whether the safer, already-shipped Result shape
could be usable for schema validation too, since the abstraction clearly exists in v4.

For reference, the current signature:

  // Schema.d.ts (effect@4.0.0-beta.98)
  declare const makeFilter: <T>(
    filter: (input: T, ast, options) => FilterOutput,
    ...
  ) => Filter<T>

  type FilterOutput = undefined | boolean | FilterIssue | ReadonlyArray<FilterIssue>
  //                                         ^^^^^^^^^^^ FilterIssue includes `string`

Version: effect@4.0.0-beta.98

[nikelborm]

It is probably an artifact that came from effect v3.

import * as Record from 'effect/Record'
import * as Schema from 'effect/Schema'

export const NonEmptyRecord = <
  K extends Schema.Schema.All,
  V extends Schema.Schema.All,
>(
  key: K,
  value: V,
) =>
  Schema.Record({ key, value }).pipe(
    Schema.filter(
      record =>
        !!Record.size(record) || 'Record must contain at least some keys',
    ),
  )

const GitHubSlugStringSchema = Schema.NonEmptyString.pipe(
  Schema.filter(s => isGitHubSlug(s) || invalidGitHubSlugMessage),
)

I assume it was intended to be used like this, but I agree it might be a silent footgun in some cases and support the idea of writing a bit more code for clarity in this case