- Add tests

alexdima · eamodio · commit d233bf34bc24 · 2020-08-25T13:04:49.000-04:00
- RHS of `in` expression is not a value, so don't deserialize it like a value
- avoid unnecessary undefined check
- use `hasOwnProperty` for object case
- keys() should include the RHS of `in` expression
diff --git a/src/vs/platform/contextkey/common/contextkey.ts b/src/vs/platform/contextkey/common/contextkey.ts
@@ -17,6 +17,8 @@ STATIC_VALUES.set('isWindows', isWindows);
 STATIC_VALUES.set('isWeb', isWeb);
 STATIC_VALUES.set('isMacNative', isMacintosh && !isWeb);
 
+const hasOwnProperty = Object.prototype.hasOwnProperty;
+
 export const enum ContextKeyExprType {
 	False = 0,
 	True = 1,
@@ -138,7 +140,7 @@ export abstract class ContextKeyExpr {
 
 		if (serializedOne.indexOf(' in ') >= 0) {
 			let pieces = serializedOne.split(' in ');
-			return ContextKeyInExpr.create(pieces[0].trim(), this._deserializeValue(pieces[1], strict));
+			return ContextKeyInExpr.create(pieces[0].trim(), pieces[1].trim());
 		}
 
 		if (/^\!\s*/.test(serializedOne)) {
@@ -451,8 +453,8 @@ export class ContextKeyInExpr implements IContextKeyExpression {
 			return (source.indexOf(item) >= 0);
 		}
 
-		if (typeof item === 'string' && typeof source === 'object' && source !== undefined && source !== null) {
-			return item in source;
+		if (typeof item === 'string' && typeof source === 'object' && source !== null) {
+			return hasOwnProperty.call(source, item);
 		}
 		return false;
 	}
@@ -462,7 +464,7 @@ export class ContextKeyInExpr implements IContextKeyExpression {
 	}
 
 	public keys(): string[] {
-		return [this.key];
+		return [this.key, this.valueKey];
 	}
 
 	public map(mapFnc: IContextKeyExprMapper): ContextKeyInExpr {
diff --git a/src/vs/platform/contextkey/test/common/contextkey.test.ts b/src/vs/platform/contextkey/test/common/contextkey.test.ts
@@ -150,4 +150,19 @@ suite('ContextKeyExpr', () => {
 		t('a || b', 'c && d', 'a && c && d || b && c && d');
 		t('a || b', 'c && d || e', 'a && e || b && e || a && c && d || b && c && d');
 	});
+
+	test('ContextKeyInExpr', () => {
+		const ainb = ContextKeyExpr.deserialize('a in b')!;
+		assert.equal(ainb.evaluate(createContext({ 'a': 3, 'b': [3, 2, 1] })), true);
+		assert.equal(ainb.evaluate(createContext({ 'a': 3, 'b': [1, 2, 3] })), true);
+		assert.equal(ainb.evaluate(createContext({ 'a': 3, 'b': [1, 2] })), false);
+		assert.equal(ainb.evaluate(createContext({ 'a': 3 })), false);
+		assert.equal(ainb.evaluate(createContext({ 'a': 3, 'b': null })), false);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'x', 'b': ['x'] })), true);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'x', 'b': ['y'] })), false);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'x', 'b': {} })), false);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'x', 'b': { 'x': false } })), true);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'x', 'b': { 'x': true } })), true);
+		assert.equal(ainb.evaluate(createContext({ 'a': 'prototype', 'b': {} })), false);
+	});
 });

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,8 @@ STATIC_VALUES.set('isWindows', isWindows);`
`17`	`17`	`STATIC_VALUES.set('isWeb', isWeb);`
`18`	`18`	`STATIC_VALUES.set('isMacNative', isMacintosh && !isWeb);`
`19`	`19`
	`20`	`+const hasOwnProperty = Object.prototype.hasOwnProperty;`
	`21`	`+`
`20`	`22`	`export const enum ContextKeyExprType {`
`21`	`23`	`False = 0,`
`22`	`24`	`True = 1,`
`@@ -138,7 +140,7 @@ export abstract class ContextKeyExpr {`
`138`	`140`
`139`	`141`	`if (serializedOne.indexOf(' in ') >= 0) {`
`140`	`142`	`let pieces = serializedOne.split(' in ');`
`141`		`- return ContextKeyInExpr.create(pieces[0].trim(), this._deserializeValue(pieces[1], strict));`
	`143`	`+ return ContextKeyInExpr.create(pieces[0].trim(), pieces[1].trim());`
`142`	`144`	`}`
`143`	`145`
`144`	`146`	`if (/^\!\s*/.test(serializedOne)) {`
`@@ -451,8 +453,8 @@ export class ContextKeyInExpr implements IContextKeyExpression {`
`451`	`453`	`return (source.indexOf(item) >= 0);`
`452`	`454`	`}`
`453`	`455`
`454`		`- if (typeof item === 'string' && typeof source === 'object' && source !== undefined && source !== null) {`
`455`		`- return item in source;`
	`456`	`+ if (typeof item === 'string' && typeof source === 'object' && source !== null) {`
	`457`	`+ return hasOwnProperty.call(source, item);`
`456`	`458`	`}`
`457`	`459`	`return false;`
`458`	`460`	`}`
`@@ -462,7 +464,7 @@ export class ContextKeyInExpr implements IContextKeyExpression {`
`462`	`464`	`}`
`463`	`465`
`464`	`466`	`public keys(): string[] {`
`465`		`- return [this.key];`
	`467`	`+ return [this.key, this.valueKey];`
`466`	`468`	`}`
`467`	`469`
`468`	`470`	`public map(mapFnc: IContextKeyExprMapper): ContextKeyInExpr {`