Skip to content

Commit

Permalink
KAS: try to type some things (#1449)
Browse files Browse the repository at this point in the history
* make parser-generator ts

* make compare happy with types

* add return type for compare

* move index to ts

* undo that

* changeset

* respond to Jeremys feedback
  • Loading branch information
handeyeco authored Jul 30, 2024
1 parent 92b77fe commit ca31afb
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .changeset/shy-scissors-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/kas": patch
---

Add some types to the KAS package
2 changes: 1 addition & 1 deletion packages/kas/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"dist"
],
"scripts": {
"gen:parsers": "node src/parser-generator.js",
"gen:parsers": "node src/parser-generator.ts",
"test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'"
},
"dependencies": {
Expand Down
27 changes: 12 additions & 15 deletions packages/kas/src/compare.js → packages/kas/src/compare.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,30 @@
/* eslint-disable */
import _ from "underscore";
import type {CompareOptions, CompareResult, Expression} from "./types";

// Assumes that both expressions have already been parsed
// TODO(alex): be able to pass a random() function to compare()
export const compare = function (expr1, expr2, options) {
var defaults = {
form: false, // Check that the two expressions have the same form
simplify: false, // Check that the second expression is simplified
export const compare = function (
expr1: Expression,
expr2: Expression,
options: CompareOptions,
): CompareResult {
const defaults: CompareOptions = {
form: false,
simplify: false,
};

/* Options that could be added in the future:
* - Allow ratios: e.g. 3/1 and 3 should both be accepted for something
* like slope
* - Allow student to choose their own variable names
*/

if (options !== undefined) {
// eslint-disable-next-line no-undef
options = _.extend(defaults, options);
} else {
options = defaults;
}
options = {...defaults, ...options};

// TODO(CP-1614): Figure out how to make these messages translatable

// Variable check
var vars = expr1.sameVars(expr2);
const vars = expr1.sameVars(expr2);
if (!vars.equal) {
var message;
let message;
if (vars.equalIgnoringCase) {
message =
"Check your variables; one or more are using " +
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
#!/usr/bin/env node
/* eslint-disable @typescript-eslint/no-var-requires */
/* eslint-disable comma-dangle */
/* eslint-disable comma-spacing */
/* eslint-disable import/no-commonjs */
/* eslint-disable max-len */
/* eslint-disable no-var */
/* eslint-disable prettier/prettier */

const fs = require("fs");
const path = require("path");

const jison = require("jison");

var grammar = {
const grammar = {
lex: {
rules: [
["\\s+", "/* skip whitespace */"],
Expand Down Expand Up @@ -204,32 +200,32 @@ var grammar = {
},
};

var prelude =
const prelude =
"// This is a @gene" + "rated file\n" + 'import _ from "underscore";\n\n';
var parser = new jison.Generator(grammar).generate({moduleType: "js"});
let parser = new jison.Generator(grammar).generate({moduleType: "js"});
// NOTE(jeresig): We need to comment out these two labels as they appear to be
// invalid ES5 (they also aren't referenced anywhere so this seems safe).
parser = parser.replace(/(_token_stack:)/g, "//$1");
var postlude = "\n\nexport {parser};\n";
const postlude = "\n\nexport {parser};\n";

fs.writeFileSync(
path.resolve(__dirname, "__genfiles__", "parser.js"),
prelude + parser + postlude,
);

var unitPrelude = "// this is a @gene" + "rated file\n\n";
var unitEpilogue = "\n\nexport const unitParser = parser;\n";
const unitPrelude = "// this is a @gene" + "rated file\n\n";
const unitEpilogue = "\n\nexport const unitParser = parser;\n";

var unitParserInfile = path.resolve(__dirname, "unitvalue.jison");
var unitParserOutfile = path.resolve(
const unitParserInfile = path.resolve(__dirname, "unitvalue.jison");
const unitParserOutfile = path.resolve(
__dirname,
"__genfiles__",
"unitparser.js",
);

var unitParserSource = fs.readFileSync(unitParserInfile);
var unitParser = new jison.Generator(unitParserSource.toString());
var generatedParser = unitParser.generate({moduleType: "js"});
const unitParserSource = fs.readFileSync(unitParserInfile);
const unitParser = new jison.Generator(unitParserSource.toString());
let generatedParser = unitParser.generate({moduleType: "js"});
// NOTE(jeresig): We need to comment out these two labels as they appear to be
// invalid ES5 (they also aren't referenced anywhere so this seems safe).
generatedParser = generatedParser.replace(/(_token_stack:)/g, "//$1");
Expand Down
25 changes: 25 additions & 0 deletions packages/kas/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export type CompareOptions = {
// Check that the two expressions have the same form
form: boolean;
// Check that the second expression is simplified
simplify: boolean;
};

export type CompareResult = {
equal: boolean;
wrongVariableCase?: boolean;
wrongVariableNames?: boolean;
message: string | null;
};

export type ExpressionVars = {
equal: boolean;
equalIgnoringCase: boolean;
};

export type Expression = {
compare: (expr: Expression) => boolean;
sameVars: (expr: Expression) => ExpressionVars;
sameForm: (expr: Expression) => unknown;
isSimplified: () => boolean;
};

0 comments on commit ca31afb

Please sign in to comment.