Skip to content

Fix unassignable properties by adding undefined with exactOptionalPropertyTypes #45032

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.

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 23 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
664d749
Simple first version
sandersn Jul 13, 2021
d1008eb
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Jul 13, 2021
671096b
Lots of cases work
sandersn Jul 14, 2021
a728852
Support variable declarations, property assignments, destructuring
sandersn Jul 14, 2021
715f235
More cleanup
sandersn Jul 14, 2021
cfea6e1
skip all d.ts, not just node_modules/lib
sandersn Jul 15, 2021
d058191
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Jul 15, 2021
334ca65
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Jul 30, 2021
41f0525
Offer a codefix for a lot more cases
sandersn Aug 3, 2021
09c916f
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Aug 3, 2021
cdd40d3
remove incorrect tuple check
sandersn Aug 3, 2021
f018b8e
Use getSymbolId instead of converting to string
sandersn Aug 3, 2021
960e657
add test + switch to tracking number symbol ids
sandersn Aug 3, 2021
cdbe969
Address PR comments
sandersn Aug 3, 2021
22b9e7a
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Aug 4, 2021
e9607f1
Exclude tuples from suggestion
sandersn Aug 4, 2021
2c1982f
Better way to get error node
sandersn Aug 4, 2021
3ffe9db
fix semicolon lint
sandersn Aug 4, 2021
3e70798
fix another crash
sandersn Aug 5, 2021
4b7b3fe
Simplify: add undefined to all optional propertie
sandersn Aug 5, 2021
d535509
remove fix-all
sandersn Aug 5, 2021
e027d1e
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Aug 6, 2021
cc51d23
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn Aug 10, 2021
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
63 changes: 49 additions & 14 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ namespace ts {
isTupleType,
isArrayLikeType,
isTypeInvalidDueToUnionDiscriminant,
getExactOptionalProperties,
getAllPossiblePropertiesOfTypes,
getSuggestedSymbolForNonexistentProperty,
getSuggestionForNonexistentProperty,
Expand Down Expand Up @@ -16768,24 +16769,29 @@ namespace ts {
let sourcePropType = getIndexedAccessTypeOrUndefined(source, nameType);
if (!sourcePropType) continue;
const propName = getPropertyNameFromIndex(nameType, /*accessNode*/ undefined);
const targetIsOptional = !!(propName && (getPropertyOfType(target, propName) || unknownSymbol).flags & SymbolFlags.Optional);
const sourceIsOptional = !!(propName && (getPropertyOfType(source, propName) || unknownSymbol).flags & SymbolFlags.Optional);
targetPropType = removeMissingType(targetPropType, targetIsOptional);
sourcePropType = removeMissingType(sourcePropType, targetIsOptional && sourceIsOptional);
if (!checkTypeRelatedTo(sourcePropType, targetPropType, relation, /*errorNode*/ undefined)) {
const elaborated = next && elaborateError(next, sourcePropType, targetPropType, relation, /*headMessage*/ undefined, containingMessageChain, errorOutputContainer);
if (elaborated) {
reportedError = true;
}
else {
reportedError = true;
if (!elaborated) {
// Issue error on the prop itself, since the prop couldn't elaborate the error
const resultObj: { errors?: Diagnostic[] } = errorOutputContainer || {};
// Use the expression type, if available
const specificSource = next ? checkExpressionForMutableLocationWithContextualType(next, sourcePropType) : sourcePropType;
const result = checkTypeRelatedTo(specificSource, targetPropType, relation, prop, errorMessage, containingMessageChain, resultObj);
if (result && specificSource !== sourcePropType) {
// If for whatever reason the expression type doesn't yield an error, make sure we still issue an error on the sourcePropType
checkTypeRelatedTo(sourcePropType, targetPropType, relation, prop, errorMessage, containingMessageChain, resultObj);
if (exactOptionalPropertyTypes && isExactOptionalPropertyMismatch(specificSource, targetPropType)) {
const diag = createDiagnosticForNode(prop, Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_type_of_the_target, typeToString(specificSource), typeToString(targetPropType));
diagnostics.add(diag);
resultObj.errors = [diag];
}
else {
const targetIsOptional = !!(propName && (getPropertyOfType(target, propName) || unknownSymbol).flags & SymbolFlags.Optional);
const sourceIsOptional = !!(propName && (getPropertyOfType(source, propName) || unknownSymbol).flags & SymbolFlags.Optional);
targetPropType = removeMissingType(targetPropType, targetIsOptional);
sourcePropType = removeMissingType(sourcePropType, targetIsOptional && sourceIsOptional);
const result = checkTypeRelatedTo(specificSource, targetPropType, relation, prop, errorMessage, containingMessageChain, resultObj);
if (result && specificSource !== sourcePropType) {
// If for whatever reason the expression type doesn't yield an error, make sure we still issue an error on the sourcePropType
checkTypeRelatedTo(sourcePropType, targetPropType, relation, prop, errorMessage, containingMessageChain, resultObj);
}
}
if (resultObj.errors) {
const reportedDiag = resultObj.errors[resultObj.errors.length - 1];
Expand Down Expand Up @@ -16813,7 +16819,6 @@ namespace ts {
}
}
}
reportedError = true;
}
}
}
Expand Down Expand Up @@ -17679,10 +17684,18 @@ namespace ts {
else if (sourceType === targetType) {
message = Diagnostics.Type_0_is_not_assignable_to_type_1_Two_different_types_with_this_name_exist_but_they_are_unrelated;
}
else if (exactOptionalPropertyTypes && getExactOptionalUnassignableProperties(source, target).length) {
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;
}
else {
message = Diagnostics.Type_0_is_not_assignable_to_type_1;
}
}
else if (message === Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1
&& exactOptionalPropertyTypes
&& getExactOptionalUnassignableProperties(source, target).length) {
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;
}

reportError(message, generalizedSourceType, targetType);
}
Expand Down Expand Up @@ -19598,6 +19611,20 @@ namespace ts {
return isUnitType(type) || !!(type.flags & TypeFlags.TemplateLiteral);
}

function getExactOptionalUnassignableProperties(source: Type, target: Type) {
if (isTupleType(source) && isTupleType(target)) return emptyArray;
return getPropertiesOfType(target)
.filter(targetProp => isExactOptionalPropertyMismatch(getTypeOfPropertyOfType(source, targetProp.escapedName), getTypeOfSymbol(targetProp)));
}

function isExactOptionalPropertyMismatch(source: Type | undefined, target: Type | undefined) {
return !!source && !!target && maybeTypeOfKind(source, TypeFlags.Undefined) && !!containsMissingType(target);
}

function getExactOptionalProperties(type: Type) {
return getPropertiesOfType(type).filter(targetProp => containsMissingType(getTypeOfSymbol(targetProp)));
}

function getBestMatchingType(source: Type, target: UnionOrIntersectionType, isRelatedTo = compareTypesAssignable) {
return findMatchingDiscriminantType(source, target, isRelatedTo, /*skipPartial*/ true) ||
findMatchingTypeReferenceOrTypeAliasReference(source, target) ||
Expand Down Expand Up @@ -32479,8 +32506,16 @@ namespace ts {
Diagnostics.The_left_hand_side_of_an_assignment_expression_must_be_a_variable_or_a_property_access,
Diagnostics.The_left_hand_side_of_an_assignment_expression_may_not_be_an_optional_property_access)
&& (!isIdentifier(left) || unescapeLeadingUnderscores(left.escapedText) !== "exports")) {

let headMessage: DiagnosticMessage | undefined;
if (exactOptionalPropertyTypes && isPropertyAccessExpression(left) && maybeTypeOfKind(valueType, TypeFlags.Undefined)) {
const target = getTypeOfPropertyOfType(getTypeOfExpression(left.expression), left.name.escapedText);
if (isExactOptionalPropertyMismatch(valueType, target)) {
headMessage = Diagnostics.Type_0_is_not_assignable_to_type_1_with_exactOptionalPropertyTypes_Colon_true_Consider_adding_undefined_to_the_type_of_the_target;
}
}
// to avoid cascading errors check assignability only if 'isReference' check succeeded and no errors were reported
checkTypeAssignableToAndOptionallyElaborate(valueType, leftType, left, right);
checkTypeAssignableToAndOptionallyElaborate(valueType, leftType, left, right, headMessage);
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,10 @@
"category": "Error",
"code": 2374
},
"Type '{0}' is not assignable to type '{1}' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.": {
"category": "Error",
"code": 2375
},
"A 'super' call must be the first statement in the constructor when a class contains initialized properties, parameter properties, or private identifiers.": {
"category": "Error",
"code": 2376
Expand All @@ -1750,6 +1754,10 @@
"category": "Error",
"code": 2378
},
"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.": {
"category": "Error",
"code": 2379
},
"The return type of a 'get' accessor must be assignable to its 'set' accessor type": {
"category": "Error",
"code": 2380
Expand Down Expand Up @@ -1874,6 +1882,10 @@
"category": "Error",
"code": 2410
},
"Type '{0}' is not assignable to type '{1}' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the type of the target.": {
"category": "Error",
"code": 2412
},
"Property '{0}' of type '{1}' is not assignable to '{2}' index type '{3}'.": {
"category": "Error",
"code": 2411
Expand Down Expand Up @@ -7118,6 +7130,10 @@
"category": "Message",
"code": 95168
},
"Add 'undefined' to optional property type": {
"category": "Message",
"code": 95169
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4308,6 +4308,7 @@ namespace ts {
* e.g. it specifies `kind: "a"` and obj has `kind: "b"`.
*/
/* @internal */ isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression | JsxAttributes): boolean;
/* @internal */ getExactOptionalProperties(type: Type): Symbol[];
/**
* For a union, will include a property if it's defined in *any* of the member types.
* So for `{ a } | { b }`, this will include both `a` and `b`.
Expand Down
3 changes: 2 additions & 1 deletion src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,8 @@ namespace FourSlash {
if (errors.length) {
this.printErrorLog(/*expectErrors*/ false, errors);
const error = errors[0];
this.raiseError(`Found an error: ${this.formatPosition(error.file!, error.start!)}: ${error.messageText}`);
const message = typeof error.messageText === "string" ? error.messageText : error.messageText.messageText;
this.raiseError(`Found an error: ${this.formatPosition(error.file!, error.start!)}: ${message}`);
}
});
}
Expand Down
28 changes: 9 additions & 19 deletions src/services/codefixes/addMissingAwait.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace ts.codefix {
errorCodes,
getCodeActions: context => {
const { sourceFile, errorCode, span, cancellationToken, program } = context;
const expression = getFixableErrorSpanExpression(sourceFile, errorCode, span, cancellationToken, program);
const expression = getAwaitErrorSpanExpression(sourceFile, errorCode, span, cancellationToken, program);
if (!expression) {
return;
}
Expand All @@ -48,7 +48,7 @@ namespace ts.codefix {
const checker = context.program.getTypeChecker();
const fixedDeclarations = new Set<number>();
return codeFixAll(context, errorCodes, (t, diagnostic) => {
const expression = getFixableErrorSpanExpression(sourceFile, diagnostic.code, diagnostic, cancellationToken, program);
const expression = getAwaitErrorSpanExpression(sourceFile, diagnostic.code, diagnostic, cancellationToken, program);
if (!expression) {
return;
}
Expand All @@ -59,6 +59,13 @@ namespace ts.codefix {
},
});

function getAwaitErrorSpanExpression(sourceFile: SourceFile, errorCode: number, span: TextSpan, cancellationToken: CancellationToken, program: Program) {
const expression = getFixableErrorSpanExpression(sourceFile, span);
return expression
&& isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program)
&& isInsideAwaitableBody(expression) ? expression : undefined;
}

function getDeclarationSiteFix(context: CodeFixContext | CodeFixAllContext, expression: Expression, errorCode: number, checker: TypeChecker, trackChanges: ContextualTrackChangesFunction, fixedDeclarations?: Set<number>) {
const { sourceFile, program, cancellationToken } = context;
const awaitableInitializers = findAwaitableInitializers(expression, sourceFile, cancellationToken, program, checker);
Expand Down Expand Up @@ -95,23 +102,6 @@ namespace ts.codefix {
some(relatedInformation, related => related.code === Diagnostics.Did_you_forget_to_use_await.code));
}

function getFixableErrorSpanExpression(sourceFile: SourceFile, errorCode: number, span: TextSpan, cancellationToken: CancellationToken, program: Program): Expression | undefined {
const token = getTokenAtPosition(sourceFile, span.start);
// Checker has already done work to determine that await might be possible, and has attached
// related info to the node, so start by finding the expression that exactly matches up
// with the diagnostic range.
const expression = findAncestor(token, node => {
if (node.getStart(sourceFile) < span.start || node.getEnd() > textSpanEnd(span)) {
return "quit";
}
return isExpression(node) && textSpansEqual(span, createTextSpanFromNode(node, sourceFile));
}) as Expression | undefined;

return expression
&& isMissingAwaitError(sourceFile, errorCode, span, cancellationToken, program)
&& isInsideAwaitableBody(expression) ? expression : undefined;
}

interface AwaitableInitializer {
expression: Expression;
declarationSymbol: Symbol;
Expand Down
Loading