Skip to content

Commit 132496f

Browse files
committed
Refactor
1 parent eefb63a commit 132496f

File tree

3 files changed

+76
-45
lines changed

3 files changed

+76
-45
lines changed

src/interpreter/ArithmeticHelper.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class ArithmeticHelper {
4747
constructor(
4848
private readonly config: Config,
4949
private readonly dateTimeHelper: DateTimeHelper,
50-
public readonly numberLiteralsHelper: NumberLiteralHelper,
50+
private readonly numberLiteralsHelper: NumberLiteralHelper,
5151
) {
5252
this.collator = collatorFromConfig(config)
5353
this.actualEps = config.smartRounding ? config.precisionEpsilon : 0
@@ -222,6 +222,41 @@ export class ArithmeticHelper {
222222
)
223223
}
224224

225+
/**
226+
* Parses a string to a number, supporting percentages, currencies, numeric strings, and date/time formats.
227+
* Unlike coerceNonDateScalarToMaybeNumber, this also handles scientific notation with uppercase E.
228+
*/
229+
public parseStringToNumber(input: string): Maybe<ExtendedNumber> {
230+
const trimmedInput = input.trim()
231+
232+
// Try percentage
233+
const percentResult = this.coerceStringToMaybePercentNumber(trimmedInput)
234+
if (percentResult !== undefined) {
235+
return percentResult
236+
}
237+
238+
// Try currency
239+
const currencyResult = this.coerceStringToMaybeCurrencyNumber(trimmedInput)
240+
if (currencyResult !== undefined) {
241+
return currencyResult
242+
}
243+
244+
// Try plain number (normalize scientific notation E to e)
245+
const normalizedInput = trimmedInput.replace(/E/g, 'e')
246+
const numberResult = this.numberLiteralsHelper.numericStringToMaybeNumber(normalizedInput)
247+
if (numberResult !== undefined) {
248+
return numberResult
249+
}
250+
251+
// Try date/time
252+
const dateTimeResult = this.dateTimeHelper.dateStringToDateNumber(trimmedInput)
253+
if (dateTimeResult !== undefined) {
254+
return dateTimeResult
255+
}
256+
257+
return undefined
258+
}
259+
225260
public coerceNonDateScalarToMaybeNumber(arg: InternalScalarValue): Maybe<ExtendedNumber> {
226261
if (arg === EmptyValue) {
227262
return 0
@@ -250,7 +285,7 @@ export class ArithmeticHelper {
250285
}
251286
}
252287

253-
public coerceStringToMaybePercentNumber(input: string): Maybe<PercentNumber> {
288+
private coerceStringToMaybePercentNumber(input: string): Maybe<PercentNumber> {
254289
const trimmedInput = input.trim()
255290

256291
if (trimmedInput.endsWith('%')) {
@@ -264,7 +299,7 @@ export class ArithmeticHelper {
264299
return undefined
265300
}
266301

267-
public coerceStringToMaybeCurrencyNumber(input: string): Maybe<CurrencyNumber> {
302+
private coerceStringToMaybeCurrencyNumber(input: string): Maybe<CurrencyNumber> {
268303
const matchedCurrency = this.currencyMatcher(input.trim())
269304

270305
if (matchedCurrency !== undefined) {

src/interpreter/plugin/TextPlugin.ts

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Copyright (c) 2025 Handsoncode. All rights reserved.
44
*/
55

6-
import {CellError, ErrorType} from '../../Cell'
6+
import {CellError, ErrorType} from '../../Cell'
77
import {ErrorMessage} from '../../error-message'
88
import {ProcedureAst} from '../../parser'
99
import {InterpreterState} from '../InterpreterState'
@@ -379,14 +379,14 @@ export class TextPlugin extends FunctionPlugin implements FunctionPluginTypechec
379379
// Try parsing parentheses notation for negative numbers: "(123)" -> -123
380380
const parenthesesMatch = /^\(([^()]+)\)$/.exec(trimmedArg)
381381
if (parenthesesMatch) {
382-
const innerValue = this.parseStringToNumber(parenthesesMatch[1])
382+
const innerValue = this.arithmeticHelper.parseStringToNumber(parenthesesMatch[1])
383383
if (innerValue !== undefined) {
384384
return -innerValue
385385
}
386386
}
387387

388388
// Try standard parsing
389-
const parsedValue = this.parseStringToNumber(trimmedArg)
389+
const parsedValue = this.arithmeticHelper.parseStringToNumber(trimmedArg)
390390
if (parsedValue !== undefined) {
391391
return parsedValue
392392
}
@@ -398,37 +398,6 @@ export class TextPlugin extends FunctionPlugin implements FunctionPluginTypechec
398398
})
399399
}
400400

401-
/**
402-
* Parses a string to a number, supporting percentages, currencies, numeric strings, and date/time formats.
403-
*/
404-
private parseStringToNumber(input: string): ExtendedNumber | undefined {
405-
// Try percentage
406-
const percentResult = this.arithmeticHelper.coerceStringToMaybePercentNumber(input)
407-
if (percentResult !== undefined) {
408-
return percentResult
409-
}
410-
411-
// Try currency
412-
const currencyResult = this.arithmeticHelper.coerceStringToMaybeCurrencyNumber(input)
413-
if (currencyResult !== undefined) {
414-
return currencyResult
415-
}
416-
417-
// Try plain number
418-
const numberResult = this.arithmeticHelper.numberLiteralsHelper.numericStringToMaybeNumber(input.trim())
419-
if (numberResult !== undefined) {
420-
return numberResult
421-
}
422-
423-
// Try date/time
424-
const dateTimeResult = this.dateTimeHelper.dateStringToDateNumber(input)
425-
if (dateTimeResult !== undefined) {
426-
return dateTimeResult
427-
}
428-
429-
return undefined
430-
}
431-
432401
private escapeRegExpSpecialCharacters(text: string): string {
433402
return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
434403
}

test/unit/interpreter/function-value.spec.ts

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('Function VALUE', () => {
8383
it('should convert string with thousand separator', () => {
8484
const engine = HyperFormula.buildFromArray([
8585
['=VALUE("1,234")'],
86-
])
86+
], {thousandSeparator: ',', functionArgSeparator: ';'})
8787

8888
expect(engine.getCellValue(adr('A1'))).toBe(1234)
8989
})
@@ -95,6 +95,22 @@ describe('Function VALUE', () => {
9595

9696
expect(engine.getCellValue(adr('A1'))).toBe(-123)
9797
})
98+
99+
it('should return VALUE error for nested parentheses', () => {
100+
const engine = HyperFormula.buildFromArray([
101+
['=VALUE("((123))")'],
102+
])
103+
104+
expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.VALUE, ErrorMessage.NumberCoercion))
105+
})
106+
107+
it('should return VALUE error for unbalanced parentheses', () => {
108+
const engine = HyperFormula.buildFromArray([
109+
['=VALUE("(123")'],
110+
])
111+
112+
expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.VALUE, ErrorMessage.NumberCoercion))
113+
})
98114
})
99115

100116
describe('percentage strings', () => {
@@ -119,7 +135,7 @@ describe('Function VALUE', () => {
119135
it('should convert currency string with thousand separator and decimal', () => {
120136
const engine = HyperFormula.buildFromArray([
121137
['=VALUE("$1,234.56")'],
122-
])
138+
], {thousandSeparator: ',', functionArgSeparator: ';'})
123139

124140
expect(engine.getCellValue(adr('A1'))).toBe(1234.56)
125141
})
@@ -204,12 +220,15 @@ describe('Function VALUE', () => {
204220
expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.VALUE, ErrorMessage.NumberCoercion))
205221
})
206222

207-
it('should return VALUE error for 12-hour time format without proper config', () => {
223+
})
224+
225+
describe('12-hour time format', () => {
226+
it('should parse 12-hour time format with am/pm', () => {
208227
const engine = HyperFormula.buildFromArray([
209228
['=VALUE("3:00pm")'],
210229
])
211230

212-
expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.VALUE, ErrorMessage.NumberCoercion))
231+
expect(engine.getCellValue(adr('A1'))).toBeCloseTo(0.625, 6)
213232
})
214233
})
215234

@@ -238,9 +257,17 @@ describe('Function VALUE', () => {
238257
expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.DIV_BY_ZERO))
239258
})
240259

241-
it('should convert cell reference with numeric string', () => {
260+
it('should convert cell reference with string value', () => {
261+
const engine = HyperFormula.buildFromArray([
262+
["'123", '=VALUE(A1)'],
263+
])
264+
265+
expect(engine.getCellValue(adr('B1'))).toBe(123)
266+
})
267+
268+
it('should pass through cell reference with numeric value', () => {
242269
const engine = HyperFormula.buildFromArray([
243-
['123', '=VALUE(A1)'],
270+
[123, '=VALUE(A1)'],
244271
])
245272

246273
expect(engine.getCellValue(adr('B1'))).toBe(123)
@@ -251,15 +278,15 @@ describe('Function VALUE', () => {
251278
it('should respect decimal separator config', () => {
252279
const engine = HyperFormula.buildFromArray([
253280
['=VALUE("123,45")'],
254-
], {decimalSeparator: ',', thousandSeparator: ' '})
281+
], {decimalSeparator: ',', thousandSeparator: ' ', functionArgSeparator: ';'})
255282

256283
expect(engine.getCellValue(adr('A1'))).toBe(123.45)
257284
})
258285

259286
it('should respect thousand separator config', () => {
260287
const engine = HyperFormula.buildFromArray([
261288
['=VALUE("1 234,56")'],
262-
], {decimalSeparator: ',', thousandSeparator: ' '})
289+
], {decimalSeparator: ',', thousandSeparator: ' ', functionArgSeparator: ';'})
263290

264291
expect(engine.getCellValue(adr('A1'))).toBe(1234.56)
265292
})

0 commit comments

Comments
 (0)