Skip to content

Commit 31f70f9

Browse files
authored
Fix bug where the OFFSET function was ignoring the sheet reference in the provided address (#1548)
* Add unit tests that reproduce the issue * Add changelog entry * Make OFFSET read the sheet ref in the provided address * Adjust eslint rules * Fix TS type warning * Fix eslint warning
1 parent 3cc65df commit 31f70f9

File tree

5 files changed

+41
-9
lines changed

5 files changed

+41
-9
lines changed

.eslintrc.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ module.exports = {
9090
'jsdoc/empty-tags': 'warn',
9191
'jsdoc/implements-on-classes': 'warn',
9292
'jsdoc/multiline-blocks': 'warn',
93-
'jsdoc/tag-lines': 'warn',
93+
'jsdoc/tag-lines': ['warn', 'never', {
94+
startLines: 1,
95+
}],
9496
'jsdoc/no-multi-asterisks': 'warn',
9597
'jsdoc/require-param-description': 'warn',
9698
'jsdoc/require-param-name': 'warn',
@@ -104,8 +106,9 @@ module.exports = {
104106
'jsdoc/require-yields-check': 'warn',
105107
'jsdoc/valid-types': 'warn',
106108
'jsdoc/require-jsdoc': ['warn', {
109+
checkConstructors: false,
107110
require: {
108-
ArrowFunctionExpression: true,
111+
ArrowFunctionExpression: false,
109112
ClassDeclaration: true,
110113
ClassExpression: true,
111114
FunctionDeclaration: true,

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fixed an issue where the `OFFSET` function was ignoring the sheet reference in the provided address. [#1477](https://github.com/handsontable/hyperformula/issues/1477)
13+
1014
## [3.0.1] - 2025-08-11
1115

1216
### Fixed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
"test": "npm-run-all lint test:unit test:browser test:compatibility",
7979
"test:unit": "cross-env NODE_ICU_DATA=node_modules/full-icu jest",
8080
"test:watch": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch",
81-
"test:watch-tdd": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch xlookup",
81+
"test:tdd": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --watch offset",
8282
"test:coverage": "npm run test:unit -- --coverage",
8383
"test:logMemory": "cross-env NODE_ICU_DATA=node_modules/full-icu jest --runInBand --logHeapUsage",
8484
"test:unit.ci": "cross-env NODE_ICU_DATA=node_modules/full-icu node --expose-gc ./node_modules/jest/bin/jest --forceExit",

src/parser/FormulaParser.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export class FormulaParser extends EmbeddedActionsParser {
140140
return this.OR([
141141
{ALT: () => this.SUBRULE(this.booleanExpression)},
142142
{ALT: EMPTY_ALT(buildEmptyArgAst())}
143-
])
143+
]) as Ast
144144
})
145145

146146
/**
@@ -318,7 +318,7 @@ export class FormulaParser extends EmbeddedActionsParser {
318318
}
319319
},
320320
},
321-
])
321+
]) as Ast
322322
})
323323

324324
/**
@@ -371,7 +371,7 @@ export class FormulaParser extends EmbeddedActionsParser {
371371
}
372372
},
373373
},
374-
])
374+
]) as Ast
375375
})
376376

377377
/**
@@ -448,7 +448,7 @@ export class FormulaParser extends EmbeddedActionsParser {
448448
{
449449
ALT: () => this.SUBRULE(this.parenthesisExpression)
450450
}
451-
])
451+
]) as Ast
452452
})
453453

454454
constructor(lexerConfig: LexerConfig, sheetMapping: SheetMappingFn) {
@@ -556,7 +556,7 @@ export class FormulaParser extends EmbeddedActionsParser {
556556
}
557557
},
558558
},
559-
]))
559+
])) as Ast
560560
})
561561

562562
private rightUnaryOpAtomicExpression: AstRule = this.RULE('rightUnaryOpAtomicExpression', () => {
@@ -595,7 +595,7 @@ export class FormulaParser extends EmbeddedActionsParser {
595595
{
596596
ALT: () => this.SUBRULE2(this.rightUnaryOpAtomicExpression),
597597
},
598-
])
598+
]) as Ast
599599
})
600600

601601
/**
@@ -817,6 +817,7 @@ export class FormulaParser extends EmbeddedActionsParser {
817817
cellArg.reference.col + colShift,
818818
cellArg.reference.row + rowShift,
819819
cellArg.reference.type,
820+
cellArg.reference.sheet ?? undefined,
820821
)
821822

822823
let absoluteCol = topLeftCorner.col

test/unit/parser/offset-translation.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1+
import { HyperFormula } from '../../../src'
12
import {CellError, ErrorType} from '../../../src/Cell'
23
import {Config} from '../../../src/Config'
4+
import { SheetMapping } from '../../../src/DependencyGraph'
35
import {ErrorMessage} from '../../../src/error-message'
6+
import { buildTranslationPackage } from '../../../src/i18n'
7+
import { enUS } from '../../../src/i18n/languages'
48
import {AstNodeType, CellAddress, CellRangeAst, CellReferenceAst, ErrorAst} from '../../../src/parser'
59
import {adr} from '../testUtils'
610
import {buildEmptyParserWithCaching} from './common'
@@ -175,4 +179,24 @@ describe('Parser - OFFSET to reference translation', () => {
175179
expect(ast.type).toBe(AstNodeType.CELL_REFERENCE)
176180
expect(ast.reference).toEqual(CellAddress.relative(4, 13))
177181
})
182+
183+
it('parser returns address with a sheet reference', () => {
184+
const sheetMapping = new SheetMapping(buildTranslationPackage(enUS))
185+
sheetMapping.addSheet('Sheet1')
186+
sheetMapping.addSheet('Sheet2')
187+
const parser = buildEmptyParserWithCaching(new Config(), sheetMapping)
188+
189+
const ast = parser.parse('=OFFSET(Sheet1!A1, 0, 0)', adr('A1', 1)).ast as CellReferenceAst
190+
expect(ast.type).toBe(AstNodeType.CELL_REFERENCE)
191+
expect(ast.reference).toEqual(CellAddress.relative(0, 0, 0))
192+
})
193+
194+
it('function OFFSET can reference a different sheet', () => {
195+
const engine = HyperFormula.buildFromSheets({
196+
Sheet1: [['sheet1']],
197+
Sheet2: [['sheet2', '=OFFSET(Sheet1!A1, 0, 0)']],
198+
})
199+
200+
expect(engine.getCellValue(adr('B1', engine.getSheetId('Sheet2')))).toEqual('sheet1')
201+
})
178202
})

0 commit comments

Comments
 (0)