Promote FunctionParameter.type to TPowerQueryType; fix isCompatible left-side Any/AnyUnion and two related bugs - #414
Open
JordanBoltonMN wants to merge 4 commits into
Conversation
…eft-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
…mpatible 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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39041e13-1ee5-4083-a312-1101ec33bc23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Promotes
FunctionParameter.typefromTypeKind | undefinedto the fullTPowerQueryType | undefined, and fixes a family of latent bugs inisCompatiblearound bareAny/AnyUnionon the left-hand side (plus two additional pre-existing bugs discovered nearby during review).Motivation
isValidInvocation/typeCheckInvocationpreviously collapsedisCompatible's three-state result (true/false/undefined) incorrectly in several branches, andFunctionParametercould only express a primitiveTypeKind, losing nullability/extended-type fidelity (e.g.list,record,tableparameter types). This blocked a downstream ask frompowerquery-language-servicesfor richer parameter type information in hover/signature-help/diagnostics.Changes
isCompatible.ts: Added explicit handling forAny/AnyUnionon the left-hand side.Anyon the left →undefined(indeterminate) in general — it represents absence of constraint, not proof of compatibility, so it must not short-circuit totrue. Two deterministic exceptions are special-cased:right = AnyNonNull→true(a non-nullableAnytrivially satisfies "anything but null"),right = Null→false(bareAnycan never be exactlynull).AnyUnionon the left → newisAnyUnionCompatibleWithhelper using all-semantics (falseon any definite mismatch,undefinedif any member is indeterminate, elsetrue).isCompatibleWithAnyUnion→isRightAnyUnionCompatiblefor clarity (unioned type on the right retains existing some-semantics), and fixed a pre-existing bug: it previously used a truthy check that collapsed an indeterminate member to a hardfalse; it now correctly returnsundefinedwhen no member is definitelytruebut at least one is indeterminate.isCompatibleWithFunctionParameter(superseded by directisCompatiblecalls intypeCheckInvocation).isDefinedListTypeCompatibleWithListType: it usedBoolean(array.find(v => v === undefined || v === false)), which is unconditionallyfalseregardless of input (Array.findreturnsundefinedon "not found," andBooleanofundefined/falseis alwaysfalse). Replaced with.every(v => v === true), so this compatibility check — previously non-functional — now actually reportstruewhen every list item type matches.type.ts:FunctionParameter.typeis nowTPowerQueryType | undefined;isNullableis removed and folded intotype— an optional parameter's implied nullability is now baked into the constructed type (optionalimplies nullable).typeUtils.ts:inspectAstParameter/inspectContextParameterupdated to construct full types per the above. RemovedisValidInvocation— dead code with zero callers in this repo and inpowerquery-language-services(superseded entirely bytypeCheckInvocation).typeCheck.ts:CheckedInvocationgained anindeterminate: ReadonlyArray<number>bucket.typeCheckInvocationrewritten to explicitly bucket each argument index intovalid/invalid/missing/indeterminatebased onisCompatible's tri-state result, rather than conflating them viaisCompatibleWithFunctionParameter. Also made two pre-existing truthiness collapses explicit (=== true) now thatisCompatiblecan returnundefined.nameOf.ts/isEqualType.ts: Updated to work against the newFunctionParameter.typeshape.isCompatible.test.ts,isEqualType.test.ts,nameOf.test.ts,typeCheck.test.ts,typeUtils.test.tsto match, including new coverage for theAny/AnyUnion-on-the-left fix and both additional bug fixes above.Compatibility note
CheckedInvocation's newindeterminatefield andFunctionParameter's shape change are public API surface changes.powerquery-language-servicespins a specific parser version, so this is non-breaking until it explicitly bumps its dependency (tracked separately).Testing
npm run build— cleannpm run lint— cleannpm test— 705 passing, 0 failing, 1 pending