Skip to content

Commit

Permalink
fix(formula): fix parsing of =, equality check, lookupByKey equality …
Browse files Browse the repository at this point in the history
…check + add custom unequal operator
  • Loading branch information
pjanik committed Sep 27, 2023
1 parent cb04c57 commit 6a6a94b
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 30 deletions.
64 changes: 35 additions & 29 deletions v3/src/models/data/formula-fn-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,41 +48,47 @@ const aggregateFnWithFilterFactory = (fn: (values: number[]) => number) => {
// count(attribute) will return a count of valid data values, since 0 is a valid numeric value.
export const isValueTruthy = (value: any) => value !== "" && value !== false && value !== null && value !== undefined

export const equal = (a: any, b: any) => {
if (Array.isArray(a) && Array.isArray(b)) {
return a.map((v, i) => v === b[i])
}
if (Array.isArray(a) && !Array.isArray(b)) {
return a.map((v) => v === b)
}
if (!Array.isArray(a) && Array.isArray(b)) {
return b.map((v) => v === a)
}
// Checks below might seem redundant once the data set cases start using typed values, but they are not.
// Note that user might still compare a string with a number unintentionally, and it makes sense to try to cast
// values when possible, so that the comparison can be performed without forcing users to think about types.
// Also, there's more ifs than needed, but it lets us avoid unnecessary casts.
if (typeof a === "number" && typeof b !== "number") {
return a === Number(b)
}
if (typeof a !== "number" && typeof b === "number") {
return Number(a) === b
}
if (typeof a === "boolean" && typeof b !== "boolean") {
return a === (b === "true")
}
if (typeof a !== "boolean" && typeof b === "boolean") {
return (a === "true") === b
}
return a === b
}

const UNDEF_RESULT = ""

export const fnRegistry = {
// equal(a, b) or a == b
// Note that we need to override default MathJs implementation so we can compare strings like "ABC" == "CDE".
// MathJs doesn't allow that by default, as it assumes that equal operator can be used only with numbers.
equal: {
evaluateRaw: (a: any, b: any) => {
if (Array.isArray(a) && Array.isArray(b)) {
return a.map((v, i) => v === b[i])
}
if (Array.isArray(a) && !Array.isArray(b)) {
return a.map((v) => v === b)
}
if (!Array.isArray(a) && Array.isArray(b)) {
return b.map((v) => v === a)
}
// Checks below might seem redundant once the data set cases start using typed values, but they are not.
// Note that user might still compare a string with a number unintentionally, and it makes sense to try to cast
// values when possible, so that the comparison can be performed without forcing users to think about types.
// Also, there's more ifs than needed, but it lets us avoid unnecessary casts.
if (typeof a === "number" && typeof b !== "number") {
return a === Number(b)
}
if (typeof a !== "number" && typeof b === "number") {
return Number(a) === b
}
if (typeof a === "boolean" && typeof b !== "boolean") {
return a === (b === "true")
}
if (typeof a !== "boolean" && typeof b === "boolean") {
return (a === "true") === b
}
return a === b
}
evaluate: equal
},

unequal: {
evaluate: (a: any, b: any) => !equal(a, b)
},

// lookupByIndex("dataSetName", "attributeName", index)
Expand Down Expand Up @@ -162,7 +168,7 @@ export const fnRegistry = {
}
for (const c of dataSet.cases) {
const val = dataSet.getValue(c.__id__, keyAttrId)
if (val === keyAttrValue) {
if (equal(val, keyAttrValue)) {
return dataSet.getValue(c.__id__, attrId) || UNDEF_RESULT
}
}
Expand Down
5 changes: 5 additions & 0 deletions v3/src/models/data/formula-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ describe("customizeFormula", () => {
expect(customizeFormula("a = b")).toEqual("a == b")
expect(customizeFormula("a = b = c")).toEqual("a == b == c")
})
it("doesn't replace unequality operator", () => {
expect(customizeFormula("a != 1")).toEqual("a != 1")
expect(customizeFormula("a != b")).toEqual("a != b")
expect(customizeFormula("a != b = c = d != e")).toEqual("a != b == c == d != e")
})
it("replaces all the symbols enclosed between `` with safe symbol names", () => {
expect(customizeFormula("mean(`Attribute Name`)")).toEqual("mean(Attribute_Name)")
expect(customizeFormula("`Attribute Name` + `Attribute Name 2`")).toEqual("Attribute_Name + Attribute_Name_2")
Expand Down
2 changes: 1 addition & 1 deletion v3/src/models/data/formula-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const customizeFormula = (formula: string) => {
// Over time, this function might grow significantly and require more advanced parsing of the formula.
return formula
// Replace all the assignment operators with equality operators, as CODAP v2 uses a single "=" for equality check.
.replace(/=/g, "==")
.replace(/(?<!!)=(?!=)/g, "==")
// Names between `` are symbols that require special processing, as otherwise they could not be parsed by Mathjs,
// eg. names with spaces or names that start with a number.
.replace(/`([^`]+)`/g, (_, match) => safeSymbolName(match))
Expand Down

0 comments on commit 6a6a94b

Please sign in to comment.