Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handling commas and dots input in parseToken of Parser #7514

Merged
merged 24 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ebf5945
KOQ get from base property + tests
naronchen Dec 9, 2024
4e447fc
property override should not get serialized
naronchen Dec 9, 2024
fad8bf4
category property impl
naronchen Dec 9, 2024
c861f0f
priority impl
naronchen Dec 9, 2024
d49068c
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 10, 2024
6b957f1
small cleanup/fixes
naronchen Dec 10, 2024
8176507
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 10, 2024
f976605
new changelog entry
naronchen Dec 10, 2024
2240376
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 11, 2024
d8c23bc
adjustment to tests
naronchen Dec 11, 2024
36610c0
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 16, 2024
055a4be
fix on returning Nan in parseTokens + tests
naronchen Dec 24, 2024
bc47f5c
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 24, 2024
cc25c0a
rush lint fix/rush change
naronchen Dec 24, 2024
f90f9e4
input with only special characters return Error
naronchen Dec 26, 2024
89757a4
Merge branch 'master' of https://github.com/iTwin/itwinjs-core into n…
naronchen Dec 26, 2024
3c10f37
change fix to handle special characters mixed with numbers
naronchen Jan 2, 2025
2187deb
refactor to handle invalid token in async/sync parsing together
naronchen Jan 2, 2025
495c4de
more parsing tests
naronchen Jan 3, 2025
cc7eb3f
multiple comas handling
naronchen Jan 9, 2025
0f88d87
Revert "multiple comas handling"
naronchen Jan 10, 2025
5ff45ea
cleanUp
naronchen Jan 10, 2025
c17890b
Merge branch 'master' into naron/issue7424
naronchen Jan 13, 2025
fc89041
Merge branch 'master' into naron/issue7424
naronchen Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-quantity",
"comment": "minor fix in parseToken",
"type": "none"
}
],
"packageName": "@itwin/core-quantity"
}
35 changes: 25 additions & 10 deletions core/quantity/src/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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){
Expand Down Expand Up @@ -634,6 +655,7 @@ export class Parser {
}
}
}

return { ok: true, value: mag };
}

Expand Down Expand Up @@ -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`);
Expand Down
98 changes: 97 additions & 1 deletion core/quantity/src/test/Parsing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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}`);
}
}
});

});
Loading