Skip to content

feat(eslint-plugin): [no-unused-var] handle implicit exports in declaration files #10714

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

Merged
merged 8 commits into from
Mar 3, 2025
Merged

feat(eslint-plugin): [no-unused-var] handle implicit exports in declaration files #10714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7117fcc
WIP
jakebailey Mar 3, 2024
3d2fc3b
WIP
jakebailey Mar 3, 2024
f9f18c5
revert nx
jakebailey Mar 6, 2024
2264c7d
Merge remote-tracking branch 'origin/main' into implicit-export-names…
ronami Jan 24, 2025
02d1b06
initial implementation
ronami Jan 25, 2025
e307ce3
change implementation to use existing ambient code
ronami Jan 25, 2025
4eb0d58
refactor
ronami Mar 2, 2025
f84ded5
fix lint
ronami Mar 3, 2025
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
122 changes: 96 additions & 26 deletions packages/eslint-plugin/src/rules/no-unused-vars.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
} from '@typescript-eslint/scope-manager';
import { AST_NODE_TYPES, TSESLint } from '@typescript-eslint/utils';

import type { MakeRequired } from '../util';

import {
collectVariables,
createRule,
Expand Down Expand Up @@ -58,6 +60,11 @@ type VariableType =
| 'parameter'
| 'variable';

type ModuleDeclarationWithBody = MakeRequired<
TSESTree.TSModuleDeclaration,
'body'
>;

export default createRule<Options, MessageIds>({
name: 'no-unused-vars',
meta: {
Expand Down Expand Up @@ -144,7 +151,10 @@ export default createRule<Options, MessageIds>({
},
defaultOptions: [{}],
create(context, [firstOption]) {
const MODULE_DECL_CACHE = new Map<TSESTree.TSModuleDeclaration, boolean>();
const MODULE_DECL_CACHE = new Map<
ModuleDeclarationWithBody | TSESTree.Program,
boolean
>();

const options = ((): TranslatedOptions => {
const options: TranslatedOptions = {
Expand Down Expand Up @@ -557,40 +567,71 @@ export default createRule<Options, MessageIds>({
}

return {
// declaration file handling
[ambientDeclarationSelector(AST_NODE_TYPES.Program, true)](
// top-level declaration file handling
[ambientDeclarationSelector(AST_NODE_TYPES.Program)](
Copy link
Member Author

@ronami ronami Jan 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Testing various edge cases, I think that only checking `declare'd module-level values creates some incorrect reports (playground link).

Removing this didn't cause any tests to fail, though I could be missing an edge case.

node: DeclarationSelectorNode,
): void {
if (!isDefinitionFile(context.filename)) {
return;
}

const moduleDecl = nullThrows(
node.parent,
NullThrowsReasons.MissingParent,
) as TSESTree.Program;

if (checkForOverridingExportStatements(moduleDecl)) {
return;
}
Comment on lines +578 to +585
Copy link
Member Author

Choose a reason for hiding this comment

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

This addition is so the rule would report the following (playground link):

// should be reported but doesn't
declare class Foo { }

export {}

Along with not reporting on the following (playground link):

// reported but shouldn't
class Foo { }

// even though this shows up as a compiler error, this isn't marked as used
// uncomment the code below and TypeScript would mark this as unused:

// export {};


markDeclarationChildAsUsed(node);
},

// children of a namespace that is a child of a declared namespace are auto-exported
[ambientDeclarationSelector(
'TSModuleDeclaration[declare = true] > TSModuleBlock TSModuleDeclaration > TSModuleBlock',
false,
)](node: DeclarationSelectorNode): void {
const moduleDecl = nullThrows(
node.parent.parent,
NullThrowsReasons.MissingParent,
) as ModuleDeclarationWithBody;

if (checkForOverridingExportStatements(moduleDecl)) {
return;
}

markDeclarationChildAsUsed(node);
},

// declared namespace handling
[ambientDeclarationSelector(
'TSModuleDeclaration[declare = true] > TSModuleBlock',
false,
)](node: DeclarationSelectorNode): void {
const moduleDecl = nullThrows(
node.parent.parent,
NullThrowsReasons.MissingParent,
) as TSESTree.TSModuleDeclaration;
) as ModuleDeclarationWithBody;

// declared ambient modules with an `export =` statement will only export that one thing
// all other statements are not automatically exported in this case
if (
moduleDecl.id.type === AST_NODE_TYPES.Literal &&
checkModuleDeclForExportEquals(moduleDecl)
) {
if (checkForOverridingExportStatements(moduleDecl)) {
return;
}

markDeclarationChildAsUsed(node);
},

// namespace handling in definition files
[ambientDeclarationSelector('TSModuleDeclaration > TSModuleBlock')](
node: DeclarationSelectorNode,
): void {
if (!isDefinitionFile(context.filename)) {
return;
}
const moduleDecl = nullThrows(
node.parent.parent,
NullThrowsReasons.MissingParent,
) as ModuleDeclarationWithBody;

if (checkForOverridingExportStatements(moduleDecl)) {
return;
}

Expand Down Expand Up @@ -670,21 +711,19 @@ export default createRule<Options, MessageIds>({
},
};

function checkModuleDeclForExportEquals(
node: TSESTree.TSModuleDeclaration,
function checkForOverridingExportStatements(
node: ModuleDeclarationWithBody | TSESTree.Program,
): boolean {
const cached = MODULE_DECL_CACHE.get(node);
if (cached != null) {
return cached;
}

if (node.body) {
for (const statement of node.body.body) {
if (statement.type === AST_NODE_TYPES.TSExportAssignment) {
MODULE_DECL_CACHE.set(node, true);
return true;
}
}
const body = getStatementsOfNode(node);

if (hasOverridingExportStatement(body)) {
MODULE_DECL_CACHE.set(node, true);
return true;
}

MODULE_DECL_CACHE.set(node, false);
Expand All @@ -700,10 +739,7 @@ export default createRule<Options, MessageIds>({
| TSESTree.TSModuleDeclaration
| TSESTree.TSTypeAliasDeclaration
| TSESTree.VariableDeclaration;
function ambientDeclarationSelector(
parent: string,
childDeclare: boolean,
): string {
function ambientDeclarationSelector(parent: string): string {
return [
// Types are ambiently exported
`${parent} > :matches(${[
Expand All @@ -717,7 +753,7 @@ export default createRule<Options, MessageIds>({
AST_NODE_TYPES.TSEnumDeclaration,
AST_NODE_TYPES.TSModuleDeclaration,
AST_NODE_TYPES.VariableDeclaration,
].join(', ')})${childDeclare ? '[declare = true]' : ''}`,
].join(', ')})`,
].join(', ');
}
function markDeclarationChildAsUsed(node: DeclarationSelectorNode): void {
Expand Down Expand Up @@ -774,6 +810,40 @@ export default createRule<Options, MessageIds>({
},
});

function hasOverridingExportStatement(
body: TSESTree.ProgramStatement[],
): boolean {
for (const statement of body) {
if (
(statement.type === AST_NODE_TYPES.ExportNamedDeclaration &&
statement.declaration == null) ||
statement.type === AST_NODE_TYPES.ExportAllDeclaration ||
statement.type === AST_NODE_TYPES.TSExportAssignment
) {
return true;
}

if (
statement.type === AST_NODE_TYPES.ExportDefaultDeclaration &&
statement.declaration.type === AST_NODE_TYPES.Identifier
) {
return true;
}
}

return false;
}

function getStatementsOfNode(
block: ModuleDeclarationWithBody | TSESTree.Program,
): TSESTree.ProgramStatement[] {
if (block.type === AST_NODE_TYPES.Program) {
return block.body;
}

return block.body.body;
}

/*

###### TODO ######
Expand Down
Loading