Skip to content

Commit

Permalink
fix(formula): fix some bug (#3774)
Browse files Browse the repository at this point in the history 
Co-authored-by: wpxp123456 <Wpxp1223456>
  • Loading branch information
@wpxp123456
wpxp123456 authored Oct 17, 2024
1 parent e1aeec0 commit 4c4efe4
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 54 deletions.
9 changes: 5 additions & 4 deletions packages/engine-formula/src/engine/utils/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,11 @@ export function deserializeRangeWithSheet(refString: string): IUnitRangeName {

const endGrid = singleReferenceToGrid(refEndString);

const startRow = startGrid.row;
const startColumn = startGrid.column;
const endRow = endGrid.row;
const endColumn = endGrid.column;
// range A1:B10 === B10:A1
const startRow = startGrid.row > endGrid.row ? endGrid.row : startGrid.row;
const startColumn = startGrid.column > endGrid.column ? endGrid.column : startGrid.column;
const endRow = startGrid.row > endGrid.row ? startGrid.row : endGrid.row;
const endColumn = startGrid.column > endGrid.column ? startGrid.column : endGrid.column;

let rangeType = RANGE_TYPE.NORMAL;
if (Number.isNaN(startRow) && Number.isNaN(endRow)) {
Expand Down
17 changes: 0 additions & 17 deletions packages/engine-formula/src/engine/value-object/array-value-object.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1528,13 +1528,6 @@ export class ArrayValueObject extends BaseValueObject {
}
result[r][column] = BooleanValueObject.create(true);
});
// for (let r = 0; r < rowCount; r++) {
// if (rowPositions.has(r + startRow)) {
// result[r][column] = BooleanValueObject.create(true);
// } else {
// result[r][column] = BooleanValueObject.create(false);
// }
// }
}
} else {
const rowValuePositions = CELL_INVERTED_INDEX_CACHE.getCellValuePositions(
Expand Down Expand Up @@ -1577,16 +1570,6 @@ export class ArrayValueObject extends BaseValueObject {
// }
// }
}

// else {
// for (let r = 0; r < rowCount; r++) {
// if (result[r] == null) {
// result[r] = [];
// }

// result[r][column] = BooleanValueObject.create(false);
// }
// }
}

return;
Expand Down
47 changes: 34 additions & 13 deletions packages/engine-formula/src/engine/value-object/primitive-object.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
*/

import { isRealNum } from '@univerjs/core';
import { FormulaAstLRU } from '../../basics/cache-lru';
import { reverseCompareOperator } from '../../basics/calculate';
import { BooleanValue, ConcatenateType } from '../../basics/common';
import { ErrorType } from '../../basics/error-type';
import { compareToken, operatorToken } from '../../basics/token';
import { compareWithWildcard, isWildcard } from '../utils/compare';
import { ceil, divide, equals, floor, greaterThan, greaterThanOrEquals, lessThan, lessThanOrEquals, minus, mod, multiply, plus, pow, round, sqrt } from '../utils/math-kit';
import { FormulaAstLRU } from '../../basics/cache-lru';
import { comparePatternPriority } from '../utils/numfmt-kit';
import { BaseValueObject, ErrorValueObject } from './base-value-object';

Expand Down Expand Up @@ -797,30 +797,51 @@ export class NumberValueObject extends BaseValueObject {
return valueObject.powInverse(this);
}

if (this.isError()) {
return this;
}

const currentValue = this.getValue();
const value = valueObject.getValue();

if (typeof value === 'string') {
let _valueObject = valueObject;

if (valueObject.isString()) {
_valueObject = valueObject.convertToNumberObjectValue();
}

if (_valueObject.isError()) {
return _valueObject;
}

const value = +_valueObject.getValue();

if (Number.isNaN(value)) {
return ErrorValueObject.create(ErrorType.VALUE);
}
if (typeof value === 'number') {
if (!Number.isFinite(currentValue) || !Number.isFinite(value)) {
return ErrorValueObject.create(ErrorType.NUM);
}

const result = pow(currentValue, value);
if (!Number.isFinite(currentValue) || !Number.isFinite(value)) {
return ErrorValueObject.create(ErrorType.NUM);
}

if (!Number.isFinite(result)) {
if (currentValue === 0) {
if (value < 0) {
return ErrorValueObject.create(ErrorType.DIV_BY_ZERO);
}

if (value === 0) {
return ErrorValueObject.create(ErrorType.NUM);
}

return NumberValueObject.create(result);
return NumberValueObject.create(0);
}
if (typeof value === 'boolean') {
return NumberValueObject.create(pow(currentValue, value ? 1 : 0));

const result = pow(currentValue, value);

if (!Number.isFinite(result)) {
return ErrorValueObject.create(ErrorType.NUM);
}

return this;
return NumberValueObject.create(result);
}

override sqrt(): BaseValueObject {
Expand Down
38 changes: 27 additions & 11 deletions packages/engine-formula/src/functions/math/power/__tests__/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@

import { describe, expect, it } from 'vitest';

import { ErrorType } from '../../../../basics/error-type';
import { ArrayValueObject, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { NumberValueObject, StringValueObject } from '../../../../engine/value-object/primitive-object';
import { getObjectValue } from '../../../__tests__/create-function-test-bed';
import { FUNCTION_NAMES_MATH } from '../../function-names';
import { Power } from '../index';
import { NumberValueObject, StringValueObject } from '../../../../engine/value-object/primitive-object';
import { ArrayValueObject, transformToValue, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { ErrorType } from '../../../../basics/error-type';
import { stripArrayValue } from '../../../__tests__/create-function-test-bed';

describe('Test power function', () => {
const testFunction = new Power(FUNCTION_NAMES_MATH.POWER);
Expand All @@ -31,13 +31,14 @@ describe('Test power function', () => {
const number = NumberValueObject.create(5);
const power = NumberValueObject.create(2);
const result = testFunction.calculate(number, power);
expect(result.getValue()).toBe(25);
expect(getObjectValue(result)).toBe(25);
});

it('Number is single string number, power is single string number', () => {
const number = new StringValueObject('5');
const power = new StringValueObject('2');
const result = testFunction.calculate(number, power);
expect(result.getValue()).toBe(25);
expect(getObjectValue(result)).toBe(25);
});

it('Number is single cell, power is array', () => {
Expand All @@ -55,7 +56,7 @@ describe('Test power function', () => {
column: 0,
});
const result = testFunction.calculate(number, power);
expect(transformToValue(result.getArrayValue())).toStrictEqual([
expect(getObjectValue(result)).toStrictEqual([
[2, ErrorType.VALUE, 2.3456698984637576, 2, 1, 1],
[1, 1.2676506002282294e+30, 5.063026375881119, ErrorType.VALUE, 0.125, ErrorType.VALUE],
]);
Expand All @@ -76,9 +77,9 @@ describe('Test power function', () => {
});
const power = NumberValueObject.create(2);
const result = testFunction.calculate(number, power);
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([
expect(getObjectValue(result)).toStrictEqual([
[1, ErrorType.VALUE, 1.5129, 1, 0, 0],
[0, 10000, 5.4756, ErrorType.VALUE, 9, ErrorType.VALUE],
[0, 10000, 5.475599999999999, ErrorType.VALUE, 9, ErrorType.VALUE],
]);
});

Expand Down Expand Up @@ -109,11 +110,26 @@ describe('Test power function', () => {
column: 0,
});
const result = testFunction.calculate(number, power);
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([
expect(getObjectValue(result)).toStrictEqual([
[1, ErrorType.VALUE, 1.23, 1, 0, 0],
[0, 10000, 5.4756, ErrorType.VALUE, 9, ErrorType.VALUE],
[0, 10000, 5.475599999999999, ErrorType.VALUE, 9, ErrorType.VALUE],
[ErrorType.NA, ErrorType.NA, ErrorType.NA, ErrorType.NA, ErrorType.NA, ErrorType.NA],
]);
});

it('Number is 0', () => {
const number = NumberValueObject.create(0);
const power = NumberValueObject.create(2);
const result = testFunction.calculate(number, power);
expect(getObjectValue(result)).toBe(0);

const power2 = NumberValueObject.create(0);
const result2 = testFunction.calculate(number, power2);
expect(getObjectValue(result2)).toBe(ErrorType.NUM);

const power3 = NumberValueObject.create(-1);
const result3 = testFunction.calculate(number, power3);
expect(getObjectValue(result3)).toBe(ErrorType.DIV_BY_ZERO);
});
});
});
37 changes: 37 additions & 0 deletions packages/engine-formula/src/services/runtime.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type { ArrayValueObject } from '../engine/value-object/array-value-object
import { createIdentifier, Disposable, isNullCell, ObjectMatrix } from '@univerjs/core';
import { isInDirtyRange } from '../basics/dirty';
import { ErrorType } from '../basics/error-type';
import { CELL_INVERTED_INDEX_CACHE } from '../basics/inverted-index-cache';
import { getRuntimeFeatureCell } from '../engine/utils/get-runtime-feature-cell';
import { objectValueToCellValue } from '../engine/utils/value-object';
import { type BaseValueObject, ErrorValueObject } from '../engine/value-object/base-value-object';
Expand Down Expand Up @@ -501,6 +502,15 @@ export class FormulaRuntimeService extends Disposable implements IFormulaRuntime
sheetData.setValue(row, column, valueObject);
clearArrayUnitData.setValue(row, column, valueObject);

// Formula calculation results are saved to cache
CELL_INVERTED_INDEX_CACHE.set(
unitId,
sheetId,
column,
firstCell.getValue(),
row
);

return;
}

Expand All @@ -523,6 +533,15 @@ export class FormulaRuntimeService extends Disposable implements IFormulaRuntime
sheetData.setValue(row, column, errorObject);
clearArrayUnitData.setValue(row, column, errorObject);

// Formula calculation results are saved to cache
CELL_INVERTED_INDEX_CACHE.set(
unitId,
sheetId,
column,
ErrorType.SPILL,
row
);

/**
* When there are values within the array formula range, the entire formula will result in an error.
* In this case, you need to clear the previous range data to prevent other formulas from referencing the old values.
Expand Down Expand Up @@ -550,6 +569,15 @@ export class FormulaRuntimeService extends Disposable implements IFormulaRuntime
} else {
const spillError = ErrorValueObject.create(ErrorType.SPILL);
objectValueRefOrArray.iterator((valueObject, rowIndex, columnIndex) => {
// Formula calculation results are saved to cache
CELL_INVERTED_INDEX_CACHE.set(
unitId,
sheetId,
column - startColumn + columnIndex,
!valueObject ? 0 : valueObject.getValue(),
row - startRow + rowIndex
);

const value = objectValueToCellValue(valueObject);
if (rowIndex === startRow && columnIndex === startColumn) {
/**
Expand All @@ -574,6 +602,15 @@ export class FormulaRuntimeService extends Disposable implements IFormulaRuntime
const valueObject = objectValueToCellValue(functionVariant as BaseValueObject);
sheetData.setValue(row, column, valueObject);

// Formula calculation results are saved to cache
CELL_INVERTED_INDEX_CACHE.set(
unitId,
sheetId,
column,
(functionVariant as BaseValueObject).getValue(),
row
);

clearArrayUnitData.setValue(row, column, valueObject);
}
}
Expand Down
11 changes: 9 additions & 2 deletions packages/sheets-formula-ui/src/controllers/formula-alert-render.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { Workbook } from '@univerjs/core';
import type { IRenderContext, IRenderModule } from '@univerjs/engine-render';
import { Disposable, Inject, isICellData, LocaleService } from '@univerjs/core';
import { ErrorType } from '@univerjs/engine-formula';
import { ErrorType, FormulaDataModel } from '@univerjs/engine-formula';
import { CellAlertManagerService, CellAlertType, HoverManagerService } from '@univerjs/sheets-ui';
import { IZenZoneService } from '@univerjs/ui';
import { extractFormulaError } from './utils/utils';
Expand Down Expand Up @@ -45,6 +45,7 @@ export class FormulaAlertRenderController extends Disposable implements IRenderM
@Inject(HoverManagerService) private readonly _hoverManagerService: HoverManagerService,
@Inject(CellAlertManagerService) private readonly _cellAlertManagerService: CellAlertManagerService,
@Inject(LocaleService) private readonly _localeService: LocaleService,
@Inject(FormulaDataModel) private readonly _formulaDataModel: FormulaDataModel,
@IZenZoneService private readonly _zenZoneService: IZenZoneService
) {
super();
Expand All @@ -66,9 +67,15 @@ export class FormulaAlertRenderController extends Disposable implements IRenderM

const cellData = worksheet.getCell(cellPos.location.row, cellPos.location.col);

const arrayFormulaCellData = this._formulaDataModel.getArrayFormulaCellData()?.
[cellPos.location.unitId]?.
[cellPos.location.subUnitId]?.
[cellPos.location.row]?.
[cellPos.location.col];

// Preventing blank object
if (isICellData(cellData)) {
const errorType = extractFormulaError(cellData);
const errorType = extractFormulaError(cellData, !!arrayFormulaCellData);

if (!errorType) {
return;
Expand Down
12 changes: 10 additions & 2 deletions packages/sheets-formula-ui/src/controllers/formula-render.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { Inject, InterceptorEffectEnum, RxDisposable } from '@univerjs/core';
import { FormulaDataModel } from '@univerjs/engine-formula';
import { INTERCEPTOR_POINT, SheetInterceptorService } from '@univerjs/sheets';
import { extractFormulaError } from './utils/utils';

Expand All @@ -27,7 +28,8 @@ const FORMULA_ERROR_MARK = {

export class FormulaRenderManagerController extends RxDisposable {
constructor(
@Inject(SheetInterceptorService) private readonly _sheetInterceptorService: SheetInterceptorService
@Inject(SheetInterceptorService) private readonly _sheetInterceptorService: SheetInterceptorService,
@Inject(FormulaDataModel) private readonly _formulaDataModel: FormulaDataModel
) {
super();

Expand All @@ -36,7 +38,13 @@ export class FormulaRenderManagerController extends RxDisposable {
{
effect: InterceptorEffectEnum.Style,
handler: (cell, pos, next) => {
const errorType = extractFormulaError(cell);
const arrayFormulaCellData = this._formulaDataModel.getArrayFormulaCellData()?.
[pos.unitId]?.
[pos.subUnitId]?.
[pos.row]?.
[pos.col];

const errorType = extractFormulaError(cell, !!arrayFormulaCellData);
if (!errorType) {
return next(cell);
}
Expand Down
8 changes: 6 additions & 2 deletions packages/sheets-formula-ui/src/controllers/utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ export function whenEditorStandalone(contextService: IContextService) {
* @param cell
* @returns
*/
export function extractFormulaError(cell: Nullable<ICellData>) {
export function extractFormulaError(cell: Nullable<ICellData>, isArrayFormulaCell: boolean = false) {
// Must contain a formula
if ((isFormulaString(cell?.f) || isFormulaId(cell?.si)) && typeof cell?.v === 'string' && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
if (!isArrayFormulaCell && !(isFormulaString(cell?.f) || isFormulaId(cell?.si))) {
return null;
}

if (typeof cell?.v === 'string' && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
return cell.v as ErrorType;
}

Expand Down
10 changes: 7 additions & 3 deletions packages/sheets-formula/src/controllers/utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/

import type { ICellData, IContextService, Nullable } from '@univerjs/core';
import { CellValueType, FOCUSING_DOC, FOCUSING_UNIVER_EDITOR, FOCUSING_UNIVER_EDITOR_STANDALONE_SINGLE_MODE, isFormulaId, isFormulaString } from '@univerjs/core';
import type { ErrorType } from '@univerjs/engine-formula';
import { CellValueType, FOCUSING_DOC, FOCUSING_UNIVER_EDITOR, FOCUSING_UNIVER_EDITOR_STANDALONE_SINGLE_MODE, isFormulaId, isFormulaString } from '@univerjs/core';
import { ERROR_TYPE_SET, stripErrorMargin } from '@univerjs/engine-formula';

export function whenEditorStandalone(contextService: IContextService) {
Expand All @@ -32,9 +32,13 @@ export function whenEditorStandalone(contextService: IContextService) {
* @param cell
* @returns
*/
export function extractFormulaError(cell: Nullable<ICellData>) {
export function extractFormulaError(cell: Nullable<ICellData>, isArrayFormulaCell: boolean = false) {
// Must contain a formula
if ((isFormulaString(cell?.f) || isFormulaId(cell?.si)) && typeof cell?.v === 'string' && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
if (!isArrayFormulaCell && !(isFormulaString(cell?.f) || isFormulaId(cell?.si))) {
return null;
}

if (typeof cell?.v === 'string' && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
return cell.v as ErrorType;
}

Expand Down

0 comments on commit 4c4efe4

Please sign in to comment.