Fixes microsoft#101015: Distribute OR expressions to avoid the need for introducing paranthesis

alexdima · alexdima · commit 350a3c69f5bf · 2020-06-29T10:50:06.000+02:00
diff --git a/src/vs/platform/contextkey/common/contextkey.ts b/src/vs/platform/contextkey/common/contextkey.ts
@@ -637,16 +637,7 @@ export class ContextKeyNotRegexExpr implements IContextKeyExpression {
 export class ContextKeyAndExpr implements IContextKeyExpression {
 
 	public static create(_expr: ReadonlyArray<ContextKeyExpression | null | undefined>): ContextKeyExpression | undefined {
-		const expr = ContextKeyAndExpr._normalizeArr(_expr);
-		if (expr.length === 0) {
-			return undefined;
-		}
-
-		if (expr.length === 1) {
-			return expr[0];
-		}
-
-		return new ContextKeyAndExpr(expr);
+		return ContextKeyAndExpr._normalizeArr(_expr);
 	}
 
 	public readonly type = ContextKeyExprType.And;
@@ -697,7 +688,7 @@ export class ContextKeyAndExpr implements IContextKeyExpression {
 		return true;
 	}
 
-	private static _normalizeArr(arr: ReadonlyArray<ContextKeyExpression | null | undefined>): ContextKeyExpression[] {
+	private static _normalizeArr(arr: ReadonlyArray<ContextKeyExpression | null | undefined>): ContextKeyExpression | undefined {
 		const expr: ContextKeyExpression[] = [];
 		let hasTrue = false;
 
@@ -714,29 +705,56 @@ export class ContextKeyAndExpr implements IContextKeyExpression {
 
 			if (e.type === ContextKeyExprType.False) {
 				// anything && false ==> false
-				return [ContextKeyFalseExpr.INSTANCE];
+				return ContextKeyFalseExpr.INSTANCE;
 			}
 
 			if (e.type === ContextKeyExprType.And) {
 				expr.push(...e.expr);
 				continue;
 			}
 
-			if (e.type === ContextKeyExprType.Or) {
-				// Not allowed, because we don't have parens!
-				throw new Error(`It is not allowed to have an or expression here due to lack of parens! For example "a && (b||c)" is not supported, use "(a&&b) || (a&&c)" instead.`);
-			}
-
 			expr.push(e);
 		}
 
 		if (expr.length === 0 && hasTrue) {
-			return [ContextKeyTrueExpr.INSTANCE];
+			return ContextKeyTrueExpr.INSTANCE;
+		}
+
+		if (expr.length === 0) {
+			return undefined;
+		}
+
+		if (expr.length === 1) {
+			return expr[0];
 		}
 
 		expr.sort(cmp);
 
-		return expr;
+		// We must distribute any OR expression because we don't support parens
+		// OR extensions will be at the end (due to sorting rules)
+		while (expr.length > 1) {
+			const lastElement = expr[expr.length - 1];
+			if (lastElement.type !== ContextKeyExprType.Or) {
+				break;
+			}
+			// pop the last element
+			expr.pop();
+
+			// pop the second to last element
+			const secondToLastElement = expr.pop()!;
+
+			// distribute `lastElement` over `secondToLastElement`
+			const resultElement = ContextKeyOrExpr.create(
+				lastElement.expr.map(el => ContextKeyAndExpr.create([el, secondToLastElement]))
+			);
+
+			if (resultElement) {
+				expr.push(resultElement);
+				expr.sort(cmp);
+			}
+		}
+
+		return new ContextKeyAndExpr(expr);
 	}
 
 	public serialize(): string {
diff --git a/src/vs/platform/contextkey/test/common/contextkey.test.ts b/src/vs/platform/contextkey/test/common/contextkey.test.ts
@@ -136,4 +136,18 @@ suite('ContextKeyExpr', () => {
 		testNormalize('isLinux', isLinux ? 'true' : 'false');
 		testNormalize('isWindows', isWindows ? 'true' : 'false');
 	});
+
+	test('issue #101015: distribute OR', () => {
+		function t(expr1: string, expr2: string, expected: string | undefined): void {
+			const e1 = ContextKeyExpr.deserialize(expr1);
+			const e2 = ContextKeyExpr.deserialize(expr2);
+			const actual = ContextKeyExpr.and(e1, e2)?.serialize();
+			assert.strictEqual(actual, expected);
+		}
+		t('a', 'b', 'a && b');
+		t('a || b', 'c', 'a && c || b && c');
+		t('a || b', 'c || d', 'a && c || b && c || a && d || b && d');
+		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');
+	});
 });
