Skip to content

Commit

Permalink
fix(eslint-plugin): [no-shadow] fix static class method generics shad…
Browse files Browse the repository at this point in the history
…owing class generics (#3393)

Fixes #2592

Unfortunately I think this is impossible to solve from the scope analysis side.. So I opted instead to just ignore it from the lint rule.
  • Loading branch information
bradzacher committed May 15, 2021
1 parent b85261c commit b1e1c8a
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 0 deletions.
85 changes: 85 additions & 0 deletions packages/eslint-plugin/src/rules/no-shadow.ts
Expand Up @@ -126,6 +126,84 @@ export default util.createRule<Options, MessageIds>({
return util.isFunctionType(id.parent);
}

function isGenericOfStaticMethod(
variable: TSESLint.Scope.Variable,
): boolean {
if (!('isTypeVariable' in variable)) {
// this shouldn't happen...
return false;
}

if (!variable.isTypeVariable) {
return false;
}

if (variable.identifiers.length === 0) {
return false;
}

const typeParameter = variable.identifiers[0].parent;
if (typeParameter?.type !== AST_NODE_TYPES.TSTypeParameter) {
return false;
}
const typeParameterDecl = typeParameter.parent;
if (
typeParameterDecl?.type !== AST_NODE_TYPES.TSTypeParameterDeclaration
) {
return false;
}
const functionExpr = typeParameterDecl.parent;
if (
!functionExpr ||
(functionExpr.type !== AST_NODE_TYPES.FunctionExpression &&
functionExpr.type !== AST_NODE_TYPES.TSEmptyBodyFunctionExpression)
) {
return false;
}
const methodDefinition = functionExpr.parent;
if (methodDefinition?.type !== AST_NODE_TYPES.MethodDefinition) {
return false;
}
return methodDefinition.static;
}

function isGenericOfClassDecl(variable: TSESLint.Scope.Variable): boolean {
if (!('isTypeVariable' in variable)) {
// this shouldn't happen...
return false;
}

if (!variable.isTypeVariable) {
return false;
}

if (variable.identifiers.length === 0) {
return false;
}

const typeParameter = variable.identifiers[0].parent;
if (typeParameter?.type !== AST_NODE_TYPES.TSTypeParameter) {
return false;
}
const typeParameterDecl = typeParameter.parent;
if (
typeParameterDecl?.type !== AST_NODE_TYPES.TSTypeParameterDeclaration
) {
return false;
}
const classDecl = typeParameterDecl.parent;
return classDecl?.type === AST_NODE_TYPES.ClassDeclaration;
}

function isGenericOfAStaticMethodShadow(
variable: TSESLint.Scope.Variable,
shadowed: TSESLint.Scope.Variable,
): boolean {
return (
isGenericOfStaticMethod(variable) && isGenericOfClassDecl(shadowed)
);
}

/**
* Check if variable name is allowed.
* @param variable The variable to check.
Expand Down Expand Up @@ -321,6 +399,13 @@ export default util.createRule<Options, MessageIds>({
continue;
}

// ignore static class method generic shadowing class generic
// this is impossible for the scope analyser to understand
// so we have to handle this manually in this rule
if (isGenericOfAStaticMethodShadow(variable, shadowed)) {
continue;
}

const isESLintGlobal = 'writeable' in shadowed;
if (
(shadowed.identifiers.length > 0 ||
Expand Down
13 changes: 13 additions & 0 deletions packages/eslint-plugin/tests/rules/no-shadow.test.ts
Expand Up @@ -159,6 +159,19 @@ type Fn = (Foo: string) => typeof Foo;
`,
options: [{ ignoreFunctionTypeParameterNameValueShadow: false }],
},
`
export class Wrapper<Wrapped> {
private constructor(private readonly wrapped: Wrapped) {}
unwrap(): Wrapped {
return this.wrapped;
}
static create<Wrapped>(wrapped: Wrapped) {
return new Wrapper<Wrapped>(wrapped);
}
}
`,
],
invalid: [
{
Expand Down

0 comments on commit b1e1c8a

Please sign in to comment.