Skip to content

Commit 671096b

Browse files
committed
Lots of cases work
Destructuring does not. But - skipping node_modules and lib.* does. - call expressions does - property access, including with private identifiers, does
1 parent d1008eb commit 671096b

File tree

7 files changed

+196
-31
lines changed

7 files changed

+196
-31
lines changed

src/compiler/checker.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17657,10 +17657,18 @@ namespace ts {
1765717657
else if (sourceType === targetType) {
1765817658
message = Diagnostics.Type_0_is_not_assignable_to_type_1_Two_different_types_with_this_name_exist_but_they_are_unrelated;
1765917659
}
17660+
else if (exactOptionalPropertyTypes && getExactOptionalUnassignableProperties(source, target).length) {
17661+
message = Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties
17662+
}
1766017663
else {
1766117664
message = Diagnostics.Type_0_is_not_assignable_to_type_1;
1766217665
}
1766317666
}
17667+
else if (message === Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1
17668+
&& exactOptionalPropertyTypes
17669+
&& getExactOptionalUnassignableProperties(source, target).length) {
17670+
message = Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties
17671+
}
1766417672

1766517673
reportError(message, generalizedSourceType, targetType);
1766617674
}
@@ -17875,7 +17883,6 @@ namespace ts {
1787517883

1787617884
function reportErrorResults(source: Type, target: Type, result: Ternary, isComparingJsxAttributes: boolean) {
1787717885
if (!result && reportErrors) {
17878-
let message = headMessage
1787917886
const sourceHasBase = !!getSingleBaseForNonAugmentingSubtype(originalSource);
1788017887
const targetHasBase = !!getSingleBaseForNonAugmentingSubtype(originalTarget);
1788117888
source = (originalSource.aliasSymbol || sourceHasBase) ? originalSource : source;
@@ -17907,18 +17914,15 @@ namespace ts {
1790717914
return result;
1790817915
}
1790917916
}
17910-
else if (exactOptionalPropertyTypes && getExactOptionalUnassignableProperties(source, target).length) {
17911-
message = Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties
17912-
}
1791317917
else {
1791417918
errorInfo = elaborateNeverIntersection(errorInfo, originalTarget);
1791517919
}
17916-
if (!message && maybeSuppress) {
17920+
if (!headMessage && maybeSuppress) {
1791717921
lastSkippedInfo = [source, target];
1791817922
// Used by, eg, missing property checking to replace the top-level message with a more informative one
1791917923
return result;
1792017924
}
17921-
reportRelationError(message, source, target);
17925+
reportRelationError(headMessage, source, target);
1792217926
}
1792317927
}
1792417928
}
@@ -19580,7 +19584,6 @@ namespace ts {
1958019584
return isUnitType(type) || !!(type.flags & TypeFlags.TemplateLiteral);
1958119585
}
1958219586

19583-
// TODO: Only issue with source has undefined, target does not, and they are otherwise assignable/the same (or who cares)
1958419587
function getExactOptionalUnassignableProperties(source: Type, target: Type) {
1958519588
return checker.getPropertiesOfType(target).filter(targetProp => {
1958619589
const sourceProp = getPropertyOfType(source, targetProp.escapedName)

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,6 +1714,10 @@
17141714
"category": "Error",
17151715
"code": 2378
17161716
},
1717+
"Argument of type '{0}' is not assignable to parameter of type '{1}' with exactOptionalPropertyTypes: true. Consider adding 'undefined' to the types of the target's properties.": {
1718+
"category": "Error",
1719+
"code": 2379
1720+
},
17171721
"The return type of a 'get' accessor must be assignable to its 'set' accessor type": {
17181722
"category": "Error",
17191723
"code": 2380

src/harness/fourslashImpl.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,8 @@ namespace FourSlash {
641641
if (errors.length) {
642642
this.printErrorLog(/*expectErrors*/ false, errors);
643643
const error = errors[0];
644-
this.raiseError(`Found an error: ${this.formatPosition(error.file!, error.start!)}: ${error.messageText}`);
644+
const message = typeof error.messageText === "string" ? error.messageText : error.messageText.messageText;
645+
this.raiseError(`Found an error: ${this.formatPosition(error.file!, error.start!)}: ${message}`);
645646
}
646647
});
647648
}

src/services/codefixes/addOptionalPropertyUndefined.ts

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ namespace ts.codefix {
33
const addOptionalPropertyUndefined = "addOptionalPropertyUndefined";
44

55
const errorCodes = [
6-
Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties.code
6+
Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties.code,
7+
Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_types_of_the_target_s_properties.code
78
];
89

910
registerCodeFix({
@@ -43,23 +44,47 @@ namespace ts.codefix {
4344
},
4445
});
4546

46-
function getInfo(sourceFile: SourceFile, tokenPos: number, checker: TypeChecker): Symbol[] {
47+
function getTarget(file: SourceFile, pos: number) {
48+
const start = getTokenAtPosition(file, pos)
49+
if (isPropertyAccessExpression(start.parent) && start.parent.expression === start) {
50+
return start.parent
51+
}
52+
else if (isIdentifier(start) || isPrivateIdentifier(start)) {
53+
return start
54+
}
55+
return undefined
56+
}
57+
58+
function getSourceTarget(file: SourceFile, pos: number, checker: TypeChecker) {
59+
const target = getTarget(file, pos)
60+
if (!target) return undefined
61+
if (isBinaryExpression(target.parent) && target.parent.operatorToken.kind === SyntaxKind.EqualsToken) {
62+
return [target.parent.right, target]
63+
}
64+
else if (isCallExpression(target.parent)) {
65+
const n = checker.getSymbolAtLocation(target.parent.expression)
66+
if (!n?.valueDeclaration) return undefined
67+
if (!isIdentifier(target)) return undefined;
68+
const i = target.parent.arguments.indexOf(target)
69+
const name = (n.valueDeclaration as any as SignatureDeclaration).parameters[i].name
70+
if (isIdentifier(name)) return [target, name]
71+
}
72+
// It's possible to handle destructuring by recording the path up through the structure
73+
// and following the reverse path down through the right-hand-side, but that's not worth the effort
74+
return undefined
75+
}
76+
77+
function getInfo(file: SourceFile, pos: number, checker: TypeChecker): Symbol[] {
4778
// The target of the incorrect assignment
4879
// eg
4980
// this.definite = 1; -OR- definite = source
5081
// ^^^^ ^^^^^^^^
51-
const targetToken = getTokenAtPosition(sourceFile, tokenPos);
52-
const isOK = (isIdentifier(targetToken) || isPrivateIdentifier(targetToken))
53-
&& isBinaryExpression(targetToken.parent)
54-
&& targetToken.parent.operatorToken.kind === SyntaxKind.EqualsToken;
55-
if (!isOK) {
56-
// TODO: Walk up through lhs instead
57-
return [];
58-
}
59-
const sourceNode = targetToken.parent.right;
60-
// TODO: Also can apply to function calls, and then you have to get the signature, then its parameters, then the type of a particular parameter
61-
// TODO: Also skip 'any' and node_modules and if target is not in node_modules or is built-in
62-
return checker.getExactOptionalUnassignableProperties(checker.getTypeAtLocation(sourceNode), checker.getTypeAtLocation(targetToken))
82+
const sourceTarget = getSourceTarget(file, pos, checker)
83+
if (!sourceTarget) return []
84+
const [sourceNode, targetNode] = sourceTarget
85+
const target = checker.getTypeAtLocation(targetNode)
86+
if (target.symbol?.declarations?.some(d => getSourceFileOfNode(d).fileName.match(/(node_modules|^lib)/))) return [];
87+
return checker.getExactOptionalUnassignableProperties(checker.getTypeAtLocation(sourceNode), target)
6388
}
6489

6590
function addUndefinedToOptionalProperty(changes: textChanges.ChangeTracker, toAdd: Symbol[]) {

tests/cases/fourslash/fixExactOptionalUnassignableProperties1.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
// @strictNullChecks: true
44
// @exactOptionalPropertyTypes: true
5-
////interface I {
6-
//// a?: number
7-
////}
8-
////interface J {
9-
//// a?: number | undefined
10-
////}
11-
////declare var i: I
12-
////declare var j: J
13-
////i/**/ = j
5+
//// interface I {
6+
//// a?: number
7+
//// }
8+
//// interface J {
9+
//// a?: number | undefined
10+
//// }
11+
//// declare var i: I
12+
//// declare var j: J
13+
//// i/**/ = j
1414
verify.codeFixAvailable([
1515
{ description: ts.Diagnostics.Add_undefined_to_optional_property_type.message }
1616
]);
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @strictNullChecks: true
4+
// @exactOptionalPropertyTypes: true
5+
6+
//// interface I {
7+
//// a?: number
8+
//// }
9+
//// interface IAny {
10+
//// a?: any
11+
//// }
12+
//// interface IF {
13+
//// a?: number
14+
//// }
15+
//// interface IC {
16+
//// a?: number
17+
//// }
18+
//// interface IC2 {
19+
//// a?: number
20+
//// }
21+
//// interface ICP {
22+
//// a?: number
23+
//// }
24+
//// interface ID {
25+
//// a?: number
26+
//// }
27+
//// interface J {
28+
//// a?: number | undefined
29+
//// }
30+
//// declare var i: I
31+
//// declare var iany: IAny
32+
//// declare var if: IF
33+
//// declare var j: J
34+
//// i = j
35+
//// iany = j
36+
//// function fi(if_: IF) { return if_ }
37+
//// fi(j)
38+
//// class C {
39+
//// ic: IC
40+
//// ic2: IC2
41+
//// m() { this.ic = j }
42+
//// }
43+
//// var c = new C()
44+
//// c.ic2 = j
45+
//// class CP {
46+
//// #icp: ICP
47+
//// m() { this.#icp = j }
48+
//// }
49+
//// declare var id: ID
50+
//// ({ id } = { id: j })
51+
//// declare var pd: PropertyDescriptor
52+
//// interface PartialWrongPropertyDescriptor {
53+
//// configurable?: boolean | undefined
54+
//// }
55+
//// declare var pwpd: PartialWrongPropertyDescriptor
56+
//// pd = pwpd
57+
58+
verify.codeFixAll({
59+
fixId: "addOptionalPropertyUndefined",
60+
fixAllDescription: ts.Diagnostics.Add_undefined_to_all_optional_properties.message,
61+
newFileContent:
62+
`interface I {
63+
a?: number | undefined
64+
}
65+
interface IAny {
66+
a?: any
67+
}
68+
interface IF {
69+
a?: number | undefined
70+
}
71+
interface IC {
72+
a?: number | undefined
73+
}
74+
interface IC2 {
75+
a?: number | undefined
76+
}
77+
interface ICP {
78+
a?: number | undefined
79+
}
80+
interface ID {
81+
a?: number
82+
}
83+
interface J {
84+
a?: number | undefined
85+
}
86+
declare var i: I
87+
declare var iany: IAny
88+
declare var if: IF
89+
declare var j: J
90+
i = j
91+
iany = j
92+
function fi(if_: IF) { return if_ }
93+
fi(j)
94+
class C {
95+
ic: IC
96+
ic2: IC2
97+
m() { this.ic = j }
98+
}
99+
var c = new C()
100+
c.ic2 = j
101+
class CP {
102+
#icp: ICP
103+
m() { this.#icp = j }
104+
}
105+
declare var id: ID
106+
({ id } = { id: j })
107+
declare var pd: PropertyDescriptor
108+
interface PartialWrongPropertyDescriptor {
109+
configurable?: boolean | undefined
110+
}
111+
declare var pwpd: PartialWrongPropertyDescriptor
112+
pd = pwpd`,
113+
});
114+
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @strictNullChecks: true
4+
// @exactOptionalPropertyTypes: true
5+
// @Filename: fixExactOptionalUnassignableProperties2.ts
6+
//// import { INodeModules } from 'foo'
7+
//// interface J {
8+
//// a?: number | undefined
9+
//// }
10+
//// declare var inm: INodeModules
11+
//// declare var j: J
12+
//// inm/**/ = j
13+
//// console.log(inm)
14+
// @Filename: node_modules/@types/foo/index.d.ts
15+
//// export interface INodeModules {
16+
//// a?: number
17+
//// }
18+
verify.codeFixAvailable([]);

0 commit comments

Comments
 (0)