Skip to content

Commit

Permalink
Parse PerseusItem data on input (#1785)
Browse files Browse the repository at this point in the history
See [ADR #773][1] for context.

NOTE TO REVIEWERS: The best place to start reading this code is probably `parse-perseus-json/parser-types.ts`, followed by the files in `parse-perseus-json/general-purpose-parsers`.

I have not written parsers for all widget types yet, but I want to get this foundational code merged because this PR is already way too big. The parser is not hooked up in production yet, so this change should have no effect.

If you have a suggestion for a term I should use other than "parse", I'd love to hear it. Some other ideas are listed [here](https://khanacademy.atlassian.net/wiki/spaces/ENG/pages/3358883970/Public+notes+on+ADR+773+-+Validate+widget+data+on+input+in+Perseus#Words).

[1]: https://khanacademy.atlassian.net/wiki/spaces/ENG/pages/3318349891/ADR+773+Validate+widget+data+on+input+in+Perseus

Issue: https://khanacademy.atlassian.net/browse/LEMS-2521

## Test plan:

CI should pass.

Author: benchristel

Reviewers: benchristel, jeremywiebe, handeyeco

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1785
  • Loading branch information
benchristel authored Nov 8, 2024
1 parent 97e4da6 commit 32b2732
Show file tree
Hide file tree
Showing 67 changed files with 2,797 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/fair-wombats-begin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Internal: add a parsing layer to typecheck PerseusItem data at runtime (ADR #773). The parsing code is not used yet.
4 changes: 2 additions & 2 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export const ItemExtras = [
] as const;
export type PerseusAnswerArea = Record<(typeof ItemExtras)[number], boolean>;

type WidgetOptions<Type extends string, Options> = {
export type WidgetOptions<Type extends string, Options> = {
// The "type" of widget which will define what the Options field looks like
type: Type;
// Whether this widget is displayed with the values and is immutable. For display only
Expand Down Expand Up @@ -666,7 +666,7 @@ export type PerseusInteractiveGraphWidgetOptions = {
fullGraphAriaDescription?: string;
};

const lockedFigureColorNames = [
export const lockedFigureColorNames = [
"blue",
"green",
"grayH",
Expand Down
10 changes: 0 additions & 10 deletions packages/perseus/src/util/parse-perseus-json.test.ts

This file was deleted.

20 changes: 0 additions & 20 deletions packages/perseus/src/util/parse-perseus-json.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {ErrorTrackingParseContext} from "./error-tracking-parse-context";
import {failure} from "./result";

describe("ErrorTrackingParseContext", () => {
it("adds its `path` to reported failures", () => {
const path = ["foo", 1, "bar"];
const ctx = new ErrorTrackingParseContext(path);

expect(ctx.failure("a million bucks", 4)).toEqual(
failure([
{
expected: ["a million bucks"],
badValue: 4,
path: ["foo", 1, "bar"],
},
]),
);
});

it("spawns a new context for a subtree of the object being parsed", () => {
const rootCtx = new ErrorTrackingParseContext(["one"]);
const subCtx = rootCtx.forSubtree("two");
expect(subCtx.failure("", 99).detail[0].path).toEqual(["one", "two"]);
});

it("is not modified by spawning a subcontext", () => {
const rootCtx = new ErrorTrackingParseContext(["original", "path"]);
rootCtx.forSubtree("changed");
expect(rootCtx.failure("", 99).detail[0].path).toEqual([
"original",
"path",
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {failure, success} from "./result";

import type {ParseContext, Mismatch, PathSegment} from "./parser-types";
import type {Failure, Success} from "./result";

export class ErrorTrackingParseContext implements ParseContext {
constructor(private readonly path: PathSegment[]) {}

failure(
expected: string | string[],
badValue: unknown,
): Failure<Mismatch[]> {
return failure([
{
expected: wrapInArray(expected),
badValue,
path: this.path,
},
]);
}

forSubtree(key: PathSegment): ParseContext {
return new ErrorTrackingParseContext([...this.path, key]);
}

success<T>(value: T): Success<T> {
return success(value);
}
}

function wrapInArray(a: string | string[]): string[] {
return Array.isArray(a) ? a : [a];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type {Parser} from "../parser-types";

export const any: Parser<any> = (rawValue, ctx) => ctx.success(rawValue);
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import {assertFailure, success} from "../result";

import {array} from "./array";
import {string} from "./string";
import {anyFailure, ctx, parseFailureWith} from "./test-helpers";

describe("array", () => {
const arrayOfStrings = array(string);

it("accepts an empty array", () => {
expect(arrayOfStrings([], ctx())).toEqual(success([]));
});

it("accepts a array of one element", () => {
expect(arrayOfStrings(["hi"], ctx())).toEqual(success(["hi"]));
});

it("accepts a array of multiple elements", () => {
expect(arrayOfStrings(["a", "b", "c"], ctx())).toEqual(
success(["a", "b", "c"]),
);
});

it("rejects a string", () => {
expect(arrayOfStrings("blah", ctx())).toEqual(anyFailure);
});

it("rejects a array with one element of the wrong type", () => {
expect(arrayOfStrings([99], ctx())).toEqual(anyFailure);
});

it("rejects a array with multiple elements if any has the wrong type", () => {
expect(arrayOfStrings(["ok", 99, "ok"], ctx())).toEqual(anyFailure);
});

it("indicates the bad element in the failure message", () => {
const theArray = [
"ok",
"ok",
99, // index 2
];

const result = arrayOfStrings(theArray, ctx());

expect(result).toEqual(
parseFailureWith({
expected: ["string"],
badValue: 99,
path: [2],
}),
);
});

it("lists all mismatches", () => {
const theArray = ["ok", 4, 99];

const result = arrayOfStrings(theArray, ctx());

assertFailure(result);

expect(result.detail).toEqual([
{
expected: ["string"],
badValue: 4,
path: [1],
},
{
expected: ["string"],
badValue: 99,
path: [2],
},
]);
});

it("pinpoints mismatches in nested arrays", () => {
const arrayOfArrayOfStrings = array(array(string));
const theArray = [["", ""], [""], [], ["", 99, ""]];

const result = arrayOfArrayOfStrings(theArray, ctx());

expect(result).toEqual(parseFailureWith({path: [3, 1]}));
});

it("lists multiple mismatches in nested arrays", () => {
const arrayOfArrayOfStrings = array(array(string));
const theArray = [["", "", 4], [9, ""], [], ["", 99, ""]];

const result = arrayOfArrayOfStrings(theArray, ctx());

assertFailure(result);

expect(result.detail.map((d) => d.path)).toEqual([
[0, 2],
[1, 0],
[3, 1],
]);
});

it("describes the problem if given a non-array", () => {
const result = arrayOfStrings(99, ctx());

expect(result).toEqual(
parseFailureWith({
expected: ["array"],
badValue: 99,
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as Result from "../result";

import type {ParseContext, Parser} from "../parser-types";

export function array<T>(elementParser: Parser<T>): Parser<T[]> {
return (rawValue: unknown, ctx: ParseContext) => {
if (!Array.isArray(rawValue)) {
return ctx.failure("array", rawValue);
}
const elementResults = rawValue.map((elem, i) =>
elementParser(elem, ctx.forSubtree(i)),
);
return Result.all(elementResults, concat);
};
}

function concat<T>(a: T[], b: T[]): T[] {
return [...a, ...b];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import {array} from "./array";
import {number} from "./number";
import {string} from "./string";
import {summonParsedValue} from "./test-helpers";

// Test: return type is correctly assignable
{
const arrayOfStrings = array(string);
const parsed = summonParsedValue<typeof arrayOfStrings>();

parsed satisfies string[];

// @ts-expect-error - parsed is string[]
parsed satisfies number[];
// @ts-expect-error - parsed is string[]
parsed satisfies string;
}

// Test: nested array parsers
{
const arrayOfArraysOfNumbers = array(array(number));
const parsed = summonParsedValue<typeof arrayOfArraysOfNumbers>();
parsed satisfies number[][];

// @ts-expect-error - parsed is number[][]
parsed satisfies number[];
// @ts-expect-error - parsed is number[][]
parsed satisfies string[][];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {success} from "../result";

import {boolean} from "./boolean";
import {ctx, parseFailureWith} from "./test-helpers";

describe("boolean()", () => {
it("accepts `true`", () => {
expect(boolean(true, ctx())).toEqual(success(true));
});

it("accepts `false`", () => {
expect(boolean(false, ctx())).toEqual(success(false));
});

it("rejects `null`", () => {
expect(boolean(null, ctx())).toEqual(
parseFailureWith({
expected: ["boolean"],
badValue: null,
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type {ParseContext, ParseResult} from "../parser-types";

export function boolean(
rawValue: unknown,
ctx: ParseContext,
): ParseResult<boolean> {
if (typeof rawValue === "boolean") {
return ctx.success(rawValue);
}
return ctx.failure("boolean", rawValue);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {isFailure} from "../result";

import type {
ParsedValue,
PartialParser,
ParseContext,
Parser,
} from "../parser-types";

export function composeParsers<
A extends Parser<any>,
B extends PartialParser<ParsedValue<A>, any>,
>(parserA: A, parserB: B): Parser<ParsedValue<B>> {
return (rawValue: unknown, ctx: ParseContext) => {
const partialResult = parserA(rawValue, ctx);
if (isFailure(partialResult)) {
return partialResult;
}

return parserB(partialResult.value, ctx);
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {success} from "../result";

import {constant} from "./constant";
import {ctx, parseFailureWith} from "./test-helpers";

describe("constant()", function () {
const fooParser = constant("foo");

it("creates a parser that accepts a single value", () => {
expect(fooParser("foo", ctx())).toEqual(success("foo"));
});

it("rejects any other value", () => {
expect(fooParser("bar", ctx())).toEqual(
parseFailureWith({
expected: [`"foo"`],
badValue: "bar",
}),
);
});

it("formats the error message correctly when the accepted value is `undefined`", () => {
const undefinedParser = constant(undefined);
expect(undefinedParser(null, ctx())).toEqual(
parseFailureWith({
expected: ["undefined"],
badValue: null,
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type {Parser} from "../parser-types";

type Primitive = string | number | boolean | null | undefined;

export function constant<T extends Primitive>(acceptedValue: T): Parser<T> {
return (rawValue, ctx) => {
if (rawValue !== acceptedValue) {
return ctx.failure(String(JSON.stringify(acceptedValue)), rawValue);
}
return ctx.success(acceptedValue);
};
}
Loading

0 comments on commit 32b2732

Please sign in to comment.