Skip to content

Fix: parser rejects formulas with Excel's internal prefixes (#1655) - #1771

Open
marcin-kordas-hoc wants to merge 11 commits into
developfrom
fix/hf-217-excel-internal-prefixes
Open

marcin-kordas-hoc wants to merge 11 commits into
developfrom
fix/hf-217-excel-internal-prefixes

Conversation

@marcin-kordas-hoc

@marcin-kordas-hoc marcin-kordas-hoc commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

Context

Excel writes an internal prefix in front of some function names when it saves an .xlsx file, for example _xlfn.IFS(...) for every function added after Excel 2007. The parser requires a function name to start with a letter, so it rejected these formulas with a parsing error. Whether a user meets this depends on the import library, because some libraries pass the prefixes through. Registering the prefixed name as a custom function does not help, because the lexer rejects the leading underscore before names are resolved.

The fix:

  • The ProcedureName token accepts an optional prefix: _xlfn., _xlfn._xlws., _xlws., _xlpm. or _xludf.. canonicalProcedureNameFromToken drops it, so _xlfn.IFS(...) parses to the same function call as IFS(...). Prefixes are matched in lower case, as Excel writes them.
  • OFFSET has its own token. It accepts a prefix only when an opening parenthesis follows, so a named expression such as _xlfn.OFFSET_RATE keeps working. Its pattern is case-insensitive for the translated name, so it also accepts an upper-case prefix.
  • ParserWithCaching computes the cache hash through the same helpers, so a prefixed formula and its unprefixed form share one cache entry, OFFSET included.
  • getCellFormula() and getCellSerialized() return the formula without the prefix.

The prefix is removed in the lexer, not by rewriting the formula string. A string replace, as sketched in the issue, would also change text inside string literals: ="_xlfn.IFS is a literal" would become ="IFS is a literal".

docs/guide/file-import.md documents the prefixes, and hyperformula/dev-docs/PARSER.md tells contributors to read function names through the new helpers.

How did you test your changes?

  • Paired branch fix/hf-217-excel-internal-prefixes in hyperformula-tests (hyperformula-tests#55), unit/parser/excel-internal-function-prefixes.spec.ts: 31 tests pass. With hyperformula/src/parser/ reverted to develop, 23 of them fail.
  • The tests cover each prefix on a call, several kinds of functions, nested and repeated prefixes, the stored formula and the cache entry, string literals and named expressions that must stay untouched, and the error for an unknown function.
  • A script run against this branch gives "Fail" for =_xlfn.IFS(A1>B1,"Pass",A1<=B1,"Fail") with A1 = 3 and B1 = 5, 3 for =_xlfn._xlws.SORT(A1:B1), and 42 for a named expression _xlfn.OFFSET_RATE set to 42.

Compatibility with Excel and Google Sheets

Not applicable: the formulas behave as their unprefixed form, which this PR does not change. A prefix does not add a function: =_xlfn.CONCAT("a","b") returns #NAME?, because HyperFormula has no CONCAT.

Public API changes

None. Formulas that used to fail with a parsing error now parse.

Out of scope and follow-ups

  • A bare _xlpm.x (a LAMBDA or LET parameter, not a call) is not stripped. HyperFormula supports neither function, with or without the prefixes.

Types of changes

  • Breaking change (existing functionality no longer works as before) – agreed with the maintainers
  • New feature or improvement (a non-breaking change that adds functionality)
  • Bug fix (a non-breaking change that fixes an issue)
  • Additional language file, or a change to an existing language file (translations)
  • Change to the documentation

Related issues

  1. Fixes [Bug]: Parser fails on formulas with _xlfn.xxx prefix added by Excel for backwards compatibility #1655
  2. HF-217

Checklist

🤖 Generated with Claude Code


Note

Medium Risk
Touches core lexer/parser tokenization and parse-cache hashing; behavior change is localized to prefixed function calls but mistakes could affect formula parsing broadly.

Overview
Fixes formulas imported from .xlsx that Excel serializes with internal function-name prefixes (e.g. _xlfn.IFS, _xlws.SORT), which previously failed parsing because names had to start with a letter.

The lexer now accepts optional _xlfn., _xlfn._xlws., _xlws., _xlpm., and _xludf. prefixes on ProcedureName and (with lookahead for () on OFFSET. canonicalProcedureNameFromToken / canonicalOffsetProcedureNameFromToken strip prefixes before name mapping, and ParserWithCaching hashes the canonical names so prefixed and unprefixed formulas share cache entries. Stored/read-back formulas omit the prefix via the AST, without rewriting the raw string (so literals like "_xlfn.IFS" stay intact).

Docs add an XLSX import section on these prefixes; CHANGELOG and PARSER.md note the behavior for contributors.

Reviewed by Cursor Bugbot for commit cc890aa. Bugbot is set up for automated code reviews on this repo. Configure here.

Excel marks every function added after the original OOXML specification with an
internal prefix when it serializes a workbook, so an imported file can contain
`_xlfn.IFS(...)` where the user typed `IFS(...)`. The ProcedureName token required
the name to start with a letter, so none of the prefixes matched and the formula
failed with "Parsing error. Redundant input, expecting EOF but found: (".

The prefixes are now an optional part of the ProcedureName and OffsetProcedureName
patterns, and are dropped when the token is turned into a function name. Doing this
in the lexer rather than by rewriting the formula string keeps the prefixes intact
wherever they are not a function name, such as inside a string literal.

FormulaParser and ParserWithCaching derive the name through one shared helper, so a
prefixed formula and its unprefixed equivalent also produce the same cache hash.

Closes #1655

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
hyperformula-docs cc890aa Commit Preview URL

Branch Preview URL
Oct 23 2026, 08:25 AM

@github-actions

github-actions Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Performance comparison of head (cc890aa) vs base (d143456)

                                     testName |    base |    head | change
--------------------------------------------------------------------------
                                      Sheet A |  478.47 |   484.1 | +1.18%
                                      Sheet B |  151.36 |  154.08 | +1.80%
                                      Sheet T |   134.7 |  137.92 | +2.39%
                                Column ranges |  459.06 |  459.97 | +0.20%
                                Sorted lookup | 13342.4 | 14382.4 | +7.79%
Sheet A:  change value, add/remove row/column |   14.18 |   14.96 | +5.50%
 Sheet B: change value, add/remove row/column |  123.34 |  118.48 | -3.94%
                   Column ranges - add column |  139.76 |  144.96 | +3.72%
                Column ranges - without batch |  422.96 |  449.14 | +6.19%
                        Column ranges - batch |  109.52 |  111.42 | +1.73%

@marcin-kordas-hoc
marcin-kordas-hoc marked this pull request as ready for review September 12, 2026 06:42

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a697a2a. Configure here.

Comment thread CHANGELOG.md Outdated
marcin-kordas-hoc and others added 3 commits September 12, 2026 14:23
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
computeHashFromTokens only special-cased ProcedureName tokens; OffsetProcedureName
is a separate, uncategorized token type (OFFSET has its own grammar rule), so it
fell through to the generic branch that hashes the raw token image — meaning
_xlfn.OFFSET(...) and OFFSET(...) did NOT share a cache entry, contradicting this
fix's own stated design goal. Verified empirically before and after (hashes differ
pre-fix, match post-fix) using the parser's own tokenizeFormula(), not a separately
built lexer — a second buildLexerConfig() call creates a distinct OffsetProcedureName
token-type object, so testing this via an external lexer silently never exercises
the real bug (fixed the same latent issue in the existing 'shares one cache entry'
spec test).

Also adds a cheap leading-underscore guard to canonicalProcedureNameFromToken/
canonicalOffsetProcedureNameFromToken before invoking the prefix-stripping regex,
since every prefix starts with '_' and the overwhelming majority of tokens don't
have one. Verified as a real, correctness-preserving improvement for the common
case with a standalone microbenchmark; a reorder alternative suggested by review
was tried and measured to NOT help (and slightly hurt the prefixed case), so it
was not adopted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- parser-consts.ts / LexerConfig.ts: the 'prefixes are matched in lower case only'
  claim is true for ProcedureName but not for OffsetProcedureName, which reuses the
  prefix pattern inside an already case-insensitive regex (the 'i' flag exists for
  the translated OFFSET name, pre-dating this fix). Replaced 'harmless' with the
  actual asymmetry and why it's not worth a dedicated case-sensitive-prefix matcher:
  OFFSET predates the OOXML cutoff this prefix scheme exists for, so Excel can never
  emit a prefixed OFFSET call in any case — there is no reachable real input, only
  hand-typed formulas.
- docs/guide/file-import.md: corrected the claim that HyperFormula 'ignores' the
  _xlpm. prefix. It only strips a prefix in front of a function call; _xlpm. never
  appears there in a real .xlsx — it always prefixes a bare LAMBDA/LET parameter
  name, which this fix deliberately does not touch (HF has no LAMBDA/LET, so such a
  formula is #NAME? either way).
- LexerConfig.ts: added {type} JSDoc annotations to match the majority convention
  already used elsewhere in src/parser/.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@GreenFlux GreenFlux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the quick response!

@Tobiadefami Tobiadefami left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we restrict the prefixed OFFSET token to function calls?

Comment thread src/parser/LexerConfig.ts Outdated
// cutoff this whole prefix scheme exists for, so Excel can never actually emit a prefixed OFFSET call
// in any case — the asymmetry has no reachable real input, only hand-typed formulas (see the
// "an upper-cased prefix is accepted on OFFSET, unlike everywhere else" test).
const OffsetProcedureName = createToken({ name: 'OffsetProcedureName', pattern: new RegExp(`(?:${EXCEL_INTERNAL_FUNCTION_PREFIX_PATTERN})?${offsetProcedureNameLiteral}`, 'i') })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently breaks an existing named-expression reference:

const engine = HyperFormula.buildFromArray([[null]], {
  licenseKey: 'gpl-v3',
})
const address = {sheet: 0, row: 0, col: 0}

engine.addNamedExpression('_xlfn.OFFSET_RATE', 42)
engine.setCellContents(address, [['=_xlfn.OFFSET_RATE']])

engine.getCellValue(address) // Expected: 42

The regression test passes on base c920375c, but fails on head a2185233 with:

Expected: 42
Received: #ERROR!
Parsing error: expected LParen, found '_RATE'

The new pattern recognizes _xlfn.OFFSET without requiring (, splitting the complete name into an OFFSET token and _RATE. Could we preserve the named-expression reference when there is no function call?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eaaab7c. The prefixed OFFSET pattern now looks ahead for an opening parenthesis (whitespace allowed, as for the unprefixed form), so _xlfn.OFFSET_RATE lexes as one named expression again and your repro returns 42. A bare =_xlfn.OFFSET now yields #NAME? instead of a parsing error. The unprefixed pattern is unchanged. Regression tests are in hyperformula-tests#55 (fecaded); I confirmed they fail on the previous head and pass now.

Comment thread docs/guide/file-import.md Outdated

| Prefix | Example |
| --- | --- |
| `_xlfn.` | `=_xlfn.IFS(A1>B1,"Pass","Fail")` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we correct the IFS example here and in the getCellFormula() explanation on line 43?

The current formula has three arguments:

=_xlfn.IFS(A1>B1,"Pass","Fail")

IFS expects condition/result pairs, so this returns #N/A with “Wrong number of arguments.” Could we use:

=_xlfn.IFS(A1>B1,"Pass",A1<=B1,"Fail")

The corresponding getCellFormula() output would be:

=IFS(A1>B1,"Pass",A1<=B1,"Fail")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in eaaab7c: both the table row and the getCellFormula() explanation now use =_xlfn.IFS(A1>B1,"Pass",A1<=B1,"Fail") / =IFS(A1>B1,"Pass",A1<=B1,"Fail").

marcin-kordas-hoc and others added 4 commits September 23, 2026 11:10
The prefixed OffsetProcedureName pattern matched `_xlfn.OFFSET` without a
following parenthesis, so it split a named expression such as
`_xlfn.OFFSET_RATE` into an OFFSET token and `_RATE`, and turned a bare
`=_xlfn.OFFSET` into a parsing error instead of #NAME?. The prefixed form now
looks ahead for an opening parenthesis, allowing whitespace before it as the
grammar does for unprefixed OFFSET. The unprefixed pattern is unchanged.

Also correct the IFS example in the file-import guide: it had three
arguments and evaluated to #N/A.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
marcin-kordas-hoc and others added 3 commits October 23, 2026 00:32
…xes in the parser guide

The changelog rules ask for the public issue where one exists. The parser guide
said OFFSET is hashed exactly as typed; a prefix is now dropped first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t import libraries

A LAMBDA called in place is a parsing error, not #NAME?. Which libraries keep
the prefixes was not measured, so the guide no longer names them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.28%. Comparing base (d143456) to head (cc890aa).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1771   +/-   ##
========================================
  Coverage    97.28%   97.28%           
========================================
  Files          207      207           
  Lines        16218    16230   +12     
  Branches      3556     3589   +33     
========================================
+ Hits         15777    15789   +12     
  Misses         433      433           
  Partials         8        8           
Files with missing lines Coverage Δ
hyperformula/src/parser/FormulaParser.ts 97.37% <100.00%> (-0.01%) ⬇️
hyperformula/src/parser/LexerConfig.ts 100.00% <100.00%> (ø)
hyperformula/src/parser/ParserWithCaching.ts 95.08% <100.00%> (+0.02%) ⬆️
hyperformula/src/parser/parser-consts.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants