handling commas and dots input in parseToken of Parser (#7514)

Co-authored-by: naronchen <[email protected]>

common/changes/@itwin/core-quantity/naron-issue7424_2024-12-24-23-01.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/core-quantity",
+      "comment": "minor fix in parseToken",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/core-quantity"
+}

core/quantity/src/Parser.ts

@@ -85,6 +85,10 @@ class ParseToken {
 
   public get isString(): boolean { return !this.isOperator && typeof this.value === "string"; }
   public get isNumber(): boolean { return typeof this.value === "number"; }
+  public get isSpecialCharacter(): boolean {
+    const format = /^[!@#$%^&*()_+\-=\[\]{};':"\\|,.<>\/?]+$/;
+    return this.isString && this.value.toString().match(format) !== null;
+  }
 }
 
 /** A ScientificToken holds an index and string representing the exponent.
@@ -356,7 +360,11 @@ export class Parser {
         if (signToken.length > 0) {
           wipToken = signToken + wipToken;
         }
-        tokens.push(new ParseToken(parseFloat(wipToken)));
+        if (isNaN(Number(wipToken))) {
+          tokens.push(new ParseToken(NaN));
+        } else {
+          tokens.push(new ParseToken(parseFloat(wipToken)));
+        }
       } else {
         tokens.push(new ParseToken(wipToken));
       }
@@ -468,8 +476,6 @@ export class Parser {
    */
   public static async parseIntoQuantity(inString: string, format: Format, unitsProvider: UnitsProvider, altUnitLabelsProvider?: AlternateUnitLabelsProvider): Promise<QuantityProps> {
     const tokens: ParseToken[] = Parser.parseQuantitySpecification(inString, format);
-    if (tokens.length === 0 || (!format.allowMathematicOperations && Parser.isMathematicOperation(tokens)))
-      return new Quantity();
 
     return Parser.createQuantityFromParseTokens(tokens, format, unitsProvider, altUnitLabelsProvider);
   }
@@ -572,6 +578,19 @@ export class Parser {
    * Accumulate the given list of tokens into a single quantity value. Formatting the tokens along the way.
    */
   private static getQuantityValueFromParseTokens(tokens: ParseToken[], format: Format, unitsConversions: UnitConversionSpec[], defaultUnitConversion?: UnitConversionProps ): QuantityParseResult {
+    if (tokens.length === 0)
+      return { ok: false, error: ParseError.UnableToGenerateParseTokens };
+
+    if(!format.allowMathematicOperations && Parser.isMathematicOperation(tokens))
+      return { ok: false, error: ParseError.MathematicOperationFoundButIsNotAllowed };
+
+    if (
+      tokens.some((token) => token.isNumber && isNaN(token.value as number)) ||
+      tokens.every((token) => token.isSpecialCharacter)
+    ) {
+      return { ok: false, error: ParseError.UnableToConvertParseTokensToQuantity };
+    }
+
     const defaultUnit = format.units && format.units.length > 0 ? format.units[0][0] : undefined;
     defaultUnitConversion = defaultUnitConversion ? defaultUnitConversion : Parser.getDefaultUnitConversion(tokens, unitsConversions, defaultUnit);
 
@@ -582,6 +601,8 @@ export class Parser {
     let sign: 1 | -1 = 1;
 
     let compositeUnitIndex = 0;
+
+
     for (let i = 0; i < tokens.length; i = i + increment) {
       tokenPair = this.getNextTokenPair(i, tokens);
       if(!tokenPair || tokenPair.length === 0){
@@ -634,6 +655,7 @@ export class Parser {
         }
       }
     }
+
     return { ok: true, value: mag };
   }
 
@@ -880,13 +902,6 @@ export class Parser {
 
   private static parseAndProcessTokens(inString: string, format: Format, unitsConversions: UnitConversionSpec[]): QuantityParseResult {
     const tokens: ParseToken[] = Parser.parseQuantitySpecification(inString, format);
-    if (tokens.length === 0)
-      return { ok: false, error: ParseError.UnableToGenerateParseTokens };
-
-    if(!format.allowMathematicOperations && Parser.isMathematicOperation(tokens)){
-      return { ok: false, error: ParseError.MathematicOperationFoundButIsNotAllowed };
-    }
-
     if (Parser._log) {
       // eslint-disable-next-line no-console
       console.log(`Parse tokens`);

core/quantity/src/test/Parsing.test.ts

@@ -595,6 +595,45 @@ describe("Parsing tests:", () => {
     }
   });
 
+  it("should return parseError when parsing only special characters", async () => {
+    const formatData = {
+      formatTraits: ["keepSingleZero", "applyRounding", "showUnitLabel"],
+      precision: 4,
+      type: "Decimal",
+      uomSeparator: "",
+      composite: {
+        units: [
+          {
+            label: "m",
+            name: "Units.M",
+          },
+        ],
+      },
+      allowMathematicOperations: true,
+    };
+
+    const testData = [
+      ".",
+      ",",
+      ",,",
+      "..",
+      ",,,",
+      "...",
+      ".,",
+      ",.",
+      "!@#$%^&*()_", // special characters
+    ];
+
+    const unitsProvider = new TestUnitsProvider();
+    const format = new Format("test");
+    await format.fromJSON(unitsProvider, formatData).catch(() => { });
+
+    for (const testEntry of testData) {
+      const parseResult = await Parser.parseIntoQuantity(testEntry, format, unitsProvider);
+      expect(parseResult.isValid).to.eql(false);
+    }
+  });
+
 });
 
 describe("Synchronous Parsing tests:", async () => {
@@ -927,4 +966,61 @@ describe("Synchronous Parsing tests:", async () => {
     }
   });
 
-});
+  it("should return parseError when parsing only special characters", async () => {
+    const testData = [
+      ".",
+      ",",
+      ",,",
+      "..",
+      ",,,",
+      "...",
+      ".,",
+      ",.",
+      "!@#$%^&*()_",
+    ];
+
+    for (const testEntry of testData) {
+      const parseResult = Parser.parseQuantityString(testEntry, parserSpec);
+      if (Parser.isParseError(parseResult)){
+        expect(parseResult.error).to.eql(ParseError.UnableToConvertParseTokensToQuantity);
+      } else {
+        assert.fail(`Expected a ParseError with input: ${testEntry}`);
+      }
+    }
+  });
+
+  it("should return parseError when parsing a string with invalid special characters mixed into numbers", async () => {
+    const testData = [
+      "10..",
+      "1.2,,3..4...7",
+      ",,3..",
+      "12..34",
+      "1.2.3",
+      "..10",
+      "1...2",
+      "1..",
+      "1..,",
+      "10,20,30..40",
+      "1..e2",
+      "1e..2",
+      "1e2..",
+      "1...2e3",
+      "1e2..,3",
+      // "1,,2", // the parsing skips comas for loose checking, returns 12
+      // "1.,2", // returns 1.2
+    ];
+
+    const unitMeter = await unitsProvider.findUnitByName("Units.FT");
+    const parserSpecExp = await ParserSpec.create(format, unitsProvider, unitMeter, unitsProvider);
+
+    for (const testEntry of testData) {
+      const parseResult = Parser.parseQuantityString(testEntry, parserSpecExp);
+      if (Parser.isParseError(parseResult)){
+        expect(parseResult.error).to.eql(ParseError.UnableToConvertParseTokensToQuantity);
+      } else {
+        assert.fail(`Expected a ParseError with input: ${testEntry}`);
+      }
+    }
+    });
+
+});