Skip to content

fix: range 'a':'b'!A1 is mistaken for sheet range a:b!A1 #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gthb wants to merge 6 commits into from
Closed

fix: range 'a':'b'!A1 is mistaken for sheet range a:b!A1 #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f154a05
fix: a quoted scope in an operand position does not parse
gthb Sep 3, 2026
f022b88
Pin that the corrected type parses
gthb Sep 3, 2026
a7e5d7f
Say that the parse test pins fx's reading, not Excel's
gthb Sep 3, 2026
92fc49d
Excel reads these forms as the range operator as well
gthb Sep 3, 2026
265280e
Trim the retyping comment to the rule it implements
gthb Sep 3, 2026
cbb6d53
fix: a sheet range whose both ends are quoted is a range operation
gthb Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions lib/addTokenMeta.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, test, expect } from 'vitest';
import { FX_PREFIX, OPERATOR, NUMBER, REF_RANGE, REF_BEAM, FUNCTION, WHITESPACE, REF_STRUCT } from './constants.ts';
import { FX_PREFIX, OPERATOR, NUMBER, REF_RANGE, REF_BEAM, FUNCTION, WHITESPACE, REF_STRUCT, REF_NAMED } from './constants.ts';
import { addTokenMeta } from './addTokenMeta.ts';
import { tokenize } from './tokenize.ts';

Expand Down Expand Up @@ -85,15 +85,21 @@ describe('add extra meta to operators', () => {
{ index: 2, depth: 0, type: OPERATOR, value: ',' },
{ index: 3, depth: 0, type: REF_RANGE, value: "'jan:dec'!B11", groupId: 'fxg1' },
{ index: 4, depth: 0, type: OPERATOR, value: ',' },
{ index: 5, depth: 0, type: REF_RANGE, value: "'jan':'dec'!B11", groupId: 'fxg1' },
{ index: 6, depth: 0, type: OPERATOR, value: ',' },
{ index: 7, depth: 0, type: REF_RANGE, value: "'jan':dec!B11", groupId: 'fxg1' },
// `'jan':'dec'!B11` is not a sheet range: the quote on the second name makes the colon a
// range operator, so the reference in it is `'dec'!B11` alone. References share a group
// when they resolve to the same sheet and cells, wherever they sit in the formula, so it
// shares `Dec!B11`'s group and not the sheet ranges'.
{ index: 5, depth: 0, type: REF_NAMED, value: "'jan'" },
{ index: 6, depth: 0, type: OPERATOR, value: ':' },
{ index: 7, depth: 0, type: REF_RANGE, value: "'dec'!B11", groupId: 'fxg2' },
{ index: 8, depth: 0, type: OPERATOR, value: ',' },
{ index: 9, depth: 0, type: REF_RANGE, value: 'JAN:dEc!B11', groupId: 'fxg1' },
{ index: 9, depth: 0, type: REF_RANGE, value: "'jan':dec!B11", groupId: 'fxg1' },
{ index: 10, depth: 0, type: OPERATOR, value: ',' },
{ index: 11, depth: 0, type: REF_RANGE, value: 'Jan!B11', groupId: 'fxg2' },
{ index: 11, depth: 0, type: REF_RANGE, value: 'JAN:dEc!B11', groupId: 'fxg1' },
{ index: 12, depth: 0, type: OPERATOR, value: ',' },
{ index: 13, depth: 0, type: REF_RANGE, value: 'Dec!B11', groupId: 'fxg3' }
{ index: 13, depth: 0, type: REF_RANGE, value: 'Jan!B11', groupId: 'fxg3' },
{ index: 14, depth: 0, type: OPERATOR, value: ',' },
{ index: 15, depth: 0, type: REF_RANGE, value: 'Dec!B11', groupId: 'fxg2' }
], { sheetName: 'Sheet1', workbookName: 'foo' });
});

Expand Down
4 changes: 2 additions & 2 deletions lib/fixRanges.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ describe('fixRanges prefixes', () => {

test('a quote around the second end is the range operator, not a 3D reference', () => {
isFixed("=foo:'bar baz'!A1", "=foo:'bar baz'!A1");
// Excel fixes this to `=foo:'bar baz'!A1`
isFixed("='foo':'bar baz'!A1", "='foo:bar baz'!A1");
// Excel leaves this one as written on the next save; its formula bar drops the second quote
isFixed("='foo':'bar baz'!A1", "='foo':'bar baz'!A1");
isFixed("=foo:'bar baz'!A1", "=foo:'bar baz'!A1", { xlsx: true });
isFixed("=Jan:'[1]Nope'!A1", '=Jan:[1]Nope!A1');
});
Expand Down
23 changes: 13 additions & 10 deletions lib/mergeRefTokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,10 @@ const validRunsMerge = [
[ [ CONTEXT, CONTEXT_QUOTE ], '!', REF_BEAM ],
[ [ CONTEXT, CONTEXT_QUOTE ], '!', REF_TERNARY ],

// 'Sheet1':Sheet2!A1 | 'Sheet1':'Sheet2'!A1
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_CELL, ':', REF_CELL ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_CELL, '.:', REF_CELL ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_CELL, ':.', REF_CELL ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_CELL, '.:.', REF_CELL ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_CELL ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_RANGE ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_BEAM ],
[ CONTEXT_QUOTE, ':', [ CONTEXT, CONTEXT_QUOTE ], '!', REF_TERNARY ],

// Sheet1:Sheet2!A1 | 'Sheet1':Sheet2!A1
//
// The second name must be unquoted. A quote on it makes the colon a range operator rather than a
// sheet-range marker, so `a:'b'!A1` and `'a':'b'!A1` are two references and are not merged.
[ [ REF_NAMED, CONTEXT, CONTEXT_QUOTE ], ':', CONTEXT, '!', REF_CELL, ':', REF_CELL ],
[ [ REF_NAMED, CONTEXT, CONTEXT_QUOTE ], ':', CONTEXT, '!', REF_CELL, '.:', REF_CELL ],
[ [ REF_NAMED, CONTEXT, CONTEXT_QUOTE ], ':', CONTEXT, '!', REF_CELL, ':.', REF_CELL ],
Expand Down Expand Up @@ -193,6 +186,11 @@ const matcher = (
return best;
};

function isBangAfter (tokenlist: Token[], i: number): boolean {
const next = tokenlist[i + 1];
return !!next && next.type === OPERATOR && next.value === '!';
}

function commonMergeRefTokens (tokenlist: Token[], xlsx: boolean): Token[] {
const finalTokens = [];
// this seeks backwards because it's really the range part
Expand All @@ -217,6 +215,11 @@ function commonMergeRefTokens (tokenlist: Token[], xlsx: boolean): Token[] {
i -= valid - 1;
}
}
// A quoted scope with no `!` after it is a name, not a scope. One that has its `!` stays a
// scope, merged or not.
if (token.type === CONTEXT_QUOTE && !isBangAfter(tokenlist, i)) {
token = { ...token, type: REF_NAMED };
}
finalTokens[finalTokens.length] = token;
}
return finalTokens.reverse();
Expand Down
22 changes: 22 additions & 0 deletions lib/parse.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,28 @@ describe('parser', () => {
isParsed("'A1:B2'!C3", { type: 'ReferenceIdentifier', value: "'A1:B2'!C3", kind: 'range' });
});

// Excel reads both as the range operator over the quoted name, and stores them with the
// second name quoted: `'Alpha:Beta':'Gamma'!A1` and `Alpha:'[1]Gamma'!A1`. In the second it
// is the workbook bracket that rules out a sheet range, not the quote, which Excel drops.
test('a quoted scope with no bang after it is a name operand', () => {
isParsed("'Alpha':[Book.xlsx]Gamma!A1", {

@borgar borgar Sep 14, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I disagree that this should parse, it is not a valid expression. 'Alpha' is not a valid name so it should not be turned into a name ref node in the AST.

type: 'BinaryExpression',
operator: ':',
arguments: [
{ type: 'ReferenceIdentifier', value: "'Alpha'", kind: 'name' },
{ type: 'ReferenceIdentifier', value: '[Book.xlsx]Gamma!A1', kind: 'range' }
]
});
isParsed("'Alpha:Beta':Gamma!A1", {
type: 'BinaryExpression',
operator: ':',
arguments: [
{ type: 'ReferenceIdentifier', value: "'Alpha:Beta'", kind: 'name' },
{ type: 'ReferenceIdentifier', value: 'Gamma!A1', kind: 'range' }
]
});
});

test('"$" is not allowed on an unquoted sheet name', () => {
isInvalidExpr('=SUM($Jan:$Mar!A1)');
isParsed("'$Jan:$Mar'!A1", { type: 'ReferenceIdentifier', value: "'$Jan:$Mar'!A1", kind: 'range' });
Expand Down
8 changes: 5 additions & 3 deletions lib/parseA1Ref.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,6 @@ describe('parse A1 references', () => {
isA1Equal("'[Book.xlsx]foo':bar!A1", { context: [ 'Book.xlsx', 'foo:bar' ], range });
isA1Equal("'foo':bar!A1", { sheetName: 'foo:bar', range }, { xlsx: true });
isA1Equal("'[1]foo':bar!A1", { workbookName: '1', sheetName: 'foo:bar', range }, { xlsx: true });
isA1Equal("'foo':'bar'!A1", { sheetName: 'foo:bar', range }, { xlsx: true });
isA1Equal("'[1]foo':'bar'!A1", { workbookName: '1', sheetName: 'foo:bar', range }, { xlsx: true });
isA1Equal("'foo bar':'baz'!A1", { sheetName: 'foo bar:baz', range }, { xlsx: true });
});

test('a quote around the second end is the range operator, not a sheet range', () => {
Expand All @@ -147,6 +144,11 @@ describe('parse A1 references', () => {
isA1Equal("foo:'bar baz'!A1", undefined, { xlsx: true });
isA1Equal("1:'Dec'!A1", undefined, { xlsx: true });
isA1Equal("5:'a b'!A1", undefined, { xlsx: true });
isA1Equal("'foo':'bar'!A1", undefined);
isA1Equal("'foo bar':'baz'!A1", undefined);
isA1Equal("'foo':'bar'!A1", undefined, { xlsx: true });
isA1Equal("'[1]foo':'bar'!A1", undefined, { xlsx: true });
isA1Equal("'foo bar':'baz'!A1", undefined, { xlsx: true });
});

test('a workbook specifier is not allowed in the second section of a sheet-range', () => {
Expand Down
13 changes: 6 additions & 7 deletions lib/parseRef.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,13 @@ const pContextNames: RefParserPart = (t, data, xlsx) => {
}
}
};
const pExtendedContext: RefParserPart = (t, data, xlsx, r1c1, tokens) => {
const type = t?.type;
// We don't allow quoted sheet ranges if the prev context was unquoted:
// ✅ a:b ✅ 'a':'b' ✅ 'a':b ⛔️ a:'b'
if (type === CONTEXT || (type === CONTEXT_QUOTE && tokens[0].type === CONTEXT_QUOTE)) {
const pExtendedContext: RefParserPart = (t, data, xlsx) => {
// The second name must be unquoted. A quote on it makes the colon a range operator rather than
// a sheet-range marker, whatever stands to its left:
// ✅ a:b ✅ 'a':b ⛔️ a:'b' ⛔️ 'a':'b'
if (t?.type === CONTEXT) {
const d: Partial<RefParseDataCtx & RefParseDataXls> = {};
const value = type === CONTEXT_QUOTE ? unquotePrefix(t.value) : t.value;
splitContext(value, d, xlsx);
splitContext(t.value, d, xlsx);
if (xlsx && d.sheetName && !d.workbookName) {
data.sheetName += ':' + d.sheetName;
return 1;
Expand Down
26 changes: 16 additions & 10 deletions lib/tokenize-3d.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,20 @@ describe('lexer: 3d ranges', () => {
{ type: REF_RANGE, value: "'Gamma'!A1" }
]);
});
test('Both sections quoted independently', () => {
test('a quote on the second section leaves two references', () => {
expect(tokenize("'Alpha':'Gamma'!A1", { mergeRefs: false })).toEqual([
{ type: CONTEXT_QUOTE, value: "'Alpha'" },
{ type: OPERATOR, value: ':' },
{ type: CONTEXT_QUOTE, value: "'Gamma'" },
{ type: OPERATOR, value: '!' },
{ type: REF_RANGE, value: 'A1' }
]);
// The quote on the second name makes the colon a range operator, so these are two
// references and are not merged.
expect(tokenize("'Alpha':'Gamma'!A1")).toEqual([
{ type: REF_RANGE, value: "'Alpha':'Gamma'!A1" }
{ type: REF_NAMED, value: "'Alpha'" },
{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: "'Gamma'!A1" }
]);
});
test('Both sections quoted together', () => {
Expand All @@ -132,7 +136,7 @@ describe('lexer: 3d ranges', () => {
{ type: REF_RANGE, value: 'A1' }
]);
expect(tokenize("'Alpha:Beta':Gamma!A1")).toEqual([
{ type: CONTEXT_QUOTE, value: "'Alpha:Beta'" },
{ type: REF_NAMED, value: "'Alpha:Beta'" },
{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: 'Gamma!A1' }
]);
Expand Down Expand Up @@ -200,7 +204,7 @@ describe('lexer: 3d ranges', () => {
expect(tokenize("Alpha:'Beta:Gamma':Delta!A1")).toEqual([
{ type: REF_NAMED, value: 'Alpha' },
{ type: OPERATOR, value: ':' },
{ type: CONTEXT_QUOTE, value: "'Beta:Gamma'" },
{ type: REF_NAMED, value: "'Beta:Gamma'" },
{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: 'Delta!A1' }
]);
Expand Down Expand Up @@ -262,9 +266,7 @@ describe('lexer: 3d ranges', () => {
]);
});

test('both sections quoted, but independently', () => {
// XXX: ensure fixranges deals with this
// Excel will correct this to `'[Book.xlsx]Alpha:Gamma'!A1`
test('a quote on the second section leaves two references, with a workbook', () => {
expect(tokenize("'[Book.xlsx]Alpha':'Gamma'!A1", { mergeRefs: false })).toEqual([
{ type: CONTEXT_QUOTE, value: "'[Book.xlsx]Alpha'" },
{ type: OPERATOR, value: ':' },
Expand All @@ -273,7 +275,9 @@ describe('lexer: 3d ranges', () => {
{ type: REF_RANGE, value: 'A1' }
]);
expect(tokenize("'[Book.xlsx]Alpha':'Gamma'!A1")).toEqual([
{ type: REF_RANGE, value: "'[Book.xlsx]Alpha':'Gamma'!A1" }
{ type: REF_NAMED, value: "'[Book.xlsx]Alpha'" },
{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: "'Gamma'!A1" }
]);
});

Expand Down Expand Up @@ -317,8 +321,10 @@ describe('lexer: 3d ranges', () => {
{ type: OPERATOR, value: '!' },
{ type: REF_RANGE, value: 'A1' }
]);
// Nothing merges the first scope, because a workbook specifier is not allowed in the
// second. What is left has no `!` after it, so it is the operand Excel reads as a name.
expect(tokenize("'Alpha':[Book.xlsx]Gamma!A1")).toEqual([
{ type: CONTEXT_QUOTE, value: "'Alpha'" },
{ type: REF_NAMED, value: "'Alpha'" },

@borgar borgar Sep 14, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is still a context token, ' is not an allowed character in a name. This is an invalid expression and could be fixed by cleaning "unmerged" context tokens, but we shouldn't emit invalid name tokens to force this to parse.

{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: '[Book.xlsx]Gamma!A1' }
]);
Expand Down Expand Up @@ -380,7 +386,7 @@ describe('lexer: 3d ranges', () => {
{ type: REF_RANGE, value: 'A1' }
]);
expect(tokenize("'Alpha':'[Book.xlsx]Gamma'!A1")).toEqual([
{ type: CONTEXT_QUOTE, value: "'Alpha'" },
{ type: REF_NAMED, value: "'Alpha'" },
{ type: OPERATOR, value: ':' },
{ type: REF_RANGE, value: "'[Book.xlsx]Gamma'!A1" }
]);
Expand Down