Skip to content

Commit 8da53d1

Browse files
johnsoncodehkjohnsoncodehk
committed
gen-tm: scope an import default-binding named like a modifier keyword (fixes TS #950)
`import type from "x"` is a DEFAULT import whose binding is named `type` (tsc: isTypeOnly=false) — so `type` is a variable, not the type-only modifier. Monogram used to mis-fire its type-alias declaration region on the `type <name>` shape (`type from`, with `from` a valid alias name). Derive an `import-default-binding` rule: `import <ident>(?=\s*(?:from|,))` -> the ident is `variable.other.readwrite`. Everything is read from the grammar — the `import` keyword (scope keyword.control.import) and the `from` connector (keyword.control.from); the disambiguator is the grammar's own shape (a default binding precedes the connector, whereas the `type` MODIFIER precedes the clause ident). Leftmost-match beats the `type <name>` declaration region (one token to its right). Emitted ONLY when a declaration keyword is also non-reserved (can legally stand in the binding slot — TS `type`/`interface`), so plain JS (whose `function`/`class` are reserved) emits nothing and stays byte-identical. This also correctly scopes every default binding now (`import foo from`). Disproves the earlier "agnostic ceiling" framing. TS ledger 25->26/27 (only-official 0); agnostic 8/8, test-issues 349/0, highlight-bench per-snippet 84.1->85.0%, tsx 56/56; JS/HTML/Vue/JSX byte-identical.
1 parent 3855de1 commit 8da53d1

5 files changed

Lines changed: 133 additions & 12 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ The same question, every language at once: take the bugs reported against each *
4646

4747
<!-- issues:start -->
4848
<!-- generated by `npm run bench:issues` — do not edit by hand -->
49-
_Each hand-written **official** grammar vs Monogram's **derived** one, on the bugs filed against it: **TypeScript 25/27** (official 9/27) · **TSX 10/11** (official 6/11) · **HTML 19/20** (official 13/20) · **Vue 19/19** (official 15/19). Per-issue detail below — auto-generated by `npm run bench:issues`._
49+
_Each hand-written **official** grammar vs Monogram's **derived** one, on the bugs filed against it: **TypeScript 26/27** (official 9/27) · **TSX 10/11** (official 6/11) · **HTML 19/20** (official 13/20) · **Vue 19/19** (official 15/19). Per-issue detail below — auto-generated by `npm run bench:issues`._
5050

5151
#### TypeScript
5252
| issue | Monogram | official |
@@ -67,7 +67,7 @@ _Each hand-written **official** grammar vs Monogram's **derived** one, on the bu
6767
| [#995](https://github.com/microsoft/TypeScript-TmLanguage/issues/995) — paren-wrapped `as keyof typeof` assertion tokenizes | ✓ | · |
6868
| [#891](https://github.com/microsoft/TypeScript-TmLanguage/issues/891) — `from` as an ordinary variable is not a keyword | ✓ | · |
6969
| [#814](https://github.com/microsoft/TypeScript-TmLanguage/issues/814) — `a instanceof B & c` keeps the operand a value, not a type | ✓ | · |
70-
| [#950](https://github.com/microsoft/TypeScript-TmLanguage/issues/950) — default import named `type` — the binding is a variable, not the `type` keyword | · | · |
70+
| [#950](https://github.com/microsoft/TypeScript-TmLanguage/issues/950) — default import named `type` — the binding is a variable, not the `type` keyword | ✓ | · |
7171
| [#1058](https://github.com/microsoft/TypeScript-TmLanguage/issues/1058) — `import defer` should scope `defer` as a keyword | · | · |
7272

7373
<details><summary>… and 9 more both grammars already handle (✓ / ✓)</summary>

‎src/gen-tm.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4741,6 +4741,84 @@ export function generateTmLanguage(grammar: CstGrammar, langName: string): TmGra
47414741
topPatterns.push({ include: '#import-export-all' });
47424742
}
47434743

4744+
// ── 4a-bis. Default-import binding name → variable.other.readwrite ──
4745+
// The identifier directly after an import keyword that is immediately followed
4746+
// by the module-source connector (`import X from "m"`) — or a `,` that begins a
4747+
// mixed clause (`import X, { a } from "m"`) — is a BOUND NAME (the default
4748+
// import), exactly like a `const` binding, NOT a type/keyword. The catch-all
4749+
// already scopes a plain identifier `variable.other.readwrite`, but a *contextual
4750+
// keyword* sitting in that slot (e.g. `import type from "m"`, where `type` is the
4751+
// default binding literally named `type`, tsc isTypeOnly=false) is otherwise
4752+
// claimed by its own keyword/declaration rule — the type-alias `type <name>`
4753+
// region fires on `type from`. This rule restores the binding role.
4754+
//
4755+
// Fully derived, no hardcoded words:
4756+
// • trigger keywords = the import keywords (scope keyword.control.import*) whose
4757+
// rule places a bare identifier directly after them — directly, or through a
4758+
// ref whose first alternative starts with the identifier token (the default
4759+
// branch of an import-clause rule). Keywords that only ever take `{…}`/`*`
4760+
// (no default form) are excluded.
4761+
// • module-source connector = the keyword(s) scoped keyword.control.from.
4762+
// The connector lookahead is the disambiguator the grammar itself uses: a default
4763+
// binding is followed by the source connector or a `,`; a *modifier* keyword
4764+
// (`import type X from`) is followed by the clause identifier instead, so the
4765+
// modifier is left untouched (its own keyword/declaration rule still paints it).
4766+
// Leftmost-match makes this win over the later `type <name>` region (which begins
4767+
// one token further right), and the order rank (below) wins the position-0 tie
4768+
// over the flat import-keyword / storage-keyword matches.
4769+
//
4770+
// GATED on the collision actually being possible: the only reason this is needed
4771+
// is that a DECLARATION keyword (one with its own `keyword <name>` region) is also
4772+
// a NON-reserved word, so it can legally stand as the bound default-import name and
4773+
// its region would otherwise fire inside the import (TS `type`/`interface`/…). A
4774+
// language whose declaration keywords are all reserved (plain JS: `function`/`class`)
4775+
// has no such word in the binding slot — a plain identifier already gets readwrite
4776+
// from the catch-all — so the rule is not emitted and the output stays byte-identical.
4777+
const declReservedWords = collectReservedWords(grammar);
4778+
const hasContextualDeclKeyword = declarations.some(d => !declReservedWords.has(d.keyword));
4779+
const defaultBindImportKws = new Set<string>();
4780+
{
4781+
// True when the rule-ref `b` is the identifier token itself, or a rule whose
4782+
// first alternative starts with it (`ruleStartsWithIdent` returns true for the
4783+
// token name directly, so this covers both `import Ident` and `import ImportClause`).
4784+
const startsWithIdentClause = (refName: string): boolean =>
4785+
identToken ? ruleStartsWithIdent(refName, identToken.name, grammar.rules) : false;
4786+
const walk = (e: RuleExpr | undefined): void => {
4787+
if (!e) return;
4788+
for (const alt of expandAlts(e)) {
4789+
for (let i = 0; i < alt.length - 1; i++) {
4790+
const a = alt[i], b = alt[i + 1];
4791+
if (a.type !== 'literal' || !importExportKws.has(a.value)) continue;
4792+
// A bare identifier directly after the keyword, or reached through the
4793+
// next rule-ref whose first alternative is the identifier token.
4794+
if (b.type === 'ref' && startsWithIdentClause(b.name)) {
4795+
defaultBindImportKws.add(a.value);
4796+
}
4797+
}
4798+
for (const item of alt) {
4799+
if (item.type === 'quantifier' || item.type === 'group') walk(item.body);
4800+
else if (item.type === 'sep') walk(item.element);
4801+
}
4802+
}
4803+
};
4804+
for (const rule of grammar.rules) walk(rule.body);
4805+
}
4806+
const moduleSourceKws = [...scopeOverrides]
4807+
.filter(([, scopes]) => scopes.some(s => /(^|\.)keyword\.control\.from\b/.test(s)))
4808+
.map(([lit]) => lit);
4809+
if (hasContextualDeclKeyword && defaultBindImportKws.size > 0 && moduleSourceKws.length > 0 && identToken) {
4810+
const kwScope = getScope(scopeOverrides, [...defaultBindImportKws][0]) ?? 'keyword.control.import';
4811+
const fromAlt = moduleSourceKws.map(escapeRegex).join('|');
4812+
repository['import-default-binding'] = {
4813+
match: `\\b(${[...defaultBindImportKws].map(escapeRegex).join('|')})\\s+(${identPattern})(?=\\s*(?:(?:${fromAlt})\\b|,))`,
4814+
captures: {
4815+
'1': { name: `${kwScope}.${langName}` },
4816+
'2': { name: `variable.other.readwrite.${langName}` },
4817+
},
4818+
};
4819+
topPatterns.push({ include: '#import-default-binding' });
4820+
}
4821+
47444822
// ── 4b. Function call detection ──
47454823
const hasCallExpr = detectCallExpression(grammar);
47464824
if (hasCallExpr) {
@@ -5453,6 +5531,11 @@ export function generateTmLanguage(grammar: CstGrammar, langName: string): TmGra
54535531
// import/export namespace `*` must beat both the import/export keyword group
54545532
// (which would consume the keyword alone) and the arithmetic-operator match.
54555533
if (key === 'import-export-all') return 2;
5534+
// A default-import binding name (`import X from`, `import type from`) must beat
5535+
// the flat import keyword group AND the `type <name>` declaration/storage match
5536+
// for the contextual-keyword-as-binding case; leftmost-match handles the region
5537+
// one token to the right, this rank wins the position-0 tie.
5538+
if (key === 'import-default-binding') return 2;
54565539
if (scope.includes('constant.numeric')) return 3; // stable sort preserves DSL token order
54575540
if (scope.includes('keyword.operator') && key.startsWith('scope-')) return 4;
54585541
if (scope.includes('keyword.control')) return 5;

‎test/issue-cases.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -723,18 +723,22 @@ export const tests: TestCase[] = [
723723
],
724724
},
725725
// `import type from "./type"` is a DEFAULT import whose binding is named `type` (tsc:
726-
// isTypeOnly=false, importClause.name=`type`) — so `type` should be a variable. A PROVEN both-miss
727-
// at the agnostic ceiling: Monogram's PARSER accepts it (the CFG distinguishes the binding), but
728-
// the pattern-highlighter's type-alias DECLARATION region fires on the `type <name>` shape — here
729-
// `type from`, with `from` a perfectly valid contextual alias name (`type from = X` is a real type
730-
// alias). The only TM disambiguator is the preceding `import`, i.e. a negative lookbehind hardcoding
731-
// that one keyword — which the agnostic generator can't emit (it has no parse-context notion of
732-
// "statement position", and `import` is just data). The official hardcodes `import type` and STILL
733-
// misses this (scopes `type`→keyword.control.type); Monogram mis-fires its type-alias region
734-
// (`type`→storage.type.type). Different wrong answers, same miss.
726+
// isTypeOnly=false, importClause.name=`type`) — so `type` should be a variable. Monogram's
727+
// PARSER already distinguishes it (the CFG admits `type` as the default binding), and the
728+
// DERIVED highlighter now scopes it variable.other too: gen-tm emits an `import-default-binding`
729+
// rule — the identifier directly after an import keyword that is immediately followed by the
730+
// module-source connector (or a `,`) is a bound name, exactly like a `const` binding. All three
731+
// ingredients are read from the grammar, not hardcoded: the import keyword(s) (scope
732+
// keyword.control.import*, placing a bare identifier after them), the `from` connector (scope
733+
// keyword.control.from), and the disambiguator (a default binding is FOLLOWED by the connector,
734+
// whereas the `type` MODIFIER in `import type X from` is followed by the clause identifier — so
735+
// `import type { A }` / `import type X` / `import type * as ns` keep `type` as the modifier).
736+
// The rule is gated on the language having a contextual (non-reserved) declaration keyword that
737+
// can stand in the binding slot — plain JS has none, so its grammar stays byte-identical. The
738+
// official hardcodes `import type` and STILL misses this (scopes `type`→keyword.control.type);
739+
// Monogram now gets it right (`type`→variable.other.readwrite).
735740
{
736741
label: '#950: default import named `type` — the binding is a variable, not the `type` keyword',
737-
monoGap: true,
738742
input: `import type from "./type";`,
739743
checks: [
740744
{ text: 'type', scope: 'variable.other' },

‎typescript.tmLanguage.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@
111111
{
112112
"include": "#import-export-all"
113113
},
114+
{
115+
"include": "#import-default-binding"
116+
},
114117
{
115118
"include": "#is-typekw"
116119
},
@@ -2369,6 +2372,17 @@
23692372
}
23702373
}
23712374
},
2375+
"import-default-binding": {
2376+
"match": "\\b(import)\\s+((?:[a-zA-Z_$\\p{L}\\p{Nl}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$\\p{L}\\p{Nl}\\p{Nd}\\p{Mn}\\p{Mc}\\p{Pc}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*(?:(?:from)\\b|,))",
2377+
"captures": {
2378+
"1": {
2379+
"name": "keyword.control.import.ts"
2380+
},
2381+
"2": {
2382+
"name": "variable.other.readwrite.ts"
2383+
}
2384+
}
2385+
},
23722386
"function-call": {
23732387
"match": "((?:[a-zA-Z_$\\p{L}\\p{Nl}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$\\p{L}\\p{Nl}\\p{Nd}\\p{Mn}\\p{Mc}\\p{Pc}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*\\()",
23742388
"captures": {
@@ -3047,6 +3061,9 @@
30473061
{
30483062
"include": "#import-export-all"
30493063
},
3064+
{
3065+
"include": "#import-default-binding"
3066+
},
30503067
{
30513068
"include": "#is-typekw"
30523069
},

‎typescriptreact.tmLanguage.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@
117117
{
118118
"include": "#import-export-all"
119119
},
120+
{
121+
"include": "#import-default-binding"
122+
},
120123
{
121124
"include": "#is-typekw"
122125
},
@@ -2757,6 +2760,17 @@
27572760
}
27582761
}
27592762
},
2763+
"import-default-binding": {
2764+
"match": "\\b(import)\\s+((?:[a-zA-Z_$\\p{L}\\p{Nl}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$\\p{L}\\p{Nl}\\p{Nd}\\p{Mn}\\p{Mc}\\p{Pc}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*(?:(?:from)\\b|,))",
2765+
"captures": {
2766+
"1": {
2767+
"name": "keyword.control.import.tsx"
2768+
},
2769+
"2": {
2770+
"name": "variable.other.readwrite.tsx"
2771+
}
2772+
}
2773+
},
27602774
"function-call": {
27612775
"match": "((?:[a-zA-Z_$\\p{L}\\p{Nl}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$\\p{L}\\p{Nl}\\p{Nd}\\p{Mn}\\p{Mc}\\p{Pc}]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*\\()",
27622776
"captures": {
@@ -3441,6 +3455,9 @@
34413455
{
34423456
"include": "#import-export-all"
34433457
},
3458+
{
3459+
"include": "#import-default-binding"
3460+
},
34443461
{
34453462
"include": "#is-typekw"
34463463
},

0 commit comments

Comments
 (0)