Skip to content
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

Fix: checkAliasSymbol crash when checking for @deprecated #42971

Merged
merged 2 commits into from Feb 26, 2021

Conversation

@sandersn
Copy link
Member

@sandersn sandersn commented Feb 25, 2021

It's possible that we shouldn't be creating symbol with no declarations from non-homomorphic mapped types, but for 4.2, the right fix is to make the @deprecated-check in checkAliasSymbol ensure that target.declarations is defined.

Fixes #42957, test case based on @types/http-errors

sandersn added 2 commits Feb 25, 2021
It's possible that we shouldn't be creating symbol with no declarations
from non-homomorphic mapped types, but for 4.2, the right fix is to make
the @deprecated-check in checkAliasSymbol ensure that
target.declarations is defined.
@sandersn
Copy link
Member Author

@sandersn sandersn commented Feb 25, 2021

@RyanCavanaugh @DanielRosenwasser you are probably interested in this too.

@sandersn
Copy link
Member Author

@sandersn sandersn commented Feb 25, 2021

We've been creating symbols this way since at least 3.0. The right thing to do is to put undefined into Symbol.declarations' type.

@@ -37124,7 +37124,7 @@ namespace ts {
error(node, Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type);
}

if (isImportSpecifier(node) && every(target.declarations, d => !!(getCombinedNodeFlags(d) & NodeFlags.Deprecated))) {

This comment has been minimized.

@weswigham

weswigham Feb 25, 2021
Member

Our every helper doesn't handle undefined? Most of our other array helpers treat undefined as an empty array input. Maybe we should change the helper?

This comment has been minimized.

@sandersn

sandersn Feb 26, 2021
Author Member

It does handle undefined, but every(undefined, _) ==> true, and then addDeprecatedSuggestion crashes on the next line.

This fix is shorter than target.declarations && every(target.declarations, ... and uses the standard utility, so I think it's better.

This comment has been minimized.

@weswigham

weswigham Feb 26, 2021
Member

Pffffff okay. Yeah.

@sandersn sandersn merged commit 2c5cee5 into master Feb 26, 2021
10 checks passed
10 checks passed
@github-actions
build (10.x)
Details
@github-actions
CodeQL-Build CodeQL-Build
Details
@github-actions
CodeQL-Build CodeQL-Build
Details
@github-actions
build (12.x)
Details
@github-actions
build (14.x)
Details
@github-code-scanning
CodeQL No new or fixed alerts
Details
@microsoft-cla
license/cla All CLA requirements met.
Details
@azure-pipelines
node10 Build #97028 succeeded
Details
@azure-pipelines
node12 Build #97026 succeeded
Details
@azure-pipelines
node14 Build #97027 succeeded
Details
@sandersn
Copy link
Member Author

@sandersn sandersn commented Feb 26, 2021

@typescript-bot cherry-pick this to 4.2

@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Feb 26, 2021

Heya @sandersn, I couldn't find the branch '4.2' on Microsoft/TypeScript. You may need to make it and try again.

@sandersn
Copy link
Member Author

@sandersn sandersn commented Feb 26, 2021

@typescript-bot cherry-pick this to release-4.2

@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Feb 26, 2021

Heya @sandersn, I've started to run the task to cherry-pick this into release-4.2 on this PR at 75449de. You can monitor the build here.

@sandersn sandersn deleted the fix-checkAliasSymbol-crash-on-checking-deprecated branch Feb 26, 2021
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Feb 26, 2021

Hey @sandersn, I've opened #42972 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Feb 26, 2021
Component commits:
9f9825a Fix: checkAliasSymbol crash when checking for @deprecated
It's possible that we shouldn't be creating symbol with no declarations
from non-homomorphic mapped types, but for 4.2, the right fix is to make
the @deprecated-check in checkAliasSymbol ensure that
target.declarations is defined.

75449de Add bug number and accept baselines
sandersn added a commit that referenced this pull request Feb 26, 2021
Component commits:
9f9825a Fix: checkAliasSymbol crash when checking for @deprecated
It's possible that we shouldn't be creating symbol with no declarations
from non-homomorphic mapped types, but for 4.2, the right fix is to make
the @deprecated-check in checkAliasSymbol ensure that
target.declarations is defined.

75449de Add bug number and accept baselines

Co-authored-by: Nathan Shively-Sanders <nathansa@microsoft.com>
This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants