feat(typescript-estree)!: throw error on file not in project when project set (#760)

uniqueiniquity · JamesHenry · commit 3777b7706704 · 2019-07-28T14:15:07.000-04:00
BREAKING CHANGE: by default we will now throw when a file is not in the `project` provided
diff --git a/.prettierignore b/.prettierignore
@@ -1,4 +1,5 @@
 **/tests/fixtures/**/*
+**/tests/fixture-project/**/*
 **/dist
 **/coverage
 **/shared-fixtures
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/1.ts b/packages/eslint-plugin-tslint/tests/fixture-project/1.ts
@@ -0,0 +1 @@
+var foo = true;
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/2.ts b/packages/eslint-plugin-tslint/tests/fixture-project/2.ts
@@ -0,0 +1 @@
+throw 'should be ok because rule is not loaded';
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/3.ts b/packages/eslint-plugin-tslint/tests/fixture-project/3.ts
@@ -0,0 +1 @@
+throw 'err'; // no-string-throw
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/4.ts b/packages/eslint-plugin-tslint/tests/fixture-project/4.ts
@@ -0,0 +1 @@
+var foo = true // semicolon
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/5.ts b/packages/eslint-plugin-tslint/tests/fixture-project/5.ts
@@ -0,0 +1 @@
+var foo = true; // fail
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/6.ts b/packages/eslint-plugin-tslint/tests/fixture-project/6.ts
@@ -0,0 +1 @@
+foo;
diff --git a/packages/eslint-plugin-tslint/tests/fixture-project/tsconfig.json b/packages/eslint-plugin-tslint/tests/fixture-project/tsconfig.json
@@ -0,0 +1 @@
+{}
diff --git a/packages/eslint-plugin-tslint/tests/index.spec.ts b/packages/eslint-plugin-tslint/tests/index.spec.ts
@@ -12,7 +12,7 @@ const ruleTester = new TSESLint.RuleTester({
      * Project is needed to generate the parserServices
      * within @typescript-eslint/parser
      */
-    project: './tests/tsconfig.json',
+    project: './tests/fixture-project/tsconfig.json',
   },
   parser: require.resolve('@typescript-eslint/parser'),
 });
@@ -47,6 +47,7 @@ ruleTester.run('tslint/config', rule, {
     {
       code: 'var foo = true;',
       options: tslintRulesConfig,
+      filename: './tests/fixture-project/1.ts',
     },
     {
       filename: './tests/test-project/file-spec.ts',
@@ -62,13 +63,15 @@ ruleTester.run('tslint/config', rule, {
     {
       code: 'throw "should be ok because rule is not loaded";',
       options: tslintRulesConfig,
+      filename: './tests/fixture-project/2.ts',
     },
   ],
 
   invalid: [
     {
       options: [{ lintFile: './tests/test-project/tslint.json' }],
       code: 'throw "err" // no-string-throw',
+      filename: './tests/fixture-project/3.ts',
       errors: [
         {
           messageId: 'failure',
@@ -84,6 +87,7 @@ ruleTester.run('tslint/config', rule, {
       code: 'var foo = true // semicolon',
       options: tslintRulesConfig,
       output: 'var foo = true // semicolon',
+      filename: './tests/fixture-project/4.ts',
       errors: [
         {
           messageId: 'failure',
@@ -100,6 +104,7 @@ ruleTester.run('tslint/config', rule, {
       code: 'var foo = true // fail',
       options: tslintRulesDirectoryConfig,
       output: 'var foo = true // fail',
+      filename: './tests/fixture-project/5.ts',
       errors: [
         {
           messageId: 'failure',
@@ -174,26 +179,30 @@ describe('tslint/error', () => {
     });
   });
 
-  it('should not crash if there is no tslint rules specified', () => {
+  it('should not crash if there are no tslint rules specified', () => {
     const linter = new TSESLint.Linter();
     jest.spyOn(console, 'warn').mockImplementation();
     linter.defineRule('tslint/config', rule);
     linter.defineParser('@typescript-eslint/parser', parser);
     expect(() =>
-      linter.verify('foo;', {
-        parserOptions: {
-          project: `${__dirname}/test-project/tsconfig.json`,
-        },
-        rules: {
-          'tslint/config': [2, {}],
+      linter.verify(
+        'foo;',
+        {
+          parserOptions: {
+            project: `${__dirname}/test-project/tsconfig.json`,
+          },
+          rules: {
+            'tslint/config': [2, {}],
+          },
+          parser: '@typescript-eslint/parser',
         },
-        parser: '@typescript-eslint/parser',
-      }),
+        `${__dirname}/test-project/extra.ts`,
+      ),
     ).not.toThrow();
 
     expect(console.warn).toHaveBeenCalledWith(
       expect.stringContaining(
-        'Tried to lint <input> but found no valid, enabled rules for this file type and file path in the resolved configuration.',
+        `Tried to lint ${__dirname}/test-project/extra.ts but found no valid, enabled rules for this file type and file path in the resolved configuration.`,
       ),
     );
     jest.resetAllMocks();
diff --git a/packages/eslint-plugin-tslint/tests/test-project/extra.ts b/packages/eslint-plugin-tslint/tests/test-project/extra.ts
@@ -0,0 +1 @@
+foo;
diff --git a/packages/eslint-plugin/tests/RuleTester.ts b/packages/eslint-plugin/tests/RuleTester.ts
@@ -7,6 +7,8 @@ type RuleTesterConfig = Omit<TSESLint.RuleTesterConfig, 'parser'> & {
   parser: typeof parser;
 };
 class RuleTester extends TSESLint.RuleTester {
+  private filename: string | undefined = undefined;
+
   // as of eslint 6 you have to provide an absolute path to the parser
   // but that's not as clean to type, this saves us trying to manually enforce
   // that contributors require.resolve everything
@@ -15,6 +17,10 @@ class RuleTester extends TSESLint.RuleTester {
       ...options,
       parser: require.resolve(options.parser),
     });
+
+    if (options.parserOptions && options.parserOptions.project) {
+      this.filename = path.join(getFixturesRootDir(), 'file.ts');
+    }
   }
 
   // as of eslint 6 you have to provide an absolute path to the parser
@@ -26,17 +32,36 @@ class RuleTester extends TSESLint.RuleTester {
     tests: TSESLint.RunTests<TMessageIds, TOptions>,
   ): void {
     const errorMessage = `Do not set the parser at the test level unless you want to use a parser other than ${parser}`;
+
+    if (this.filename) {
+      tests.valid = tests.valid.map(test => {
+        if (typeof test === 'string') {
+          return {
+            code: test,
+            filename: this.filename,
+          };
+        }
+        return test;
+      });
+    }
+
     tests.valid.forEach(test => {
       if (typeof test !== 'string') {
         if (test.parser === parser) {
           throw new Error(errorMessage);
         }
+        if (!test.filename) {
+          test.filename = this.filename;
+        }
       }
     });
     tests.invalid.forEach(test => {
       if (test.parser === parser) {
         throw new Error(errorMessage);
       }
+      if (!test.filename) {
+        test.filename = this.filename;
+      }
     });
 
     super.run(name, rule, tests);
diff --git a/packages/eslint-plugin/tests/fixtures/file.ts b/packages/eslint-plugin/tests/fixtures/file.ts
diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts
@@ -199,7 +199,6 @@ import * as Foo from './foo';
 declare module './foo' {
   const x: Foo.T = 3;
 }`,
-      filename: path.join(rootPath, 'bar.ts'),
       errors: [
         {
           messageId,
diff --git a/packages/parser/README.md b/packages/parser/README.md
@@ -50,8 +50,23 @@ The following additional configuration options are available by specifying them
 
 - **`project`** - default `undefined`. This option allows you to provide a path to your project's `tsconfig.json`. **This setting is required if you want to use rules which require type information**. You may want to use this setting in tandem with the `tsconfigRootDir` option below.
 
+  - Note that if this setting is specified and `createDefaultProgram` is not, you must only lint files that are included in the projects as defined by the provided `tsconfig.json` files. If your existing configuration does not include all of the files you would like to lint, you can create a separate `tsconfig.eslint.json` as follows:
+
+  ```ts
+  {
+    "extends": "./tsconfig.json", // path to existing tsconfig
+    "includes": [
+      "src/**/*.ts",
+      "test/**/*.ts",
+      // etc
+    ]
+  }
+  ```
+
 - **`tsconfigRootDir`** - default `undefined`. This option allows you to provide the root directory for relative tsconfig paths specified in the `project` option above.
 
+- **`createDefaultProgram`** - default `false`. This option allows you to request that when the `project` setting is specified, files will be allowed when not included in the projects defined by the provided `tsconfig.json` files. However, this may incur significant performance costs, so this option is primarily included for backwards-compatibility. See the **`project`** section for more information.
+
 - **`extraFileExtensions`** - default `undefined`. This option allows you to provide one or more additional file extensions which should be considered in the TypeScript Program compilation. E.g. a `.vue` file
 
 - **`warnOnUnsupportedTypeScriptVersion`** - default `true`. This option allows you to toggle the warning that the parser will give you if you use a version of TypeScript which is not explicitly supported
diff --git a/packages/parser/tests/lib/parser.ts b/packages/parser/tests/lib/parser.ts
@@ -50,12 +50,12 @@ describe('parser', () => {
         jsx: false,
       },
       // ts-estree specific
-      filePath: 'test/foo',
+      filePath: 'tests/fixtures/services/isolated-file.src.ts',
       project: 'tsconfig.json',
       useJSXTextNode: false,
       errorOnUnknownASTType: false,
       errorOnTypeScriptSyntacticAndSemanticIssues: false,
-      tsconfigRootDir: './',
+      tsconfigRootDir: 'tests/fixtures/services',
       extraFileExtensions: ['foo'],
     };
     parseForESLint(code, config);
diff --git a/packages/typescript-estree/src/parser-options.ts b/packages/typescript-estree/src/parser-options.ts
@@ -18,6 +18,7 @@ export interface Extra {
   tsconfigRootDir: string;
   extraFileExtensions: string[];
   preserveNodeMaps?: boolean;
+  createDefaultProgram: boolean;
 }
 
 export interface TSESTreeOptions {
@@ -35,6 +36,7 @@ export interface TSESTreeOptions {
   tsconfigRootDir?: string;
   extraFileExtensions?: string[];
   preserveNodeMaps?: boolean;
+  createDefaultProgram?: boolean;
 }
 
 // This lets us use generics to type the return value, and removes the need to
diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts
@@ -58,6 +58,7 @@ function resetExtra(): void {
     tsconfigRootDir: process.cwd(),
     extraFileExtensions: [],
     preserveNodeMaps: undefined,
+    createDefaultProgram: false,
   };
 }
 
@@ -66,20 +67,27 @@ function resetExtra(): void {
  * @param options The config object
  * @returns If found, returns the source file corresponding to the code and the containing program
  */
-function getASTFromProject(code: string, options: TSESTreeOptions) {
-  return firstDefined(
-    calculateProjectParserOptions(
-      code,
-      options.filePath || getFileName(options),
-      extra,
-    ),
+function getASTFromProject(
+  code: string,
+  options: TSESTreeOptions,
+  createDefaultProgram: boolean,
+) {
+  const filePath = options.filePath || getFileName(options);
+  const astAndProgram = firstDefined(
+    calculateProjectParserOptions(code, filePath, extra),
     currentProgram => {
-      const ast = currentProgram.getSourceFile(
-        options.filePath || getFileName(options),
-      );
+      const ast = currentProgram.getSourceFile(filePath);
       return ast && { ast, program: currentProgram };
     },
   );
+
+  if (!astAndProgram && !createDefaultProgram) {
+    throw new Error(
+      `If "parserOptions.project" has been set for @typescript-eslint/parser, ${filePath} must be included in at least one of the projects provided.`,
+    );
+  }
+
+  return astAndProgram;
 }
 
 /**
@@ -161,10 +169,14 @@ function getProgramAndAST(
   code: string,
   options: TSESTreeOptions,
   shouldProvideParserServices: boolean,
+  createDefaultProgram: boolean,
 ) {
   return (
-    (shouldProvideParserServices && getASTFromProject(code, options)) ||
-    (shouldProvideParserServices && getASTAndDefaultProject(code, options)) ||
+    (shouldProvideParserServices &&
+      getASTFromProject(code, options, createDefaultProgram)) ||
+    (shouldProvideParserServices &&
+      createDefaultProgram &&
+      getASTAndDefaultProject(code, options)) ||
     createNewProgram(code)
   );
 }
@@ -254,6 +266,10 @@ function applyParserOptionsToExtra(options: TSESTreeOptions): void {
   if (options.preserveNodeMaps === undefined && extra.projects.length > 0) {
     extra.preserveNodeMaps = true;
   }
+
+  extra.createDefaultProgram =
+    typeof options.createDefaultProgram === 'boolean' &&
+    options.createDefaultProgram;
 }
 
 function warnAboutTSVersion(): void {
@@ -386,6 +402,7 @@ export function parseAndGenerateServices<
     code,
     options,
     shouldProvideParserServices,
+    extra.createDefaultProgram,
   );
   /**
    * Determine whether or not two-way maps of converted AST nodes should be preserved
diff --git a/packages/typescript-estree/src/tsconfig-parser.ts b/packages/typescript-estree/src/tsconfig-parser.ts
@@ -30,6 +30,16 @@ const watchCallbackTrackingMap = new Map<string, ts.FileWatcherCallback>();
 
 const parsedFilesSeen = new Set<string>();
 
+/**
+ * Clear tsconfig caches.
+ * Primarily used for testing.
+ */
+export function clearCaches() {
+  knownWatchProgramMap.clear();
+  watchCallbackTrackingMap.clear();
+  parsedFilesSeen.clear();
+}
+
 /**
  * Holds information about the file currently being linted
  */
diff --git a/packages/typescript-estree/tests/fixtures/simpleProject/file.ts b/packages/typescript-estree/tests/fixtures/simpleProject/file.ts
diff --git a/packages/typescript-estree/tests/fixtures/simpleProject/tsconfig.json b/packages/typescript-estree/tests/fixtures/simpleProject/tsconfig.json
@@ -0,0 +1 @@
+{}
diff --git a/packages/typescript-estree/tests/lib/parse.ts b/packages/typescript-estree/tests/lib/parse.ts
diff --git a/packages/typescript-estree/tests/lib/semanticInfo.ts b/packages/typescript-estree/tests/lib/semanticInfo.ts
diff --git a/tests/integration/utils/.eslintrc.js b/tests/integration/utils/.eslintrc.js
diff --git a/tests/integration/utils/jsconfig.json b/tests/integration/utils/jsconfig.json

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`/tests/fixtures//*`
	`2`	`+/tests/fixture-project//*`
`2`	`3`	`**/dist`
`3`	`4`	`**/coverage`
`4`	`5`	`**/shared-fixtures`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+throw 'should be ok because rule is not loaded';`