Skip to content

Commit ea2ae36

Browse files
JamesHenryJamesHenry
committed
feat(builder): support reportUnusedDisableDirectives option
1 parent c62eb08 commit ea2ae36

9 files changed

+154
-99
lines changed

‎packages/builder/package.json

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
"LICENSE",
1818
"builders.json"
1919
],
20+
"dependencies": {
21+
"@nx/devkit": "16.0.0-beta.8"
22+
},
2023
"builders": "./builders.json",
2124
"peerDependencies": {
2225
"eslint": "^7.20.0 || ^8.0.0",

‎packages/builder/src/lint.impl.spec.ts

+26-29
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,7 @@ import type { Schema } from './schema';
99
// eslint-disable-next-line @typescript-eslint/no-var-requires
1010
const fs = require('fs');
1111
jest.spyOn(fs, 'writeFileSync').mockImplementation();
12-
13-
const mockCreateDirectory = jest.fn();
14-
jest.mock('./utils/create-directory', () => ({
15-
createDirectory: mockCreateDirectory,
16-
}));
12+
jest.spyOn(fs, 'mkdirSync').mockImplementation();
1713

1814
const mockFormatter = {
1915
format: jest
@@ -35,6 +31,7 @@ class MockESLint {
3531
static version = VALID_ESLINT_VERSION;
3632
static outputFixes = mockOutputFixes;
3733
loadFormatter = mockLoadFormatter;
34+
isPathIgnored = jest.fn().mockReturnValue(false);
3835
}
3936

4037
let mockReports: unknown[] = [
@@ -73,6 +70,7 @@ function createValidRunBuilderOptions(
7370
noEslintrc: false,
7471
rulesdir: [],
7572
resolvePluginsRelativeTo: null,
73+
reportUnusedDisableDirectives: null,
7674
...additionalOptions,
7775
};
7876
}
@@ -167,29 +165,26 @@ describe('Linter Builder', () => {
167165
resolvePluginsRelativeTo: null,
168166
}),
169167
);
170-
expect(mockLint).toHaveBeenCalledWith(
171-
resolve('/root'),
172-
resolve('/root/.eslintrc'),
173-
{
174-
lintFilePatterns: [],
175-
eslintConfig: './.eslintrc',
176-
exclude: ['excludedFile1'],
177-
fix: true,
178-
quiet: false,
179-
cache: true,
180-
cacheLocation: 'cacheLocation1',
181-
cacheStrategy: 'content',
182-
format: 'stylish',
183-
force: false,
184-
silent: false,
185-
maxWarnings: -1,
186-
outputFile: null,
187-
ignorePath: null,
188-
noEslintrc: false,
189-
rulesdir: [],
190-
resolvePluginsRelativeTo: null,
191-
},
192-
);
168+
expect(mockLint).toHaveBeenCalledWith(resolve('/root/.eslintrc'), {
169+
lintFilePatterns: [],
170+
eslintConfig: './.eslintrc',
171+
exclude: ['excludedFile1'],
172+
fix: true,
173+
quiet: false,
174+
cache: true,
175+
cacheLocation: 'cacheLocation1/<???>',
176+
cacheStrategy: 'content',
177+
format: 'stylish',
178+
force: false,
179+
silent: false,
180+
maxWarnings: -1,
181+
outputFile: null,
182+
ignorePath: null,
183+
noEslintrc: false,
184+
rulesdir: [],
185+
resolvePluginsRelativeTo: null,
186+
reportUnusedDisableDirectives: null,
187+
});
193188
});
194189

195190
it('should throw if no reports generated', async () => {
@@ -632,7 +627,9 @@ describe('Linter Builder', () => {
632627
outputFile: 'a/b/c/outputFile1',
633628
}),
634629
);
635-
expect(mockCreateDirectory).toHaveBeenCalledWith('/root/a/b/c');
630+
expect(fs.mkdirSync).toHaveBeenCalledWith('/root/a/b/c', {
631+
recursive: true,
632+
});
636633
expect(fs.writeFileSync).toHaveBeenCalledWith(
637634
'/root/a/b/c/outputFile1',
638635
mockFormatter.format(mockReports),

‎packages/builder/src/lint.impl.ts

+55-30
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1-
import type { BuilderContext, BuilderOutput } from '@angular-devkit/architect';
2-
import { createBuilder } from '@angular-devkit/architect';
1+
import type { BuilderOutput } from '@angular-devkit/architect';
2+
import { convertNxExecutor } from '@nx/devkit';
33
import type { ESLint } from 'eslint';
4-
import { writeFileSync } from 'fs';
4+
import { mkdirSync, writeFileSync } from 'fs';
55
import { dirname, join, resolve } from 'path';
66
import type { Schema } from './schema';
7-
import { createDirectory } from './utils/create-directory';
87
import { lint, loadESLint } from './utils/eslint-utils';
98

10-
export default createBuilder(
11-
async (options: Schema, context: BuilderContext): Promise<BuilderOutput> => {
12-
const workspaceRoot = context.workspaceRoot;
13-
process.chdir(workspaceRoot);
9+
export default convertNxExecutor(
10+
async (options: Schema, context): Promise<BuilderOutput> => {
11+
const systemRoot = context.root;
1412

15-
const projectName = context.target?.project || '<???>';
13+
// eslint resolves files relative to the current working directory.
14+
// We want these paths to always be resolved relative to the workspace
15+
// root to be able to run the lint executor from any subfolder.
16+
process.chdir(systemRoot);
17+
18+
const projectName = context.projectName || '<???>';
1619
const printInfo = options.format && !options.silent;
17-
const reportOnlyErrors = options.quiet;
18-
const maxWarnings = options.maxWarnings;
1920

2021
if (printInfo) {
2122
console.info(`\nLinting ${JSON.stringify(projectName)}...`);
2223
}
2324

24-
const projectESLint = await loadESLint();
25+
const projectESLint: { ESLint: typeof ESLint } = await loadESLint();
2526
const version = projectESLint.ESLint?.version?.split('.');
2627
if (
2728
!version ||
@@ -36,16 +37,20 @@ export default createBuilder(
3637

3738
/**
3839
* We want users to have the option of not specifying the config path, and let
39-
* eslint automatically resolve the `.eslintrc` files in each folder.
40+
* eslint automatically resolve the `.eslintrc.json` files in each folder.
4041
*/
4142
const eslintConfigPath = options.eslintConfig
42-
? resolve(workspaceRoot, options.eslintConfig)
43+
? resolve(systemRoot, options.eslintConfig)
4344
: undefined;
4445

46+
options.cacheLocation = options.cacheLocation
47+
? join(options.cacheLocation, projectName)
48+
: null;
49+
4550
let lintResults: ESLint.LintResult[] = [];
4651

4752
try {
48-
lintResults = await lint(workspaceRoot, eslintConfigPath, options);
53+
lintResults = await lint(eslintConfigPath, options);
4954
} catch (err) {
5055
if (
5156
err instanceof Error &&
@@ -54,19 +59,18 @@ export default createBuilder(
5459
)
5560
) {
5661
let eslintConfigPathForError = `for ${projectName}`;
57-
58-
const projectMetadata = await context.getProjectMetadata(projectName);
59-
if (projectMetadata) {
60-
eslintConfigPathForError = `\`${projectMetadata.root}/.eslintrc.json\``;
62+
if (context.projectsConfigurations?.projects?.[projectName]?.root) {
63+
const { root } = context.projectsConfigurations.projects[projectName];
64+
eslintConfigPathForError = `\`${root}/.eslintrc.json\``;
6165
}
6266

6367
console.error(`
64-
Error: You have attempted to use a lint rule which requires the full TypeScript type-checker to be available, but you do not have \`parserOptions.project\` configured to point at your project tsconfig.json files in the relevant TypeScript file "overrides" block of your ESLint config ${
65-
eslintConfigPath || eslintConfigPathForError
66-
}
67-
68-
For full guidance on how to resolve this issue, please see https://github.com/angular-eslint/angular-eslint/blob/main/docs/RULES_REQUIRING_TYPE_INFORMATION.md
69-
`);
68+
Error: You have attempted to use a lint rule which requires the full TypeScript type-checker to be available, but you do not have \`parserOptions.project\` configured to point at your project tsconfig.json files in the relevant TypeScript file "overrides" block of your project ESLint config ${
69+
eslintConfigPath || eslintConfigPathForError
70+
}
71+
72+
For full guidance on how to resolve this issue, please see https://github.com/angular-eslint/angular-eslint/blob/main/docs/RULES_REQUIRING_TYPE_INFORMATION.md
73+
`);
7074

7175
return {
7276
success: false,
@@ -77,16 +81,37 @@ export default createBuilder(
7781
}
7882

7983
if (lintResults.length === 0) {
80-
throw new Error('Invalid lint configuration. Nothing to lint.');
84+
const ignoredPatterns = (
85+
await Promise.all(
86+
options.lintFilePatterns.map(async (pattern) =>
87+
(await eslint.isPathIgnored(pattern)) ? pattern : null,
88+
),
89+
)
90+
)
91+
.filter((pattern) => !!pattern)
92+
.map((pattern) => `- '${pattern}'`);
93+
if (ignoredPatterns.length) {
94+
throw new Error(
95+
`All files matching the following patterns are ignored:\n${ignoredPatterns.join(
96+
'\n',
97+
)}\n\nPlease check your '.eslintignore' file.`,
98+
);
99+
}
100+
throw new Error(
101+
'Invalid lint configuration. Nothing to lint. Please check your lint target pattern(s).',
102+
);
81103
}
82104

105+
// output fixes to disk, if applicable based on the options
106+
await projectESLint.ESLint.outputFixes(lintResults);
107+
83108
const formatter = await eslint.loadFormatter(options.format);
84109

85110
let totalErrors = 0;
86111
let totalWarnings = 0;
87112

88-
// output fixes to disk, if applicable based on the options
89-
await projectESLint.ESLint.outputFixes(lintResults);
113+
const reportOnlyErrors = options.quiet;
114+
const maxWarnings = options.maxWarnings;
90115

91116
/**
92117
* Depending on user configuration we may not want to report on all the
@@ -128,8 +153,8 @@ export default createBuilder(
128153
const formattedResults = await formatter.format(finalLintResults);
129154

130155
if (options.outputFile) {
131-
const pathToOutputFile = join(context.workspaceRoot, options.outputFile);
132-
createDirectory(dirname(pathToOutputFile));
156+
const pathToOutputFile = join(context.root, options.outputFile);
157+
mkdirSync(dirname(pathToOutputFile), { recursive: true });
133158
writeFileSync(pathToOutputFile, formattedResults);
134159
} else {
135160
console.info(formattedResults);

‎packages/builder/src/schema.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export interface Schema extends JsonObject {
1717
noEslintrc: boolean;
1818
rulesdir: string[];
1919
resolvePluginsRelativeTo: string | null;
20+
reportUnusedDisableDirectives: Linter.RuleLevel | null;
2021
}
2122

2223
type Formatter =

‎packages/builder/src/schema.json

+10-5
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
},
5555
"lintFilePatterns": {
5656
"type": "array",
57-
"description": "One or more files/dirs/globs to pass directly to ESLint's lintFiles() method.",
57+
"description": "One or more files/dirs/globs to pass directly to ESLint's `lintFiles()` method.",
5858
"default": [],
5959
"items": {
6060
"type": "string"
@@ -87,24 +87,29 @@
8787
},
8888
"ignorePath": {
8989
"type": "string",
90-
"description": "The path of the .eslintignore file."
90+
"description": "The path of the `.eslintignore` file."
9191
},
9292
"noEslintrc": {
9393
"type": "boolean",
94-
"description": "The equivalent of the --no-eslintrc flag on the ESLint CLI, it is false by default",
94+
"description": "The equivalent of the `--no-eslintrc` flag on the ESLint CLI, it is false by default",
9595
"default": false
9696
},
9797
"rulesdir": {
9898
"type": "array",
99-
"description": "The equivalent of the --rulesdir flag on the ESLint CLI, it is an empty array by default",
99+
"description": "The equivalent of the `--rulesdir` flag on the ESLint CLI, it is an empty array by default",
100100
"default": [],
101101
"items": {
102102
"type": "string"
103103
}
104104
},
105105
"resolvePluginsRelativeTo": {
106106
"type": "string",
107-
"description": "The equivalent of the --resolve-plugins-relative-to flag on the ESLint CLI"
107+
"description": "The equivalent of the `--resolve-plugins-relative-to` flag on the ESLint CLI"
108+
},
109+
"reportUnusedDisableDirectives": {
110+
"type": "string",
111+
"enum": ["off", "warn", "error"],
112+
"description": "The equivalent of the `--report-unused-disable-directives` flag on the ESLint CLI."
108113
}
109114
},
110115
"additionalProperties": false,

‎packages/builder/src/utils/create-directory.ts

-20
This file was deleted.

‎packages/builder/src/utils/eslint-utils.spec.ts

+30-5
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe('eslint-utils', () => {
2020
});
2121

2222
it('should create the ESLint instance with the proper parameters', async () => {
23-
await lint('/root', './.eslintrc.json', {
23+
await lint('./.eslintrc.json', {
2424
fix: true,
2525
cache: true,
2626
cacheLocation: '/root/cache',
@@ -42,7 +42,7 @@ describe('eslint-utils', () => {
4242
});
4343

4444
it('should create the ESLint instance with the proper parameters', async () => {
45-
await lint('/root', undefined, {
45+
await lint(undefined, {
4646
fix: true,
4747
cache: true,
4848
cacheLocation: '/root/cache',
@@ -65,7 +65,7 @@ describe('eslint-utils', () => {
6565

6666
describe('noEslintrc', () => {
6767
it('should create the ESLint instance with "useEslintrc" set to false', async () => {
68-
await lint('/root', undefined, {
68+
await lint(undefined, {
6969
fix: true,
7070
cache: true,
7171
cacheLocation: '/root/cache',
@@ -89,7 +89,7 @@ describe('eslint-utils', () => {
8989
describe('rulesdir', () => {
9090
it('should create the ESLint instance with "rulePaths" set to the given value for rulesdir', async () => {
9191
const extraRuleDirectories = ['./some-rules', '../some-more-rules'];
92-
await lint('/root', undefined, {
92+
await lint(undefined, {
9393
fix: true,
9494
cache: true,
9595
cacheLocation: '/root/cache',
@@ -113,7 +113,7 @@ describe('eslint-utils', () => {
113113

114114
describe('resolvePluginsRelativeTo', () => {
115115
it('should create the ESLint instance with "resolvePluginsRelativeTo" set to the given value for resolvePluginsRelativeTo', async () => {
116-
await lint('/root', undefined, {
116+
await lint(undefined, {
117117
fix: true,
118118
cache: true,
119119
cacheLocation: '/root/cache',
@@ -135,4 +135,29 @@ describe('eslint-utils', () => {
135135
});
136136
});
137137
});
138+
139+
describe('reportUnusedDisableDirectives', () => {
140+
it('should create the ESLint instance with "reportUnusedDisableDirectives" set to the given value for reportUnusedDisableDirectives', async () => {
141+
await lint(undefined, {
142+
fix: true,
143+
cache: true,
144+
cacheLocation: '/root/cache',
145+
cacheStrategy: 'content',
146+
reportUnusedDisableDirectives: 'warn',
147+
// eslint-disable-next-line @typescript-eslint/no-empty-function
148+
}).catch(() => {});
149+
150+
expect(ESLint).toHaveBeenCalledWith({
151+
fix: true,
152+
cache: true,
153+
cacheLocation: '/root/cache',
154+
cacheStrategy: 'content',
155+
ignorePath: undefined,
156+
useEslintrc: true,
157+
errorOnUnmatchedPattern: false,
158+
rulePaths: [],
159+
reportUnusedDisableDirectives: 'warn',
160+
});
161+
});
162+
});
138163
});

0 commit comments

Comments
 (0)