Fix a few bugs in comparison expressions

Author: J-Fields

src/error.ts

@@ -52,6 +52,7 @@ export enum ErrorCode {
   CanOnlyIndexAListDictionaryOrBlob = 689,
   CanOnlyCompareListWithList = 691,
   InvalidOperationForList = 692,
+  InvalidOperationForFuncrefs = 694,
   CannotIndexAFuncref = 695,
   UnknownFunction_funcref = 700,
   InvalidTypeForLen = 701,
@@ -87,6 +88,8 @@ export enum ErrorCode {
   SecondArgumentOfFunction = 923,
   BlobLiteralShouldHaveAnEvenNumberOfHexCharacters = 973,
   UsingABlobAsANumber = 974,
+  CanOnlyCompareBlobWithBlob = 977,
+  InvalidOperationForBlob = 978,
   CannotModifyExistingVariable = 995,
   CannotLockARegister = 996,
 }
@@ -141,6 +144,7 @@ export const ErrorMessage: IErrorMessage = {
   689: 'Can only index a List, Dictionary or Blob',
   691: 'Can only compare List with List',
   692: 'Invalid operation for List',
+  694: 'Invalid operation for Funcrefs',
   695: 'Cannot index a Funcref',
   700: 'Unknown function',
   701: 'Invalid type for len()',
@@ -176,6 +180,8 @@ export const ErrorMessage: IErrorMessage = {
   923: 'Second argument of function() must be a list or a dict',
   973: 'Blob literal should have an even number of hex characters',
   974: 'Using a Blob as a Number',
+  977: 'Can only compare Blob with Blob',
+  978: 'Invalid operation for Blob',
   995: 'Cannot modify existing variable',
   996: 'Cannot lock a register',
 };

src/vimscript/expression/evaluate.ts

@@ -247,12 +247,14 @@ export class EvaluationContext {
           toInt(this.evaluate(expression.if)) !== 0 ? expression.then : expression.else,
         );
       case 'comparison':
+        const _lhs = this.evaluate(expression.lhs);
+        const _rhs = this.evaluate(expression.rhs);
         return bool(
           this.evaluateComparison(
             expression.operator,
             expression.matchCase ?? configuration.ignorecase,
-            expression.lhs,
-            expression.rhs,
+            _lhs,
+            _rhs,
           ),
         );
       default: {
@@ -511,81 +513,79 @@ export class EvaluationContext {
   private evaluateComparison(
     operator: ComparisonOp,
     matchCase: boolean,
-    lhsExpr: Expression,
-    rhsExpr: Expression,
+    lhs: Value,
+    rhs: Value,
   ): boolean {
     switch (operator) {
       case '==':
-        return this.evaluateBasicComparison('==', matchCase, lhsExpr, rhsExpr);
+        return this.evaluateBasicComparison('==', matchCase, lhs, rhs);
       case '!=':
-        return !this.evaluateBasicComparison('==', matchCase, lhsExpr, rhsExpr);
+        return !this.evaluateBasicComparison('==', matchCase, lhs, rhs);
       case '>':
-        return this.evaluateBasicComparison('>', matchCase, lhsExpr, rhsExpr);
+        return this.evaluateBasicComparison('>', matchCase, lhs, rhs);
       case '>=':
         return (
-          this.evaluateBasicComparison('>', matchCase, lhsExpr, rhsExpr) ||
-          this.evaluateBasicComparison('==', matchCase, lhsExpr, rhsExpr)
+          this.evaluateBasicComparison('>', matchCase, lhs, rhs) ||
+          this.evaluateBasicComparison('==', matchCase, lhs, rhs)
         );
       case '<':
-        return this.evaluateBasicComparison('>', matchCase, rhsExpr, lhsExpr);
+        return this.evaluateBasicComparison('>', matchCase, rhs, lhs);
       case '<=':
-        return !this.evaluateBasicComparison('>', matchCase, lhsExpr, rhsExpr);
+        return !this.evaluateBasicComparison('>', matchCase, lhs, rhs);
       case '=~':
-        return this.evaluateBasicComparison('=~', matchCase, lhsExpr, rhsExpr);
+        return this.evaluateBasicComparison('=~', matchCase, lhs, rhs);
       case '!~':
-        return !this.evaluateBasicComparison('=~', matchCase, lhsExpr, rhsExpr);
+        return !this.evaluateBasicComparison('=~', matchCase, lhs, rhs);
       case 'is':
-        return this.evaluateBasicComparison('is', matchCase, lhsExpr, rhsExpr);
+        return this.evaluateBasicComparison('is', matchCase, lhs, rhs);
       case 'isnot':
-        return !this.evaluateBasicComparison('is', matchCase, lhsExpr, rhsExpr);
+        return !this.evaluateBasicComparison('is', matchCase, lhs, rhs);
     }
   }
 
   private evaluateBasicComparison(
     operator: '==' | '>' | '=~' | 'is',
     matchCase: boolean,
-    lhsExpr: Expression,
-    rhsExpr: Expression,
+    lhs: Value,
+    rhs: Value,
     topLevel: boolean = true,
   ): boolean {
-    if (operator === 'is' && lhsExpr.type !== rhsExpr.type) {
+    if (operator === 'is' && lhs.type !== rhs.type) {
       return false;
     }
 
-    if (lhsExpr.type === 'list') {
-      if (rhsExpr.type === 'list') {
+    if (lhs.type === 'list') {
+      if (rhs.type === 'list') {
         switch (operator) {
           case '==':
+            const rhsItems = rhs.items;
             return (
-              lhsExpr.items.length === rhsExpr.items.length &&
-              lhsExpr.items.every((left, idx) =>
-                this.evaluateBasicComparison('==', matchCase, left, rhsExpr.items[idx], false),
+              lhs.items.length === rhsItems.length &&
+              lhs.items.every((left, idx) =>
+                this.evaluateBasicComparison('==', matchCase, left, rhsItems[idx], false),
               )
             );
           case 'is':
-            return lhsExpr.items === rhsExpr.items;
+            return lhs.items === rhs.items;
           default:
             throw VimError.fromCode(ErrorCode.InvalidOperationForList);
         }
       } else {
         throw VimError.fromCode(ErrorCode.CanOnlyCompareListWithList);
       }
-    } else if (rhsExpr.type === 'list') {
+    } else if (rhs.type === 'list') {
       throw VimError.fromCode(ErrorCode.CanOnlyCompareListWithList);
-    } else if (lhsExpr.type === 'dictionary') {
-      if (rhsExpr.type === 'dictionary') {
-        const [lhs, rhs] = [this.evaluate(lhsExpr), this.evaluate(rhsExpr)] as [
-          DictionaryValue,
-          DictionaryValue,
-        ];
+    } else if (lhs.type === 'dict_val') {
+      if (rhs.type === 'dict_val') {
         switch (operator) {
           case '==':
+            const rhsItems = rhs.items;
             return (
-              lhs.items.size === rhs.items.size &&
+              lhs.items.size === rhsItems.size &&
               [...lhs.items.entries()].every(
                 ([key, value]) =>
-                  rhs.items.has(key) &&
-                  this.evaluateBasicComparison('==', matchCase, value, rhs.items.get(key)!, false),
+                  rhsItems.has(key) &&
+                  this.evaluateBasicComparison('==', matchCase, value, rhsItems.get(key)!, false),
               )
             );
           case 'is':
@@ -596,14 +596,43 @@ export class EvaluationContext {
       } else {
         throw VimError.fromCode(ErrorCode.CanOnlyCompareDictionaryWithDictionary);
       }
-    } else if (rhsExpr.type === 'dictionary') {
+    } else if (rhs.type === 'dict_val') {
       throw VimError.fromCode(ErrorCode.CanOnlyCompareDictionaryWithDictionary);
+    } else if (lhs.type === 'funcref') {
+      if (rhs.type === 'funcref') {
+        switch (operator) {
+          case '==':
+            return lhs.name === rhs.name && lhs.dict === rhs.dict;
+          case 'is':
+            return lhs === rhs;
+          default:
+            throw VimError.fromCode(ErrorCode.InvalidOperationForFuncrefs);
+        }
+      } else {
+        return false;
+      }
+    } else if (rhs.type === 'funcref') {
+      return false;
+    } else if (lhs.type === 'blob') {
+      if (rhs.type === 'blob') {
+        switch (operator) {
+          case '==':
+            const [_lhs, _rhs] = [new Uint8Array(lhs.data), new Uint8Array(rhs.data)];
+            return _lhs.length === _rhs.length && _lhs.every((byte, idx) => byte === _rhs[idx]);
+          case 'is':
+            return lhs.data === rhs.data;
+          default:
+            throw VimError.fromCode(ErrorCode.InvalidOperationForBlob);
+        }
+      } else {
+        throw VimError.fromCode(ErrorCode.CanOnlyCompareBlobWithBlob);
+      }
+    } else if (rhs.type === 'blob') {
+      throw VimError.fromCode(ErrorCode.CanOnlyCompareBlobWithBlob);
     } else {
-      let [lhs, rhs] = [this.evaluate(lhsExpr), this.evaluate(rhsExpr)] as [
-        NumberValue | StringValue,
-        NumberValue | StringValue,
-      ];
-      if (lhs.type === 'number' || rhs.type === 'number') {
+      if (lhs.type === 'float' || rhs.type === 'float') {
+        [lhs, rhs] = [float(toFloat(lhs)), float(toFloat(rhs))];
+      } else if (lhs.type === 'number' || rhs.type === 'number') {
         if (topLevel) {
           // Strings are automatically coerced to numbers, except within a list/dict
           // i.e. 4 == "4" but [4] != ["4"]

test/vimscript/expression.test.ts

@@ -426,12 +426,19 @@ suite('Vimscript expressions', () => {
       exprTest("'abc' ==? 'Abc'", { value: bool(true) });
     });
 
-    suite('Misc', () => {
+    suite('Different types', () => {
       exprTest("4 == '4'", { value: bool(true) });
       exprTest("4 is '4'", { value: bool(false) });
       exprTest('0 is []', { value: bool(false) });
       exprTest('0 is {}', { value: bool(false) });
       exprTest('[4] == ["4"]', { value: bool(false) });
+      exprTest('3.2 > 3', { value: bool(true) });
+      exprTest('5 == [5]', { error: ErrorCode.CanOnlyCompareListWithList });
+      exprTest('[] == {}', { error: ErrorCode.CanOnlyCompareListWithList });
+      exprTest('{} == []', { error: ErrorCode.CanOnlyCompareListWithList });
+      exprTest('{} == 10', { error: ErrorCode.CanOnlyCompareDictionaryWithDictionary });
+      exprTest('0 == 0z00', { error: ErrorCode.CanOnlyCompareBlobWithBlob });
+      exprTest('2 == function("abs")', { value: bool(false) });
     });
   });