Skip to content

Commit

Permalink
[IMP] evaluation: catchable invalid ranges
Browse files Browse the repository at this point in the history
Steps to reproduce:
- make sure no sheet is named qsqdfq
- type in a cell: `=IFERROR(qsqdfq!A1, 45)`
=> the cell value is #ERROR instead of 45

closes #4088

Task: 3597039
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
LucasLefevre authored and rrahir committed Apr 23, 2024
1 parent eb9058e commit 34e6b94
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
12 changes: 12 additions & 0 deletions src/functions/module_lookup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ export const COLUMN = {
],
returns: ["NUMBER"],
compute: function (cellReference: Maybe<{ value: string }>): number {
if (isEvaluationError(cellReference?.value)) {
throw cellReference;
}
const _cellReference =
cellReference === undefined ? this.__originCellXC?.() : cellReference.value;
assert(
Expand All @@ -140,6 +143,9 @@ export const COLUMNS = {
args: [arg("range (meta)", _t("The range whose column count will be returned."))],
returns: ["NUMBER"],
compute: function (range: { value: string }): number {
if (isEvaluationError(range?.value)) {
throw range;
}
const zone = toZone(range.value);
return zone.right - zone.left + 1;
},
Expand Down Expand Up @@ -482,6 +488,9 @@ export const ROW = {
],
returns: ["NUMBER"],
compute: function (cellReference: Maybe<{ value: string }>): number {
if (isEvaluationError(cellReference?.value)) {
throw cellReference;
}
const _cellReference =
cellReference === undefined ? this.__originCellXC?.() : cellReference.value;
assert(
Expand All @@ -503,6 +512,9 @@ export const ROWS = {
args: [arg("range (meta)", _t("The range whose row count will be returned."))],
returns: ["NUMBER"],
compute: function (range: { value: string }): number {
if (isEvaluationError(range?.value)) {
throw range;
}
const zone = toZone(range.value);
return zone.bottom - zone.top + 1;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ class CompilationParametersBuilder {
* The `compute` of the formula's function must process it completely
*/
private refFn(range: Range, isMeta: boolean): FPayload {
this.assertRangeValid(range);
const rangeError = this.getRangeError(range);
if (rangeError) {
return rangeError;
}
if (isMeta) {
// Use zoneToXc of zone instead of getRangeString to avoid sending unbounded ranges
const sheetName = this.getters.getSheetName(range.sheetId);
Expand All @@ -90,7 +93,10 @@ class CompilationParametersBuilder {
* that are actually present in the grid.
*/
private range(range: Range): Matrix<FPayload> {
this.assertRangeValid(range);
const rangeError = this.getRangeError(range);
if (rangeError) {
return [[rangeError]];
}
const sheetId = range.sheetId;
const zone = range.zone;

Expand Down Expand Up @@ -124,12 +130,13 @@ class CompilationParametersBuilder {
return matrix;
}

private assertRangeValid(range: Range): void {
private getRangeError(range: Range): EvaluationError | undefined {
if (!isZoneValid(range.zone)) {
throw new InvalidReferenceError();
return new InvalidReferenceError();
}
if (range.invalidSheetName) {
throw new EvaluationError(_t("Invalid sheet name: %s", range.invalidSheetName));
return new EvaluationError(_t("Invalid sheet name: %s", range.invalidSheetName));
}
return undefined;
}
}
2 changes: 2 additions & 0 deletions tests/functions/module_logical.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ describe("IFERROR formula", () => {
expect(evaluateCell("A1", { A1: "=IFERROR(IFERROR(1/0, 1/0), A1)" })).toBe("#CYCLE");
expect(evaluateCell("A1", { A1: "=IFERROR(1, 1/0) + IFERROR(1, 1/0)" })).toBe(2);
expect(evaluateCell("A1", { A1: "=IFERROR(ADD(B1:B2,2), 1)" })).toBe(1);
expect(evaluateCell("A1", { A1: "=IFERROR(doesNotExist!B1, 1)" })).toBe(1);
expect(evaluateCell("A1", { A1: "=IFERROR(doesNotExist!B1:B2, 1)" })).toBe(1);
});

test("format is preserved from value", () => {
Expand Down

0 comments on commit 34e6b94

Please sign in to comment.