Skip to content

Commit 542c7a5

Browse files
ulugbeknaulugbekna
committed
context keys: parsing: stronger typing on tokens
1 parent df2db87 commit 542c7a5

File tree

2 files changed

+190
-121
lines changed

2 files changed

+190
-121
lines changed

‎src/vs/platform/contextkey/common/contextkey.ts

Lines changed: 132 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { illegalState } from 'vs/base/common/errors';
76
import { Event } from 'vs/base/common/event';
87
import { isChrome, isEdge, isFirefox, isLinux, isMacintosh, isSafari, isWeb, isWindows } from 'vs/base/common/platform';
98
import { isFalsyOrWhitespace } from 'vs/base/common/strings';
@@ -407,94 +406,109 @@ export class Parser {
407406

408407
private _term(): ContextKeyExpression | undefined {
409408
if (this._matchOne(TokenType.Neg)) {
410-
if (this._matchAny(TokenType.Str, TokenType.True, TokenType.False)) {
411-
const expr = this._previous();
412-
switch (expr.type) {
413-
case TokenType.Str:
414-
return ContextKeyExpr.not(expr.lexeme!);
415-
case TokenType.True:
416-
return ContextKeyExpr.false();
417-
case TokenType.False:
418-
return ContextKeyExpr.true();
419-
default:
420-
throw illegalState(`must've seen only token types matched above`);
421-
}
409+
const expr = this._peek();
410+
switch (expr.type) {
411+
case TokenType.Str:
412+
this._advance();
413+
return ContextKeyExpr.not(expr.lexeme!);
414+
case TokenType.True:
415+
this._advance();
416+
return ContextKeyExpr.false();
417+
case TokenType.False:
418+
this._advance();
419+
return ContextKeyExpr.true();
420+
default:
421+
throw this._errExpectedButGot(`KEY, 'true', or 'false'`, expr);
422422
}
423-
throw this._errExpectedButGot(`KEY, 'true', or 'false'`, this._peek());
424423
}
425424
return this._primary();
426425
}
427426

428427
private _primary(): ContextKeyExpression | undefined {
429428

430-
if (this._matchOne(TokenType.True)) {
431-
return ContextKeyExpr.true();
429+
const peek = this._peek();
430+
switch (peek.type) {
431+
case TokenType.True:
432+
this._advance();
433+
return ContextKeyExpr.true();
432434

433-
} else if (this._matchOne(TokenType.False)) {
434-
return ContextKeyExpr.false();
435+
case TokenType.False:
436+
this._advance();
437+
return ContextKeyExpr.false();
435438

436-
} else if (this._matchOne(TokenType.LParen)) {
437-
const expr = this._expr();
438-
this._consume(TokenType.RParen, `')'`);
439-
return expr;
439+
case TokenType.LParen: {
440+
this._advance();
441+
const expr = this._expr();
442+
this._consume(TokenType.RParen, `')'`);
443+
return expr;
444+
}
445+
446+
case TokenType.Str: {
447+
// KEY
448+
const key = peek.lexeme;
449+
this._advance();
440450

441-
} else if (this._matchOne(TokenType.Str)) {
442-
// KEY
443-
const key = this._previous().lexeme!;
444-
445-
// =~ regex
446-
if (this._matchOne(TokenType.RegexOp)) {
447-
448-
if (this._matchOne(TokenType.RegexStr)) { // expected tokens
449-
const regexLexeme = this._previous().lexeme!; // /REGEX/ or /REGEX/FLAGS
450-
const closingSlashIndex = regexLexeme.lastIndexOf('/');
451-
const flags = closingSlashIndex === regexLexeme.length - 1 ? undefined : regexLexeme.substring(closingSlashIndex + 1);
452-
return ContextKeyExpr.regex(key, new RegExp(regexLexeme.substring(1, closingSlashIndex), flags));
453-
} if (this._matchOne(TokenType.QuotedStr)) {
454-
// replicate old regex parsing behavior
455-
456-
const serializedValue = this._previous().lexeme!;
457-
458-
let regex: RegExp | null = null;
459-
460-
if (isFalsyOrWhitespace(serializedValue)) {
461-
console.warn('missing regexp-value for =~-expression');
462-
} else {
463-
const start = serializedValue.indexOf('/');
464-
const end = serializedValue.lastIndexOf('/');
465-
if (start === end || start < 0 /* || to < 0 */) {
466-
console.warn(`bad regexp-value '${serializedValue}', missing /-enclosure`);
467-
} else {
468-
469-
const value = serializedValue.slice(start + 1, end);
470-
const caseIgnoreFlag = serializedValue[end + 1] === 'i' ? 'i' : '';
471-
try {
472-
regex = new RegExp(value, caseIgnoreFlag);
473-
} catch (e) {
474-
console.warn(`bad regexp-value '${serializedValue}', parse error: ${e}`);
451+
// =~ regex
452+
if (this._matchOne(TokenType.RegexOp)) {
453+
454+
const expr = this._peek();
455+
switch (expr.type) {
456+
case TokenType.RegexStr: {
457+
const regexLexeme = expr.lexeme; // /REGEX/ or /REGEX/FLAGS
458+
this._advance();
459+
const closingSlashIndex = regexLexeme.lastIndexOf('/');
460+
const flags = closingSlashIndex === regexLexeme.length - 1 ? undefined : regexLexeme.substring(closingSlashIndex + 1);
461+
return ContextKeyExpr.regex(key, new RegExp(regexLexeme.substring(1, closingSlashIndex), flags));
462+
}
463+
464+
case TokenType.QuotedStr: {
465+
const serializedValue = expr.lexeme;
466+
this._advance();
467+
// replicate old regex parsing behavior
468+
469+
let regex: RegExp | null = null;
470+
471+
if (isFalsyOrWhitespace(serializedValue)) {
472+
console.warn('missing regexp-value for =~-expression');
473+
} else {
474+
const start = serializedValue.indexOf('/');
475+
const end = serializedValue.lastIndexOf('/');
476+
if (start === end || start < 0 /* || to < 0 */) {
477+
console.warn(`bad regexp-value '${serializedValue}', missing /-enclosure`);
478+
} else {
479+
480+
const value = serializedValue.slice(start + 1, end);
481+
const caseIgnoreFlag = serializedValue[end + 1] === 'i' ? 'i' : '';
482+
try {
483+
regex = new RegExp(value, caseIgnoreFlag);
484+
} catch (e) {
485+
console.warn(`bad regexp-value '${serializedValue}', parse error: ${e}`);
486+
}
487+
}
475488
}
489+
490+
return ContextKeyRegexExpr.create(key, regex);
476491
}
477-
}
478492

479-
return ContextKeyRegexExpr.create(key, regex);
480-
} else {
481-
throw this._errExpectedButGot('REGEX', this._peek());
493+
default:
494+
throw this._errExpectedButGot('REGEX', this._peek());
495+
}
482496
}
483-
}
484497

485-
// [ 'not' 'in' value ]
486-
if (this._matchOne(TokenType.Not)) {
487-
this._consume(TokenType.In, `'in' after 'not'`);
488-
const right = this._value();
489-
return ContextKeyExpr.notIn(key, right);
490-
}
498+
// [ 'not' 'in' value ]
499+
if (this._matchOne(TokenType.Not)) {
500+
this._consume(TokenType.In, `'in' after 'not'`);
501+
const right = this._value();
502+
return ContextKeyExpr.notIn(key, right);
503+
}
491504

492-
// [ ('==' | '!=' | '<' | '<=' | '>' | '>=' | 'in') value ]
493-
if (this._matchAny(TokenType.Eq, TokenType.NotEq, TokenType.Lt, TokenType.LtEq, TokenType.Gt, TokenType.GtEq, TokenType.In)) {
494-
const op = this._previous().type;
495-
const right = this._value();
496-
switch (op) {
505+
// [ ('==' | '!=' | '<' | '<=' | '>' | '>=' | 'in') value ]
506+
const maybeOp = this._peek().type;
507+
switch (maybeOp) {
497508
case TokenType.Eq: {
509+
this._advance();
510+
511+
const right = this._value();
498512
switch (right) {
499513
case 'true':
500514
return ContextKeyExpr.has(key);
@@ -504,7 +518,11 @@ export class Parser {
504518
return ContextKeyExpr.equals(key, right);
505519
}
506520
}
521+
507522
case TokenType.NotEq: {
523+
this._advance();
524+
525+
const right = this._value();
508526
switch (right) {
509527
case 'true':
510528
return ContextKeyExpr.not(key);
@@ -514,38 +532,59 @@ export class Parser {
514532
return ContextKeyExpr.notEquals(key, right);
515533
}
516534
}
517-
518535
// TODO: ContextKeyExpr.smaller(key, right) accepts only `number` as `right` AND during eval of this node, we just eval to `false` if `right` is not a number
519536
// consequently, package.json linter should _warn_ the user if they're passing undesired things to ops
520-
case TokenType.Lt: return ContextKeySmallerExpr.create(key, right);
521-
case TokenType.LtEq: return ContextKeySmallerEqualsExpr.create(key, right);
522-
case TokenType.Gt: return ContextKeyGreaterExpr.create(key, right);
523-
case TokenType.GtEq: return ContextKeyGreaterEqualsExpr.create(key, right);
537+
case TokenType.Lt:
538+
this._advance();
539+
return ContextKeySmallerExpr.create(key, this._value());
540+
541+
case TokenType.LtEq:
542+
this._advance();
543+
return ContextKeySmallerEqualsExpr.create(key, this._value());
544+
545+
case TokenType.Gt:
546+
this._advance();
547+
return ContextKeyGreaterExpr.create(key, this._value());
548+
549+
case TokenType.GtEq:
550+
this._advance();
551+
return ContextKeyGreaterEqualsExpr.create(key, this._value());
524552

525-
case TokenType.In: return ContextKeyExpr.in(key, right);
553+
case TokenType.In:
554+
this._advance();
555+
return ContextKeyExpr.in(key, this._value());
526556

527557
default:
528-
throw illegalState(`must've matched the op with this._match() call above`);
558+
return ContextKeyExpr.has(key);
529559
}
530560
}
531561

532-
return ContextKeyExpr.has(key);
533-
} else {
534-
throw this._errExpectedButGot(`'true', 'false', '(', KEY, KEY '=~' regex, KEY [ ('==' | '!=' | '<' | '<=' | '>' | '>=' | 'in' | 'not' 'in') value ]`, this._peek());
562+
default:
563+
throw this._errExpectedButGot(`'true', 'false', '(', KEY, KEY '=~' regex, KEY [ ('==' | '!=' | '<' | '<=' | '>' | '>=' | 'in' | 'not' 'in') value ]`, this._peek());
564+
535565
}
536566
}
537567

538-
private _value(): string { // TODO@ulugbekna: match all at once and then `switch` on the type
539-
if (this._matchAny(TokenType.Str, TokenType.QuotedStr)) {
540-
return this._previous().lexeme!;
541-
} if (this._matchOne(TokenType.True)) {
542-
return 'true';
543-
} if (this._matchOne(TokenType.False)) {
544-
return 'false';
545-
} if (this._matchOne(TokenType.In)) { // we support `in` as a value, e.g., "when": "languageId == in" - exists in existing extensions
546-
return 'in';
547-
} else {
548-
return ''; // this allows "when": "foo == " which's used by existing extensions
568+
private _value(): string {
569+
const token = this._peek();
570+
switch (token.type) {
571+
case TokenType.Str:
572+
case TokenType.QuotedStr:
573+
this._advance();
574+
return token.lexeme;
575+
case TokenType.True:
576+
this._advance();
577+
return 'true';
578+
case TokenType.False:
579+
this._advance();
580+
return 'false';
581+
case TokenType.In: // we support `in` as a value, e.g., "when": "languageId == in" - exists in existing extensions
582+
this._advance();
583+
return 'in';
584+
default:
585+
// this allows "when": "foo == " which's used by existing extensions
586+
// we do not call `_advance` on purpose - we don't want to eat unintended tokens
587+
return '';
549588
}
550589
}
551590

@@ -563,17 +602,6 @@ export class Parser {
563602
return false;
564603
}
565604

566-
private _matchAny(...tokens: TokenType[]) {
567-
for (const token of tokens) {
568-
if (this._check(token)) {
569-
this._advance();
570-
return true;
571-
}
572-
}
573-
574-
return false;
575-
}
576-
577605
private _advance() {
578606
if (!this._isAtEnd()) {
579607
this._current++;

0 commit comments

Comments
 (0)