Skip to content

Commit 6541fae

Browse files
committed
Improve JS highlighter fidelity: bare-param scoping + regex after contextual keywords
Two language-agnostic gen-tm fixes surfaced by the JavaScript coverage gate: - Unannotated parameters (plain function f(x), methods, constructors) now get variable.parameter instead of variable.other. Previously only arrow params and type-annotated params (x: T) triggered param scoping, so the copy+strip'd JS grammar -- which has no annotations -- lost it. A structural declaration-param-name matcher inside the params scope handles bare params; annotated params still defer to the type-annotation path. JS highlighter visual accuracy 96.4% -> 98.0% (real-gap 16 -> 9 tokens). - Regex-vs-division detection now flattens alt()/opt()/group() when collecting the keywords that may precede an expression, so a keyword reachable only through an alternative (e.g. alt('in','of') Expr in the for-head) is seen. Fixes regex literals after `of` (for (x of /re/.exec(s)), upstream #883) in both the JS and TS TextMate output; a / b still parses as division. Both engine changes stay agnostic (no hardcoded language tokens; agnostic 5/5). TS unaffected: coverage 99.3%, valid-code 100% (FN=0), bidirectional 97.84%. Regenerated both grammars' artifacts.
1 parent cb39056 commit 6541fae

4 files changed

Lines changed: 61 additions & 4 deletions

File tree

examples/javascript.tmLanguage.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@
184184
"repository": {
185185
"regex-literal": {
186186
"name": "string.regexp.javascript",
187-
"begin": "(?:(?<=[=|\\^&<>+\\-*%!~(,.\\[?:{;])|(?<=\\binstanceof)|(?<=\\bin)|(?<=\\bnew)|(?<=\\bextends)|(?<=\\belse)|(?<=\\bdo)|(?<=\\bthrow)|(?<=\\btry)|(?<=\\bfinally)|(?<=\\bcase)|(?<=\\bstatic)|(?<=\\btypeof)|(?<=\\bvoid)|(?<=\\bdelete)|(?<=\\bawait)|(?<=\\byield)|(?<=^))\\s*(/)(?![*/])",
187+
"begin": "(?:(?<=[=|\\^&<>+\\-*%!~(,.\\[?:{;])|(?<=\\binstanceof)|(?<=\\bin)|(?<=\\bnew)|(?<=\\bextends)|(?<=\\byield)|(?<=\\bget)|(?<=\\bset)|(?<=\\basync)|(?<=\\belse)|(?<=\\bdo)|(?<=\\breturn)|(?<=\\bthrow)|(?<=\\btry)|(?<=\\bfinally)|(?<=\\bcatch)|(?<=\\bof)|(?<=\\bcase)|(?<=\\bexport)|(?<=\\bdefault)|(?<=\\bimport)|(?<=\\bstatic)|(?<=\\baccessor)|(?<=\\btypeof)|(?<=\\bvoid)|(?<=\\bdelete)|(?<=\\bawait)|(?<=^))\\s*(/)(?![*/])",
188188
"beginCaptures": {
189189
"1": {
190190
"name": "punctuation.definition.string.begin.regexp.javascript"
@@ -418,6 +418,17 @@
418418
}
419419
]
420420
},
421+
"declaration-param-name": {
422+
"match": "(?<=[,(])\\s*(\\.\\.\\.)?\\s*((?:[a-zA-Z_$]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*[,)=])",
423+
"captures": {
424+
"1": {
425+
"name": "keyword.operator.spread.javascript"
426+
},
427+
"2": {
428+
"name": "variable.parameter.javascript"
429+
}
430+
}
431+
},
421432
"declaration-params": {
422433
"name": "meta.parameters.javascript",
423434
"begin": "\\(",
@@ -433,6 +444,9 @@
433444
}
434445
},
435446
"patterns": [
447+
{
448+
"include": "#declaration-param-name"
449+
},
436450
{
437451
"include": "#nested-parens"
438452
},

examples/typescript.tmLanguage.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@
399399
},
400400
"regex-literal": {
401401
"name": "string.regexp.typescript",
402-
"begin": "(?:(?<=[=|\\^&<>+\\-*%!~,\\[(?:{;.])|(?<=\\bis)|(?<=\\bkeyof)|(?<=\\btypeof)|(?<=\\breadonly)|(?<=\\bnew)|(?<=\\bextends)|(?<=\\bin)|(?<=\\bas)|(?<=\\binstanceof)|(?<=\\bclass)|(?<=\\bsatisfies)|(?<=\\belse)|(?<=\\bdo)|(?<=\\bthrow)|(?<=\\btry)|(?<=\\bfinally)|(?<=\\bcase)|(?<=\\btype)|(?<=\\bstatic)|(?<=\\bvoid)|(?<=\\bdelete)|(?<=\\bawait)|(?<=\\byield)|(?<=^))\\s*(/)(?![*/])",
402+
"begin": "(?:(?<=[=|\\^&<>+\\-*%!~,\\[(?:{;.])|(?<=\\bis)|(?<=\\bkeyof)|(?<=\\btypeof)|(?<=\\breadonly)|(?<=\\bnew)|(?<=\\bextends)|(?<=\\bin)|(?<=\\bas)|(?<=\\binstanceof)|(?<=\\bclass)|(?<=\\basync)|(?<=\\byield)|(?<=\\bsatisfies)|(?<=\\bfunction)|(?<=\\bget)|(?<=\\bset)|(?<=\\belse)|(?<=\\bdo)|(?<=\\breturn)|(?<=\\bthrow)|(?<=\\btry)|(?<=\\bfinally)|(?<=\\bcatch)|(?<=\\bpublic)|(?<=\\bprivate)|(?<=\\bprotected)|(?<=\\bof)|(?<=\\bcase)|(?<=\\bdeclare)|(?<=\\bexport)|(?<=\\bdefault)|(?<=\\bimport)|(?<=\\btype)|(?<=\\bstatic)|(?<=\\babstract)|(?<=\\boverride)|(?<=\\baccessor)|(?<=\\bvoid)|(?<=\\bdelete)|(?<=\\bawait)|(?<=^))\\s*(/)(?![*/])",
403403
"beginCaptures": {
404404
"1": {
405405
"name": "punctuation.definition.string.begin.regexp.typescript"
@@ -794,6 +794,17 @@
794794
}
795795
]
796796
},
797+
"declaration-param-name": {
798+
"match": "(?<=[,(])\\s*(\\.\\.\\.)?\\s*((?:[a-zA-Z_$]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})(?:[a-zA-Z0-9_$]|\\\\u[0-9a-fA-F]{4}|\\\\u\\{[0-9a-fA-F]+\\})*)(?=\\s*[,)=])",
799+
"captures": {
800+
"1": {
801+
"name": "keyword.operator.spread.typescript"
802+
},
803+
"2": {
804+
"name": "variable.parameter.typescript"
805+
}
806+
}
807+
},
797808
"declaration-params": {
798809
"name": "meta.parameters.typescript",
799810
"begin": "\\(",
@@ -812,6 +823,9 @@
812823
{
813824
"include": "#param-type-annotation"
814825
},
826+
{
827+
"include": "#declaration-param-name"
828+
},
815829
{
816830
"include": "#nested-parens"
817831
},

src/gen-tm.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,8 +981,15 @@ function detectRegexLiteral(grammar: CstGrammar, tokenNames: Set<string>): Regex
981981
}
982982
}
983983

984+
// Walk every expression body, checking each flattened alternative for a
985+
// keyword→expr-ref adjacency. `expandAlts` resolves `alt`/`opt`/`group` so a
986+
// keyword tucked inside an alternative (e.g. `alt('in','of') Expr` in a for-head)
987+
// is seen adjacent to the following ref — without it, only keywords written as a
988+
// bare literal directly before the ref (`'in' Expr` as an infix op) are collected,
989+
// so `of` would be missed. `sep`/quantifier inner bodies stay opaque to
990+
// `expandAlts`, so recurse into them to keep nested adjacencies reachable.
984991
function walk(expr: RuleExpr) {
985-
if (expr.type === 'seq') checkSeq(expr.items);
992+
for (const seq of expandAlts(expr)) checkSeq(seq);
986993
if (expr.type === 'alt' || expr.type === 'seq') expr.items.forEach(walk);
987994
if (expr.type === 'quantifier' || expr.type === 'group') walk(expr.body);
988995
if (expr.type === 'sep') walk(expr.element);
@@ -1777,6 +1784,28 @@ export function generateTmLanguage(grammar: CstGrammar, langName: string): TmGra
17771784
};
17781785
}
17791786

1787+
// Bare parameter name (no type annotation): the identifier at a
1788+
// param-start position — directly after '(' or ',', and immediately
1789+
// followed by a delimiter that ends a *bare* param: ',' (next param),
1790+
// ')' (end of list), or '=' (default value). The lookbehind keeps this
1791+
// from matching default-value expressions (e.g. `x = foo` → only `x`
1792+
// matches; `foo` is preceded by '=', not '('/','). This gives
1793+
// unannotated params in function/method/constructor signatures the
1794+
// variable.parameter scope (the type-annotation path handles annotated
1795+
// ones). The lookahead deliberately omits ':' and '?' so an annotated
1796+
// param (`x: T`, `x?: T`, `...args: T[]`) is left for
1797+
// #param-type-annotation — which is listed first AND, because this
1798+
// matcher swallows leading whitespace (TextMate prefers the leftmost
1799+
// match), would otherwise be pre-empted on rest params like `...args:`.
1800+
repository['declaration-param-name'] = {
1801+
match: `(?<=[,(])\\s*(\\.\\.\\.)?\\s*(${identPattern})(?=\\s*[,)=])`,
1802+
captures: {
1803+
'1': { name: `keyword.operator.spread.${langName}` },
1804+
'2': { name: `variable.parameter.${langName}` },
1805+
},
1806+
};
1807+
paramsInnerPatterns.push({ include: '#declaration-param-name' });
1808+
17801809
paramsInnerPatterns.push({ include: '#nested-parens' });
17811810
paramsInnerPatterns.push({ include: '$self' });
17821811

test/test-issues.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ const tests: TestCase[] = [
315315
input: 'function f(a = 1) {}',
316316
checks: [
317317
{ text: 'f', scope: 'entity.name.function' },
318-
{ text: 'a', scope: 'variable.other' },
318+
{ text: 'a', scope: 'variable.parameter' },
319319
{ text: '=', scope: 'keyword.operator.assignment' },
320320
{ text: '1', scope: 'constant.numeric' },
321321
],

0 commit comments

Comments
 (0)