-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
664d749
Simple first version
sandersn d1008eb
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn 671096b
Lots of cases work
sandersn a728852
Support variable declarations, property assignments, destructuring
sandersn 715f235
More cleanup
sandersn cfea6e1
skip all d.ts, not just node_modules/lib
sandersn d058191
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn 334ca65
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn 41f0525
Offer a codefix for a lot more cases
sandersn 09c916f
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn cdd40d3
remove incorrect tuple check
sandersn f018b8e
Use getSymbolId instead of converting to string
sandersn 960e657
add test + switch to tracking number symbol ids
sandersn cdbe969
Address PR comments
sandersn 22b9e7a
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn e9607f1
Exclude tuples from suggestion
sandersn 2c1982f
Better way to get error node
sandersn 3ffe9db
fix semicolon lint
sandersn 3e70798
fix another crash
sandersn 4b7b3fe
Simplify: add undefined to all optional propertie
sandersn d535509
remove fix-all
sandersn e027d1e
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn cc51d23
Merge branch 'main' into fix-exact-optional-unassignable-properties
sandersn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -634,6 +634,8 @@ namespace ts { | |
| isTupleType, | ||
| isArrayLikeType, | ||
| isTypeInvalidDueToUnionDiscriminant, | ||
| getExactOptionalUnassignableProperties, | ||
| isExactOptionalPropertyMismatch, | ||
| getAllPossiblePropertiesOfTypes, | ||
| getSuggestedSymbolForNonexistentProperty, | ||
| getSuggestionForNonexistentProperty, | ||
|
|
@@ -16735,24 +16737,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); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this verbatim inside the new |
||
| if (resultObj.errors) { | ||
| const reportedDiag = resultObj.errors[resultObj.errors.length - 1]; | ||
|
|
@@ -16780,7 +16787,6 @@ namespace ts { | |
| } | ||
| } | ||
| } | ||
| reportedError = true; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -17646,10 +17652,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); | ||
| } | ||
|
|
@@ -19565,6 +19579,16 @@ 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 getBestMatchingType(source: Type, target: UnionOrIntersectionType, isRelatedTo = compareTypesAssignable) { | ||
| return findMatchingDiscriminantType(source, target, isRelatedTo, /*skipPartial*/ true) || | ||
| findMatchingTypeReferenceOrTypeAliasReference(source, target) || | ||
|
|
@@ -32426,8 +32450,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); | ||
| } | ||
| } | ||
| } | ||
|
|
||
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
driveby fix; reportedError was always set to true in this block, regardless of whether
elaboratedis true, and it's not checked anywhere in this block, so it's fine to set it at the top.