Skip to content

Commit 7be47ab

Browse files
rbucktonrbuckton
committed
PR feedback
1 parent 24de747 commit 7be47ab

20 files changed

+928
-54
lines changed

‎src/compiler/checker.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15025,15 +15025,12 @@ namespace ts {
1502515025
if (parent.questionDotToken) {
1502615026
// If we have a questionDotToken then we are an OptionalExpression and should remove `null` and
1502715027
// `undefined` from the type and add the optionalType to the result, if needed.
15028-
if (isNullableType(type)) {
15029-
isOptional = true;
15030-
type = getNonNullableType(type);
15031-
}
15032-
return { isOptional, type };
15028+
isOptional = isNullableType(type);
15029+
return { isOptional, type: isOptional ? getNonNullableType(type) : type };
1503315030
}
1503415031

1503515032
// If we do not have a questionDotToken, then we are an OptionalChain and we remove the optionalType and
15036-
// add it back into the result, if needed.
15033+
// indicate whether we need to add optionalType back into the result.
1503715034
const nonOptionalType = removeOptionalTypeMarker(type);
1503815035
if (nonOptionalType !== type) {
1503915036
isOptional = true;

‎src/compiler/types.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ namespace ts {
530530
NestedNamespace = 1 << 2, // Namespace declaration
531531
Synthesized = 1 << 3, // Node was synthesized during transformation
532532
Namespace = 1 << 4, // Namespace declaration
533-
OptionalChain = 1 << 5, // Chained MemberExpression rooted to a pseudo-OptionalExpression
533+
OptionalChain = 1 << 5, // Chained MemberExpression rooted to a pseudo-OptionalExpression
534534
ExportContext = 1 << 6, // Export context (initialized by binding)
535535
ContainsThis = 1 << 7, // Interface contains references to "this"
536536
HasImplicitReturn = 1 << 8, // If function implicitly returns on one of codepaths (initialized by binding)
@@ -1788,7 +1788,6 @@ namespace ts {
17881788

17891789
export interface PropertyAccessChain extends PropertyAccessExpression {
17901790
_optionalChainBrand: any;
1791-
kind: SyntaxKind.PropertyAccessExpression;
17921791
}
17931792

17941793
export interface SuperPropertyAccessExpression extends PropertyAccessExpression {
@@ -1810,7 +1809,6 @@ namespace ts {
18101809

18111810
export interface ElementAccessChain extends ElementAccessExpression {
18121811
_optionalChainBrand: any;
1813-
kind: SyntaxKind.ElementAccessExpression;
18141812
}
18151813

18161814
export interface SuperElementAccessExpression extends ElementAccessExpression {
@@ -1830,7 +1828,6 @@ namespace ts {
18301828

18311829
export interface CallChain extends CallExpression {
18321830
_optionalChainBrand: any;
1833-
kind: SyntaxKind.CallExpression;
18341831
}
18351832

18361833
export type OptionalChain =

‎src/services/completions.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,11 @@ namespace ts.Completions {
256256
// Somehow there was a global with a non-identifier name. Hopefully someone will complain about getting a "foo bar" global completion and provide a repro.
257257
else if ((origin && originIsSymbolMember(origin) || needsConvertPropertyAccess) && propertyAccessToConvert) {
258258
insertText = needsConvertPropertyAccess ? `[${quote(name, preferences)}]` : `[${name}]`;
259-
const dot = findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile)!;
259+
const dot = findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile) ||
260+
findChildOfKind(propertyAccessToConvert, SyntaxKind.QuestionDotToken, sourceFile);
261+
if (!dot) {
262+
return undefined;
263+
}
260264
// If the text after the '.' starts with this name, write over it. Else, add new text.
261265
const end = startsWith(name, propertyAccessToConvert.name.text) ? propertyAccessToConvert.name.end : dot.end;
262266
replacementSpan = createTextSpanFromBounds(dot.getStart(sourceFile), end);

‎tests/baselines/reference/callChain.3.errors.txt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts(3,7): error TS2322: Type 'number | undefined' is not assignable to type 'number'.
2+
Type 'undefined' is not assignable to type 'number'.
3+
tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts(4,7): error TS2322: Type 'number | undefined' is not assignable to type 'number'.
4+
Type 'undefined' is not assignable to type 'number'.
5+
6+
7+
==== tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts (2 errors) ====
8+
declare function absorb<T>(): T;
9+
declare const a: { m?<T>(obj: {x: T}): T } | undefined;
10+
const n1: number = a?.m?.({x: 12 }); // should be an error (`undefined` is not assignable to `number`)
11+
~~
12+
!!! error TS2322: Type 'number | undefined' is not assignable to type 'number'.
13+
!!! error TS2322: Type 'undefined' is not assignable to type 'number'.
14+
const n2: number = a?.m?.({x: absorb()}); // likewise
15+
~~
16+
!!! error TS2322: Type 'number | undefined' is not assignable to type 'number'.
17+
!!! error TS2322: Type 'undefined' is not assignable to type 'number'.
18+
const n3: number | undefined = a?.m?.({x: 12}); // should be ok
19+
const n4: number | undefined = a?.m?.({x: absorb()}); // likewise
20+
21+
// Also a test showing `!` vs `?` for good measure
22+
let t1 = a?.m?.({x: 12});
23+
t1 = a!.m!({x: 12});

‎tests/baselines/reference/callChain.3.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//// [callChain.3.ts]
2+
declare function absorb<T>(): T;
3+
declare const a: { m?<T>(obj: {x: T}): T } | undefined;
4+
const n1: number = a?.m?.({x: 12 }); // should be an error (`undefined` is not assignable to `number`)
5+
const n2: number = a?.m?.({x: absorb()}); // likewise
6+
const n3: number | undefined = a?.m?.({x: 12}); // should be ok
7+
const n4: number | undefined = a?.m?.({x: absorb()}); // likewise
8+
9+
// Also a test showing `!` vs `?` for good measure
10+
let t1 = a?.m?.({x: 12});
11+
t1 = a!.m!({x: 12});
12+
13+
//// [callChain.3.js]
14+
"use strict";
15+
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m, _o, _p, _q;
16+
var n1 = (_c = (_a = a) === null || _a === void 0 ? void 0 : (_b = _a).m) === null || _c === void 0 ? void 0 : _c.call(_b, { x: 12 }); // should be an error (`undefined` is not assignable to `number`)
17+
var n2 = (_f = (_d = a) === null || _d === void 0 ? void 0 : (_e = _d).m) === null || _f === void 0 ? void 0 : _f.call(_e, { x: absorb() }); // likewise
18+
var n3 = (_j = (_g = a) === null || _g === void 0 ? void 0 : (_h = _g).m) === null || _j === void 0 ? void 0 : _j.call(_h, { x: 12 }); // should be ok
19+
var n4 = (_m = (_k = a) === null || _k === void 0 ? void 0 : (_l = _k).m) === null || _m === void 0 ? void 0 : _m.call(_l, { x: absorb() }); // likewise
20+
// Also a test showing `!` vs `?` for good measure
21+
var t1 = (_q = (_o = a) === null || _o === void 0 ? void 0 : (_p = _o).m) === null || _q === void 0 ? void 0 : _q.call(_p, { x: 12 });
22+
t1 = a.m({ x: 12 });

‎tests/baselines/reference/callChain.3.symbols

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts ===
2+
declare function absorb<T>(): T;
3+
>absorb : Symbol(absorb, Decl(callChain.3.ts, 0, 0))
4+
>T : Symbol(T, Decl(callChain.3.ts, 0, 24))
5+
>T : Symbol(T, Decl(callChain.3.ts, 0, 24))
6+
7+
declare const a: { m?<T>(obj: {x: T}): T } | undefined;
8+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
9+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
10+
>T : Symbol(T, Decl(callChain.3.ts, 1, 22))
11+
>obj : Symbol(obj, Decl(callChain.3.ts, 1, 25))
12+
>x : Symbol(x, Decl(callChain.3.ts, 1, 31))
13+
>T : Symbol(T, Decl(callChain.3.ts, 1, 22))
14+
>T : Symbol(T, Decl(callChain.3.ts, 1, 22))
15+
16+
const n1: number = a?.m?.({x: 12 }); // should be an error (`undefined` is not assignable to `number`)
17+
>n1 : Symbol(n1, Decl(callChain.3.ts, 2, 5))
18+
>a?.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
19+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
20+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
21+
>x : Symbol(x, Decl(callChain.3.ts, 2, 27))
22+
23+
const n2: number = a?.m?.({x: absorb()}); // likewise
24+
>n2 : Symbol(n2, Decl(callChain.3.ts, 3, 5))
25+
>a?.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
26+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
27+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
28+
>x : Symbol(x, Decl(callChain.3.ts, 3, 27))
29+
>absorb : Symbol(absorb, Decl(callChain.3.ts, 0, 0))
30+
31+
const n3: number | undefined = a?.m?.({x: 12}); // should be ok
32+
>n3 : Symbol(n3, Decl(callChain.3.ts, 4, 5))
33+
>a?.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
34+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
35+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
36+
>x : Symbol(x, Decl(callChain.3.ts, 4, 39))
37+
38+
const n4: number | undefined = a?.m?.({x: absorb()}); // likewise
39+
>n4 : Symbol(n4, Decl(callChain.3.ts, 5, 5))
40+
>a?.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
41+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
42+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
43+
>x : Symbol(x, Decl(callChain.3.ts, 5, 39))
44+
>absorb : Symbol(absorb, Decl(callChain.3.ts, 0, 0))
45+
46+
// Also a test showing `!` vs `?` for good measure
47+
let t1 = a?.m?.({x: 12});
48+
>t1 : Symbol(t1, Decl(callChain.3.ts, 8, 3))
49+
>a?.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
50+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
51+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
52+
>x : Symbol(x, Decl(callChain.3.ts, 8, 17))
53+
54+
t1 = a!.m!({x: 12});
55+
>t1 : Symbol(t1, Decl(callChain.3.ts, 8, 3))
56+
>a!.m : Symbol(m, Decl(callChain.3.ts, 1, 18))
57+
>a : Symbol(a, Decl(callChain.3.ts, 1, 13))
58+
>m : Symbol(m, Decl(callChain.3.ts, 1, 18))
59+
>x : Symbol(x, Decl(callChain.3.ts, 9, 12))
60+

‎tests/baselines/reference/callChain.3.types

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts ===
2+
declare function absorb<T>(): T;
3+
>absorb : <T>() => T
4+
5+
declare const a: { m?<T>(obj: {x: T}): T } | undefined;
6+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
7+
>m : (<T>(obj: { x: T; }) => T) | undefined
8+
>obj : { x: T; }
9+
>x : T
10+
11+
const n1: number = a?.m?.({x: 12 }); // should be an error (`undefined` is not assignable to `number`)
12+
>n1 : number
13+
>a?.m?.({x: 12 }) : number | undefined
14+
>a?.m : (<T>(obj: { x: T; }) => T) | undefined
15+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
16+
>m : (<T>(obj: { x: T; }) => T) | undefined
17+
>{x: 12 } : { x: number; }
18+
>x : number
19+
>12 : 12
20+
21+
const n2: number = a?.m?.({x: absorb()}); // likewise
22+
>n2 : number
23+
>a?.m?.({x: absorb()}) : number | undefined
24+
>a?.m : (<T>(obj: { x: T; }) => T) | undefined
25+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
26+
>m : (<T>(obj: { x: T; }) => T) | undefined
27+
>{x: absorb()} : { x: number; }
28+
>x : number
29+
>absorb() : number
30+
>absorb : <T>() => T
31+
32+
const n3: number | undefined = a?.m?.({x: 12}); // should be ok
33+
>n3 : number | undefined
34+
>a?.m?.({x: 12}) : number | undefined
35+
>a?.m : (<T>(obj: { x: T; }) => T) | undefined
36+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
37+
>m : (<T>(obj: { x: T; }) => T) | undefined
38+
>{x: 12} : { x: number; }
39+
>x : number
40+
>12 : 12
41+
42+
const n4: number | undefined = a?.m?.({x: absorb()}); // likewise
43+
>n4 : number | undefined
44+
>a?.m?.({x: absorb()}) : number | undefined
45+
>a?.m : (<T>(obj: { x: T; }) => T) | undefined
46+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
47+
>m : (<T>(obj: { x: T; }) => T) | undefined
48+
>{x: absorb()} : { x: number | undefined; }
49+
>x : number | undefined
50+
>absorb() : number | undefined
51+
>absorb : <T>() => T
52+
53+
// Also a test showing `!` vs `?` for good measure
54+
let t1 = a?.m?.({x: 12});
55+
>t1 : number | undefined
56+
>a?.m?.({x: 12}) : number | undefined
57+
>a?.m : (<T>(obj: { x: T; }) => T) | undefined
58+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
59+
>m : (<T>(obj: { x: T; }) => T) | undefined
60+
>{x: 12} : { x: number; }
61+
>x : number
62+
>12 : 12
63+
64+
t1 = a!.m!({x: 12});
65+
>t1 = a!.m!({x: 12}) : number
66+
>t1 : number | undefined
67+
>a!.m!({x: 12}) : number
68+
>a!.m! : <T>(obj: { x: T; }) => T
69+
>a!.m : (<T>(obj: { x: T; }) => T) | undefined
70+
>a! : { m?<T>(obj: { x: T; }): T; }
71+
>a : { m?<T>(obj: { x: T; }): T; } | undefined
72+
>m : (<T>(obj: { x: T; }) => T) | undefined
73+
>{x: 12} : { x: number; }
74+
>x : number
75+
>12 : 12
76+

‎tests/baselines/reference/callChain.js

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,70 @@
11
//// [callChain.ts]
2-
declare const o1: undefined | (() => number);
2+
declare const o1: undefined | ((...args: any[]) => number);
33
o1?.();
4+
o1?.(1);
5+
o1?.(...[1, 2]);
6+
o1?.(1, ...[2, 3], 4);
47

5-
declare const o2: undefined | { b: () => number };
8+
declare const o2: undefined | { b: (...args: any[]) => number };
69
o2?.b();
10+
o2?.b(1);
11+
o2?.b(...[1, 2]);
12+
o2?.b(1, ...[2, 3], 4);
13+
o2?.["b"]();
14+
o2?.["b"](1);
15+
o2?.["b"](...[1, 2]);
16+
o2?.["b"](1, ...[2, 3], 4);
717

8-
declare const o3: { b: (() => { c: string }) | undefined };
18+
declare const o3: { b: ((...args: any[]) => { c: string }) | undefined };
919
o3.b?.().c;
10-
20+
o3.b?.(1).c;
21+
o3.b?.(...[1, 2]).c;
22+
o3.b?.(1, ...[2, 3], 4).c;
23+
o3.b?.()["c"];
24+
o3.b?.(1)["c"];
25+
o3.b?.(...[1, 2])["c"];
26+
o3.b?.(1, ...[2, 3], 4)["c"];
27+
o3["b"]?.().c;
28+
o3["b"]?.(1).c;
29+
o3["b"]?.(...[1, 2]).c;
30+
o3["b"]?.(1, ...[2, 3], 4).c;
31+
32+
declare const o4: undefined | (<T>(f: (a: T) => T) => T);
33+
declare function incr(x: number): number;
34+
const v: number | undefined = o4?.(incr);
1135

1236
//// [callChain.js]
1337
"use strict";
14-
var _a, _b, _c, _d;
38+
var __spreadArrays = (this && this.__spreadArrays) || function () {
39+
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
40+
for (var r = Array(s), k = 0, i = 0; i < il; i++)
41+
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
42+
r[k] = a[j];
43+
return r;
44+
};
45+
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m, _o, _p, _q, _r, _s, _t, _u, _v, _w, _x, _y, _z, _0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12;
1546
(_a = o1) === null || _a === void 0 ? void 0 : _a();
16-
(_b = o2) === null || _b === void 0 ? void 0 : _b.b();
17-
(_d = (_c = o3).b) === null || _d === void 0 ? void 0 : _d.call(_c).c;
47+
(_b = o1) === null || _b === void 0 ? void 0 : _b(1);
48+
(_c = o1) === null || _c === void 0 ? void 0 : _c.apply(void 0, [1, 2]);
49+
(_d = o1) === null || _d === void 0 ? void 0 : _d.apply(void 0, __spreadArrays([1], [2, 3], [4]));
50+
(_e = o2) === null || _e === void 0 ? void 0 : _e.b();
51+
(_f = o2) === null || _f === void 0 ? void 0 : _f.b(1);
52+
(_g = o2) === null || _g === void 0 ? void 0 : _g.b.apply(_g, [1, 2]);
53+
(_h = o2) === null || _h === void 0 ? void 0 : _h.b.apply(_h, __spreadArrays([1], [2, 3], [4]));
54+
(_j = o2) === null || _j === void 0 ? void 0 : _j["b"]();
55+
(_k = o2) === null || _k === void 0 ? void 0 : _k["b"](1);
56+
(_l = o2) === null || _l === void 0 ? void 0 : _l["b"].apply(_l, [1, 2]);
57+
(_m = o2) === null || _m === void 0 ? void 0 : _m["b"].apply(_m, __spreadArrays([1], [2, 3], [4]));
58+
(_p = (_o = o3).b) === null || _p === void 0 ? void 0 : _p.call(_o).c;
59+
(_r = (_q = o3).b) === null || _r === void 0 ? void 0 : _r.call(_q, 1).c;
60+
(_t = (_s = o3).b) === null || _t === void 0 ? void 0 : _t.call.apply(_t, __spreadArrays([_s], [1, 2])).c;
61+
(_v = (_u = o3).b) === null || _v === void 0 ? void 0 : _v.call.apply(_v, __spreadArrays([_u, 1], [2, 3], [4])).c;
62+
(_x = (_w = o3).b) === null || _x === void 0 ? void 0 : _x.call(_w)["c"];
63+
(_z = (_y = o3).b) === null || _z === void 0 ? void 0 : _z.call(_y, 1)["c"];
64+
(_1 = (_0 = o3).b) === null || _1 === void 0 ? void 0 : _1.call.apply(_1, __spreadArrays([_0], [1, 2]))["c"];
65+
(_3 = (_2 = o3).b) === null || _3 === void 0 ? void 0 : _3.call.apply(_3, __spreadArrays([_2, 1], [2, 3], [4]))["c"];
66+
(_5 = (_4 = o3)["b"]) === null || _5 === void 0 ? void 0 : _5.call(_4).c;
67+
(_7 = (_6 = o3)["b"]) === null || _7 === void 0 ? void 0 : _7.call(_6, 1).c;
68+
(_9 = (_8 = o3)["b"]) === null || _9 === void 0 ? void 0 : _9.call.apply(_9, __spreadArrays([_8], [1, 2])).c;
69+
(_11 = (_10 = o3)["b"]) === null || _11 === void 0 ? void 0 : _11.call.apply(_11, __spreadArrays([_10, 1], [2, 3], [4])).c;
70+
var v = (_12 = o4) === null || _12 === void 0 ? void 0 : _12(incr);

0 commit comments

Comments
 (0)