Improve validate() function for plugin options (#1323)

* Add `validateConfigParameter` and `validateConfigParameterFromArray` functions to validate user-provided parameters from py-config

* Add units tests for `validateConfigParameter` and `validateConfigParameterFromArray`

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

pyscriptjs/src/plugin.ts

@@ -1,6 +1,6 @@
 import type { PyScriptApp } from './main';
 import type { AppConfig } from './pyconfig';
-import type { UserError } from './exceptions';
+import { UserError, ErrorCode } from './exceptions';
 import { getLogger } from './logger';
 import { make_PyScript } from './components/pyscript';
 import { InterpreterClient } from './interpreter_client';
@@ -266,3 +266,74 @@ export function define_custom_element(tag: string, pyElementClass: PyElementClas
 
     customElements.define(tag, ProxyCustomElement);
 }
+
+// Members of py-config in plug that we want to validate must be one of these types
+type BaseConfigObject = string | boolean | number | undefined;
+
+/**
+ * Validate that parameter the user provided to py-config conforms to the specified validation function;
+ * if not, throw an error explaining the bad value. If no value is provided, set the parameter
+ * to the provided default value
+ * This is the most generic validation function; other validation functions for common situations follow
+ * @param options.config - The (extended) AppConfig object from py-config
+ * @param {string} options.name - The name of the key in py-config to be checked
+ * @param {(b:BaseConfigObject) => boolean} options.validator - the validation function used to test the user-supplied value
+ * @param {BaseConfigObject} options.defaultValue - The default value for this parameter, if none is provided
+ * @param {string} [options.hintMessage] - The message to show in a user error if the supplied value isn't valid
+ */
+export function validateConfigParameter(options: {
+    config: AppConfig;
+    name: string;
+    validator: (b: BaseConfigObject) => boolean;
+    defaultValue: BaseConfigObject;
+    hintMessage?: string;
+}) {
+    //Validate that the default value is acceptable, at runtime
+    if (!options.validator(options.defaultValue)) {
+        throw Error(
+            `Default value ${JSON.stringify(options.defaultValue)} for ${options.name} is not a valid argument, ` +
+                `according to the provided validator function. ${options.hintMessage ? options.hintMessage : ''}`,
+        );
+    }
+
+    const value = options.config[options.name] as BaseConfigObject;
+    if (value !== undefined && !options.validator(value)) {
+        //Use default hint message if none is provided:
+        const hintOutput = `Invalid value ${JSON.stringify(value)} for config.${options.name}. ${
+            options.hintMessage ? options.hintMessage : ''
+        }`;
+        throw new UserError(ErrorCode.BAD_CONFIG, hintOutput);
+    }
+    if (value === undefined) {
+        options.config[options.name] = options.defaultValue;
+    }
+}
+
+/**
+ * Validate that parameter the user provided to py-config is one of the acceptable values in
+ * the given Array; if not, throw an error explaining the bad value. If no value is provided,
+ * set the parameter to the provided default value
+ * @param options.config - The (extended) AppConfig object from py-config
+ * @param {string} options.name - The name of the key in py-config to be checked
+ * @param {Array<BaseConfigObject>} options.possibleValues: The acceptable values for this parameter
+ * @param {BaseConfigObject} options.defaultValue: The default value for this parameter, if none is provided
+ */
+export function validateConfigParameterFromArray(options: {
+    config: AppConfig;
+    name: string;
+    possibleValues: Array<BaseConfigObject>;
+    defaultValue: BaseConfigObject;
+}) {
+    const validator = (b: BaseConfigObject) => options.possibleValues.includes(b);
+    const hint = `The only accepted values are: [${options.possibleValues
+        .map(item => JSON.stringify(item))
+        .join(', ')}]`;
+
+    validateConfigParameter({
+        config: options.config,
+        name: options.name,
+        validator: validator,
+        defaultValue: options.defaultValue,
+        hintMessage: hint,
+    });
+}

pyscriptjs/src/plugins/pyterminal.ts

@@ -1,28 +1,15 @@
 import type { PyScriptApp } from '../main';
 import type { AppConfig } from '../pyconfig';
-import { Plugin } from '../plugin';
-import { UserError, ErrorCode } from '../exceptions';
+import { Plugin, validateConfigParameterFromArray } from '../plugin';
 import { getLogger } from '../logger';
 import { type Stdio } from '../stdio';
 import { InterpreterClient } from '../interpreter_client';
 
-type AppConfigStyle = AppConfig & { terminal?: boolean | 'auto'; docked?: boolean | 'docked' };
-
 const logger = getLogger('py-terminal');
 
-const validate = (config: AppConfigStyle, name: string, default_: string) => {
-    const value = config[name] as undefined | boolean | string;
-    if (value !== undefined && value !== true && value !== false && value !== default_) {
-        const got = JSON.stringify(value);
-        throw new UserError(
-            ErrorCode.BAD_CONFIG,
-            `Invalid value for config.${name}: the only accepted` +
-                `values are true, false and "${default_}", got "${got}".`,
-        );
-    }
-    if (value === undefined) {
-        config[name] = default_;
-    }
+type AppConfigStyle = AppConfig & {
+    terminal?: string | boolean;
+    docked?: string | boolean;
 };
 
 export class PyTerminalPlugin extends Plugin {
@@ -35,8 +22,18 @@ export class PyTerminalPlugin extends Plugin {
 
     configure(config: AppConfigStyle) {
         // validate the terminal config and handle default values
-        validate(config, 'terminal', 'auto');
-        validate(config, 'docked', 'docked');
+        validateConfigParameterFromArray({
+            config: config,
+            name: 'terminal',
+            possibleValues: [true, false, 'auto'],
+            defaultValue: 'auto',
+        });
+        validateConfigParameterFromArray({
+            config: config,
+            name: 'docked',
+            possibleValues: [true, false, 'docked'],
+            defaultValue: 'docked',
+        });
     }
 
     beforeLaunch(config: AppConfigStyle) {

pyscriptjs/tests/unit/plugin.test.ts

@@ -0,0 +1,116 @@
+import { validateConfigParameter, validateConfigParameterFromArray } from '../../src/plugin';
+import { UserError } from '../../src/exceptions';
+
+describe('validateConfigParameter', () => {
+    const validator = a => a.charAt(0) === 'a';
+
+    it('should not change a matching config option', () => {
+        const pyconfig = { a: 'a1', dummy: 'dummy' };
+        validateConfigParameter({
+            config: pyconfig,
+            name: 'a',
+            validator: validator,
+            defaultValue: 'a_default',
+            hintMessage: "Should start with 'a'",
+        });
+        expect(pyconfig).toStrictEqual({ a: 'a1', dummy: 'dummy' });
+    });
+
+    it('should set the default value if no value is present', () => {
+        const pyconfig = { dummy: 'dummy' };
+        validateConfigParameter({
+            config: pyconfig,
+            name: 'a',
+            validator: validator,
+            defaultValue: 'a_default',
+            hintMessage: "Should start with 'a'",
+        });
+        expect(pyconfig).toStrictEqual({ a: 'a_default', dummy: 'dummy' });
+    });
+
+    it('should error if the provided value is not valid', () => {
+        const pyconfig = { a: 'NotValidValue', dummy: 'dummy' };
+        const func = () =>
+            validateConfigParameter({
+                config: pyconfig,
+                name: 'a',
+                validator: validator,
+                defaultValue: 'a_default',
+                hintMessage: "Should start with 'a'",
+            });
+        expect(func).toThrow(UserError);
+        expect(func).toThrow('(PY1000): Invalid value "NotValidValue" for config.a. Should start with \'a\'');
+    });
+
+    it('should error if the provided default is not valid', () => {
+        const pyconfig = { a: 'a1', dummy: 'dummy' };
+        const func = () =>
+            validateConfigParameter({
+                config: pyconfig,
+                name: 'a',
+                validator: validator,
+                defaultValue: 'NotValidDefault',
+                hintMessage: "Should start with 'a'",
+            });
+        expect(func).toThrow(Error);
+        expect(func).toThrow(
+            'Default value "NotValidDefault" for a is not a valid argument, according to the provided validator function. Should start with \'a\'',
+        );
+    });
+});
+
+describe('validateConfigParameterFromArray', () => {
+    const possibilities = ['a1', 'a2', true, 42, 'a_default'];
+
+    it('should not change a matching config option', () => {
+        const pyconfig = { a: 'a1', dummy: 'dummy' };
+        validateConfigParameterFromArray({
+            config: pyconfig,
+            name: 'a',
+            possibleValues: possibilities,
+            defaultValue: 'a_default',
+        });
+        expect(pyconfig).toStrictEqual({ a: 'a1', dummy: 'dummy' });
+    });
+
+    it('should set the default value if no value is present', () => {
+        const pyconfig = { dummy: 'dummy' };
+        validateConfigParameterFromArray({
+            config: pyconfig,
+            name: 'a',
+            possibleValues: possibilities,
+            defaultValue: 'a_default',
+        });
+        expect(pyconfig).toStrictEqual({ a: 'a_default', dummy: 'dummy' });
+    });
+
+    it('should error if the provided value is not in possible_values', () => {
+        const pyconfig = { a: 'NotValidValue', dummy: 'dummy' };
+        const func = () =>
+            validateConfigParameterFromArray({
+                config: pyconfig,
+                name: 'a',
+                possibleValues: possibilities,
+                defaultValue: 'a_default',
+            });
+        expect(func).toThrow(Error);
+        expect(func).toThrow(
+            '(PY1000): Invalid value "NotValidValue" for config.a. The only accepted values are: ["a1", "a2", true, 42, "a_default"]',
+        );
+    });
+
+    it('should error if the provided default is not in possible_values', () => {
+        const pyconfig = { a: 'a1', dummy: 'dummy' };
+        const func = () =>
+            validateConfigParameterFromArray({
+                config: pyconfig,
+                name: 'a',
+                possibleValues: possibilities,
+                defaultValue: 'NotValidDefault',
+            });
+        expect(func).toThrow(Error);
+        expect(func).toThrow(
+            'Default value "NotValidDefault" for a is not a valid argument, according to the provided validator function. The only accepted values are: ["a1", "a2", true, 42, "a_default"]',
+        );
+    });
+});