From 4bc028c36a4dec1ccb3a3afa652b2e6e36bd940d Mon Sep 17 00:00:00 2001 From: "Jordan Bolton (jobolton)" Date: Mon, 24 Aug 2026 14:10:07 -0500 Subject: [PATCH 1/4] Promote FunctionParameter.type to TPowerQueryType, fix isCompatible left-side Any/AnyUnion handling - isCompatible: bare Any and AnyUnion on the left now resolve correctly instead of short-circuiting to true/false (fixes several latent isValidInvocation bugs around indeterminate results). - FunctionParameter.type is now TPowerQueryType | undefined instead of TypeKind | undefined; isNullable is folded into type (optional implies nullable). - typeCheckInvocation/CheckedInvocation gained an indeterminate bucket for arguments that can't be definitively ruled valid or invalid. - Migrated nameOf/isEqualType/typeCheck consumers and all affected parser test literals to the new model. - Removed isValidInvocation (dead code, superseded by typeCheckInvocation, zero callers in this repo or powerquery-language-services). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23 --- src/powerquery-parser/language/type/type.ts | 8 +- .../language/type/typeUtils/isCompatible.ts | 81 +++++++++++----- .../language/type/typeUtils/isEqualType.ts | 13 +-- .../language/type/typeUtils/nameOf.ts | 8 +- .../language/type/typeUtils/typeCheck.ts | 43 ++++++--- .../language/type/typeUtils/typeUtils.ts | 95 +++++-------------- .../language/typeUtils/isCompatible.test.ts | 69 ++++++++++++++ .../language/typeUtils/isEqualType.test.ts | 3 +- .../language/typeUtils/nameOf.test.ts | 26 ++--- .../language/typeUtils/typeCheck.test.ts | 62 ++++++------ .../language/typeUtils/typeUtils.test.ts | 24 ++--- 11 files changed, 250 insertions(+), 182 deletions(-) diff --git a/src/powerquery-parser/language/type/type.ts b/src/powerquery-parser/language/type/type.ts index ad4745ce..fd498ae6 100644 --- a/src/powerquery-parser/language/type/type.ts +++ b/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; } // ------------------------------------------- diff --git a/src/powerquery-parser/language/type/typeUtils/isCompatible.ts b/src/powerquery-parser/language/type/typeUtils/isCompatible.ts index 3e19314a..2e821373 100644 --- a/src/powerquery-parser/language/type/typeUtils/isCompatible.ts +++ b/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,12 @@ export function isCompatible( result = false; } else if (left.kind === Type.TypeKind.Null && right.isNullable) { result = true; + } else if (left.kind === Type.TypeKind.Any) { + result = + left.extendedKind === Type.ExtendedTypeKind.AnyUnion + ? isAnyUnionCompatibleWith(left, right, traceManager, trace.id) + : // Bare `Any` is top: no enumerated evidence of a mismatch, so no hard claim. + undefined; } else { switch (right.kind) { case Type.TypeKind.Action: @@ -115,25 +133,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 +153,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,7 +165,10 @@ 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, @@ -174,7 +176,7 @@ function isCompatibleWithAnyUnion( ): boolean | undefined { const trace: Trace = traceManager.entry( TypeUtilsTraceConstant.IsCompatible, - isCompatibleWithAnyUnion.name, + isRightAnyUnionCompatible.name, correlationId, ); @@ -191,6 +193,41 @@ function isCompatibleWithAnyUnion( return 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 sawIndeterminate ? undefined : true; +} + function isCompatibleWithDefinedList( left: Type.TPowerQueryType, right: Type.DefinedList, diff --git a/src/powerquery-parser/language/type/typeUtils/isEqualType.ts b/src/powerquery-parser/language/type/typeUtils/isEqualType.ts index ddaccbbf..266da2a8 100644 --- a/src/powerquery-parser/language/type/typeUtils/isEqualType.ts +++ b/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( diff --git a/src/powerquery-parser/language/type/typeUtils/nameOf.ts b/src/powerquery-parser/language/type/typeUtils/nameOf.ts index 78d557eb..a254d5f7 100644 --- a/src/powerquery-parser/language/type/typeUtils/nameOf.ts +++ b/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)}`; } diff --git a/src/powerquery-parser/language/type/typeUtils/typeCheck.ts b/src/powerquery-parser/language/type/typeUtils/typeCheck.ts index 231d1189..e503a413 100644 --- a/src/powerquery-parser/language/type/typeUtils/typeCheck.ts +++ b/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; export type CheckedFunctionSignature = IChecked; -export type CheckedInvocation = IChecked; +export interface CheckedInvocation extends IChecked { + // 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; +} export type TMismatch = | DefinedFunctionMismatch @@ -123,21 +129,35 @@ export function typeCheckInvocation( const validArgs: number[] = []; const missingArgs: number[] = []; + const indeterminateArgs: number[] = []; const invalidArgs: Map = 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 = 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, { diff --git a/src/powerquery-parser/language/type/typeUtils/typeUtils.ts b/src/powerquery-parser/language/type/typeUtils/typeUtils.ts index 228b4198..79ff4e7d 100644 --- a/src/powerquery-parser/language/type/typeUtils/typeUtils.ts +++ b/src/powerquery-parser/language/type/typeUtils/typeUtils.ts @@ -1,16 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { ArrayUtils, Assert } from "../../../common"; import { Ast, AstUtils } from "../.."; import { NodeIdMap, NodeIdMapUtils, ParseContext, XorNode, XorNodeKind } from "../../../parser"; -import { Trace, TraceManager } from "../../../common/trace"; -import { isCompatible } from "./isCompatible"; +import { Assert } from "../../../common"; import { isEqualType } from "./isEqualType"; import { primitiveType } from "./factories"; import { Type } from ".."; import { typeKindFromPrimitiveTypeConstantKind } from "./primitive"; -import { TypeUtilsTraceConstant } from "./typeTraceConstant"; export function typeKindFromLiteralKind(literalKind: Ast.LiteralKind): Type.TypeKind { switch (literalKind) { @@ -74,51 +71,6 @@ export function isTypeKind(text: string): text is Type.TypeKind { } } -export function isValidInvocation( - functionType: Type.DefinedFunction, - args: ReadonlyArray, - traceManager: TraceManager, - correlationId: number | undefined, -): boolean { - const trace: Trace = traceManager.entry(TypeUtilsTraceConstant.TypeUtils, isValidInvocation.name, correlationId); - - // You can't provide more arguments than are on the function signature. - if (args.length > functionType.parameters.length) { - return false; - } - - const parameters: ReadonlyArray = functionType.parameters; - const numParameters: number = parameters.length; - - for (let index: number = 1; index < numParameters; index += 1) { - const parameter: Type.FunctionParameter = ArrayUtils.assertGet(parameters, index); - const argType: Type.TPowerQueryType | undefined = args[index]; - - if (argType !== undefined) { - const parameterType: Type.TPowerQueryType = primitiveType( - parameter.isNullable, - Assert.asDefined(parameter.type), - ); - - if (!isCompatible(argType, parameterType, traceManager, trace.id)) { - trace.exit(); - - return false; - } - } - - if (!parameter.isOptional) { - trace.exit(); - - return false; - } - } - - trace.exit(); - - return true; -} - export function inspectParameter( nodeIdMapCollection: NodeIdMap.Collection, parameter: XorNode, @@ -136,39 +88,40 @@ export function inspectParameter( } function inspectAstParameter(node: Ast.TParameter): Type.FunctionParameter { - let isNullable: boolean; - let type: Type.TypeKind | undefined; + const isOptional: boolean = node.optionalConstant !== undefined; + let type: Type.TPowerQueryType | undefined; const parameterType: Ast.TParameterType | undefined = node.parameterType; if (parameterType !== undefined) { + let simplified: AstUtils.SimplifiedType; + switch (parameterType.kind) { - case Ast.NodeKind.AsNullablePrimitiveType: { - const simplified: AstUtils.SimplifiedType = AstUtils.simplifyAsNullablePrimitiveType(parameterType); - isNullable = simplified.isNullable; - type = typeKindFromPrimitiveTypeConstantKind(simplified.primitiveTypeConstantKind); + case Ast.NodeKind.AsNullablePrimitiveType: + simplified = AstUtils.simplifyAsNullablePrimitiveType(parameterType); break; - } - case Ast.NodeKind.AsType: { - const simplified: AstUtils.SimplifiedType = AstUtils.simplifyType(parameterType.paired); - isNullable = simplified.isNullable; - type = typeKindFromPrimitiveTypeConstantKind(simplified.primitiveTypeConstantKind); + case Ast.NodeKind.AsType: + simplified = AstUtils.simplifyType(parameterType.paired); break; - } default: throw Assert.isNever(parameterType); } + + // D2: an omitted optional argument is equivalent to passing `null`, so `isOptional` + // implies nullable regardless of how the parameter was ascribed in source. + type = primitiveType( + simplified.isNullable || isOptional, + typeKindFromPrimitiveTypeConstantKind(simplified.primitiveTypeConstantKind), + ); } else { - isNullable = true; type = undefined; } return { nameLiteral: node.name.literal, - isNullable, - isOptional: node.optionalConstant !== undefined, + isOptional, type, }; } @@ -177,8 +130,7 @@ function inspectContextParameter( nodeIdMapCollection: NodeIdMap.Collection, parameter: ParseContext.Node, ): Type.FunctionParameter | undefined { - let isNullable: boolean; - let type: Type.TypeKind | undefined; + let type: Type.TPowerQueryType | undefined; const name: Ast.Identifier | undefined = NodeIdMapUtils.nthChildAstChecked( nodeIdMapCollection, @@ -209,17 +161,20 @@ function inspectContextParameter( if (parameterType !== undefined) { const simplified: AstUtils.SimplifiedType = AstUtils.simplifyAsNullablePrimitiveType(parameterType); - isNullable = simplified.isNullable; - type = typeKindFromPrimitiveTypeConstantKind(simplified.primitiveTypeConstantKind); + + // D2: an omitted optional argument is equivalent to passing `null`, so `isOptional` + // implies nullable regardless of how the parameter was ascribed in source. + type = primitiveType( + simplified.isNullable || isOptional, + typeKindFromPrimitiveTypeConstantKind(simplified.primitiveTypeConstantKind), + ); } else { - isNullable = true; type = undefined; } return { nameLiteral: name.literal, isOptional, - isNullable, type, }; } diff --git a/src/test/libraryTest/language/typeUtils/isCompatible.test.ts b/src/test/libraryTest/language/typeUtils/isCompatible.test.ts index c811c4c9..24f83911 100644 --- a/src/test/libraryTest/language/typeUtils/isCompatible.test.ts +++ b/src/test/libraryTest/language/typeUtils/isCompatible.test.ts @@ -114,6 +114,75 @@ describe(`TypeUtils.isCompatible`, () => { }); }); + // Left-side `Any` / `AnyUnion` handling (D1). See the tri-state contract documented above + // `isCompatible` in isCompatible.ts. + describe(`AnyUnion on the left`, () => { + it(`all members compatible -> true`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([ + TypeUtils.numberLiteral(false, `1`), + TypeUtils.numberLiteral(false, `2`), + ]); + + expect(noopIsCompatible(left, Type.NumberInstance)).to.equal(true, undefined); + }); + + it(`one member incompatible -> false`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([ + TypeUtils.textLiteral(false, `"x"`), + TypeUtils.numberLiteral(false, `1`), + ]); + + expect(noopIsCompatible(left, Type.NumberInstance)).to.equal(false, undefined); + }); + + it(`one member indeterminate, rest compatible -> undefined`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([ + Type.UnknownInstance, + TypeUtils.numberLiteral(false, `1`), + ]); + + expect(noopIsCompatible(left, Type.NumberInstance)).to.equal(undefined, undefined); + }); + + it(`right is bare any -> true (right fast path wins)`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([Type.TextInstance, Type.NumberInstance]); + expect(noopIsCompatible(left, Type.AnyInstance)).to.equal(true, undefined); + }); + + it(`nullable member against non-nullable right -> false`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([Type.NullInstance, Type.NumberInstance]); + expect(noopIsCompatible(left, Type.NumberInstance)).to.equal(false, undefined); + }); + + it(`nullable member against nullable right -> true`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([Type.NullInstance, Type.NumberInstance]); + expect(noopIsCompatible(left, Type.NullableNumberInstance)).to.equal(true, undefined); + }); + + it(`${Type.TypeKind.None} member is a hard mismatch even against a required parameter`, () => { + const left: Type.TPowerQueryType = noopCreateAnyUnion([ + TypeUtils.primitiveType(false, Type.TypeKind.None), + Type.NumberInstance, + ]); + + expect(noopIsCompatible(left, Type.NumberInstance)).to.equal(false, undefined); + }); + }); + + describe(`bare any on the left`, () => { + it(`any -> number is undefined (absence of constraint, not a hard mismatch)`, () => { + expect(noopIsCompatible(Type.AnyInstance, Type.NumberInstance)).to.equal(undefined, undefined); + }); + + it(`any -> any is true (right-side fast path wins)`, () => { + expect(noopIsCompatible(Type.AnyInstance, Type.AnyInstance)).to.equal(true, undefined); + }); + + it(`number -> any is true (unchanged)`, () => { + expect(noopIsCompatible(Type.NumberInstance, Type.AnyInstance)).to.equal(true, undefined); + }); + }); + describe(`${Type.ExtendedTypeKind.DefinedList}`, () => { describe(`identity`, () => { it(`empty`, () => { diff --git a/src/test/libraryTest/language/typeUtils/isEqualType.test.ts b/src/test/libraryTest/language/typeUtils/isEqualType.test.ts index 1a69e9c9..164c0057 100644 --- a/src/test/libraryTest/language/typeUtils/isEqualType.test.ts +++ b/src/test/libraryTest/language/typeUtils/isEqualType.test.ts @@ -25,9 +25,8 @@ describe(`TypeUtils.isEqualType`, () => { false, [ { - isNullable: false, isOptional: true, - type: Type.TypeKind.Text, + type: Type.TextInstance, nameLiteral: `x`, }, ], diff --git a/src/test/libraryTest/language/typeUtils/nameOf.test.ts b/src/test/libraryTest/language/typeUtils/nameOf.test.ts index 1647325a..2e5d2791 100644 --- a/src/test/libraryTest/language/typeUtils/nameOf.test.ts +++ b/src/test/libraryTest/language/typeUtils/nameOf.test.ts @@ -270,13 +270,11 @@ describe(`TypeUtils.nameOf`, () => { false, [ { - isNullable: false, isOptional: false, type: undefined, nameLiteral: "x", }, { - isNullable: false, isOptional: true, type: undefined, nameLiteral: "y", @@ -295,27 +293,23 @@ describe(`TypeUtils.nameOf`, () => { false, [ { - isNullable: false, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param1", }, { - isNullable: true, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param2", }, { - isNullable: false, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param3", }, { - isNullable: true, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param4", }, ], @@ -477,27 +471,23 @@ describe(`TypeUtils.nameOf`, () => { false, [ { - isNullable: false, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param1", }, { - isNullable: true, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param2", }, { - isNullable: false, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param3", }, { - isNullable: true, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param4", }, ], diff --git a/src/test/libraryTest/language/typeUtils/typeCheck.test.ts b/src/test/libraryTest/language/typeUtils/typeCheck.test.ts index 88a4ceba..0c7f8bef 100644 --- a/src/test/libraryTest/language/typeUtils/typeCheck.test.ts +++ b/src/test/libraryTest/language/typeUtils/typeCheck.test.ts @@ -66,6 +66,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [0], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -78,9 +79,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -94,6 +94,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [0], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -106,9 +107,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: true, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -122,19 +122,19 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); }); - it(`type === null translates to any`, () => { + it(`an un-ascribed parameter accepts any argument`, () => { const args: ReadonlyArray = [Language.Type.NumberInstance]; const definedFunction: Language.Type.DefinedFunction = TypeUtils.definedFunction( false, [ { - isNullable: true, isOptional: false, type: undefined, nameLiteral: "foo", @@ -150,6 +150,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -162,9 +163,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Any, + type: Language.Type.AnyInstance, nameLiteral: "foo", }, ], @@ -178,21 +178,24 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); }); - it(`an any argument allowed for non-any parameters`, () => { + // D1 canary: this used to be (incorrectly) `valid`. A bare `any` argument is unconstrained, + // not proven-compatible, so per the tri-state contract it must land in `indeterminate` + // rather than `valid` or `invalid`. This is the exact bug the north star fixed. + it(`an any argument is indeterminate for non-any parameters`, () => { const args: ReadonlyArray = [Language.Type.AnyInstance]; const definedFunction: Language.Type.DefinedFunction = TypeUtils.definedFunction( false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Text, + type: Language.Type.TextInstance, nameLiteral: "foo", }, ], @@ -202,10 +205,11 @@ describe(`TypeUtils.typeCheck`, () => { const actual: TypeUtils.CheckedInvocation = noopTypeCheckInvocation(args, definedFunction); const expected: TypeUtils.CheckedInvocation = { - valid: [0], + valid: [], invalid: new Map(), extraneous: [], missing: [], + indeterminate: [0], }; expect(actual).to.deep.equal(expected); @@ -218,9 +222,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -234,6 +237,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -249,15 +253,13 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Text, + type: Language.Type.TextInstance, nameLiteral: "bar", }, ], @@ -271,6 +273,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -283,9 +286,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -307,6 +309,7 @@ describe(`TypeUtils.typeCheck`, () => { ]), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -319,9 +322,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: true, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NullableNumberInstance, nameLiteral: "foo", }, ], @@ -335,6 +337,7 @@ describe(`TypeUtils.typeCheck`, () => { invalid: new Map(), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -347,9 +350,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -365,6 +367,7 @@ describe(`TypeUtils.typeCheck`, () => { ]), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -377,9 +380,8 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, ], @@ -395,6 +397,7 @@ describe(`TypeUtils.typeCheck`, () => { ]), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); @@ -410,15 +413,13 @@ describe(`TypeUtils.typeCheck`, () => { false, [ { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Number, + type: Language.Type.NumberInstance, nameLiteral: "foo", }, { - isNullable: false, isOptional: false, - type: Language.Type.TypeKind.Text, + type: Language.Type.TextInstance, nameLiteral: "bar", }, ], @@ -447,6 +448,7 @@ describe(`TypeUtils.typeCheck`, () => { ]), extraneous: [], missing: [], + indeterminate: [], }; expect(actual).to.deep.equal(expected); diff --git a/src/test/libraryTest/language/typeUtils/typeUtils.test.ts b/src/test/libraryTest/language/typeUtils/typeUtils.test.ts index 71726b71..61d41004 100644 --- a/src/test/libraryTest/language/typeUtils/typeUtils.test.ts +++ b/src/test/libraryTest/language/typeUtils/typeUtils.test.ts @@ -545,27 +545,23 @@ describe(`TypeUtils`, () => { false, [ { - isNullable: false, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param1", }, { - isNullable: true, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param2", }, { - isNullable: false, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param3", }, { - isNullable: true, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param4", }, ], @@ -712,27 +708,23 @@ describe(`TypeUtils`, () => { false, [ { - isNullable: false, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param1", }, { - isNullable: true, isOptional: false, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param2", }, { - isNullable: false, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NumberInstance, nameLiteral: "param3", }, { - isNullable: true, isOptional: true, - type: Type.TypeKind.Number, + type: Type.NullableNumberInstance, nameLiteral: "param4", }, ], From 6e8c00b222afac5922c6d7b25865aba88d751110 Mon Sep 17 00:00:00 2001 From: "Jordan Bolton (jobolton)" Date: Mon, 24 Aug 2026 14:53:15 -0500 Subject: [PATCH 2/4] Fix left-side Any/AnyNonNull precision loss and two pre-existing isCompatible bugs - isCompatible: bare Any on the left now special-cases right=AnyNonNull (true) and right=Null (false) before falling back to indeterminate, restoring precise pre-refactor results for these two deterministic cases. - isRightAnyUnionCompatible: fixed truthy-check collapsing an indeterminate union member to a hard false; now correctly returns undefined when no member is definitely true but one is indeterminate. - isDefinedListTypeCompatibleWithListType: fixed an always-false Boolean(array.find(...)) result; now uses .every() so a defined list type can actually be reported compatible with a list type. - Removed inline D1/D2/D3 planning-phase labels from code comments. - Added regression tests for all of the above. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23 --- .../language/type/typeUtils/isCompatible.ts | 36 +++++++--- .../language/type/typeUtils/typeUtils.ts | 4 +- .../language/typeUtils/isCompatible.test.ts | 67 ++++++++++++++++++- .../language/typeUtils/typeCheck.test.ts | 4 +- 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/src/powerquery-parser/language/type/typeUtils/isCompatible.ts b/src/powerquery-parser/language/type/typeUtils/isCompatible.ts index 2e821373..5574a88a 100644 --- a/src/powerquery-parser/language/type/typeUtils/isCompatible.ts +++ b/src/powerquery-parser/language/type/typeUtils/isCompatible.ts @@ -51,11 +51,19 @@ export function isCompatible( } else if (left.kind === Type.TypeKind.Null && right.isNullable) { result = true; } else if (left.kind === Type.TypeKind.Any) { - result = - left.extendedKind === Type.ExtendedTypeKind.AnyUnion - ? isAnyUnionCompatibleWith(left, right, traceManager, trace.id) - : // Bare `Any` is top: no enumerated evidence of a mismatch, so no hard claim. - undefined; + 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: @@ -180,17 +188,23 @@ function isRightAnyUnionCompatible( 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 false; + return sawIndeterminate ? undefined : false; } // `all`-semantics: is every member of the union on the *left* compatible with `right`? @@ -924,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(); diff --git a/src/powerquery-parser/language/type/typeUtils/typeUtils.ts b/src/powerquery-parser/language/type/typeUtils/typeUtils.ts index 79ff4e7d..ca93deb3 100644 --- a/src/powerquery-parser/language/type/typeUtils/typeUtils.ts +++ b/src/powerquery-parser/language/type/typeUtils/typeUtils.ts @@ -109,7 +109,7 @@ function inspectAstParameter(node: Ast.TParameter): Type.FunctionParameter { throw Assert.isNever(parameterType); } - // D2: an omitted optional argument is equivalent to passing `null`, so `isOptional` + // An omitted optional argument is equivalent to passing `null`, so `isOptional` // implies nullable regardless of how the parameter was ascribed in source. type = primitiveType( simplified.isNullable || isOptional, @@ -162,7 +162,7 @@ function inspectContextParameter( if (parameterType !== undefined) { const simplified: AstUtils.SimplifiedType = AstUtils.simplifyAsNullablePrimitiveType(parameterType); - // D2: an omitted optional argument is equivalent to passing `null`, so `isOptional` + // An omitted optional argument is equivalent to passing `null`, so `isOptional` // implies nullable regardless of how the parameter was ascribed in source. type = primitiveType( simplified.isNullable || isOptional, diff --git a/src/test/libraryTest/language/typeUtils/isCompatible.test.ts b/src/test/libraryTest/language/typeUtils/isCompatible.test.ts index 24f83911..55074e51 100644 --- a/src/test/libraryTest/language/typeUtils/isCompatible.test.ts +++ b/src/test/libraryTest/language/typeUtils/isCompatible.test.ts @@ -112,9 +112,27 @@ describe(`TypeUtils.isCompatible`, () => { expect(actual).to.equal(true, undefined); }); + + it(`AnyUnion on the right, no member is true but one is indeterminate -> undefined`, () => { + const actual: boolean | undefined = noopIsCompatible( + Type.NumberInstance, + noopCreateAnyUnion([Type.UnknownInstance, Type.TextInstance]), + ); + + expect(actual).to.equal(undefined, undefined); + }); + + it(`AnyUnion on the right, no member is true or indeterminate -> false`, () => { + const actual: boolean | undefined = noopIsCompatible( + Type.NumberInstance, + noopCreateAnyUnion([Type.TextInstance, Type.TextInstance]), + ); + + expect(actual).to.equal(false, undefined); + }); }); - // Left-side `Any` / `AnyUnion` handling (D1). See the tri-state contract documented above + // Left-side `Any` / `AnyUnion` handling. See the tri-state contract documented above // `isCompatible` in isCompatible.ts. describe(`AnyUnion on the left`, () => { it(`all members compatible -> true`, () => { @@ -181,6 +199,18 @@ describe(`TypeUtils.isCompatible`, () => { it(`number -> any is true (unchanged)`, () => { expect(noopIsCompatible(Type.NumberInstance, Type.AnyInstance)).to.equal(true, undefined); }); + + it(`any -> anynonnull is true (non-nullable any trivially satisfies anything-but-null)`, () => { + expect(noopIsCompatible(Type.AnyInstance, Type.AnyNonNullInstance)).to.equal(true, undefined); + }); + + it(`nullable any -> anynonnull is false (unchanged, caught by the nullability check)`, () => { + expect(noopIsCompatible(Type.NullableAnyInstance, Type.AnyNonNullInstance)).to.equal(false, undefined); + }); + + it(`any -> null is false (bare any can never be exactly null)`, () => { + expect(noopIsCompatible(Type.AnyInstance, Type.NullInstance)).to.equal(false, undefined); + }); }); describe(`${Type.ExtendedTypeKind.DefinedList}`, () => { @@ -244,6 +274,41 @@ describe(`TypeUtils.isCompatible`, () => { }); }); + describe(`${Type.ExtendedTypeKind.DefinedListType}`, () => { + it(`compatible with a list type of matching item type`, () => { + const definedListType: Type.DefinedListType = TypeUtils.definedListType(false, [ + Type.NumberInstance, + Type.NumberInstance, + ]); + + const listType: Type.ListType = TypeUtils.listType(false, Type.NumberInstance); + + expect(noopIsCompatible(definedListType, listType)).to.equal(true, undefined); + }); + + it(`not compatible with a list type of mismatched item type`, () => { + const definedListType: Type.DefinedListType = TypeUtils.definedListType(false, [ + Type.NumberInstance, + Type.NumberInstance, + ]); + + const listType: Type.ListType = TypeUtils.listType(false, Type.TextInstance); + + expect(noopIsCompatible(definedListType, listType)).to.equal(false, undefined); + }); + + it(`not compatible with a list type if any item type mismatches`, () => { + const definedListType: Type.DefinedListType = TypeUtils.definedListType(false, [ + Type.NumberInstance, + Type.TextInstance, + ]); + + const listType: Type.ListType = TypeUtils.listType(false, Type.NumberInstance); + + expect(noopIsCompatible(definedListType, listType)).to.equal(false, undefined); + }); + }); + describe(`${Type.ExtendedTypeKind.DefinedRecord}`, () => { describe(`identity`, () => { it(`empty`, () => { diff --git a/src/test/libraryTest/language/typeUtils/typeCheck.test.ts b/src/test/libraryTest/language/typeUtils/typeCheck.test.ts index 0c7f8bef..f3430f56 100644 --- a/src/test/libraryTest/language/typeUtils/typeCheck.test.ts +++ b/src/test/libraryTest/language/typeUtils/typeCheck.test.ts @@ -184,9 +184,9 @@ describe(`TypeUtils.typeCheck`, () => { expect(actual).to.deep.equal(expected); }); - // D1 canary: this used to be (incorrectly) `valid`. A bare `any` argument is unconstrained, + // Canary: this used to be (incorrectly) `valid`. A bare `any` argument is unconstrained, // not proven-compatible, so per the tri-state contract it must land in `indeterminate` - // rather than `valid` or `invalid`. This is the exact bug the north star fixed. + // rather than `valid` or `invalid`. it(`an any argument is indeterminate for non-any parameters`, () => { const args: ReadonlyArray = [Language.Type.AnyInstance]; From 6cef3678c5da45da790e3f8e5715a8f457fb215a Mon Sep 17 00:00:00 2001 From: "Jordan Bolton (jobolton)" Date: Mon, 24 Aug 2026 16:18:45 -0500 Subject: [PATCH 3/4] Bump parser package version to 1.1.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 08b9df1e..561f423a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/powerquery-parser", - "version": "1.0.0", + "version": "1.1.0", "description": "A parser for the Power Query/M formula language.", "author": "Microsoft", "license": "MIT", From aaea899b8713d1bc6152b6e5016cbc339fe5d424 Mon Sep 17 00:00:00 2001 From: "Jordan Bolton (jobolton)" Date: Mon, 24 Aug 2026 16:20:50 -0500 Subject: [PATCH 4/4] Bump parser package version to 2.0.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f6010dbc..0808ed45 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@microsoft/powerquery-parser", - "version": "1.0.0", + "version": "2.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@microsoft/powerquery-parser", - "version": "1.0.0", + "version": "2.0.0", "license": "MIT", "dependencies": { "grapheme-splitter": "^1.0.4", diff --git a/package.json b/package.json index 561f423a..198f675c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/powerquery-parser", - "version": "1.1.0", + "version": "2.0.0", "description": "A parser for the Power Query/M formula language.", "author": "Microsoft", "license": "MIT",