Skip to content
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.

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

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

Merged
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 @@
{}