no-unused-variable pollutes module resolution cache with bad data, breaking fork-ts-checker-webpack-plugin

[univerio]

Bug Report

• TSLint version: 5.9.1
• TypeScript version: 2.6.2
• Running TSLint via: Realytics/fork-ts-checker-webpack-plugin (Node.js API)

TypeScript code being linted

Full details are here: TypeStrong/fork-ts-checker-webpack-plugin#74

Actual behavior

The CompilerHost created by no-unused-variable does not resolve package.json files, which results in the cached module resolution result for modules that specify typings in their package.json file to become undefined. The observed behavior is module resolution failure for packages that specify typings in their package.json and are not part of the set of packages that are re-resolved from the set of changed files, after reusing SourceFile instances that have been passed through no-unused-variable.

Expected behavior

No such module resolution failures should occur. The "fake" CompilerHost should correctly resolve package.json files. A simple fix that seems to work for me is just to delegate to a normal host, but I'm not sure if this is the correct thing to do:

diff --git src/rules/noUnusedVariableRule.ts src/rules/noUnusedVariableRule.ts
index 5326e480..ed1355ab 100644
--- src/rules/noUnusedVariableRule.ts
+++ src/rules/noUnusedVariableRule.ts
@@ -408,10 +408,23 @@ function makeUnusedCheckedProgram(program: ts.Program, checkParameters: boolean)
         program.getSourceFiles().map<[string, ts.SourceFile]>((s) => [getCanonicalFileName(s.fileName), s]));
 
     // tslint:disable object-literal-sort-keys
+    const host = ts.createCompilerHost(options);
     return ts.createProgram(Array.from(sourceFilesByName.keys()), options, {
-        fileExists: (f) => sourceFilesByName.has(getCanonicalFileName(f)),
-        readFile: (f) => sourceFilesByName.get(getCanonicalFileName(f))!.text,
-        getSourceFile: (f) => sourceFilesByName.get(getCanonicalFileName(f))!,
+        fileExists: (f) => sourceFilesByName.has(getCanonicalFileName(f)) || host.fileExists(f),
+        readFile: (f) => {
+            const sourceFile = sourceFilesByName.get(getCanonicalFileName(f));
+            if (sourceFile) {
+                return sourceFile.text;
+            }
+            return host.readFile(f);
+        },
+        getSourceFile: (fileName, languageVersion, onError, shouldCreateNewSourceFile) => {
+            const sourceFile = sourceFilesByName.get(getCanonicalFileName(fileName));
+            if (sourceFile) {
+                return sourceFile;
+            }
+            return host.getSourceFile(fileName, languageVersion, onError, shouldCreateNewSourceFile);
+        },
         getDefaultLibFileName: () => ts.getDefaultLibFileName(options),
         writeFile: () => undefined,
         getCurrentDirectory: () => "",

[JoshuaKGoldberg]

FWIW, no-unused-variable is deprecated as of TypeScript 2.9 - if you see this error, it's probably best to just disable the rule altogether in your tslint.json.

That being said, it'd be good to have someone with knowledge in this area take a look and see if the rule can be fixed.

[JoshuaKGoldberg]

💀 It's time! 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #4534. ☠️
We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋