Skip to content

Prevent substitution types from leaking out of non-deferred conditionals #54894

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

Closed
wants to merge 2 commits into from

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Jul 6, 2023

fixes #54886

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jul 6, 2023
@Andarist Andarist changed the title Fixed an issue with substitution type not always being usable as index type Prevent substitution types from leaking non-deferred conditionals Jul 6, 2023
@Andarist Andarist changed the title Prevent substitution types from leaking non-deferred conditionals Prevent substitution types from leaking out of non-deferred conditionals Jul 6, 2023
@sandersn sandersn requested review from weswigham and ahejlsberg July 19, 2023 20:41
@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jul 19, 2023
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't quite feel like the right fix - I'm actually OK with substitution types exiting conditional boundaries, it's useful in many ways - in fact, it's sometimes the only way chained conditionals like SplitString<Cast<T, string>> typecheck. Naturally, using the intersection means these cases will still work... but the ux when you see these "narrowed" types will get a bit (ok, a lot) worse.

Honestly, a much better fix is noting that in getPropertyTypeForIndexType we use isTypeAssignableToKind to make these errors, which is... not great (any type-kind based analysis is suspect, given how generics work). But so long as we are, updating isTypeAssignableToKind to intersect substitutions is probably ok.

@@ -17951,6 +17951,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
result.aliasTypeArguments = aliasSymbol ? aliasTypeArguments : instantiateTypes(root.aliasTypeArguments, mapper!); // TODO: GH#18217
break;
}
if (result.flags & TypeFlags.Substitution) {
const substitution = result as SubstitutionType;
result = getIntersectionType([substitution.baseType, substitution.constraint]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use getSubstitutionIntersection, if this sticks around.

@Andarist
Copy link
Contributor Author

superseded by #57113

@Andarist Andarist closed this Jul 13, 2024
@sandersn sandersn removed this from PR Backlog Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extends causing string literal can't be used as index
3 participants