Configs are not decoded as well as validated by io-ts. Improvements to config validation, error messages, and TSafeRegex type.

src/configValidator.ts

@@ -5,7 +5,7 @@ import { fold } from "fp-ts/lib/Either";
 import { PathReporter } from "io-ts/lib/PathReporter";
 import { availablePlugins } from "./plugins/availablePlugins";
 import { ZeppelinPlugin } from "./plugins/ZeppelinPlugin";
-import { validateStrict } from "./validatorUtils";
+import { decodeAndValidateStrict, StrictValidationError } from "./validatorUtils";
 
 const pluginNameToClass = new Map<string, typeof ZeppelinPlugin>();
 for (const pluginClass of availablePlugins) {
@@ -29,8 +29,8 @@ const globalConfigRootSchema = t.type({
 const partialMegaTest = t.partial({ name: t.string });
 
 export function validateGuildConfig(config: any): string[] | null {
-  const rootErrors = validateStrict(partialGuildConfigRootSchema, config);
+  const validationResult = decodeAndValidateStrict(partialGuildConfigRootSchema, config);
-  if (rootErrors) return rootErrors;
+  if (validationResult instanceof StrictValidationError) return validationResult.getErrors();
 
   if (config.plugins) {
     for (const [pluginName, pluginOptions] of Object.entries(config.plugins)) {

src/plugins/GlobalZeppelinPlugin.ts

@@ -4,10 +4,11 @@ import * as t from "io-ts";
 import { pipe } from "fp-ts/lib/pipeable";
 import { fold } from "fp-ts/lib/Either";
 import { PathReporter } from "io-ts/lib/PathReporter";
-import { isSnowflake, isUnicodeEmoji, resolveMember, resolveUser, UnknownUser } from "../utils";
+import { deepKeyIntersect, isSnowflake, isUnicodeEmoji, resolveMember, resolveUser, UnknownUser } from "../utils";
 import { Member, User } from "eris";
 import { performance } from "perf_hooks";
-import { validateStrict } from "../validatorUtils";
+import { decodeAndValidateStrict, StrictValidationErrors } from "../validatorUtils";
+import { mergeConfig } from "knub/dist/configUtils";
 
 const SLOW_RESOLVE_THRESHOLD = 1500;
 
@@ -15,25 +16,90 @@ export class GlobalZeppelinPlugin<TConfig extends {} = IBasePluginConfig> extend
   protected static configSchema: t.TypeC<any>;
   public static dependencies = [];
 
-  public static validateOptions(options: IPluginOptions): string[] | null {
+  /**
+   * Since we want to do type checking without creating instances of every plugin,
+   * we need a static version of getDefaultOptions(). This static version is then,
+   * by turn, called from getDefaultOptions() so everything still works as expected.
+   */
+  protected static getStaticDefaultOptions() {
+    // Implemented by plugin
+    return {};
+  }
+
+  /**
+   * Wrapper to fetch the real default options from getStaticDefaultOptions()
+   */
+  protected getDefaultOptions(): IPluginOptions<TConfig> {
+    return (this.constructor as typeof GlobalZeppelinPlugin).getStaticDefaultOptions() as IPluginOptions<TConfig>;
+  }
+
+  /**
+   * Merges the given options and default options and decodes them according to the config schema of the plugin (if any).
+   * Throws on any decoding/validation errors.
+   *
+   * Intended as an augmented, static replacement for Plugin.getMergedConfig() which is why this is also called from
+   * getMergedConfig().
+   *
+   * Like getStaticDefaultOptions(), we also want to use this function for type checking without creating an instance of
+   * the plugin, which is why this has to be a static function.
+   */
+  protected static mergeAndDecodeStaticOptions(options: any): IPluginOptions {
+    const defaultOptions: any = this.getStaticDefaultOptions();
+    const mergedConfig = mergeConfig({}, defaultOptions.config || {}, options.config || {});
+    const mergedOverrides = options["=overrides"]
+      ? options["=overrides"]
+      : (options.overrides || []).concat(defaultOptions.overrides || []);
+
+    const decodedConfig = this.configSchema ? decodeAndValidateStrict(this.configSchema, mergedConfig) : mergedConfig;
+    if (decodedConfig instanceof StrictValidationErrors) {
+      throw new Error(decodedConfig.getErrors().join("\n"));
+    }
+
+    const decodedOverrides = [];
+    for (const override of mergedOverrides) {
+      const overrideConfigMergedWithBaseConfig = mergeConfig({}, mergedConfig, override.config);
+      const decodedOverrideConfig = this.configSchema
+        ? decodeAndValidateStrict(this.configSchema, overrideConfigMergedWithBaseConfig)
+        : overrideConfigMergedWithBaseConfig;
+      if (decodedOverrideConfig instanceof StrictValidationErrors) {
+        throw new Error(decodedConfig.getErrors().join("\n"));
+      }
+      decodedOverrides.push({ ...override, config: deepKeyIntersect(decodedOverrideConfig, override.config) });
+    }
+
+    return {
+      config: decodedConfig,
+      overrides: decodedOverrides,
+    };
+  }
+
+  /**
+   * Wrapper that calls mergeAndValidateStaticOptions()
+   */
+  protected getMergedOptions(): IPluginOptions<TConfig> {
+    if (!this.mergedPluginOptions) {
+      this.mergedPluginOptions = ((this
+        .constructor as unknown) as typeof GlobalZeppelinPlugin).mergeAndDecodeStaticOptions(this.pluginOptions);
+    }
+
+    return this.mergedPluginOptions as IPluginOptions<TConfig>;
+  }
+
+  /**
+   * Run static type checks and other validations on the given options
+   */
+  public static validateOptions(options: any): string[] | null {
     // Validate config values
     if (this.configSchema) {
-      if (options.config) {
+      this.mergeAndDecodeStaticOptions(options);
-        const errors = validateStrict(this.configSchema, options.config);
-        if (errors) return errors;
-      }
-
-      if (options.overrides) {
-        for (const override of options.overrides) {
-          if (override.config) {
-            const errors = validateStrict(this.configSchema, override.config);
-            if (errors) return errors;
-          }
-        }
-      }
     }
 
     // No errors, return null
     return null;
   }
+
+  public async runLoad(): Promise<any> {
+    const mergedOptions = this.getMergedOptions(); // This implicitly also validates the config
+    return super.runLoad();
+  }
 }

src/plugins/ZeppelinPlugin.ts

@@ -4,10 +4,19 @@ import * as t from "io-ts";
 import { pipe } from "fp-ts/lib/pipeable";
 import { fold } from "fp-ts/lib/Either";
 import { PathReporter } from "io-ts/lib/PathReporter";
-import { isSnowflake, isUnicodeEmoji, resolveMember, resolveUser, resolveUserId, UnknownUser } from "../utils";
+import {
+  deepKeyIntersect,
+  isSnowflake,
+  isUnicodeEmoji,
+  resolveMember,
+  resolveUser,
+  resolveUserId,
+  UnknownUser,
+} from "../utils";
 import { Member, User } from "eris";
 import { performance } from "perf_hooks";
-import { validateStrict } from "../validatorUtils";
+import { decodeAndValidateStrict, StrictValidationErrors } from "../validatorUtils";
+import { mergeConfig } from "knub/dist/configUtils";
 
 const SLOW_RESOLVE_THRESHOLD = 1500;
 
@@ -29,50 +38,83 @@ export class ZeppelinPlugin<TConfig extends {} = IBasePluginConfig> extends Plug
     return ourLevel > memberLevel;
   }
 
+  /**
+   * Since we want to do type checking without creating instances of every plugin,
+   * we need a static version of getDefaultOptions(). This static version is then,
+   * by turn, called from getDefaultOptions() so everything still works as expected.
+   */
   protected static getStaticDefaultOptions() {
     // Implemented by plugin
     return {};
   }
 
+  /**
+   * Wrapper to fetch the real default options from getStaticDefaultOptions()
+   */
   protected getDefaultOptions(): IPluginOptions<TConfig> {
     return (this.constructor as typeof ZeppelinPlugin).getStaticDefaultOptions() as IPluginOptions<TConfig>;
   }
 
+  /**
+   * Merges the given options and default options and decodes them according to the config schema of the plugin (if any).
+   * Throws on any decoding/validation errors.
+   *
+   * Intended as an augmented, static replacement for Plugin.getMergedConfig() which is why this is also called from
+   * getMergedConfig().
+   *
+   * Like getStaticDefaultOptions(), we also want to use this function for type checking without creating an instance of
+   * the plugin, which is why this has to be a static function.
+   */
+  protected static mergeAndDecodeStaticOptions(options: any): IPluginOptions {
+    const defaultOptions: any = this.getStaticDefaultOptions();
+    const mergedConfig = mergeConfig({}, defaultOptions.config || {}, options.config || {});
+    const mergedOverrides = options["=overrides"]
+      ? options["=overrides"]
+      : (options.overrides || []).concat(defaultOptions.overrides || []);
+
+    const decodedConfig = this.configSchema ? decodeAndValidateStrict(this.configSchema, mergedConfig) : mergedConfig;
+    if (decodedConfig instanceof StrictValidationErrors) {
+      throw new Error(decodedConfig.getErrors().join("\n"));
+    }
+
+    const decodedOverrides = [];
+    for (const override of mergedOverrides) {
+      const overrideConfigMergedWithBaseConfig = mergeConfig({}, mergedConfig, override.config || {});
+      const decodedOverrideConfig = this.configSchema
+        ? decodeAndValidateStrict(this.configSchema, overrideConfigMergedWithBaseConfig)
+        : overrideConfigMergedWithBaseConfig;
+      if (decodedOverrideConfig instanceof StrictValidationErrors) {
+        throw new Error(decodedConfig.getErrors().join("\n"));
+      }
+      decodedOverrides.push({ ...override, config: deepKeyIntersect(decodedOverrideConfig, override.config || {}) });
+    }
+
+    return {
+      config: decodedConfig,
+      overrides: decodedOverrides,
+    };
+  }
+
+  /**
+   * Wrapper that calls mergeAndValidateStaticOptions()
+   */
+  protected getMergedOptions(): IPluginOptions<TConfig> {
+    if (!this.mergedPluginOptions) {
+      this.mergedPluginOptions = ((this.constructor as unknown) as typeof ZeppelinPlugin).mergeAndDecodeStaticOptions(
+        this.pluginOptions,
+      );
+    }
+
+    return this.mergedPluginOptions as IPluginOptions<TConfig>;
+  }
+
+  /**
+   * Run static type checks and other validations on the given options
+   */
   public static validateOptions(options: any): string[] | null {
     // Validate config values
     if (this.configSchema) {
-      if (options.config) {
+      this.mergeAndDecodeStaticOptions(options);
-        const merged = configUtils.mergeConfig(
-          {},
-          (this.getStaticDefaultOptions() as any).config || {},
-          options.config,
-        );
-        const errors = validateStrict(this.configSchema, merged);
-        if (errors) {
-          return errors;
-        }
-      }
-
-      if (options.overrides) {
-        for (const [i, override] of options.overrides.entries()) {
-          if (override.config) {
-            // For type checking overrides, apply default config + supplied config + any overrides preceding this override + finally this override
-            // Exhaustive type checking would require checking against all combinations of preceding overrides but that's... costy. This will do for now.
-            // TODO: Override default config retrieval functions and do some sort of memoized checking there?
-            const merged = configUtils.mergeConfig(
-              {},
-              (this.getStaticDefaultOptions() as any).config || {},
-              options.config || {},
-              ...options.overrides.slice(0, i).map(o => o.config || {}),
-              override.config,
-            );
-            const errors = validateStrict(this.configSchema, merged);
-            if (errors) {
-              return errors;
-            }
-          }
-        }
-      }
     }
 
     // No errors, return null
@@ -80,12 +122,7 @@ export class ZeppelinPlugin<TConfig extends {} = IBasePluginConfig> extends Plug
   }
 
   public async runLoad(): Promise<any> {
-    const mergedOptions = this.getMergedOptions();
+    const mergedOptions = this.getMergedOptions(); // This implicitly also validates the config
-    const validationErrors = ((this.constructor as unknown) as typeof ZeppelinPlugin).validateOptions(mergedOptions);
-    if (validationErrors) {
-      throw new Error(validationErrors.join("\n"));
-    }
-
     return super.runLoad();
   }
 
@@ -103,10 +140,20 @@ export class ZeppelinPlugin<TConfig extends {} = IBasePluginConfig> extends Plug
     }
   }
 
+  /**
+   * Intended for cross-plugin functionality
+   */
   public getRegisteredCommands() {
     return this.commands.commands;
   }
 
+  /**
+   * Intended for cross-plugin functionality
+   */
+  public getRuntimeOptions() {
+    return this.getMergedOptions();
+  }
+
   /**
    * Resolves a user from the passed string. The passed string can be a user id, a user mention, a full username (with discrim), etc.
    * If the user is not found in the cache, it's fetched from the API.

src/utils.ts

@@ -37,7 +37,7 @@ export const MINUTES = 60 * SECONDS;
 export const HOURS = 60 * MINUTES;
 export const DAYS = 24 * HOURS;
 
-export function tNullable(type: t.Mixed) {
+export function tNullable<T extends t.Type<any, any, unknown>>(type: T) {
   return t.union([type, t.undefined, t.null]);
 }
 
@@ -574,6 +574,19 @@ export class UnknownUser {
   }
 }
 
+export function deepKeyIntersect(obj, keyReference) {
+  const result = {};
+  for (const [key, value] of Object.entries(obj)) {
+    if (!keyReference.hasOwnProperty(key)) continue;
+    if (value != null && typeof value === "object" && typeof keyReference[key] === "object") {
+      result[key] = deepKeyIntersect(value, keyReference[key]);
+    } else {
+      result[key] = value;
+    }
+  }
+  return result;
+}
+
 const unknownUsers = new Set();
 const unknownMembers = new Set();
 

src/validatorUtils.ts

@@ -5,14 +5,23 @@ import { noop } from "./utils";
 import deepDiff from "deep-diff";
 import safeRegex from "safe-regex";
 
-export const TSafeRegexString = new t.Type(
+const regexWithFlags = /^\/(.*?)\/([i]*)$/;
-  "TSafeRegexString",
+
-  (s): s is string => typeof s === "string",
+/**
+ * The TSafeRegex type supports two syntaxes for regexes: /<regex>/<flags> and just <regex>
+ * The value is then checked for "catastrophic exponential-time regular expressions" by
+ * https://www.npmjs.com/package/safe-regex
+ */
+export const TSafeRegex = new t.Type<RegExp, string>(
+  "TSafeRegex",
+  (s): s is RegExp => s instanceof RegExp,
   (from, to) =>
     either.chain(t.string.validate(from, to), s => {
-      return safeRegex(s) ? t.success(s) : t.failure(from, to, "Unsafe regex");
+      const advancedSyntaxMatch = s.match(regexWithFlags);
+      const [regexStr, flags] = advancedSyntaxMatch ? [advancedSyntaxMatch[1], advancedSyntaxMatch[2]] : [s, ""];
+      return safeRegex(regexStr) ? t.success(new RegExp(regexStr, flags)) : t.failure(from, to, "Unsafe regex");
     }),
-  s => s,
+  s => `/${s.source}/${s.flags}`,
 );
 
 // From io-ts/lib/PathReporter
@@ -42,24 +51,38 @@ function getContextPath(context) {
 }
 // tslint:enable
 
-const report = fold((errors: any) => {
+export class StrictValidationError extends Error {
-  return errors.map(err => {
+  private errors;
-    if (err.message) return err.message;
+
+  constructor(errors: string[]) {
+    errors = Array.from(new Set(errors));
+    super(errors.join("\n"));
+    this.errors = errors;
+  }
+  getErrors() {
+    return this.errors;
+  }
+}
+
+const report = fold((errors: any): StrictValidationError | void => {
+  const errorStrings = errors.map(err => {
     const context = err.context.map(c => c.key).filter(k => k && !k.startsWith("{"));
-    if (context.length > 0 && !isNaN(context[context.length - 1])) context.splice(-1);
+    while (context.length > 0 && !isNaN(context[context.length - 1])) context.splice(-1);
 
     const value = stringify(err.value);
     return value === undefined
       ? `<${context.join("/")}> is required`
-      : `Invalid value supplied to <${context.join("/")}>`;
+      : `Invalid value supplied to <${context.join("/")}>${err.message ? `: ${err.message}` : ""}`;
   });
+
+  return new StrictValidationError(errorStrings);
 }, noop);
 
 /**
- * Validates the given value against the given schema while also disallowing extra properties
+ * Decodes and validates the given value against the given schema while also disallowing extra properties
  * See: https://github.com/gcanti/io-ts/issues/322
  */
-export function validateStrict(schema: t.HasProps, value: any): string[] | null {
+export function decodeAndValidateStrict(schema: t.HasProps, value: any): StrictValidationError | any {
   const validationResult = t.exact(schema).decode(value);
   return pipe(
     validationResult,
@@ -70,10 +93,10 @@ export function validateStrict(schema: t.HasProps, value: any): string[] | null
         if (JSON.stringify(value) !== JSON.stringify(result)) {
           const diff = deepDiff(result, value);
           const errors = diff.filter(d => d.kind === "N").map(d => `Unknown property <${d.path.join(".")}>`);
-          return errors.length ? errors : ["Found unknown properties"];
+          if (errors.length) return new StrictValidationError(errors);
         }
 
-        return null;
+        return result;
       },
     ),
   );