feat: Number options may require min and max values (#1278)

TypeStrong · Apr 25, 2020 · 42de2af · 42de2af
1 parent 4067f2c
commit 42de2af
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 7 deletions.
diff --git a/src/lib/utils/options/declaration.ts b/src/lib/utils/options/declaration.ts
@@ -160,6 +160,16 @@ export interface StringDeclarationOption extends DeclarationOptionBase {
 export interface NumberDeclarationOption extends DeclarationOptionBase {
     type: ParameterType.Number;
 
+    /**
+     * Lowest possible value.
+     */
+    minValue?: number;
+
+    /**
+     * Highest possible value.
+     */
+    maxValue?: number;
+
     /**
      * If not specified defaults to 0.
      */
@@ -235,9 +245,9 @@ export type DeclarationOptionToOptionType<T extends DeclarationOption> =
  * The default conversion function used by the Options container. Readers may
  * re-use this conversion function or implement their own. The arguments reader
  * implements its own since 'false' should not be converted to true for a boolean option.
- *
- * @param value
- * @param option
+ * @param value The value to convert.
+ * @param option The option for which the value should be converted.
+ * @returns The result of the conversion. Might be the value or an error.
  */
 export function convert<T extends DeclarationOption>(value: unknown, option: T): Result<DeclarationOptionToOptionType<T>, string>;
 export function convert<T extends DeclarationOption>(value: unknown, option: T): Result<unknown, string> {
@@ -246,7 +256,13 @@ export function convert<T extends DeclarationOption>(value: unknown, option: T):
         case ParameterType.String:
             return Result.Ok(value == null ? '' : String(value));
         case ParameterType.Number:
-            return Result.Ok(parseInt(String(value), 10) || 0);
+            const numberOption = option as NumberDeclarationOption;
+            const numValue = parseInt(String(value), 10) || 0;
+            if (hasBounds(numberOption) &&
+                !valueIsWithinBounds(numValue, numberOption.minValue, numberOption.maxValue)) {
+                return Result.Err(getBoundsError(numberOption.name, numberOption.minValue, numberOption.maxValue));
+            }
+            return Result.Ok(numValue);
         case ParameterType.Boolean:
             return Result.Ok(Boolean(value));
         case ParameterType.Array:
@@ -280,7 +296,13 @@ export function convert<T extends DeclarationOption>(value: unknown, option: T):
     }
 }
 
-function getMapError(map: MapDeclarationOption<unknown>['map'], name: string) {
+/**
+ * Returns an error message for a map option, indicating that a given value was not one of the values within the map.
+ * @param map The values for the option.
+ * @param name The name of the option.
+ * @returns The error message.
+ */
+function getMapError(map: MapDeclarationOption<unknown>['map'], name: string): string {
     let keys = map instanceof Map ? [...map.keys()] : Object.keys(map);
     const getString = (key: string) => String(map instanceof Map ? map.get(key) : map[key]);
 
@@ -293,3 +315,59 @@ function getMapError(map: MapDeclarationOption<unknown>['map'], name: string) {
 
     return `${name} must be one of ${keys.join(', ')}`;
 }
+
+/**
+ * Returns an error message for a value that is out ot bounds of the given min and/or max values.
+ * @param name The name of the thing the value represents.
+ * @param minValue The lower bound of the range of allowed values.
+ * @param maxValue The upper bound of the range of allowed values.
+ * @returns The error message.
+ */
+function getBoundsError(name: string, minValue?: number, maxValue?: number): string {
+    if (isFiniteNumber(minValue) && isFiniteNumber(maxValue)) {
+        return `${name} must be between ${minValue} and ${maxValue}`;
+    } else if (isFiniteNumber(minValue)) {
+        return `${name} must be >= ${minValue}`;
+    } else if (isFiniteNumber(maxValue)) {
+        return `${name} must be <= ${maxValue}`;
+    } else {
+        return '';
+    }
+}
+
+/**
+ * Checks if the given number option has bounds specified.
+ * @param numberOption The number option being checked.
+ * @retursn True, if the given number option has bounds specified, otherwise false.
+ */
+function hasBounds(numberOption: NumberDeclarationOption): boolean {
+    return isFiniteNumber(numberOption.minValue) || isFiniteNumber(numberOption.maxValue);
+}
+
+/**
+ * Checks if the given value is a finite number.
+ * @param value The value being checked.
+ * @returns True, if the value is a finite number, otherwise false.
+ */
+function isFiniteNumber(value?: unknown): value is number {
+    return typeof value === 'number' && isFinite(value);
+}
+
+/**
+ * Checks if a value is between the bounds of the given min and/or max values.
+ * @param value The value being checked.
+ * @param minValue The lower bound of the range of allowed values.
+ * @param maxValue The upper bound of the range of allowed values.
+ * @returns True, if the value is within the given bounds, otherwise false.
+ */
+function valueIsWithinBounds(value: number, minValue?: number, maxValue?: number): boolean {
+    if (isFiniteNumber(minValue) && isFiniteNumber(maxValue)) {
+        return minValue <= value && value <= maxValue;
+    } else if (isFiniteNumber(minValue)) {
+        return minValue >= value;
+    } else if (isFiniteNumber(maxValue)) {
+        return value <= maxValue;
+    } else {
+        return true;
+    }
+}
diff --git a/src/test/utils/options/declaration.test.ts b/src/test/utils/options/declaration.test.ts
@@ -1,4 +1,4 @@
-import { convert, DeclarationOption, ParameterType, MapDeclarationOption } from '../../../lib/utils/options/declaration';
+import { convert, DeclarationOption, ParameterType, MapDeclarationOption, NumberDeclarationOption } from '../../../lib/utils/options/declaration';
 import { deepStrictEqual as equal } from 'assert';
 import { Result } from '../../../lib/utils';
 
@@ -16,6 +16,54 @@ describe('Options - Default convert function', () => {
         equal(convert(NaN, optionWithType(ParameterType.Number)), Result.Ok(0));
     });
 
+    it('Converts to number if value is the lowest allowed value for a number option', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 1
+        };
+        equal(convert(1, declaration), Result.Ok(1));
+    });
+
+    it('Generates an error if value is too low for a number option', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 1
+        };
+        equal(convert(0, declaration), Result.Err('test must be between 1 and 10'));
+    });
+
+    it('Converts to number if value is the highest allowed value for a number option', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 1
+        };
+        equal(convert(10, declaration), Result.Ok(10));
+    });
+
+    it('Generates an error if value is too high for a number option', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 1
+        };
+        equal(convert(11, declaration), Result.Err('test must be between 1 and 10'));
+    });
+
     it('Converts to strings', () => {
         equal(convert('123', optionWithType(ParameterType.String)), Result.Ok('123'));
         equal(convert(123, optionWithType(ParameterType.String)), Result.Ok('123'));

diff --git a/src/test/utils/options/options.test.ts b/src/test/utils/options/options.test.ts
@@ -1,5 +1,6 @@
 import { Logger, Options, ParameterType, ParameterScope } from '../../../lib/utils';
-import { deepStrictEqual as equal, throws } from 'assert';
+import { NumberDeclarationOption } from '../../../lib/utils/options';
+import { deepStrictEqual as equal, doesNotThrow, throws } from 'assert';
 
 describe('Options', () => {
     const logger = new Logger();
@@ -25,6 +26,43 @@ describe('Options', () => {
         options.removeDeclarationByName(declaration.name);
     });
 
+    it('Does not throw if number declaration has no min and max values', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test-number-declaration',
+            help: '',
+            type: ParameterType.Number,
+            defaultValue: 1
+        };
+        doesNotThrow(() => options.addDeclaration(declaration));
+        options.removeDeclarationByName(declaration.name);
+    });
+
+    it('Does not throw if default value is within range for number declaration', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test-number-declaration',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 5
+        };
+        doesNotThrow(() => options.addDeclaration(declaration));
+        options.removeDeclarationByName(declaration.name);
+    });
+
+    it('Throws if default value is out of range for number declaration', () => {
+        const declaration: NumberDeclarationOption = {
+            name: 'test-number-declaration',
+            help: '',
+            type: ParameterType.Number,
+            minValue: 1,
+            maxValue: 10,
+            defaultValue: 0
+        };
+        throws(() => options.addDeclaration(declaration));
+        options.removeDeclarationByName(declaration.name);
+    });
+
     it('Does not error if a map declaration has a default value that is not part of the map of possible values', () => {
         logger.resetErrors();
         options.addDeclaration({