TSAbstractClassDeclaration are not defined in the scope (no-undef) #20
Description
This issue was initially reported here: eslint/typescript-eslint-parser#578
What version of TypeScript are you using?
3.1.3
What version of typescript-eslint-parser
are you using?
21.0.2
What code were you trying to parse?
export abstract class Foo {}
export class FooBar extends Foo {}
What did you expect to happen?
No lint errors
What happened?
2:29 error 'Foo' is not defined no-undef
While I understand that Typescript already has scope checks that seem to make no-undef unnecessary and can be disabled, it has been useful to ensure that name
and length
are not accidentally used, see: microsoft/TypeScript#18433
Potential Fix 1
eslint-scope/lib/referencer.js:Referencer.visitClass adds a definition into the scope for "ClassDeclaration", which does not happen for "TSAbstractClassDeclaration". I think this can be fixed by doing:
TSAbstractClassDeclaration(node) {
this.currentScope().__define(
node.id,
new Definition("ClassName", node.id, node, null, null, null)
);
this.ClassDeclaration(node);
}
However this then causes:
1:23 error 'Foo' is already declared in the upper scope no-shadow
Because of this special casing of ClassDeclaration in eslint/lib/rules/no-shadow.js:
/**
* Checks if a variable of the class name in the class scope of ClassDeclaration.
*
* ClassDeclaration creates two variables of its name into its outer scope and its class scope.
* So we should ignore the variable in the class scope.
*
* @param {Object} variable The variable to check.
* @returns {boolean} Whether or not the variable of the class name in the class scope of ClassDeclaration.
*/
function isDuplicatedClassNameVariable(variable) {
const block = variable.scope.block;
return block.type === "ClassDeclaration" && block.id === variable.identifiers[0];
}
From here, I suppose you could just say that no-shadow should be disabled on typescript files, in favor of using tslint's no-shadowed-variables
Potential Fix 2
In typescript-eslint-parser/parser.js, we can simply re-map the abstract class node:
case "TSAbstractClassDeclaration":
node.type = 'ClassDeclaration';
break;
which to me seems pretty clean, since as far as eslint is concerned, the abstract keyword doesn't affect anything.