Skip to content

Commit

Permalink
Remove Maybe.flatten function (WICG#1337)
Browse files Browse the repository at this point in the history 
It was unsound because it could be called with a Maybe<T> but its type
parameter T instantiated with Maybe<T>, resulting in an apparent return
type of Maybe<Maybe<T>>, but the instanceof check would cause the
original Maybe<T> to be returned.

For example, the following code logs "number" despite y's type
indicating that its value should be another Maybe:

```ts
const x = Maybe.some(5)
const y: Maybe<Maybe<number>> = Maybe.flatten<Maybe<number>>(x)
console.log(typeof y.value)
```
  • Loading branch information
@apasel422
apasel422 authored Jun 13, 2024
1 parent 0480842 commit ada36e1
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 30 deletions.
17 changes: 9 additions & 8 deletions ts/src/header-validator/maybe.ts
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
export type Maybeable<T> = T | Maybe<T>

export class Maybe<T> {
static readonly None = new Maybe<never>()

static some<T>(this: void, t: T): Maybe<T> {
return new Maybe(t)
}

static flatten<T>(this: void, t: Maybeable<T>): Maybe<T> {
return t instanceof Maybe ? t : Maybe.some(t)
}

private constructor(private readonly t?: T) {}

filter<C extends unknown[]>(
Expand All @@ -21,10 +15,17 @@ export class Maybe<T> {
}

map<U, C extends unknown[]>(
f: (t: T, ...args: C) => Maybeable<U>,
f: (t: T, ...args: C) => U,
...args: C
): Maybe<U> {
return this.t === undefined ? Maybe.None : new Maybe<U>(f(this.t, ...args))
}

flatMap<U, C extends unknown[]>(
f: (t: T, ...args: C) => Maybe<U>,
...args: C
): Maybe<U> {
return this.t === undefined ? Maybe.None : Maybe.flatten(f(this.t, ...args))
return this.t === undefined ? Maybe.None : f(this.t, ...args)
}

peek<C extends unknown[]>(f: (t: T, ...args: C) => void, ...args: C): this {
Expand Down
33 changes: 18 additions & 15 deletions ts/src/header-validator/validate-json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class SourceContext extends RegistrationContext {
const {
exclusive,
field,
fieldMaybeDefault,
struct: structInternal,
} = validate.make<JsonDict, Json>(
/*getAndDelete=*/ (d, name) => {
Expand All @@ -86,7 +87,7 @@ function struct<T extends object, C extends Context>(
fields: StructFields<T, C>,
warnUnknown: boolean = true
): Maybe<T> {
return object(d, ctx).map(structInternal, ctx, fields, warnUnknown)
return object(d, ctx).flatMap(structInternal, ctx, fields, warnUnknown)
}

type TypeSwitch<T, C extends Context = Context> = {
Expand Down Expand Up @@ -147,7 +148,7 @@ function keyValues<V, C extends Context = Context>(
f: CtxFunc<C, [key: string, val: Json], Maybe<V>>,
maxKeys: number = Infinity
): Maybe<Map<string, V>> {
return object(j, ctx).map((d) => {
return object(j, ctx).flatMap((d) => {
const entries = Object.entries(d)

if (entries.length > maxKeys) {
Expand Down Expand Up @@ -220,7 +221,7 @@ function destination(j: Json, ctx: Context): Maybe<Set<string>> {
return typeSwitch(j, ctx, {
string: (j) => suitableSite(j, ctx).map((s) => new Set([s])),
list: (j) =>
set(j, ctx, (j) => string(j, ctx).map(suitableSite, ctx), {
set(j, ctx, (j) => string(j, ctx).flatMap(suitableSite, ctx), {
minLength: 1,
maxLength: 3,
}),
Expand Down Expand Up @@ -311,7 +312,7 @@ function eventReportWindows(
ctx: SourceContext,
expiry: Maybe<number>
): Maybe<EventReportWindows> {
return object(j, ctx).map((j) => {
return object(j, ctx).flatMap((j) => {
const startTimeValue = field(
'start_time',
(j) => startTime(j, ctx, expiry),
Expand Down Expand Up @@ -348,7 +349,7 @@ function set<T extends number | string, C extends Context = Context>(
// checks should be performed on the resulting set, not on the list.
return list(j, ctx)
.filter((js) => isLengthValid(js.length, ctx, opts))
.map((js) => validate.set(js.entries(), ctx, f, opts?.requireDistinct))
.flatMap((js) => validate.set(js.entries(), ctx, f, opts?.requireDistinct))
}

type ArrayOpts = LengthOpts & {
Expand All @@ -363,7 +364,9 @@ function array<T, C extends Context = Context>(
): Maybe<T[]> {
return list(j, ctx)
.filter((js) => isLengthValid(js.length, ctx, opts))
.map((js) => validate.array(js.entries(), ctx, f, opts?.itemErrorAction))
.flatMap((js) =>
validate.array(js.entries(), ctx, f, opts?.itemErrorAction)
)
}

function filterDataKeyValue(
Expand Down Expand Up @@ -861,13 +864,13 @@ function triggerSpec(
)

return struct(j, ctx, {
eventReportWindows: field(
eventReportWindows: fieldMaybeDefault(
'event_report_windows',
(j) => eventReportWindows(j, ctx, deps.expiry),
deps.eventReportWindows
),

summaryBuckets: field(
summaryBuckets: fieldMaybeDefault(
'summary_buckets',
(j) => summaryBuckets(j, ctx, deps.maxEventLevelReports),
defaultSummaryBuckets
Expand Down Expand Up @@ -952,7 +955,7 @@ function defaultTriggerSpecs(
eventReportWindows: Maybe<EventReportWindows>,
maxEventLevelReports: Maybe<number>
): Maybe<TriggerSpec[]> {
return eventReportWindows.map((eventReportWindows) =>
return eventReportWindows.flatMap((eventReportWindows) =>
maxEventLevelReports.map((maxEventLevelReports) => [
{
eventReportWindows,
Expand Down Expand Up @@ -1048,7 +1051,7 @@ export type Source = CommonDebug &

function source(j: Json, ctx: SourceContext): Maybe<Source> {
return object(j, ctx)
.map((j) => {
.flatMap((j) => {
const expiryVal = field(
'expiry',
expiry,
Expand Down Expand Up @@ -1095,7 +1098,7 @@ function source(j: Json, ctx: SourceContext): Maybe<Source> {
)(j, ctx)

return struct(j, ctx, {
aggregatableReportWindow: field(
aggregatableReportWindow: fieldMaybeDefault(
'aggregatable_report_window',
(j) => singleReportWindow(j, ctx, expiryVal),
expiryVal
Expand Down Expand Up @@ -1258,7 +1261,7 @@ function aggregatableDedupKeys(
}

function enumerated<T>(j: Json, ctx: Context, e: Record<string, T>): Maybe<T> {
return string(j, ctx).map(validate.enumerated, ctx, e)
return string(j, ctx).flatMap(validate.enumerated, ctx, e)
}

export enum AggregatableSourceRegistrationTime {
Expand Down Expand Up @@ -1348,7 +1351,7 @@ function aggregationCoordinatorOrigin(
ctx: RegistrationContext
): Maybe<string> {
return string(j, ctx)
.map(suitableOrigin, ctx)
.flatMap(suitableOrigin, ctx)
.filter((s) => {
if (!ctx.vsv.aggregationCoordinatorOrigins.includes(s)) {
const allowed = ctx.vsv.aggregationCoordinatorOrigins.join(', ')
Expand All @@ -1373,7 +1376,7 @@ export type Trigger = CommonDebug &

function trigger(j: Json, ctx: RegistrationContext): Maybe<Trigger> {
return object(j, ctx)
.map((j) => {
.flatMap((j) => {
const aggregatableSourceRegTimeVal = field(
'aggregatable_source_registration_time',
aggregatableSourceRegistrationTime,
Expand Down Expand Up @@ -1483,7 +1486,7 @@ function reportDestination(j: Json, ctx: Context): Maybe<string | string[]> {
return typeSwitch<string | string[]>(j, ctx, {
string: suitableSiteNoExtraneous,
list: (j) =>
array(j, ctx, (j) => string(j, ctx).map(suitableSiteNoExtraneous), {
array(j, ctx, (j) => string(j, ctx).flatMap(suitableSiteNoExtraneous), {
minLength: 2,
maxLength: 3,
}).filter((v) => {
Expand Down
42 changes: 35 additions & 7 deletions ts/src/header-validator/validate.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as psl from 'psl'
import { Context, PathComponent } from './context'
import { Maybe, Maybeable } from './maybe'
import { Maybe } from './maybe'

export type CtxFunc<C extends Context, I, O> = (i: I, ctx: C) => O

Expand Down Expand Up @@ -52,14 +52,14 @@ type GetAndDeleteFunc<D, V> = (d: D, name: string) => V | undefined
type FieldFunc<D, V> = <T, C extends Context>(
name: string,
f: CtxFunc<C, V, Maybe<T>>,
valueIfAbsent?: Maybeable<T>
valueIfAbsent?: T
) => CtxFunc<C, D, Maybe<T>>

function field<D, V>(getAndDelete: GetAndDeleteFunc<D, V>): FieldFunc<D, V> {
return <T, C extends Context>(
name: string,
f: CtxFunc<C, V, Maybe<T>>,
valueIfAbsent?: Maybeable<T>
valueIfAbsent?: T
) =>
(d: D, ctx: C): Maybe<T> =>
ctx.scope(name, () => {
Expand All @@ -69,7 +69,33 @@ function field<D, V>(getAndDelete: GetAndDeleteFunc<D, V>): FieldFunc<D, V> {
ctx.error('required')
return Maybe.None
}
return Maybe.flatten(valueIfAbsent)
return Maybe.some(valueIfAbsent)
}
return f(v, ctx)
})
}

type FieldMaybeDefaultFunc<D, V> = <T, C extends Context>(
name: string,
f: CtxFunc<C, V, Maybe<T>>,
valueIfAbsent: Maybe<T>
) => CtxFunc<C, D, Maybe<T>>

// TODO(apaseltiner): Merge this function back into `field` since it is
// identical other than whether the default value is T or Maybe<T>.
function fieldMaybeDefault<D, V>(
getAndDelete: GetAndDeleteFunc<D, V>
): FieldMaybeDefaultFunc<D, V> {
return <T, C extends Context>(
name: string,
f: CtxFunc<C, V, Maybe<T>>,
valueIfAbsent: Maybe<T>
) =>
(d: D, ctx: C): Maybe<T> =>
ctx.scope(name, () => {
const v = getAndDelete(d, name)
if (v === undefined) {
return valueIfAbsent
}
return f(v, ctx)
})
Expand All @@ -81,15 +107,15 @@ export type Exclusive<T, V, C extends Context> = {

type ExclusiveFunc<D, V> = <T, C extends Context>(
x: Exclusive<T, V, C>,
valueIfAbsent: Maybeable<T>
valueIfAbsent: Maybe<T>
) => CtxFunc<C, D, Maybe<T>>

function exclusive<D, V>(
getAndDelete: GetAndDeleteFunc<D, V>
): ExclusiveFunc<D, V> {
return <T, C extends Context>(
x: Exclusive<T, V, C>,
valueIfAbsent: Maybeable<T>
valueIfAbsent: Maybe<T>
) =>
(d: D, ctx: C): Maybe<T> => {
const found: string[] = []
Expand All @@ -112,13 +138,14 @@ function exclusive<D, V>(
return Maybe.None
}

return Maybe.flatten(valueIfAbsent)
return valueIfAbsent
}
}

type Funcs<D, V> = {
exclusive: ExclusiveFunc<D, V>
field: FieldFunc<D, V>
fieldMaybeDefault: FieldMaybeDefaultFunc<D, V>
struct: StructFunc<D>
}

Expand All @@ -130,6 +157,7 @@ export function make<D, V>(
return {
exclusive: exclusive(getAndDelete),
field: field(getAndDelete),
fieldMaybeDefault: fieldMaybeDefault(getAndDelete),
struct: struct(unknownKeys, warnUnknownMsg),
}
}
Expand Down

0 comments on commit ada36e1

Please sign in to comment.