Skip to content

fix: resolve nested instantiations of the same generic alias #58

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.

Sign up for GitHub

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

Open
mmnavarr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: resolve nested instantiations of the same generic alias #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 27 additions & 7 deletions skills/install-anti-slop/assets/anti-slop/shared/type-alias-resolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ type TypeBinding = {
readonly scope: TypeScope;
};

type ResolvingAliases = ReadonlySet<ESTree.TSTypeAliasDeclaration>;

/**
* A type argument bound to a type parameter, with the substitutions and alias expansions
* that were active where the argument was written.
*/
type Substitution = {
readonly resolvingAliases: ResolvingAliases;
readonly substitutions: Substitutions;
readonly type: ESTree.TSType;
};
Expand Down Expand Up @@ -191,6 +198,8 @@ function aliasSubstitutions(
alias: ESTree.TSTypeAliasDeclaration,
reference: ESTree.TSTypeReference,
base: Substitutions,
baseResolving: ResolvingAliases,
aliasResolving: ResolvingAliases,
): Substitutions | null {
const parameters = alias.typeParameters?.params ?? [];
const arguments_ = reference.typeArguments?.params ?? [];
Expand All @@ -199,10 +208,12 @@ function aliasSubstitutions(
const explicitArgument = arguments_[index];
const argument = explicitArgument ?? parameter.default;
if (argument === null || argument === undefined) return null;
const argumentSubstitutions = explicitArgument === undefined ? next : base;
// Explicit arguments were written at the reference; defaults inside the alias.
const isDefault = explicitArgument === undefined;
next.set(parameter.name.name, {
type: argument,
substitutions: new Map(argumentSubstitutions),
substitutions: new Map(isDefault ? next : base),
resolvingAliases: isDefault ? aliasResolving : baseResolving,
});
}
return next;
Expand All @@ -217,25 +228,34 @@ export function resolvedTypeMatches(
const evaluate = (
current: ESTree.TSType,
substitutions: Substitutions,
resolvingAliases: ReadonlySet<ESTree.TSTypeAliasDeclaration>,
resolvingAliases: ResolvingAliases,
): boolean => {
if (current.type === "TSTypeReference") {
const name = typeReferenceName(current);
if (name !== null) {
const substitution = substitutions.get(name);
if (substitution !== undefined && !current.typeArguments?.params.length) {
// Resume in the argument's own context so a separate, finite instantiation of an
// alias that is still being expanded (`Maybe<Maybe<unknown>>`) is not mistaken for
// recursion.
return evaluate(
substitution.type,
substitution.substitutions,
resolvingAliases,
substitution.resolvingAliases,
);
}
const alias = visibleTypeAlias(name, current, environment);
if (alias !== null && !resolvingAliases.has(alias)) {
const nextSubstitutions = aliasSubstitutions(alias, current, substitutions);
const nextResolving = new Set(resolvingAliases);
nextResolving.add(alias);
const nextSubstitutions = aliasSubstitutions(
alias,
current,
substitutions,
resolvingAliases,
nextResolving,
);
if (nextSubstitutions !== null) {
const nextResolving = new Set(resolvingAliases);
nextResolving.add(alias);
return evaluate(alias.typeAnnotation, nextSubstitutions, nextResolving);
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/rules/no-unknown-type-aliases.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ tester.run("anti-slop/no-unknown-type-aliases", noUnknownTypeAliasesRule, {
"type Alias = string; type UserId = Alias;",
"type Payload = string | number;",
"type Box<T> = { readonly value: T }; type Payload = Box<unknown>;",
// Genuinely recursive aliases terminate without a finding.
"type Loop<T> = Loop<T>; type Payload = Loop<string>;",
"type Nested<T> = T | Nested<Nested<T>>; type Payload = Nested<string>;",
"type Tree<T> = { readonly value: T; readonly children: Tree<T>[] }; type Forest = Tree<unknown>;",
"type Maybe<T> = T | undefined; type Payload = Maybe<Maybe<string>>;",
],
invalid: [
{ code: "type Alias = unknown;", errors: [error] },
Expand All @@ -29,6 +34,18 @@ tester.run("anti-slop/no-unknown-type-aliases", noUnknownTypeAliasesRule, {
code: "type Identity<T> = T; type Wrapped<T> = Identity<T>; type Payload = Wrapped<unknown>;",
errors: [error],
},
{
code: "type Maybe<T> = T | undefined; type Hidden = Maybe<Maybe<unknown>>;",
errors: [error],
},
{
code: "type Identity<T> = T; type Payload = Identity<Identity<Identity<unknown>>>;",
errors: [error],
},
{
code: "type Pair<A, B> = A | B; type Payload = Pair<string, Pair<number, unknown>>;",
errors: [error],
},
{
code: "function outer() { type Payload = unknown; }",
errors: [error],
Expand Down
34 changes: 27 additions & 7 deletions src/shared/type-alias-resolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ type TypeBinding = {
readonly scope: TypeScope;
};

type ResolvingAliases = ReadonlySet<ESTree.TSTypeAliasDeclaration>;

/**
* A type argument bound to a type parameter, with the substitutions and alias expansions
* that were active where the argument was written.
*/
type Substitution = {
readonly resolvingAliases: ResolvingAliases;
readonly substitutions: Substitutions;
readonly type: ESTree.TSType;
};
Expand Down Expand Up @@ -191,6 +198,8 @@ function aliasSubstitutions(
alias: ESTree.TSTypeAliasDeclaration,
reference: ESTree.TSTypeReference,
base: Substitutions,
baseResolving: ResolvingAliases,
aliasResolving: ResolvingAliases,
): Substitutions | null {
const parameters = alias.typeParameters?.params ?? [];
const arguments_ = reference.typeArguments?.params ?? [];
Expand All @@ -199,10 +208,12 @@ function aliasSubstitutions(
const explicitArgument = arguments_[index];
const argument = explicitArgument ?? parameter.default;
if (argument === null || argument === undefined) return null;
const argumentSubstitutions = explicitArgument === undefined ? next : base;
// Explicit arguments were written at the reference; defaults inside the alias.
const isDefault = explicitArgument === undefined;
next.set(parameter.name.name, {
type: argument,
substitutions: new Map(argumentSubstitutions),
substitutions: new Map(isDefault ? next : base),
resolvingAliases: isDefault ? aliasResolving : baseResolving,
});
}
return next;
Expand All @@ -217,25 +228,34 @@ export function resolvedTypeMatches(
const evaluate = (
current: ESTree.TSType,
substitutions: Substitutions,
resolvingAliases: ReadonlySet<ESTree.TSTypeAliasDeclaration>,
resolvingAliases: ResolvingAliases,
): boolean => {
if (current.type === "TSTypeReference") {
const name = typeReferenceName(current);
if (name !== null) {
const substitution = substitutions.get(name);
if (substitution !== undefined && !current.typeArguments?.params.length) {
// Resume in the argument's own context so a separate, finite instantiation of an
// alias that is still being expanded (`Maybe<Maybe<unknown>>`) is not mistaken for
// recursion.
return evaluate(
substitution.type,
substitution.substitutions,
resolvingAliases,
substitution.resolvingAliases,
);
}
const alias = visibleTypeAlias(name, current, environment);
if (alias !== null && !resolvingAliases.has(alias)) {
const nextSubstitutions = aliasSubstitutions(alias, current, substitutions);
const nextResolving = new Set(resolvingAliases);
nextResolving.add(alias);
const nextSubstitutions = aliasSubstitutions(
alias,
current,
substitutions,
resolvingAliases,
nextResolving,
);
if (nextSubstitutions !== null) {
const nextResolving = new Set(resolvingAliases);
nextResolving.add(alias);
return evaluate(alias.typeAnnotation, nextSubstitutions, nextResolving);
}
}
Expand Down