fix: Enable lax overloads only in release

package.json

@@ -66,7 +66,8 @@
     "postbuild": "node scripts/replace_application_version.js",
     "build_and_test": "npm run build && npm run test",
     "lint": "tslint --project .",
-    "prepublishOnly": "npm run lint && npm run build_and_test",
+    "prepublishOnly": "node scripts/set_strict.js false && npm run lint && npm run build_and_test",
+    "postpublish": "node scripts/set_strict.js true",
     "prepare": "npm run build",
     "clean": "rm -rf node_modules package-lock.json lib coverage"
   },

scripts/set_strict.js

@@ -0,0 +1,18 @@
+// @ts-check
+// Sets the Strict type that TypeDoc uses to enable overloads for consumers only.
+// See the rationale in src/lib/utils/index.ts
+
+const fs = require('fs-extra');
+const { join } = require('path');
+
+const file = join(__dirname, '../src/lib/utils/index.ts');
+
+const isStrict = process.argv[2] === 'true'
+
+fs.readFile(file, { encoding: 'utf-8'})
+    .then(text => fs.writeFile(file, text.replace(/type Strict =.*/, `type Strict = ${isStrict};`)))
+    .catch(reason => {
+        console.error(reason);
+        process.exit(1);
+    });
+

src/lib/utils/index.ts

@@ -1,3 +1,35 @@
+/**
+ * This type provides a flag that can be used to turn off more lax overloads intended for
+ * plugin use only to catch type errors in the TypeDoc codebase. The prepublishOnly npm
+ * script will be used to switch this flag to false when publishing, then immediately back
+ * to true after a successful publish.
+ */
+type Strict = true;
+
+/**
+ * Helper type to convert `T` to `F` if strict mode is on.
+ *
+ * Can be used in overloads to map a parameter type to `never`. For example, the
+ * following function will work with any string argument, but to improve the type safety
+ * of internal code, we only ever want to pass 'a' or 'b' to it. Plugins on the other
+ * hand need to be able to pass any string to it. Overloads similar to this are used
+ * in the {@link Options} class.
+ *
+ * ```ts
+ * function over(flag: 'a' | 'b'): string
+ * function over(flag: IfStrict<string, never>): string
+ * function over(flag: string): string { return flag }
+ * ```
+ */
+export type IfStrict<T, F> = Strict extends true ? T : F;
+
+/**
+ * Helper type to convert `T` to `never` if strict mode is on.
+ *
+ * See {@link IfStrict} for the rationale.
+ */
+export type NeverIfStrict<T> = IfStrict<never, T>;
+
 export {
     Options,
     ParameterType,

src/lib/utils/options/declaration.ts

@@ -52,6 +52,7 @@ export interface TypeDocOptionMap {
     excludeNotExported: boolean;
     excludePrivate: boolean;
     excludeProtected: boolean;
+    excludeNotDocumented: boolean;
     ignoreCompilerErrors: boolean;
     disableSources: boolean;
     includes: string;

src/lib/utils/options/options.ts

@@ -7,6 +7,7 @@ import { Result, Ok, Err } from '../result';
 import { insertPrioritySorted } from '../array';
 import { addTSOptions, addTypeDocOptions } from './sources';
 import { Application } from '../../..';
+import { NeverIfStrict } from '..';
 
 /**
  * Describes an option reader that discovers user configuration and converts it to the
@@ -148,7 +149,7 @@ export class Options {
      * Adds an option declaration to the container.
      * @param declaration
      */
-    addDeclaration(declaration: Readonly<DeclarationOption>): void;
+    addDeclaration(declaration: NeverIfStrict<Readonly<DeclarationOption>>): void;
 
     addDeclaration(declaration: Readonly<DeclarationOption>): void {
         const names = [declaration.name];
@@ -172,10 +173,15 @@ export class Options {
     /**
      * Adds the given declarations to the container
      * @param declarations
+     *
+     * @privateRemarks
+     * This function explicitly provides a way out of the strict typing enforced
+     * by {@link addDeclaration}. It should only be used with careful validation
+     * of the declaration map. Internally, it is only used for adding TS options
      */
     addDeclarations(declarations: readonly DeclarationOption[]): void {
         for (const decl of declarations) {
-            this.addDeclaration(decl);
+            this.addDeclaration(decl as any);
         }
     }
 
@@ -217,11 +223,11 @@ export class Options {
      * Checks if the given option has a set value or if the value is the default value.
      * @param name
      */
-    isDefault(name: keyof TypeDocAndTSOptions): boolean;
-    isDefault(name: string): boolean;
+    isDefault(name: keyof TypeDocOptions): boolean;
+    isDefault(name: NeverIfStrict<string>): boolean;
     isDefault(name: string): boolean {
         // getValue will throw if the declaration does not exist.
-        return this.getValue(name) === this.getDeclaration(name)!.defaultValue;
+        return this.getValue(name as keyof TypeDocOptions) === this.getDeclaration(name)!.defaultValue;
     }
 
     /**
@@ -236,9 +242,9 @@ export class Options {
      * @param name
      */
     getValue<K extends keyof TypeDocOptions>(name: K): TypeDocOptions[K];
-    getValue(name: string): unknown;
+    getValue(name: NeverIfStrict<string>): unknown;
     getValue(name: string): unknown {
-        return this.tryGetValue(name).match({
+        return this.tryGetValue(name as keyof TypeDocOptions).match({
             ok: v => v,
             err(err) { throw err; }
         });
@@ -250,7 +256,7 @@ export class Options {
      * @param name
      */
     tryGetValue<K extends keyof TypeDocOptions>(name: K): Result<TypeDocOptions[K], Error>;
-    tryGetValue(name: string): Result<unknown, Error>;
+    tryGetValue(name: NeverIfStrict<string>): Result<unknown, Error>;
     tryGetValue(name: string): Result<unknown, Error> {
         const declaration = this.getDeclaration(name);
         if (!declaration) {
@@ -278,7 +284,7 @@ export class Options {
      * @param value
      */
     setValue<K extends keyof TypeDocAndTSOptions>(name: K, value: TypeDocAndTSOptions[K]): Result<void, Error>;
-    setValue(name: string, value: unknown): Result<void, Error>;
+    setValue(name: NeverIfStrict<string>, value: NeverIfStrict<unknown>): Result<void, Error>;
     setValue(name: string, value: unknown): Result<void, Error> {
         const declaration = this.getDeclaration(name);
         if (!declaration) {
@@ -305,7 +311,7 @@ export class Options {
     setValues(obj: Partial<TypeDocAndTSOptions>): Result<void, Error[]> {
         const errors: Error[] = [];
         for (const [name, value] of Object.entries(obj)) {
-            this.setValue(name, value).match({
+            this.setValue(name as keyof TypeDocOptions, value).match({
                 ok() {},
                 err(error) {
                     errors.push(error);
@@ -350,17 +356,17 @@ export function BindOption<K extends keyof TypeDocOptionMap>(name: K):
  * @privateRemarks
  * This overload is intended for plugin use only with looser type checks. Do not use internally.
  */
-export function BindOption(name: string):
+export function BindOption(name: NeverIfStrict<string>):
     (target: { application: Application } | { options: Options }, key: PropertyKey) => void;
 
 export function BindOption(name: string) {
     return function(target: { application: Application } | { options: Options }, key: PropertyKey) {
         Object.defineProperty(target, key, {
             get(this: { application: Application } | { options: Options }) {
                 if ('options' in this) {
-                    return this.options.getValue(name);
+                    return this.options.getValue(name as keyof TypeDocOptions);
                 } else {
-                    return this.application.options.getValue(name);
+                    return this.application.options.getValue(name as keyof TypeDocOptions);
                 }
             },
             enumerable: true,

src/lib/utils/options/readers/arguments.ts

@@ -1,6 +1,7 @@
 import { OptionsReader, Options } from '..';
 import { Logger } from '../../loggers';
 import { ParameterType } from '../declaration';
+import { Result } from '../..';
 
 /**
  * Obtains option values from command-line arguments
@@ -16,6 +17,11 @@ export class ArgumentsReader implements OptionsReader {
     }
 
     read(container: Options, logger: Logger): void {
+        // Make container's type more lax, we do the appropriate checks manually.
+        const options = container as Options & {
+            setValue(name: string, value: unknown): Result<void, Error>;
+            getValue(name: string): unknown;
+        };
         const seen = new Set<string>();
         let index = 0;
 
@@ -24,27 +30,27 @@ export class ArgumentsReader implements OptionsReader {
         while (index < this.args.length) {
             const name = this.args[index];
             const decl = name.startsWith('-')
-                ? (index++, container.getDeclaration(name.replace(/^--?/, '')))
-                : container.getDeclaration('inputFiles');
+                ? (index++, options.getDeclaration(name.replace(/^--?/, '')))
+                : options.getDeclaration('inputFiles');
 
             if (decl) {
                 if (seen.has(decl.name) && decl.type === ParameterType.Array) {
-                    container.setValue(
+                    options.setValue(
                         decl.name,
-                        (container.getValue(decl.name) as string[]).concat(this.args[index])
+                        (options.getValue(decl.name) as string[]).concat(this.args[index])
                     ).mapErr(error);
                 } else if (decl.type === ParameterType.Boolean) {
                     const value = String(this.args[index]).toLowerCase();
 
                     if (value === 'true' || value === 'false') {
-                        container.setValue(decl.name, value === 'true').mapErr(error);
+                        options.setValue(decl.name, value === 'true').mapErr(error);
                     } else {
-                        container.setValue(decl.name, true).mapErr(error);
+                        options.setValue(decl.name, true).mapErr(error);
                         // Bool option didn't consume the next argument as expected.
                         index--;
                     }
                 } else {
-                    container.setValue(decl.name, this.args[index]).mapErr(error);
+                    options.setValue(decl.name, this.args[index]).mapErr(error);
                 }
                 seen.add(decl.name);
             } else {

src/test/typedoc.test.ts

@@ -2,6 +2,7 @@ import { Application } from '..';
 import * as Path from 'path';
 import Assert = require('assert');
 import { Converter, Context } from '../lib/converter';
+import { ModuleKind } from 'typescript';
 
 describe('TypeDoc', function() {
     let application: Application;
@@ -25,22 +26,22 @@ describe('TypeDoc', function() {
         });
         it('honors the exclude argument even on a fixed directory list', function() {
             const inputFiles = Path.join(__dirname, 'converter', 'class');
-            application.options.setValue('exclude', '**/class.ts').unwrap();
+            application.options.setValue('exclude', ['**/class.ts']).unwrap();
             const expanded = application.expandInputFiles([inputFiles]);
 
             Assert(!expanded.includes(Path.join(inputFiles, 'class.ts')));
             Assert(!expanded.includes(inputFiles));
         });
         it('honors the exclude argument even on a fixed file list', function() {
             const inputFiles = Path.join(__dirname, 'converter', 'class', 'class.ts');
-            application.options.setValue('exclude', '**/class.ts').unwrap();
+            application.options.setValue('exclude', ['**/class.ts']).unwrap();
             const expanded = application.expandInputFiles([inputFiles]);
 
             Assert(!expanded.includes(inputFiles));
         });
         it('supports multiple excludes', function() {
             const inputFiles = Path.join(__dirname, 'converter');
-            application.options.setValue('exclude', '**/+(class|access).ts').unwrap();
+            application.options.setValue('exclude', ['**/+(class|access).ts']).unwrap();
             const expanded = application.expandInputFiles([inputFiles]);
 
             Assert(!expanded.includes(Path.join(inputFiles, 'class', 'class.ts')));
@@ -58,15 +59,15 @@ describe('TypeDoc', function() {
         });
         it('supports excluding directories beginning with dots', function() {
             const inputFiles = __dirname;
-            application.options.setValue('exclude', '**/+(.dot)/**').unwrap();
+            application.options.setValue('exclude', ['**/+(.dot)/**']).unwrap();
             const expanded = application.expandInputFiles([inputFiles]);
 
             Assert(!expanded.includes(Path.join(inputFiles, '.dot', 'index.ts')));
             Assert(!expanded.includes(inputFiles));
         });
         it('Honors the exclude option even if a module is imported', () => {
-            application.options.setValue('exclude', '**/b.ts').unwrap();
-            application.options.setValue('module', 'commonjs').unwrap();
+            application.options.setValue('exclude', ['**/b.ts']).unwrap();
+            application.options.setValue('module', ModuleKind.CommonJS).unwrap();
 
             function handler(context: Context) {
                 Assert.deepStrictEqual(context.fileNames, [

src/test/utils/options/options.test.ts

@@ -1,9 +1,13 @@
 import { Logger, Options, ParameterType, ParameterScope } from '../../../lib/utils';
 import { deepStrictEqual as equal, throws } from 'assert';
+import { DeclarationOption } from '../../../lib/utils/options';
 
 describe('Options', () => {
     const logger = new Logger();
-    const options = new Options(logger);
+    const options = new Options(logger) as Options & {
+        addDeclaration(declaration: Readonly<DeclarationOption>): void;
+        getValue(name: string): unknown;
+    };
     options.addDefaultDeclarations();
 
     it('Errors on duplicate declarations', () => {
@@ -66,7 +70,7 @@ describe('Options', () => {
     });
 
     it('Errors if converting a set value errors', () => {
-        throws(() => options.setValue('mode', 'nonsense').unwrap());
+        throws(() => options.setValue('mode', 'nonsense' as any).unwrap());
     });
 
     it('Supports directly getting values', () => {