[no-unused-vars] false positives for code inside declare blocks

[kripod]

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest version of the packages.
• I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-unused-vars": "warn"
  }
}

import "next-auth";

declare module "next-auth" {
  interface User {
    id: string;
    givenName: string;
    familyName: string;
  }
}

import { PrismaClient } from "@prisma/client";

// Namespace declarations are also problematic
declare namespace globalThis {
  let prisma: PrismaClient; // Value can be anything else
}

Expected Result

No warnings should be emitted.

Actual Result

Invalid warnings are emitted inside declare blocks, unlike in v4.8.

Versions

package	version
@typescript-eslint/eslint-plugin	4.9.0
@typescript-eslint/parser	4.9.0
TypeScript	4.1.2
ESLint	7.14.0
node	14.15.0

[vidaritos]

Already fixed (I think?), pending release: #2833

[kripod]

@vidaritos I saw that PR but don’t think it has anything to do with declare blocks, unfortunately. Thank you for pointing out that fixes are already being made.

[bradzacher]

I am unable to reproduce this against master

Could you please create a repro repo to help me track down the issue?

[whitetrefoil]

I've met this issue in my proj:
https://github.com/whitetrefoil/whitetrefoil-checkin-www/tree/temp/typescript-eslint-2844
It's not a minimal repro, but only 2 files are related:

/src/store/types.ts
Here declares RootState.

/src/hooks/use-root-selector.ts
And it's used here.

[bradzacher]

Thanks for that @whitetrefoil

I've figured it out.
This is a race condition between naming-convention and no-unused-vars.
If no-unused-vars runs first - it works as expected.
But if naming-convention runs first - it breaks.

Why?

Because naming-convention uses the same utility as no-unused-vars does to detect unused variables, and this utility caches the results for each AST (to save duplicate work!)

typescript-eslint/packages/eslint-plugin/src/util/collectUnusedVariables.ts

Lines 39 to 43 in aadb39f

	const program = context.getSourceCode().ast;
	const cached = this.RESULTS_CACHE.get(program);
	if (cached) {
	return cached;
	}

However - no-unused-vars has a few additional selectors it runs in order for it to not report on ambient declarations. naming-convention doesn't run these because it doesn't care.

typescript-eslint/packages/eslint-plugin/src/rules/no-unused-vars.ts

Lines 250 to 288 in a8227a6

	// declaration file handling
	[ambientDeclarationSelector(AST_NODE_TYPES.Program, true)](
	node: DeclarationSelectorNode,
	): void {
	if (!util.isDefinitionFile(filename)) {
	return;
	}
	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 {
	markDeclarationChildAsUsed(node);
	},
	// declared namespace handling
	[ambientDeclarationSelector(
	'TSModuleDeclaration[declare = true] > TSModuleBlock',
	false,
	)](node: DeclarationSelectorNode): void {
	const moduleDecl = util.nullThrows(
	node.parent?.parent,
	util.NullThrowsReasons.MissingParent,
	) as TSESTree.TSModuleDeclaration;
	// 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)
	) {
	return;
	}
	markDeclarationChildAsUsed(node);
	},

So if naming-convention runs first, it'll generate list X, which includes any ambient variables, then no-unused-vars will run, and it'll mark the ambient variables as used, but the utility will still generate list X because of caching.

This is an easy fix.

[kripod]

Hooray, thank you for figuring this out and fixing it so fast! 🙌