fix: handle empty strings to prevent unintended conversion to 0 (#469)

src/prompts/durationPrompts.ts

@@ -9,6 +9,7 @@ import input from '@inquirer/input';
 import { Duration } from '@salesforce/kit';
 import { Messages } from '@salesforce/core';
 import { FlagAnswers } from '../types.js';
+import { integerMinMaxValidation, toOptionalNumber } from './validations.js';
 import { stringToChoice } from './functions.js';
 
 export const durationPrompts = async (): Promise<
@@ -26,35 +27,30 @@ export const durationPrompts = async (): Promise<
     choices: durationUnits.map(stringToChoice),
   })) as FlagAnswers['durationUnit'];
 
-  const durationMin = Number(
+  const durationMin = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Duration.Minimum'),
       validate: (i: string): string | boolean => {
         if (!i) return true;
-        return Number.isInteger(Number(i)) ? true : messages.getMessage('error.InvalidInteger');
+        return integerMinMaxValidation(Number(i));
       },
     })
   );
-  const durationMax = Number(
+  const durationMax = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Duration.Maximum'),
       validate: (i: string): string | boolean => {
         if (!i) return true;
-        if (!Number.isInteger(Number(i))) return messages.getMessage('error.InvalidInteger');
-        return !durationMin || Number(i) > durationMin ? true : messages.getMessage('error.IntegerMaxLessThanMin');
+        return integerMinMaxValidation(Number(i), durationMin);
       },
     })
   );
-  const durationDefaultValue = Number(
+  const durationDefaultValue = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Duration.DefaultValue'),
       validate: (i: string): string | boolean => {
         if (!i) return true;
-        const num = Number(i);
-        if (!Number.isInteger(num)) return messages.getMessage('error.InvalidInteger');
-        return (!durationMin || num >= durationMin) && (!durationMax || num <= durationMax)
-          ? true
-          : messages.getMessage('error.InvalidDefaultInteger');
+        return integerMinMaxValidation(Number(i), durationMin, durationMax);
       },
     })
   );

src/prompts/integerPrompts.ts

@@ -7,43 +7,39 @@
 import input from '@inquirer/input';
 import { Messages } from '@salesforce/core';
 import { FlagAnswers } from '../types.js';
+import { integerMinMaxValidation, toOptionalNumber } from './validations.js';
 
 export const integerPrompts = async (): Promise<Pick<FlagAnswers, 'integerMin' | 'integerMax' | 'integerDefault'>> => {
   Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
   const messages = Messages.loadMessages('@salesforce/plugin-dev', 'dev.generate.flag');
 
-  const integerMin = Number(
+  const integerMin = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Integer.Minimum'),
       validate: (i: string): string | boolean => {
         if (!i) return true;
-        return Number.isInteger(Number(i)) ? true : messages.getMessage('error.InvalidInteger');
+        return integerMinMaxValidation(Number(i));
       },
     })
   );
-  const integerMax = Number(
+  const integerMax = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Integer.Maximum'),
       validate: (i: string): string | boolean => {
         if (!i) return true;
-        if (!Number.isInteger(Number(i))) return messages.getMessage('error.InvalidInteger');
-        return !integerMin || Number(i) > integerMin ? true : messages.getMessage('error.IntegerMaxLessThanMin');
+        return integerMinMaxValidation(Number(i), integerMin);
       },
     })
   );
-  const integerDefault = Number(
+  const integerDefault = toOptionalNumber(
     await input({
       message: messages.getMessage('question.Integer.Default'),
       validate: (i: string): string | boolean => {
         if (!i)
           return typeof integerMax === 'number' || typeof integerMin === 'number'
             ? messages.getMessage('error.RequiredIntegerDefault')
             : true;
-        const num = Number(i);
-        if (!Number.isInteger(num)) return messages.getMessage('error.InvalidInteger');
-        return (!integerMin || num >= integerMin) && (!integerMax || num <= integerMax)
-          ? true
-          : messages.getMessage('error.InvalidDefaultInteger');
+        return integerMinMaxValidation(Number(i), integerMin, integerMax);
       },
     })
   );

src/prompts/validations.ts

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { Messages } from '@salesforce/core';
+
+/**
+ * handle empty string as valid answer, make it mean undefined.
+ *
+ * relies on the i being validate to be convertible to an integer using the prompt `validate` function
+ */
+export const toOptionalNumber = (i: string): number | undefined => (i.length === 0 ? undefined : parseInt(i, 10));
+
+/** validation function for integers */
+export const integerMinMaxValidation = (num: number, min?: number, max?: number): boolean | string => {
+  Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
+  const messages = Messages.loadMessages('@salesforce/plugin-dev', 'dev.generate.flag');
+
+  if (!Number.isInteger(num)) return messages.getMessage('error.InvalidInteger');
+  if (min !== undefined && num < min) return messages.getMessage('error.InvalidDefaultInteger');
+  if (max !== undefined && num > max) return messages.getMessage('error.InvalidDefaultInteger');
+  return true;
+};

test/shared/prompts/validation.test.ts

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { expect } from 'chai';
+import { integerMinMaxValidation, toOptionalNumber } from '../../../src/prompts/validations.js';
+
+describe('integerMinMaxValidation', () => {
+  it('good int', () => {
+    expect(integerMinMaxValidation(1)).to.be.true;
+    expect(integerMinMaxValidation(0)).to.be.true;
+    expect(integerMinMaxValidation(-1)).to.be.true;
+    expect(integerMinMaxValidation(Number.MAX_SAFE_INTEGER)).to.be.true;
+  });
+
+  it('not int', () => {
+    expect(integerMinMaxValidation(1.1)).to.be.a('string');
+    expect(integerMinMaxValidation(-1.1)).to.be.a('string');
+    expect(integerMinMaxValidation(NaN)).to.be.a('string');
+  });
+
+  it('min ok', () => {
+    expect(integerMinMaxValidation(0, undefined)).to.be.true;
+    expect(integerMinMaxValidation(1, undefined)).to.be.true;
+    expect(integerMinMaxValidation(-1, undefined)).to.be.true;
+    expect(integerMinMaxValidation(0, 0)).to.be.true;
+    expect(integerMinMaxValidation(5, 1)).to.be.true;
+    expect(integerMinMaxValidation(-1, -5)).to.be.true;
+    expect(integerMinMaxValidation(-1, -1)).to.be.true;
+  });
+
+  it('min not ok', () => {
+    expect(integerMinMaxValidation(0, 1)).to.be.a('string');
+    expect(integerMinMaxValidation(1, 2)).to.be.a('string');
+    expect(integerMinMaxValidation(-1, 0)).to.be.a('string');
+  });
+
+  it('min max ok', () => {
+    expect(integerMinMaxValidation(0, undefined, undefined)).to.be.true;
+    expect(integerMinMaxValidation(1, undefined, 2)).to.be.true;
+    expect(integerMinMaxValidation(-1, undefined, 0)).to.be.true;
+    expect(integerMinMaxValidation(0, 0, 0)).to.be.true;
+    expect(integerMinMaxValidation(2, 2, 2)).to.be.true;
+    expect(integerMinMaxValidation(1, 0, 2)).to.be.true;
+  });
+
+  it('min max not ok', () => {
+    expect(integerMinMaxValidation(5, 1, 3)).to.be.a('string');
+    expect(integerMinMaxValidation(1, 2, 3)).to.be.a('string');
+    expect(integerMinMaxValidation(-1, 0)).to.be.a('string');
+    expect(integerMinMaxValidation(1, 2, 0)).to.be.a('string');
+    expect(integerMinMaxValidation(1, 2, 1)).to.be.a('string');
+  });
+});
+
+describe('toOptionalNumber', () => {
+  it('empty string => undefined', () => {
+    expect(toOptionalNumber('')).to.be.undefined;
+  });
+
+  it('not a number', () => {
+    expect(toOptionalNumber('foo')).to.be.NaN;
+  });
+
+  it('number string', () => {
+    expect(toOptionalNumber('1')).to.equal(1);
+    expect(toOptionalNumber('0')).to.equal(0);
+    expect(toOptionalNumber('-1')).to.equal(-1);
+    expect(toOptionalNumber('9007199254740991')).to.equal(9007199254740991);
+  });
+});