Skip to content

Add Support for Using Aliased Discriminants in Conditional Statements #56173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Add Support for Using Aliased Discriminants in Conditional Statements #56173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c914ebf
Add Support for Using Aliased Discriminants in Conditional Statements
Zzzen Oct 22, 2023
34aed6e
support nested bindings
Zzzen Oct 29, 2023
9ff723c
Apply suggestions from code review
Zzzen Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27160,6 +27160,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
case SyntaxKind.ElementAccessExpression:
// The resolvedSymbol property is initialized by checkPropertyAccess or checkElementAccess before we get here.
return isConstantReference((node as AccessExpression).expression) && isReadonlySymbol(getNodeLinks(node).resolvedSymbol || unknownSymbol);
case SyntaxKind.ObjectBindingPattern:
case SyntaxKind.ArrayBindingPattern:
const rootDeclaration = getRootDeclaration(node.parent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an alternative solution for this could look smth like this:

Suggested change
return isBindingElement(node.parent) ? isConstantReference(node.parent.parent) : isVariableDeclaration(node.parent) && isVarConstLike(node.parent);
const rootDeclaration = getRootDeclaration(node.parent);
return isVariableDeclaration(rootDeclaration) && isVarConstLike(rootDeclaration);

Copy link
Contributor

@Andarist Andarist Nov 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this should work as well: TS playground but the problem is that we are dealing with a pseudo-reference here. At this point, we can't easily check if the original symbol for which we are performing the narrowing is a non-reassigned parameter.

This could easily be seen as a separate issue though. I only mention it here since I was touching the nearby code and I noticed this now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish there was a 'const' modifier available for parameters and we could just check that!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should make that possible: #56313

return isVariableDeclaration(rootDeclaration) && isVarConstLike(rootDeclaration);
}
return false;
}
Expand Down
129 changes: 129 additions & 0 deletions tests/baselines/reference/controlFlowAliasedDiscriminants.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
controlFlowAliasedDiscriminants.ts(39,9): error TS18048: 'data1' is possibly 'undefined'.
controlFlowAliasedDiscriminants.ts(40,9): error TS18048: 'data2' is possibly 'undefined'.
controlFlowAliasedDiscriminants.ts(65,9): error TS18048: 'bar2' is possibly 'undefined'.
controlFlowAliasedDiscriminants.ts(66,9): error TS18048: 'bar3' is possibly 'undefined'.
controlFlowAliasedDiscriminants.ts(86,14): error TS1360: Type 'string | number' does not satisfy the expected type 'string'.
Type 'number' is not assignable to type 'string'.
controlFlowAliasedDiscriminants.ts(98,19): error TS1360: Type 'string | number' does not satisfy the expected type 'string'.
Type 'number' is not assignable to type 'string'.


==== controlFlowAliasedDiscriminants.ts (6 errors) ====
type UseQueryResult<T> = {
isSuccess: false;
data: undefined;
} | {
isSuccess: true;
data: T
};

function useQuery(): UseQueryResult<number> {
return {
isSuccess: false,
data: undefined,
};
}

const { data: data1, isSuccess: isSuccess1 } = useQuery();
const { data: data2, isSuccess: isSuccess2 } = useQuery();
const { data: data3, isSuccess: isSuccess3 } = useQuery();

if (isSuccess1 && isSuccess2 && isSuccess3) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}

const areSuccess = isSuccess1 && isSuccess2 && isSuccess3;
if (areSuccess) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}

{
let { data: data1, isSuccess: isSuccess1 } = useQuery();
let { data: data2, isSuccess: isSuccess2 } = useQuery();
const { data: data3, isSuccess: isSuccess3 } = useQuery();
const areSuccess = isSuccess1 && isSuccess2 && isSuccess3;
if (areSuccess) {
data1.toExponential(); // should error
~~~~~
!!! error TS18048: 'data1' is possibly 'undefined'.
data2.toExponential(); // should error
~~~~~
!!! error TS18048: 'data2' is possibly 'undefined'.
data3.toExponential(); // should ok
}
}

declare function getArrayResult(): [true, number] | [false, undefined];
{
const [foo1, bar1] = getArrayResult();
const [foo2, bar2] = getArrayResult();
const [foo3, bar3] = getArrayResult();
const arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should ok
bar3.toExponential(); // should ok
}
}

{
const [foo1, bar1] = getArrayResult();
let [foo2, bar2] = getArrayResult();
let [foo3, bar3] = getArrayResult();
const arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should error
~~~~
!!! error TS18048: 'bar2' is possibly 'undefined'.
bar3.toExponential(); // should error
~~~~
!!! error TS18048: 'bar3' is possibly 'undefined'.
}
}

type Nested = {
type: 'string';
resp: {
data: string
}
} | {
type: 'number';
resp: {
data: number;
}
}

{
let resp!: Nested;
const { resp: { data }, type } = resp;
if (type === 'string') {
data satisfies string;
~~~~~~~~~
!!! error TS1360: Type 'string | number' does not satisfy the expected type 'string'.
!!! error TS1360: Type 'number' is not assignable to type 'string'.
}
if (resp.type === 'string') {
resp.resp.data satisfies string;
}
}

{

let resp!: Nested;
const { resp: { data: dataAlias }, type } = resp;
if (type === 'string') {
dataAlias satisfies string;
~~~~~~~~~
!!! error TS1360: Type 'string | number' does not satisfy the expected type 'string'.
!!! error TS1360: Type 'number' is not assignable to type 'string'.
}
if (resp.type === 'string') {
resp.resp.data satisfies string;
}
}

182 changes: 182 additions & 0 deletions tests/baselines/reference/controlFlowAliasedDiscriminants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
//// [tests/cases/compiler/controlFlowAliasedDiscriminants.ts] ////

//// [controlFlowAliasedDiscriminants.ts]
type UseQueryResult<T> = {
isSuccess: false;
data: undefined;
} | {
isSuccess: true;
data: T
};

function useQuery(): UseQueryResult<number> {
return {
isSuccess: false,
data: undefined,
};
}

const { data: data1, isSuccess: isSuccess1 } = useQuery();
const { data: data2, isSuccess: isSuccess2 } = useQuery();
const { data: data3, isSuccess: isSuccess3 } = useQuery();

if (isSuccess1 && isSuccess2 && isSuccess3) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}

const areSuccess = isSuccess1 && isSuccess2 && isSuccess3;
if (areSuccess) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}

{
let { data: data1, isSuccess: isSuccess1 } = useQuery();
let { data: data2, isSuccess: isSuccess2 } = useQuery();
const { data: data3, isSuccess: isSuccess3 } = useQuery();
const areSuccess = isSuccess1 && isSuccess2 && isSuccess3;
if (areSuccess) {
data1.toExponential(); // should error
data2.toExponential(); // should error
data3.toExponential(); // should ok
}
}

declare function getArrayResult(): [true, number] | [false, undefined];
{
const [foo1, bar1] = getArrayResult();
const [foo2, bar2] = getArrayResult();
const [foo3, bar3] = getArrayResult();
const arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should ok
bar3.toExponential(); // should ok
}
}

{
const [foo1, bar1] = getArrayResult();
let [foo2, bar2] = getArrayResult();
let [foo3, bar3] = getArrayResult();
const arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should error
bar3.toExponential(); // should error
}
}

type Nested = {
type: 'string';
resp: {
data: string
}
} | {
type: 'number';
resp: {
data: number;
}
}

{
let resp!: Nested;
const { resp: { data }, type } = resp;
if (type === 'string') {
data satisfies string;
}
if (resp.type === 'string') {
resp.resp.data satisfies string;
}
}

{

let resp!: Nested;
const { resp: { data: dataAlias }, type } = resp;
if (type === 'string') {
dataAlias satisfies string;
}
if (resp.type === 'string') {
resp.resp.data satisfies string;
}
}


//// [controlFlowAliasedDiscriminants.js]
function useQuery() {
return {
isSuccess: false,
data: undefined,
};
}
var _a = useQuery(), data1 = _a.data, isSuccess1 = _a.isSuccess;
var _b = useQuery(), data2 = _b.data, isSuccess2 = _b.isSuccess;
var _c = useQuery(), data3 = _c.data, isSuccess3 = _c.isSuccess;
if (isSuccess1 && isSuccess2 && isSuccess3) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}
var areSuccess = isSuccess1 && isSuccess2 && isSuccess3;
if (areSuccess) {
data1.toExponential(); // should ok
data2.toExponential(); // should ok
data3.toExponential(); // should ok
}
{
var _d = useQuery(), data1_1 = _d.data, isSuccess1_1 = _d.isSuccess;
var _e = useQuery(), data2_1 = _e.data, isSuccess2_1 = _e.isSuccess;
var _f = useQuery(), data3_1 = _f.data, isSuccess3_1 = _f.isSuccess;
var areSuccess_1 = isSuccess1_1 && isSuccess2_1 && isSuccess3_1;
if (areSuccess_1) {
data1_1.toExponential(); // should error
data2_1.toExponential(); // should error
data3_1.toExponential(); // should ok
}
}
{
var _g = getArrayResult(), foo1 = _g[0], bar1 = _g[1];
var _h = getArrayResult(), foo2 = _h[0], bar2 = _h[1];
var _j = getArrayResult(), foo3 = _j[0], bar3 = _j[1];
var arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should ok
bar3.toExponential(); // should ok
}
}
{
var _k = getArrayResult(), foo1 = _k[0], bar1 = _k[1];
var _l = getArrayResult(), foo2 = _l[0], bar2 = _l[1];
var _m = getArrayResult(), foo3 = _m[0], bar3 = _m[1];
var arrayAllSuccess = foo1 && foo2 && foo3;
if (arrayAllSuccess) {
bar1.toExponential(); // should ok
bar2.toExponential(); // should error
bar3.toExponential(); // should error
}
}
{
var resp = void 0;
var data = resp.resp.data, type = resp.type;
if (type === 'string') {
data;
}
if (resp.type === 'string') {
resp.resp.data;
}
}
{
var resp = void 0;
var dataAlias = resp.resp.data, type = resp.type;
if (type === 'string') {
dataAlias;
}
if (resp.type === 'string') {
resp.resp.data;
}
}
Loading