Skip to content

Commit

Permalink
feat(typescript-estree)!: throw error on file not in project when `pr…
Browse files Browse the repository at this point in the history
…oject` set (#760)

BREAKING CHANGE: by default we will now throw when a file is not in the `project` provided
  • Loading branch information
uniqueiniquity authored and JamesHenry committed Jul 28, 2019
1 parent 4496288 commit 3777b77
Show file tree
Hide file tree
Showing 24 changed files with 173 additions and 50 deletions.
1 change: 1 addition & 0 deletions .prettierignore
@@ -1,4 +1,5 @@
**/tests/fixtures/**/*
**/tests/fixture-project/**/*
**/dist
**/coverage
**/shared-fixtures
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/1.ts
@@ -0,0 +1 @@
var foo = true;
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/2.ts
@@ -0,0 +1 @@
throw 'should be ok because rule is not loaded';
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/3.ts
@@ -0,0 +1 @@
throw 'err'; // no-string-throw
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/4.ts
@@ -0,0 +1 @@
var foo = true // semicolon
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/5.ts
@@ -0,0 +1 @@
var foo = true; // fail
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/fixture-project/6.ts
@@ -0,0 +1 @@
foo;
@@ -0,0 +1 @@
{}
31 changes: 20 additions & 11 deletions packages/eslint-plugin-tslint/tests/index.spec.ts
Expand Up @@ -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'),
});
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-tslint/tests/test-project/extra.ts
@@ -0,0 +1 @@
foo;
25 changes: 25 additions & 0 deletions packages/eslint-plugin/tests/RuleTester.ts
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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);
Expand Down
Empty file.
Expand Up @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions packages/parser/README.md
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/parser/tests/lib/parser.ts
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions packages/typescript-estree/src/parser-options.ts
Expand Up @@ -18,6 +18,7 @@ export interface Extra {
tsconfigRootDir: string;
extraFileExtensions: string[];
preserveNodeMaps?: boolean;
createDefaultProgram: boolean;
}

export interface TSESTreeOptions {
Expand All @@ -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
Expand Down
41 changes: 29 additions & 12 deletions packages/typescript-estree/src/parser.ts
Expand Up @@ -58,6 +58,7 @@ function resetExtra(): void {
tsconfigRootDir: process.cwd(),
extraFileExtensions: [],
preserveNodeMaps: undefined,
createDefaultProgram: false,
};
}

Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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)
);
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/typescript-estree/src/tsconfig-parser.ts
Expand Up @@ -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
*/
Expand Down
Empty file.
@@ -0,0 +1 @@
{}

0 comments on commit 3777b77

Please sign in to comment.