Prework for perseus-linter ticket (#1499)

## Summary:
Just trying to tidy up before addressing LEMS-2199 ([followup PR](#1531))

Issue: [LEMS-2199](https://khanacademy.atlassian.net/browse/LEMS-2199)

[LEMS-2199]: https://khanacademy.atlassian.net/browse/LEMS-2199?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Author: handeyeco

Reviewers: SonicScrewdriver, benchristel, handeyeco

Required Reviewers:

Approved By: SonicScrewdriver, benchristel

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

Pull Request URL: #1499

.changeset/fluffy-otters-join.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus-linter": minor
+---
+
+Add expression-widget lint rule

.changeset/sixty-clouds-swim.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus-linter": patch
+---
+
+Cleaning up some types in perseus-linter

packages/perseus-linter/src/__tests__/rule.test.ts

@@ -3,6 +3,8 @@ import * as PureMarkdown from "@khanacademy/pure-markdown";
 import Rule from "../rule";
 import TreeTransformer from "../tree-transformer";
 
+import type {MakeRuleOptions} from "../rule";
+
 describe("PerseusLinter lint Rules class", () => {
     const markdown = `
 ## This Heading is in Title Case
@@ -12,11 +14,11 @@ This paragraph contains an unescaped $ sign.
 #### This heading skipped a level
 `;
 
-    const ruleDescriptions = [
+    const ruleDescriptions: MakeRuleOptions[] = [
         {
             name: "heading-title-case",
             selector: "heading",
-            pattern: "\\s[A-Z][a-z]",
+            pattern: /\s[A-Z][a-z]/,
             message: `Title case in heading:
 Only capitalize the first word of headings.`,
         },
@@ -45,7 +47,7 @@ Otherwise escape it by writing \\$.`,
 this heading is level ${currentHeading.level} but
 the previous heading was level ${previousHeading.level}`;
                 }
-                return false;
+                return;
             },
         },
     ];

packages/perseus-linter/src/__tests__/rules.test.ts

@@ -4,6 +4,7 @@ import absoluteUrlRule from "../rules/absolute-url";
 import blockquotedMathRule from "../rules/blockquoted-math";
 import blockquotedWidgetRule from "../rules/blockquoted-widget";
 import doubleSpacingAfterTerminalRule from "../rules/double-spacing-after-terminal";
+import expressionWidgetRule from "../rules/expression-widget";
 import extraContentSpacingRule from "../rules/extra-content-spacing";
 import headingLevel1Rule from "../rules/heading-level-1";
 import headingLevelSkipRule from "../rules/heading-level-skip";
@@ -512,6 +513,120 @@ describe("Individual lint rules tests", () => {
         },
     });
 
+    expectWarning(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\sqrt{42}",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic"],
+                },
+            },
+        },
+    });
+
+    expectPass(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\sqrt{42}",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic", "prealgebra"],
+                },
+            },
+        },
+    });
+
+    expectWarning(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\sin\\left(42\\right)",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic"],
+                },
+            },
+        },
+    });
+
+    expectPass(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\sin\\left(42\\right)",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic", "trig"],
+                },
+            },
+        },
+    });
+
+    expectWarning(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\log\\left(5\\right)",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic"],
+                },
+            },
+        },
+    });
+
+    expectPass(expressionWidgetRule, "[[☃ expression 1]]", {
+        widgets: {
+            "expression 1": {
+                options: {
+                    answerForms: [
+                        {
+                            value: "\\log\\left(5\\right)",
+                            form: true,
+                            simplify: true,
+                            considered: "correct",
+                            key: "0",
+                        },
+                    ],
+                    buttonSets: ["basic", "logarithms"],
+                },
+            },
+        },
+    });
+
     // @ts-expect-error - TS2554 - Expected 3 arguments, but got 2.
     expectWarning(doubleSpacingAfterTerminalRule, [
         "Good times.  Great oldies.",

packages/perseus-linter/src/__tests__/tree-transformer.test.ts

@@ -56,7 +56,6 @@ describe("PerseusLinter tree transformer", () => {
     function getTraversalOrder(tree: any) {
         const order: Array<any> = [];
         new TreeTransformer(tree).traverse((n, state) => {
-            // @ts-expect-error - TS2339 - Property 'id' does not exist on type 'TreeNode'.
             order.push(n.id);
         });
         return order;

packages/perseus-linter/src/rule.ts

@@ -127,13 +127,30 @@ import Selector from "./selector";
 
 import type {TraversalState, TreeNode} from "./tree-transformer";
 
+export type MakeRuleOptions = {
+    name: string;
+    pattern?: RegExp;
+    severity?: number;
+    selector?: string;
+    locale?: string;
+    message?: string;
+    lint?: (
+        state: TraversalState,
+        content: string,
+        nodes: any,
+        match: any,
+        context: LintRuleContextObject,
+    ) => string | undefined;
+    applies?: AppliesTester;
+};
+
 // This represents the type returned by String.match(). It is an
 // array of strings, but also has index:number and input:string properties.
 // TypeScript doesn't handle it well, so we punt and just use any.
-export type PatternMatchType = any;
+type PatternMatchType = any;
 
 // This is the return type of the check() method of a Rule object
-export type RuleCheckReturnType =
+type RuleCheckReturnType =
     | {
           rule: string;
           message: string;
@@ -149,21 +166,29 @@ export type RuleCheckReturnType =
 // object containing a string and two numbers.
 // prettier-ignore
 // (prettier formats this in a way that ka-lint does not like)
-export type LintTesterReturnType = string | {
+type LintTesterReturnType = string | {
     message: string,
     start: number,
     end: number
 } | null | undefined;
 
-export type LintRuleContextObject = any | null | undefined;
+type LintRuleContextObject =
+    | {
+          content: string;
+          contentType: "article" | "exercise";
+          stack: string[];
+          widgets: any[];
+      }
+    | null
+    | undefined;
 
 // This is the type of the lint detection function that the Rule() constructor
 // expects as its fourth argument. It is passed the TraversalState object and
 // content string that were passed to check(), and is also passed the array of
 // nodes returned by the selector match and the array of strings returned by
 // the pattern match. It should return null if no lint is detected or an
 // error message or an object contining an error message.
-export type LintTester = (
+type LintTester = (
     state: TraversalState,
     content: string,
     selectorMatch: ReadonlyArray<TreeNode>,
@@ -175,7 +200,7 @@ export type LintTester = (
 // be checked by context. For example, some rules only apply in exercises,
 // and should never be applied to articles. Defaults to true, so if we
 // omit the applies function in a rule, it'll be tested everywhere.
-export type AppliesTester = (context: LintRuleContextObject) => boolean;
+type AppliesTester = (context: LintRuleContextObject) => boolean;
 
 /**
  * A Rule object describes a Perseus lint rule. See the comment at the top of
@@ -198,8 +223,8 @@ export default class Rule {
         severity: number | null | undefined,
         selector: Selector | null | undefined,
         pattern: RegExp | null | undefined,
-        lint: LintTester | string,
-        applies: AppliesTester,
+        lint: LintTester | string | undefined,
+        applies: AppliesTester | undefined,
     ) {
         if (!selector && !pattern) {
             throw new PerseusError(
@@ -233,7 +258,7 @@ export default class Rule {
 
     // A factory method for use with rules described in JSON files
     // See the documentation at the start of this file for details.
-    static makeRule(options: any): Rule {
+    static makeRule(options: MakeRuleOptions): Rule {
         return new Rule(
             options.name,
             options.severity,

packages/perseus-linter/src/rules/all-rules.ts

@@ -8,6 +8,7 @@ import AbsoluteUrl from "./absolute-url";
 import BlockquotedMath from "./blockquoted-math";
 import BlockquotedWidget from "./blockquoted-widget";
 import DoubleSpacingAfterTerminal from "./double-spacing-after-terminal";
+import ExpressionWidget from "./expression-widget";
 import ExtraContentSpacing from "./extra-content-spacing";
 import HeadingLevel1 from "./heading-level-1";
 import HeadingLevelSkip from "./heading-level-skip";
@@ -41,6 +42,7 @@ export default [
     BlockquotedMath,
     BlockquotedWidget,
     DoubleSpacingAfterTerminal,
+    ExpressionWidget,
     ExtraContentSpacing,
     HeadingLevel1,
     HeadingLevelSkip,

packages/perseus-linter/src/rules/expression-widget.ts

@@ -0,0 +1,53 @@
+import Rule from "../rule";
+
+function buttonNotInButtonSet(name: string, set: string): string {
+    return `Answer requires a button not found in the button sets: ${name} (in ${set})`;
+}
+
+const stringToButtonSet = {
+    "\\sqrt": "prealgebra",
+    "\\sin": "trig",
+    "\\cos": "trig",
+    "\\tan": "trig",
+    "\\log": "logarithms",
+    "\\ln": "logarithms",
+};
+
+/**
+ * Rule to make sure that Expression questions that require
+ * a specific math symbol to answer have that math symbol
+ * available in the keypad (desktop learners can use a keyboard,
+ * but mobile learners must use the MathInput keypad)
+ */
+export default Rule.makeRule({
+    name: "expression-widget",
+    severity: Rule.Severity.WARNING,
+    selector: "widget",
+    lint: function (state, content, nodes, match, context) {
+        // This rule only looks at image widgets
+        if (state.currentNode().widgetType !== "expression") {
+            return;
+        }
+
+        const nodeId = state.currentNode().id;
+        if (!nodeId) {
+            return;
+        }
+
+        // If it can't find a definition for the widget it does nothing
+        const widget = context?.widgets?.[nodeId];
+        if (!widget) {
+            return;
+        }
+
+        const answers = widget.options.answerForms;
+        const buttons = widget.options.buttonSets;
+        for (const answer of answers) {
+            for (const [str, set] of Object.entries(stringToButtonSet)) {
+                if (answer.value.includes(str) && !buttons.includes(set)) {
+                    return buttonNotInButtonSet(str, set);
+                }
+            }
+        }
+    },
+}) as Rule;

packages/perseus-linter/src/rules/extra-content-spacing.ts

@@ -5,7 +5,7 @@ export default Rule.makeRule({
     selector: "paragraph",
     pattern: /\s+$/,
     applies: function (context) {
-        return context.contentType === "article";
+        return context?.contentType === "article";
     },
     message: `No extra whitespace at the end of content blocks.`,
 }) as Rule;

packages/perseus-linter/src/rules/image-widget.ts

@@ -16,11 +16,13 @@ export default Rule.makeRule({
             return;
         }
 
+        const nodeId = state.currentNode().id;
+        if (!nodeId) {
+            return;
+        }
+
         // If it can't find a definition for the widget it does nothing
-        const widget =
-            context &&
-            context.widgets &&
-            context.widgets[state.currentNode().id];
+        const widget = context && context.widgets && context.widgets[nodeId];
         if (!widget) {
             return;
         }

packages/perseus-linter/src/rules/math-align-linebreaks.ts

@@ -18,7 +18,7 @@ export default Rule.makeRule({
             const index = text.indexOf("\\\\");
             if (index === -1) {
                 // No more backslash pairs, so we found no lint
-                return null;
+                return;
             }
             text = text.substring(index + 2);