Skip to content

Commit 41b414c

Browse files
alexdimaalexdima
committed
Fixes microsoft#134942: When parsing comparison expressions, try to parse the value as numbers
1 parent fcca378 commit 41b414c

File tree

2 files changed

+63
-30
lines changed

2 files changed

+63
-30
lines changed

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

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -81,50 +81,45 @@ export abstract class ContextKeyExpr {
8181
public static false(): ContextKeyExpression {
8282
return ContextKeyFalseExpr.INSTANCE;
8383
}
84-
8584
public static true(): ContextKeyExpression {
8685
return ContextKeyTrueExpr.INSTANCE;
8786
}
88-
8987
public static has(key: string): ContextKeyExpression {
9088
return ContextKeyDefinedExpr.create(key);
9189
}
92-
9390
public static equals(key: string, value: any): ContextKeyExpression {
9491
return ContextKeyEqualsExpr.create(key, value);
9592
}
96-
9793
public static notEquals(key: string, value: any): ContextKeyExpression {
9894
return ContextKeyNotEqualsExpr.create(key, value);
9995
}
100-
10196
public static regex(key: string, value: RegExp): ContextKeyExpression {
10297
return ContextKeyRegexExpr.create(key, value);
10398
}
104-
10599
public static in(key: string, value: string): ContextKeyExpression {
106100
return ContextKeyInExpr.create(key, value);
107101
}
108-
109102
public static not(key: string): ContextKeyExpression {
110103
return ContextKeyNotExpr.create(key);
111104
}
112-
113105
public static and(...expr: Array<ContextKeyExpression | undefined | null>): ContextKeyExpression | undefined {
114106
return ContextKeyAndExpr.create(expr, null);
115107
}
116-
117108
public static or(...expr: Array<ContextKeyExpression | undefined | null>): ContextKeyExpression | undefined {
118109
return ContextKeyOrExpr.create(expr, null, true);
119110
}
120-
121-
public static greater(key: string, value: any): ContextKeyExpression {
111+
public static greater(key: string, value: number): ContextKeyExpression {
122112
return ContextKeyGreaterExpr.create(key, value);
123113
}
124-
125-
public static less(key: string, value: any): ContextKeyExpression {
114+
public static greaterEquals(key: string, value: number): ContextKeyExpression {
115+
return ContextKeyGreaterEqualsExpr.create(key, value);
116+
}
117+
public static smaller(key: string, value: number): ContextKeyExpression {
126118
return ContextKeySmallerExpr.create(key, value);
127119
}
120+
public static smallerEquals(key: string, value: number): ContextKeyExpression {
121+
return ContextKeySmallerEqualsExpr.create(key, value);
122+
}
128123

129124
public static deserialize(serialized: string | null | undefined, strict: boolean = false): ContextKeyExpression | undefined {
130125
if (!serialized) {
@@ -741,17 +736,30 @@ export class ContextKeyNotExpr implements IContextKeyExpression {
741736
}
742737
}
743738

739+
function withFloatOrStr<T extends ContextKeyExpression>(value: any, callback: (value: number | string) => T): T | ContextKeyFalseExpr {
740+
if (typeof value === 'string') {
741+
const n = parseFloat(value);
742+
if (!isNaN(n)) {
743+
value = n;
744+
}
745+
}
746+
if (typeof value === 'string' || typeof value === 'number') {
747+
return callback(value);
748+
}
749+
return ContextKeyFalseExpr.INSTANCE;
750+
}
751+
744752
export class ContextKeyGreaterExpr implements IContextKeyExpression {
745753

746-
public static create(key: string, value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
747-
return new ContextKeyGreaterExpr(key, value, negated);
754+
public static create(key: string, _value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
755+
return withFloatOrStr(_value, (value) => new ContextKeyGreaterExpr(key, value, negated));
748756
}
749757

750758
public readonly type = ContextKeyExprType.Greater;
751759

752760
private constructor(
753761
private readonly key: string,
754-
private readonly value: any,
762+
private readonly value: number | string,
755763
private negated: ContextKeyExpression | null
756764
) { }
757765

@@ -774,7 +782,10 @@ export class ContextKeyGreaterExpr implements IContextKeyExpression {
774782
}
775783

776784
public evaluate(context: IContext): boolean {
777-
return (parseFloat(<any>context.getValue(this.key)) > parseFloat(this.value));
785+
if (typeof this.value === 'string') {
786+
return false;
787+
}
788+
return (parseFloat(<any>context.getValue(this.key)) > this.value);
778789
}
779790

780791
public serialize(): string {
@@ -799,15 +810,15 @@ export class ContextKeyGreaterExpr implements IContextKeyExpression {
799810

800811
export class ContextKeyGreaterEqualsExpr implements IContextKeyExpression {
801812

802-
public static create(key: string, value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
803-
return new ContextKeyGreaterEqualsExpr(key, value, negated);
813+
public static create(key: string, _value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
814+
return withFloatOrStr(_value, (value) => new ContextKeyGreaterEqualsExpr(key, value, negated));
804815
}
805816

806817
public readonly type = ContextKeyExprType.GreaterEquals;
807818

808819
private constructor(
809820
private readonly key: string,
810-
private readonly value: any,
821+
private readonly value: number | string,
811822
private negated: ContextKeyExpression | null
812823
) { }
813824

@@ -830,7 +841,10 @@ export class ContextKeyGreaterEqualsExpr implements IContextKeyExpression {
830841
}
831842

832843
public evaluate(context: IContext): boolean {
833-
return (parseFloat(<any>context.getValue(this.key)) >= parseFloat(this.value));
844+
if (typeof this.value === 'string') {
845+
return false;
846+
}
847+
return (parseFloat(<any>context.getValue(this.key)) >= this.value);
834848
}
835849

836850
public serialize(): string {
@@ -855,15 +869,15 @@ export class ContextKeyGreaterEqualsExpr implements IContextKeyExpression {
855869

856870
export class ContextKeySmallerExpr implements IContextKeyExpression {
857871

858-
public static create(key: string, value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
859-
return new ContextKeySmallerExpr(key, value, negated);
872+
public static create(key: string, _value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
873+
return withFloatOrStr(_value, (value) => new ContextKeySmallerExpr(key, value, negated));
860874
}
861875

862876
public readonly type = ContextKeyExprType.Smaller;
863877

864878
private constructor(
865879
private readonly key: string,
866-
private readonly value: any,
880+
private readonly value: number | string,
867881
private negated: ContextKeyExpression | null
868882
) {
869883
}
@@ -887,7 +901,10 @@ export class ContextKeySmallerExpr implements IContextKeyExpression {
887901
}
888902

889903
public evaluate(context: IContext): boolean {
890-
return (parseFloat(<any>context.getValue(this.key)) < parseFloat(this.value));
904+
if (typeof this.value === 'string') {
905+
return false;
906+
}
907+
return (parseFloat(<any>context.getValue(this.key)) < this.value);
891908
}
892909

893910
public serialize(): string {
@@ -912,15 +929,15 @@ export class ContextKeySmallerExpr implements IContextKeyExpression {
912929

913930
export class ContextKeySmallerEqualsExpr implements IContextKeyExpression {
914931

915-
public static create(key: string, value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
916-
return new ContextKeySmallerEqualsExpr(key, value, negated);
932+
public static create(key: string, _value: any, negated: ContextKeyExpression | null = null): ContextKeyExpression {
933+
return withFloatOrStr(_value, (value) => new ContextKeySmallerEqualsExpr(key, value, negated));
917934
}
918935

919936
public readonly type = ContextKeyExprType.SmallerEquals;
920937

921938
private constructor(
922939
private readonly key: string,
923-
private readonly value: any,
940+
private readonly value: number | string,
924941
private negated: ContextKeyExpression | null
925942
) {
926943
}
@@ -944,7 +961,10 @@ export class ContextKeySmallerEqualsExpr implements IContextKeyExpression {
944961
}
945962

946963
public evaluate(context: IContext): boolean {
947-
return (parseFloat(<any>context.getValue(this.key)) <= parseFloat(this.value));
964+
if (typeof this.value === 'string') {
965+
return false;
966+
}
967+
return (parseFloat(<any>context.getValue(this.key)) <= this.value);
948968
}
949969

950970
public serialize(): string {

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55
import * as assert from 'assert';
66
import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
7-
import { ContextKeyExpr, implies } from 'vs/platform/contextkey/common/contextkey';
7+
import { ContextKeyExpr, ContextKeyExpression, implies } from 'vs/platform/contextkey/common/contextkey';
88

99
function createContext(ctx: any) {
1010
return {
@@ -45,6 +45,19 @@ suite('ContextKeyExpr', () => {
4545
assert(a.equals(b), 'expressions should be equal');
4646
});
4747

48+
test('issue #134942: Equals in comparator expressions', () => {
49+
function testEquals(expr: ContextKeyExpression | undefined, str: string): void {
50+
const deserialized = ContextKeyExpr.deserialize(str);
51+
assert.ok(expr);
52+
assert.ok(deserialized);
53+
assert.strictEqual(expr.equals(deserialized), true, str);
54+
}
55+
testEquals(ContextKeyExpr.greater('value', 0), 'value > 0');
56+
testEquals(ContextKeyExpr.greaterEquals('value', 0), 'value >= 0');
57+
testEquals(ContextKeyExpr.smaller('value', 0), 'value < 0');
58+
testEquals(ContextKeyExpr.smallerEquals('value', 0), 'value <= 0');
59+
});
60+
4861
test('normalize', () => {
4962
let key1IsTrue = ContextKeyExpr.equals('key1', true);
5063
let key1IsNotFalse = ContextKeyExpr.notEquals('key1', false);

0 commit comments

Comments
 (0)