Promote FunctionParameter.type to TPowerQueryType; fix isCompatible left-side Any/AnyUnion and two related bugs

package.json

@@ -1,6 +1,6 @@
 {
     "name": "@microsoft/powerquery-parser",
-    "version": "1.0.0",
+    "version": "2.0.0",
     "description": "A parser for the Power Query/M formula language.",
     "author": "Microsoft",
     "license": "MIT",

src/powerquery-parser/language/type/type.ts

@@ -263,8 +263,12 @@ export interface FunctionParameter {
     // but it's useful to have when inspecting parameters.
     readonly nameLiteral: string;
     readonly isOptional: boolean;
-    readonly isNullable: boolean;
-    readonly type: TypeKind | undefined;
+    // `undefined` means the parameter was written without an ascription (eg. `(x) => ...`),
+    // and should be treated as accepting any argument. When present, nullability is already
+    // baked into the type itself (`type.isNullable`) rather than tracked as a sibling flag —
+    // this includes the case where `isOptional` is true, since omitting an optional argument
+    // is equivalent to passing `null`.
+    readonly type: TPowerQueryType | undefined;
 }
 
 // -------------------------------------------

src/powerquery-parser/language/type/typeUtils/isCompatible.ts

@@ -13,6 +13,18 @@ import { TypeUtilsTraceConstant } from "./typeTraceConstant";
 // `Type.AnyInstance is compatible with Type.TextInstance` -> false
 // `Type.NullInstance is compatible with Type.AnyNonNull` -> false
 // `Type.TextInstance is compatible with Type.AnyUnion([Type.TextInstance, Type.NumberInstance])` -> true
+//
+// Tri-state contract:
+//   `true`      left is definitely compatible with right.
+//   `false`     left is definitely NOT compatible with right. This is a hard claim and requires
+//               enumerated evidence of a mismatch (eg. a concrete kind, or every member of a union).
+//   `undefined` indeterminate. There isn't enough information to make a hard claim either way.
+//               `Unknown` on either side means insufficient information was gathered.
+//               `Any` on the left means the value is unconstrained (top type), not undetermined:
+//               there is no enumerated evidence that a mismatch exists, so a hard `false` would be
+//               an unsupported claim. This is why bare `Any` and `Unknown` both resolve to
+//               `undefined` on the left despite meaning different things: `Any` accepts everything
+//               on the right (`right.kind === Any` -> `true`), `Unknown` still doesn't.
 export function isCompatible(
     left: Type.TPowerQueryType,
     right: Type.TPowerQueryType,
@@ -38,6 +50,20 @@ export function isCompatible(
         result = false;
     } else if (left.kind === Type.TypeKind.Null && right.isNullable) {
         result = true;
+    } else if (left.kind === Type.TypeKind.Any) {
+        if (left.extendedKind === Type.ExtendedTypeKind.AnyUnion) {
+            result = isAnyUnionCompatibleWith(left, right, traceManager, trace.id);
+        } else if (right.kind === Type.TypeKind.AnyNonNull) {
+            // Bare `Any` reaching here is already known non-nullable (see the nullability
+            // check above), so it trivially satisfies "anything but null."
+            result = true;
+        } else if (right.kind === Type.TypeKind.Null) {
+            // Bare `Any` is never exactly `Null` regardless of its concrete kind.
+            result = false;
+        } else {
+            // Bare `Any` is top: no enumerated evidence of a mismatch, so no hard claim.
+            result = undefined;
+        }
     } else {
         switch (right.kind) {
             case Type.TypeKind.Action:
@@ -115,25 +141,6 @@ export function isCompatibleWithFunctionSignature(
     return isEqualFunctionSignature(left, right);
 }
 
-export function isCompatibleWithFunctionParameter(
-    left: Type.TPowerQueryType | undefined,
-    right: Type.FunctionParameter,
-): boolean {
-    if (left === undefined) {
-        return right.isOptional;
-    } else if (left.isNullable && !right.isNullable) {
-        return false;
-    } else {
-        return (
-            !right.type ||
-            right.type === Type.TypeKind.Any ||
-            left.kind === Type.TypeKind.Any ||
-            (left.kind === Type.TypeKind.Null && right.isNullable) ||
-            left.kind === right.type
-        );
-    }
-}
-
 function isCompatibleWithAny(
     left: Type.TPowerQueryType,
     right: Type.TAny,
@@ -154,7 +161,7 @@ function isCompatibleWithAny(
             break;
 
         case Type.ExtendedTypeKind.AnyUnion:
-            result = isCompatibleWithAnyUnion(left, right, traceManager, trace.id);
+            result = isRightAnyUnionCompatible(left, right, traceManager, trace.id);
             break;
 
         default:
@@ -166,29 +173,73 @@ function isCompatibleWithAny(
     return result;
 }
 
-function isCompatibleWithAnyUnion(
+// `some`-semantics: is `left` compatible with at least one member of the union on the *right*?
+// This is the mirror image of `isAnyUnionCompatibleWith`, which handles a union on the *left*
+// with `all`-semantics. Do not merge the two; they answer different questions.
+function isRightAnyUnionCompatible(
     left: Type.TPowerQueryType,
     right: Type.AnyUnion,
     traceManager: TraceManager,
     correlationId: number,
 ): boolean | undefined {
     const trace: Trace = traceManager.entry(
         TypeUtilsTraceConstant.IsCompatible,
-        isCompatibleWithAnyUnion.name,
+        isRightAnyUnionCompatible.name,
         correlationId,
     );
 
+    let sawIndeterminate: boolean = false;
+
     for (const subtype of right.unionedTypePairs) {
-        if (isCompatible(left, subtype, traceManager, trace.id)) {
+        const memberResult: boolean | undefined = isCompatible(left, subtype, traceManager, trace.id);
+
+        if (memberResult === true) {
             trace.exit();
 
             return true;
+        } else if (memberResult === undefined) {
+            sawIndeterminate = true;
+        }
+    }
+
+    trace.exit();
+
+    return sawIndeterminate ? undefined : false;
+}
+
+// `all`-semantics: is every member of the union on the *left* compatible with `right`?
+// A single definite incompatibility (`false`) makes the whole union incompatible. A member that
+// is merely indeterminate (`undefined`) downgrades the result to indeterminate rather than
+// invalidating it outright, per the tri-state contract above.
+function isAnyUnionCompatibleWith(
+    left: Type.AnyUnion,
+    right: Type.TPowerQueryType,
+    traceManager: TraceManager,
+    correlationId: number,
+): boolean | undefined {
+    const trace: Trace = traceManager.entry(
+        TypeUtilsTraceConstant.IsCompatible,
+        isAnyUnionCompatibleWith.name,
+        correlationId,
+    );
+
+    let sawIndeterminate: boolean = false;
+
+    for (const member of left.unionedTypePairs) {
+        const memberResult: boolean | undefined = isCompatible(member, right, traceManager, trace.id);
+
+        if (memberResult === false) {
+            trace.exit();
+
+            return false;
+        } else if (memberResult === undefined) {
+            sawIndeterminate = true;
         }
     }
 
     trace.exit();
 
-    return false;
+    return sawIndeterminate ? undefined : true;
 }
 
 function isCompatibleWithDefinedList(
@@ -887,9 +938,11 @@ function isDefinedListTypeCompatibleWithListType(
         (itemType: Type.TPowerQueryType) => isCompatible(itemType, listType.itemType, traceManager, trace.id),
     );
 
-    const result: boolean = Boolean(
-        itemTypeCompatabilities.find((value: boolean | undefined) => value === undefined || value === false),
-    );
+    // Every item's type must be a definite (`true`) compatibility match for the defined list to
+    // be compatible with the list type. An indeterminate (`undefined`) member is not proof of
+    // compatibility, so it's treated the same as a definite mismatch here (this function's
+    // callers only deal in `boolean`, not the tri-state result).
+    const result: boolean = itemTypeCompatabilities.every((value: boolean | undefined) => value === true);
 
     trace.exit();
 

src/powerquery-parser/language/type/typeUtils/isEqualType.ts

@@ -22,12 +22,13 @@ export function isEqualType(left: Type.TPowerQueryType, right: Type.TPowerQueryT
 }
 
 export function isEqualFunctionParameter(left: Type.FunctionParameter, right: Type.FunctionParameter): boolean {
-    return (
-        left.nameLiteral === right.nameLiteral &&
-        left.isNullable === right.isNullable &&
-        left.isOptional === right.isOptional &&
-        left.type === right.type
-    );
+    if (left.nameLiteral !== right.nameLiteral || left.isOptional !== right.isOptional) {
+        return false;
+    } else if (left.type === undefined || right.type === undefined) {
+        return left.type === right.type;
+    } else {
+        return isEqualType(left.type, right.type);
+    }
 }
 
 export function isEqualFunctionSignature(

src/powerquery-parser/language/type/typeUtils/nameOf.ts

@@ -131,12 +131,10 @@ export function nameOfFunctionParameter(
         partial += " optional";
     }
 
-    if (parameter.isNullable) {
-        partial += " nullable";
-    }
-
+    // Nullability is already baked into `parameter.type` (see the FunctionParameter doc comment),
+    // so `nameOf` below applies the `nullable` prefix on its own when required.
     if (parameter.type !== undefined) {
-        partial += ` ${nameOfTypeKind(parameter.type)}`;
+        partial += ` ${nameOf(parameter.type, traceManager, trace.id)}`;
     } else {
         partial += ` ${nameOfTypeKind(Type.TypeKind.Any)}`;
     }

src/powerquery-parser/language/type/typeUtils/typeCheck.ts

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { isCompatible, isCompatibleWithFunctionParameter } from "./isCompatible";
 import { Trace, TraceManager } from "../../../common/trace";
 import { ArrayUtils } from "../../../common";
+import { isCompatible } from "./isCompatible";
 import { isEqualFunctionParameter } from "./isEqualType";
 import { Type } from "..";
 import { TypeUtilsTraceConstant } from "./typeTraceConstant";
@@ -35,7 +35,13 @@ export type CheckedDefinedTable = IChecked<string, DefinedTableMismatch>;
 
 export type CheckedFunctionSignature = IChecked<number, FunctionSignatureMismatch>;
 
-export type CheckedInvocation = IChecked<number, InvocationMismatch>;
+export interface CheckedInvocation extends IChecked<number, InvocationMismatch> {
+    // Arguments whose compatibility with the parameter could not be determined (`isCompatible`
+    // returned `undefined`). Per the tri-state contract, indeterminate is not grounds for
+    // treating the invocation as invalid — callers that only care about hard failures can
+    // continue to read `.invalid` and get that behaviour for free.
+    readonly indeterminate: ReadonlyArray<number>;
+}
 
 export type TMismatch =
     | DefinedFunctionMismatch
@@ -123,21 +129,35 @@ export function typeCheckInvocation(
 
     const validArgs: number[] = [];
     const missingArgs: number[] = [];
+    const indeterminateArgs: number[] = [];
     const invalidArgs: Map<number, InvocationMismatch> = new Map();
 
     for (let index: number = 0; index < numParameters; index += 1) {
         const arg: Type.TPowerQueryType | undefined = args[index];
         const parameter: Type.FunctionParameter = ArrayUtils.assertGet(parameters, index);
 
-        if (isCompatibleWithFunctionParameter(arg, parameter)) {
+        if (arg === undefined) {
+            if (parameter.isOptional) {
+                validArgs.push(index);
+            } else {
+                missingArgs.push(index);
+            }
+        } else if (parameter.type === undefined) {
+            // A parameter without an ascription accepts any argument.
             validArgs.push(index);
-        } else if (arg !== undefined) {
-            invalidArgs.set(index, {
-                expected: parameter,
-                actual: arg,
-            });
         } else {
-            missingArgs.push(index);
+            const compatibility: boolean | undefined = isCompatible(arg, parameter.type, traceManager, trace.id);
+
+            if (compatibility === true) {
+                validArgs.push(index);
+            } else if (compatibility === false) {
+                invalidArgs.set(index, {
+                    expected: parameter,
+                    actual: arg,
+                });
+            } else {
+                indeterminateArgs.push(index);
+            }
         }
     }
 
@@ -146,6 +166,7 @@ export function typeCheckInvocation(
         invalid: invalidArgs,
         extraneous: extraneousArgs,
         missing: missingArgs,
+        indeterminate: indeterminateArgs,
     };
 
     trace.exit();
@@ -171,7 +192,7 @@ export function typeCheckListWithListType(
     const valueElements: ReadonlyArray<Type.TPowerQueryType> = valueType.elements;
 
     for (const [element, index] of ArrayUtils.enumerate(valueElements)) {
-        if (isCompatible(element, schemaItemType, traceManager, trace.id)) {
+        if (isCompatible(element, schemaItemType, traceManager, trace.id) === true) {
             validArgs.push(index);
         } else {
             invalidArgs.set(index, {
@@ -307,7 +328,7 @@ function typeCheckRecordOrTable(
         const schemaValueType: Type.TPowerQueryType | undefined = schemaFields.get(key);
 
         if (schemaValueType !== undefined) {
-            if (isCompatible(type, schemaValueType, traceManager, trace.id)) {
+            if (isCompatible(type, schemaValueType, traceManager, trace.id) === true) {
                 validFields.push(key);
             } else {
                 mismatches.set(key, {