From 7dd6dfc3c0f297d9c8b434c403380eace87d402a Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Tue, 30 Sep 2025 10:45:50 -0400 Subject: [PATCH 1/6] Return error result when unit labels aren't matched during quantity parsing --- ...nam-parser-label-fix_2025-09-30-14-41.json | 10 +++++++ core/quantity/src/Parser.ts | 10 ++++++- core/quantity/src/test/Parsing.test.ts | 28 +++++++++++++++++++ docs/changehistory/NextVersion.md | 9 ++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json diff --git a/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json b/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json new file mode 100644 index 000000000000..f0a48a8ae61f --- /dev/null +++ b/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@itwin/core-quantity", + "comment": "Return bad values when unit labels aren't matched during quantity parsing", + "type": "none" + } + ], + "packageName": "@itwin/core-quantity" +} \ No newline at end of file diff --git a/core/quantity/src/Parser.ts b/core/quantity/src/Parser.ts index 508555abe0a9..0478d5096398 100644 --- a/core/quantity/src/Parser.ts +++ b/core/quantity/src/Parser.ts @@ -556,6 +556,8 @@ export class Parser { if (unitConversion !== undefined) return unitConversion; } + // if there were unique unit labels but not matched to any units, throw an error + if (uniqueUnitLabels.length > 0) throw new QuantityError(QuantityStatus.UnitLabelSuppliedButNotMatched, `The unit label(s) ${uniqueUnitLabels.join(", ")} could not be matched to a known unit.`); } return unitConversion; } @@ -613,7 +615,13 @@ export class Parser { } const defaultUnit = format.units && format.units.length > 0 ? format.units[0][0] : undefined; - defaultUnitConversion = defaultUnitConversion ? defaultUnitConversion : Parser.getDefaultUnitConversion(tokens, unitsConversions, defaultUnit); + try { + defaultUnitConversion = defaultUnitConversion ? defaultUnitConversion : Parser.getDefaultUnitConversion(tokens, unitsConversions, defaultUnit); + } catch (e: unknown) { + // If we failed to get the default unit conversion, we need to return an error. + if (e instanceof QuantityError && e.errorNumber === QuantityStatus.UnitLabelSuppliedButNotMatched) + return { ok: false, error: ParseError.UnitLabelSuppliedButNotMatched }; + } if (format.type === FormatType.Bearing && format.units !== undefined && format.units.length > 0) { const units = format.units; diff --git a/core/quantity/src/test/Parsing.test.ts b/core/quantity/src/test/Parsing.test.ts index 95859d402691..8fef9aea39f3 100644 --- a/core/quantity/src/test/Parsing.test.ts +++ b/core/quantity/src/test/Parsing.test.ts @@ -986,6 +986,34 @@ describe("Synchronous Parsing tests:", async () => { } }); + it("parse returns a bad value with ParseError.UnitLabelSuppliedButNotMatched", async () => { + const formatDataUnitless = { + formatTraits: ["keepSingleZero", "showUnitLabel"], + precision: 8, + type: "Fractional", + uomSeparator: "", + allowMathematicOperations: true, + }; + const formatUnitless = new Format("test"); + await formatUnitless.fromJSON(unitsProvider, formatDataUnitless); + const unitlessParserSpec = await ParserSpec.create(formatUnitless, unitsProvider, outUnit, unitsProvider); + + const testData = [ + "100 INVALIDUNIT", + "50 BADLABEL", + "25.5 UNKNOWNUNIT", + "1.5 NOTFOUND", + "1metera + 123 + 1.65" + ]; + + for (const testEntry of testData) { + const parseResult = Parser.parseQuantityString(testEntry, unitlessParserSpec); + expect(Parser.isParseError(parseResult)).to.be.true; + if (Parser.isParseError(parseResult)) { + expect(parseResult.error).toEqual(ParseError.UnitLabelSuppliedButNotMatched); + } + } + }); it("Parse into length values using custom parse labels", () => { const testData = [ // if no quantity is provided then the format unit is used to determine unit diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md index 7745b914edd4..21116327ce3a 100644 --- a/docs/changehistory/NextVersion.md +++ b/docs/changehistory/NextVersion.md @@ -3,3 +3,12 @@ publish: false --- # NextVersion +- [NextVersion](#nextversion) + - [@itwin/core-quantity](#itwincore-quantity) + - [Changes](#changes) + +## @itwin/core-quantity + +### Changes + +- **Breaking Change**: Enhanced [Parser]($quantity) error handling to properly propagate [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity) when unit labels are provided but cannot be matched to known units. **This change may cause parsing operations that previously succeeded with invalid unit labels to now return error results instead**, providing more accurate feedback and preventing silent failures when parsing quantities with unrecognized unit labels. From 1346d13fe9a5a0e2244c4e37b348f57042d8441c Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Tue, 30 Sep 2025 10:45:58 -0400 Subject: [PATCH 2/6] typo in changelog --- .../core-quantity/nam-parser-label-fix_2025-09-30-14-41.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json b/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json index f0a48a8ae61f..16caad61dd1e 100644 --- a/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json +++ b/common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@itwin/core-quantity", - "comment": "Return bad values when unit labels aren't matched during quantity parsing", + "comment": "Return error result when unit labels aren't matched during quantity parsing", "type": "none" } ], From d343ff40a088119fd547d9c4bec1aa8e45a74e03 Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Tue, 30 Sep 2025 11:38:27 -0400 Subject: [PATCH 3/6] okay copilot --- core/quantity/src/Parser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/quantity/src/Parser.ts b/core/quantity/src/Parser.ts index 0478d5096398..f592f88016dc 100644 --- a/core/quantity/src/Parser.ts +++ b/core/quantity/src/Parser.ts @@ -617,7 +617,7 @@ export class Parser { const defaultUnit = format.units && format.units.length > 0 ? format.units[0][0] : undefined; try { defaultUnitConversion = defaultUnitConversion ? defaultUnitConversion : Parser.getDefaultUnitConversion(tokens, unitsConversions, defaultUnit); - } catch (e: unknown) { + } catch (e) { // If we failed to get the default unit conversion, we need to return an error. if (e instanceof QuantityError && e.errorNumber === QuantityStatus.UnitLabelSuppliedButNotMatched) return { ok: false, error: ParseError.UnitLabelSuppliedButNotMatched }; From 681908dc41e5e1e6d5bfc3f5b4a047da3e1b2c7c Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Wed, 8 Oct 2025 12:08:41 -0400 Subject: [PATCH 4/6] fix docs link --- docs/changehistory/NextVersion.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md index 21116327ce3a..5f74d2c1db33 100644 --- a/docs/changehistory/NextVersion.md +++ b/docs/changehistory/NextVersion.md @@ -11,4 +11,4 @@ publish: false ### Changes -- **Breaking Change**: Enhanced [Parser]($quantity) error handling to properly propagate [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity) when unit labels are provided but cannot be matched to known units. **This change may cause parsing operations that previously succeeded with invalid unit labels to now return error results instead**, providing more accurate feedback and preventing silent failures when parsing quantities with unrecognized unit labels. +- **Breaking Change**: Enhanced [Parser]($quantity) error handling to properly propagate `ParseError.UnitLabelSuppliedButNotMatched` when unit labels are provided but cannot be matched to known units. **This change may cause parsing operations that previously succeeded with invalid unit labels to now return error results instead**, providing more accurate feedback and preventing silent failures when parsing quantities with unrecognized unit labels. From 39718bd373b4b29486f6b44f9c57b07b681f5683 Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Mon, 13 Oct 2025 16:53:49 -0400 Subject: [PATCH 5/6] update documentation --- docs/changehistory/NextVersion.md | 2 +- docs/learning/quantity/index.md | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md index 5f74d2c1db33..608754ab7cfd 100644 --- a/docs/changehistory/NextVersion.md +++ b/docs/changehistory/NextVersion.md @@ -11,4 +11,4 @@ publish: false ### Changes -- **Breaking Change**: Enhanced [Parser]($quantity) error handling to properly propagate `ParseError.UnitLabelSuppliedButNotMatched` when unit labels are provided but cannot be matched to known units. **This change may cause parsing operations that previously succeeded with invalid unit labels to now return error results instead**, providing more accurate feedback and preventing silent failures when parsing quantities with unrecognized unit labels. +- Fixed a bug in [Parser]($quantity) where invalid unit labels were silently ignored during parsing when no format units were specified, leading to incorrect results. Previously, when using a unitless format with input like "12 im" (a typo "in" for inches), the parser would successfully parse it as "12 meters" when the persistence unit was set to meters. The parser now correctly returns [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity) when a unitless format is used and a unit label is provided but cannot be matched to any known unit. This fix ensures parsing errors are noticed and properly handled by the caller, preventing silent failures and ensuring data integrity. diff --git a/docs/learning/quantity/index.md b/docs/learning/quantity/index.md index c31baadb95ad..4ca9f1ca93cd 100644 --- a/docs/learning/quantity/index.md +++ b/docs/learning/quantity/index.md @@ -108,6 +108,27 @@ The [AlternateUnitLabelsProvider]($quantity) interface allows users to specify a Unit conversion is performed through a [UnitConversionSpec]($quantity). These objects are generated by a `UnitsProvider`, with the implementation determined by each specific provider. During initialization, a `ParserSpec` or `FormatterSpec` can ask for `UnitConversionSpec` objects provided via the `UnitsProvider`. During parsing and formatting, the specification will retrieve the `UnitConversionSpec` between the source and destination units to apply the unit conversion. +#### Parser Behavior + +The [Parser]($quantity) converts text strings into numeric quantity values by tokenizing the input and matching unit labels to known units. The parsing process follows these steps: + +1. __Tokenization__: The input string is broken down into tokens representing numbers, unit labels, and mathematical operators (if enabled). + +2. __Unit Label Matching__: For each unit label token found, the parser attempts to match it against: + - Units explicitly defined in the format specification + - Units from the same phenomenon (unit family) provided by the `UnitsProvider` + - Alternate labels defined through the `AlternateUnitLabelsProvider` + +3. __Error Handling__: The parser's behavior when encountering unrecognized unit labels depends on the format configuration: + - __Unitless Format__ (no units defined in format): If a unit label is provided but cannot be matched to any known unit, the parser returns [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity). This prevents silent failures where typos like "12 im" (instead of "12 in") would incorrectly parse as "12 meters" when the persistence unit is meters. + - __Format with Units__ (units explicitly defined): If an unrecognized unit label is provided (e.g., "12 ABCDEF"), the parser falls back to the format's default unit for backward compatibility. For example, with a feet format, "12 ABCDEF" would parse as "12 feet". + +4. __Default Unit Behavior__: If no unit label is provided in the input (e.g., just "12"), the parser uses the default unit specified in the format. For unitless formats, if the input contains multiple unit labels, the first successfully matched unit becomes the default for subsequent unitless values in the same expression. + +5. __Unit Conversion__: Once units are matched, the parser applies the appropriate unit conversions to produce a value in the persistence unit specified by the `ParserSpec`. + +This error handling ensures that parsing errors are caught in unitless format contexts, preventing data corruption from unrecognized or mistyped unit labels, while maintaining backward compatibility for formats with explicitly defined units. + ## Persistence We expose APIs and interfaces to support persistence of formats. Different from [KindOfQuantity](../../bis/ec/kindofquantity.md), which enables persistence of formats at the schema level, this section covers persistence at the application level. From 1bb25de249fb4a86cc71fa164a5b2d9ab587beb6 Mon Sep 17 00:00:00 2001 From: Nam Le <50554904+hl662@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:55:21 -0400 Subject: [PATCH 6/6] fix invalid links --- docs/changehistory/NextVersion.md | 2 +- docs/learning/quantity/index.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md index 608754ab7cfd..ef341277ae5a 100644 --- a/docs/changehistory/NextVersion.md +++ b/docs/changehistory/NextVersion.md @@ -11,4 +11,4 @@ publish: false ### Changes -- Fixed a bug in [Parser]($quantity) where invalid unit labels were silently ignored during parsing when no format units were specified, leading to incorrect results. Previously, when using a unitless format with input like "12 im" (a typo "in" for inches), the parser would successfully parse it as "12 meters" when the persistence unit was set to meters. The parser now correctly returns [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity) when a unitless format is used and a unit label is provided but cannot be matched to any known unit. This fix ensures parsing errors are noticed and properly handled by the caller, preventing silent failures and ensuring data integrity. +- Fixed a bug in [Parser]($quantity) where invalid unit labels were silently ignored during parsing when no format units were specified, leading to incorrect results. Previously, when using a unitless format with input like "12 im" (a typo "in" for inches), the parser would successfully parse it as "12 meters" when the persistence unit was set to meters. The parser now correctly returns `ParseError.UnitLabelSuppliedButNotMatched` when a unitless format is used and a unit label is provided but cannot be matched to any known unit. This fix ensures parsing errors are noticed and properly handled by the caller, preventing silent failures and ensuring data integrity. diff --git a/docs/learning/quantity/index.md b/docs/learning/quantity/index.md index 4ca9f1ca93cd..0bb46eaa87db 100644 --- a/docs/learning/quantity/index.md +++ b/docs/learning/quantity/index.md @@ -12,6 +12,7 @@ - [Formats Provider](#formats-provider) - [Units Provider](#units-provider) - [Unit Conversion](#unit-conversion) + - [Parser Behavior](#parser-behavior) - [Persistence](#persistence) - [FormatSet](#formatset) - [Using KindOfQuantities to Retrieve Formats](#using-kindofquantities-to-retrieve-formats) @@ -120,7 +121,7 @@ The [Parser]($quantity) converts text strings into numeric quantity values by to - Alternate labels defined through the `AlternateUnitLabelsProvider` 3. __Error Handling__: The parser's behavior when encountering unrecognized unit labels depends on the format configuration: - - __Unitless Format__ (no units defined in format): If a unit label is provided but cannot be matched to any known unit, the parser returns [`ParseError.UnitLabelSuppliedButNotMatched`]($quantity). This prevents silent failures where typos like "12 im" (instead of "12 in") would incorrectly parse as "12 meters" when the persistence unit is meters. + - __Unitless Format__ (no units defined in format): If a unit label is provided but cannot be matched to any known unit, the parser returns `ParseError.UnitLabelSuppliedButNotMatched`. This prevents silent failures where typos like "12 im" (instead of "12 in") would incorrectly parse as "12 meters" when the persistence unit is meters. - __Format with Units__ (units explicitly defined): If an unrecognized unit label is provided (e.g., "12 ABCDEF"), the parser falls back to the format's default unit for backward compatibility. For example, with a feet format, "12 ABCDEF" would parse as "12 feet". 4. __Default Unit Behavior__: If no unit label is provided in the input (e.g., just "12"), the parser uses the default unit specified in the format. For unitless formats, if the input contains multiple unit labels, the first successfully matched unit becomes the default for subsequent unitless values in the same expression.