Skip to content

Commit d53063b

Browse files
NEW(eslint): @W-18495555@: Allow users to supply their own custom "flat" ESLint configuration file (#302)
1 parent cf29fdb commit d53063b

29 files changed

+905
-224
lines changed

packages/code-analyzer-eslint-engine/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,12 @@
3333
},
3434
"devDependencies": {
3535
"@types/jest": "^29.5.14",
36+
"@types/unzipper": "^0.10.11",
3637
"cross-env": "^7.0.3",
3738
"jest": "^29.7.0",
3839
"rimraf": "^6.0.01",
39-
"ts-jest": "^29.3.3"
40+
"ts-jest": "^29.3.3",
41+
"unzipper": "^0.12.3"
4042
},
4143
"engines": {
4244
"node": ">=20.0.0"

packages/code-analyzer-eslint-engine/src/config.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,9 @@ export const ESLINT_ENGINE_CONFIG_DESCRIPTION: ConfigDescription = {
109109

110110
// See https://eslint.org/docs/latest/use/configure/configuration-files
111111
// We currently do not support Typescript config files are since they require additional setup
112-
export const FLAT_ESLINT_CONFIG_FILES: string[] =
113-
['eslint.config.js', 'eslint.config.mjs', 'eslint.config.cjs'];
112+
export const DISCOVERABLE_FLAT_ESLINT_CONFIG_FILES: string[] =
113+
['eslint.config.js', 'eslint.config.cjs', 'eslint.config.mjs'];
114+
export const FLAT_ESLINT_CONFIG_FILE_EXTS: string[] = ['.js', '.cjs', '.mjs'];
114115

115116
// See https://eslint.org/docs/v8.x/use/configure/configuration-files#configuration-file-formats
116117
export const LEGACY_ESLINT_CONFIG_FILES: string[] =
@@ -148,9 +149,11 @@ class ESLintEngineConfigValueExtractor {
148149
extractESLintConfigFileValue(): string | undefined {
149150
const eslintConfigFileField: string = 'eslint_config_file';
150151
const eslintConfigFile: string | undefined = this.delegateExtractor.extractFile(eslintConfigFileField, DEFAULT_CONFIG.eslint_config_file);
151-
if (eslintConfigFile && !LEGACY_ESLINT_CONFIG_FILES.includes(path.basename(eslintConfigFile))) {
152-
throw new Error(getMessage('InvalidLegacyConfigFileName', this.delegateExtractor.getFieldPath(eslintConfigFileField),
153-
path.basename(eslintConfigFile), JSON.stringify(LEGACY_ESLINT_CONFIG_FILES)));
152+
if (eslintConfigFile && !(isValidLegacyConfigFileName(eslintConfigFile) || isValidFlatConfigFileName(eslintConfigFile))) {
153+
throw new Error(getMessage('InvalidESLintConfigFileName',
154+
this.delegateExtractor.getFieldPath(eslintConfigFileField),
155+
JSON.stringify(FLAT_ESLINT_CONFIG_FILE_EXTS),
156+
JSON.stringify(LEGACY_ESLINT_CONFIG_FILES)));
154157
}
155158
return eslintConfigFile;
156159
}
@@ -198,3 +201,12 @@ class ESLintEngineConfigValueExtractor {
198201
return this.delegateExtractor.extractBoolean(field_name, DEFAULT_CONFIG[field_name as keyof ESLintEngineConfig] as boolean)!;
199202
}
200203
}
204+
205+
function isValidLegacyConfigFileName(eslintConfigFile: string): boolean {
206+
// For legacy config files, we only allow specific file names so we are confident it is a legacy config file.
207+
return LEGACY_ESLINT_CONFIG_FILES.includes(path.basename(eslintConfigFile).toLowerCase());
208+
}
209+
210+
function isValidFlatConfigFileName(eslintConfigFile: string): boolean {
211+
return FLAT_ESLINT_CONFIG_FILE_EXTS.includes(path.extname(eslintConfigFile).toLowerCase());
212+
}

packages/code-analyzer-eslint-engine/src/engine.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,21 @@ export class ESLintEngine extends Engine {
6060
const userConfigInfo: UserConfigInfo = this.getUserConfigInfo(describeOptions.workspace);
6161
this.emitLogEvent(LogLevel.Fine, `Detected the following state regarding the user's ESLint configuration: ${userConfigInfo}`);
6262

63-
if (this.shouldDelegateToV8(userConfigInfo)) {
63+
if (userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG) {
64+
this.emitLogEvent(LogLevel.Warn, getMessage('DetectedLegacyConfig',
65+
userConfigInfo.getChosenUserConfigFile() ?? userConfigInfo.getChosenUserIgnoreFile()!));
6466
return this.delegateV8Engine.describeRules(describeOptions);
65-
} else if (userConfigInfo.getUserIgnoreFile()) {
66-
this.emitLogEvent(LogLevel.Warn, getMessage('IgnoringLegacyIgnoreFile', userConfigInfo.getUserIgnoreFile()!));
6767
}
6868

69-
if (userConfigInfo.getUserConfigFile()) { // TODO: Remove this as soon as we allow user's to supply their own flat config file.
70-
this.emitLogEvent(LogLevel.Warn, getMessage('IgnoringFlatConfigFile', userConfigInfo.getUserConfigFile()!));
69+
if (userConfigInfo.getChosenUserConfigFile()) {
70+
this.emitLogEvent(LogLevel.Debug, getMessage('ApplyingFlatConfigFile', userConfigInfo.getChosenUserConfigFile()!));
71+
} else if (userConfigInfo.getDiscoveredConfigFile()) {
72+
this.emitLogEvent(LogLevel.Info, getMessage('UnusedESLintConfigFile',
73+
makeRelativeTo(process.cwd(), userConfigInfo.getDiscoveredConfigFile()!),
74+
makeRelativeTo(this.engineConfig.config_root, userConfigInfo.getDiscoveredConfigFile()!)));
75+
}
76+
if (userConfigInfo.getChosenUserIgnoreFile()) {
77+
this.emitLogEvent(LogLevel.Warn, getMessage('IgnoringLegacyIgnoreFile', userConfigInfo.getChosenUserIgnoreFile()!));
7178
}
7279

7380
this.emitDescribeRulesProgressEvent(10);
@@ -97,7 +104,7 @@ export class ESLintEngine extends Engine {
97104
async runRules(ruleNames: string[], runOptions: RunOptions): Promise<EngineRunResults> {
98105
this.emitRunRulesProgressEvent(0);
99106
const userConfigInfo: UserConfigInfo = this.getUserConfigInfo(runOptions.workspace);
100-
if (this.shouldDelegateToV8(userConfigInfo)) {
107+
if (userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG) {
101108
return this.delegateV8Engine.runRules(ruleNames, runOptions);
102109
}
103110

@@ -161,17 +168,14 @@ export class ESLintEngine extends Engine {
161168
return this.userConfigInfoCache.get(cacheKey)!;
162169
}
163170

164-
private shouldDelegateToV8(userConfigInfo: UserConfigInfo): boolean {
165-
return userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG;
166-
}
167-
168171
private async getESLintContext(workspace?: Workspace): Promise<ESLintContext> {
169172
const cacheKey: string = workspace?.getWorkspaceId() ?? process.cwd();
170173
if (!this.eslintContextCache.has(cacheKey)) {
171174
const userConfigInfo: UserConfigInfo = this.getUserConfigInfo(workspace);
172175
const eslintWorkspace: ESLintWorkspace = ESLintWorkspace.from(workspace,
173-
this.engineConfig.config_root, this.engineConfig.file_extensions, userConfigInfo.getUserConfigFile());
174-
const context: ESLintContext = await calculateESLintContext(this.engineConfig, eslintWorkspace);
176+
this.engineConfig.config_root, this.engineConfig.file_extensions, userConfigInfo.getChosenUserConfigFile());
177+
const context: ESLintContext = await calculateESLintContext(this.engineConfig, eslintWorkspace,
178+
userConfigInfo.getChosenUserConfigFile());
175179
this.eslintContextCache.set(cacheKey, context);
176180
}
177181
return this.eslintContextCache.get(cacheKey)!;
@@ -267,3 +271,11 @@ function normalizeEndValue(endValue: number | undefined): number | undefined {
267271
/* istanbul ignore next */
268272
return endValue && endValue > 0 ? endValue : undefined;
269273
}
274+
275+
276+
function makeRelativeTo(absFolderPath: string, absFilePath: string): string {
277+
absFolderPath = absFolderPath.endsWith(path.sep) ?
278+
/* istanbul ignore next */ absFolderPath : absFolderPath + path.sep;
279+
return absFilePath.startsWith(absFolderPath) ?
280+
absFilePath.slice(absFolderPath.length) : /* istanbul ignore next */ absFilePath;
281+
}

packages/code-analyzer-eslint-engine/src/eslint-context.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export enum ESLintRuleStatus {
1313

1414
export type ESLintContext = {
1515
baseDirectory: string,
16+
userConfigFile?: string,
1617
filesToScan: string[],
1718
ruleInfo: {
1819
[ruleName: string]: {
@@ -22,12 +23,13 @@ export type ESLintContext = {
2223
}
2324
}
2425

25-
export async function calculateESLintContext(engineConfig: ESLintEngineConfig, eslintWorkspace: ESLintWorkspace): Promise<ESLintContext> {
26+
export async function calculateESLintContext(engineConfig: ESLintEngineConfig, eslintWorkspace: ESLintWorkspace, userConfigFile?: string): Promise<ESLintContext> {
2627
const baseDirectory: string = await eslintWorkspace.getBaseDirectory();
27-
const eslint: ESLint = createESLint(engineConfig, baseDirectory);
28+
const eslint: ESLint = createESLint(engineConfig, baseDirectory, userConfigFile);
2829

2930
const context: ESLintContext = {
3031
baseDirectory: baseDirectory,
32+
userConfigFile: userConfigFile,
3133
filesToScan: await eslintWorkspace.getFilesToScan(eslint),
3234
ruleInfo: {}
3335
}
@@ -80,7 +82,9 @@ function getRuleStatusFromRuleEntry(ruleEntry: Linter.RuleEntry): ESLintRuleStat
8082
if (typeof ruleEntry === "number") {
8183
return ruleEntry === 2 ? ESLintRuleStatus.ERROR :
8284
ruleEntry === 1 ? ESLintRuleStatus.WARN : ESLintRuleStatus.OFF;
83-
} else if (typeof ruleEntry === "string") {
85+
} else /* istanbul ignore if */ if (typeof ruleEntry === "string") {
86+
// I believe ESLint 9 normalizes the ruleEntry to always be numbers when we invoke calculateConfigForFile
87+
// but just in case things change, we should keep this branch of code that handles the string case for safety.
8488
return ruleEntry.toLowerCase() === "error" ? ESLintRuleStatus.ERROR :
8589
ruleEntry.toLowerCase() === "warn" ? ESLintRuleStatus.WARN : ESLintRuleStatus.OFF;
8690
}

packages/code-analyzer-eslint-engine/src/eslint-wrapper.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import {makeStringifiable} from "./utils";
66
import { indent } from "@salesforce/code-analyzer-engine-api/utils";
77

88

9-
export function createESLint(engineConfig: ESLintEngineConfig, baseDirectory: string, rulesToRun?: Set<string>): ESLintWrapper {
9+
export function createESLint(engineConfig: ESLintEngineConfig, baseDirectory: string, userConfigFile?: string, rulesToRun?: Set<string>): ESLintWrapper {
1010
const baseConfigFactory: BaseConfigFactory = new BaseConfigFactory(engineConfig);
1111
const eslintOptions: ESLint.Options = {
1212
cwd: baseDirectory, // The base working directory. This must be an absolute path.
1313
errorOnUnmatchedPattern: false, // Unless set to false, the eslint.lintFiles() method will throw an error when no target files are found.
1414
baseConfig: baseConfigFactory.createBaseConfigArray(),
15-
overrideConfigFile: true, //TODO: Will Add in Users Config File
15+
overrideConfigFile: userConfigFile ?? true, // Oddly enough ESLint documents that "true" means don't go auto looking for a config file (which we set if we didn't find one ourselves)
1616
};
1717
if (rulesToRun) {
1818
// Using a ruleFilter ensures that we only run the rules that the user has selected. This approach is much
@@ -31,7 +31,7 @@ export class ESLintWrapper extends ESLint {
3131
try {
3232
super(options);
3333
this._options = options;
34-
} catch (error) {
34+
} catch (error) /* istanbul ignore next */ {
3535
throw wrapESLintError(error, 'ESLint', options);
3636
}
3737
}
@@ -41,7 +41,7 @@ export class ESLintWrapper extends ESLint {
4141
try {
4242
return await super.calculateConfigForFile(filePath);
4343
} catch (error) {
44-
throw wrapESLintError(error, `ESLint.calculateConfigForFile(${filePath})`, this._options);
44+
throw await wrapESLintError(error, `ESLint.calculateConfigForFile("${filePath}")`, this._options);
4545
}
4646

4747
}
@@ -50,20 +50,41 @@ export class ESLintWrapper extends ESLint {
5050
try {
5151
return await super.isPathIgnored(filePath);
5252
} catch (error) { /* istanbul ignore next */
53-
throw wrapESLintError(error, `ESLint.isPathIgnored(${filePath})`, this._options);
53+
throw await wrapESLintError(error, `ESLint.isPathIgnored("${filePath}")`, this._options);
5454
}
5555
}
5656
}
5757

58+
class WrappedError extends Error {}
59+
60+
async function wrapESLintError(rawError: unknown, fcnCallStr: string, options: ESLint.Options): Promise<Error> {
61+
if (rawError instanceof WrappedError) {
62+
return rawError; // Prevent wrapping multiple times
63+
}
64+
65+
// Before throwing the actual error message, we first want to validate the user's config file in an isolated
66+
// environment see if it is even valid when run by itself without any other configurations.
67+
// If not, then we display the simpler error and options.
68+
if (typeof options.overrideConfigFile === "string") {
69+
const simpleOptions: ESLint.Options = {overrideConfigFile: options.overrideConfigFile};
70+
try {
71+
const rawESLint: ESLint = new ESLint(simpleOptions);
72+
await rawESLint.calculateConfigForFile('dummy.js');
73+
} catch (err) {
74+
rawError = err;
75+
fcnCallStr = 'ESLint.calculateConfigForFile';
76+
options = simpleOptions;
77+
}
78+
}
5879

59-
function wrapESLintError(rawError: unknown, fcnCallStr: string, options: ESLint.Options): Error {
60-
const rawErrMsg: string = rawError instanceof Error ? rawError.message : /* istanbul ignore next */
61-
String(rawError);
62-
const eslintOptionsStr: string = indent(stringifyESLintOptions(options), ' |');
63-
const wrappedErrMsg: string = rawErrMsg.includes('conflict') ? // TODO: See if this conflict case ever happens anymore
80+
/* istanbul ignore next */
81+
const rawErrMsg: string = indent(rawError instanceof Error ?
82+
rawError.stack ?? rawError.message : String(rawError), ' | ');
83+
const eslintOptionsStr: string = indent(stringifyESLintOptions(options), ' ');
84+
const wrappedErrMsg: string = rawErrMsg.includes('Cannot redefine plugin') ? // TODO: Maybe with W-18695515 we can manually resolve conflicts
6485
getMessage('ESLintThrewExceptionWithPluginConflictMessage', fcnCallStr, rawErrMsg, eslintOptionsStr)
6586
: getMessage('ESLintThrewExceptionWithUnknownMessage', fcnCallStr, rawErrMsg, eslintOptionsStr);
66-
return new Error(wrappedErrMsg, {cause: rawError});
87+
return new WrappedError(wrappedErrMsg, {cause: rawError});
6788
}
6889

6990

packages/code-analyzer-eslint-engine/src/messages.ts

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
2323
ConfigFieldDescription_disable_javascript_base_config:
2424
`Whether to turn off the default base configuration that supplies the standard ESLint rules for JavaScript files.\n` +
2525
`The base configuration for JavaScript files adds the rules from the "eslint:all" configuration to Code Analyzer.\n` +
26-
`See https://eslint.org/docs/v8.x/rules for the list of rules.`, // TODO: This link will change when we move to v9
26+
`See https://eslint.org/docs/latest/rules for the list of rules.`,
2727

2828
ConfigFieldDescription_disable_lwc_base_config:
2929
`Whether to turn off the default base configuration that supplies the LWC rules for JavaScript files.\n` +
@@ -34,7 +34,7 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
3434
ConfigFieldDescription_disable_typescript_base_config:
3535
`Whether to turn off the default base configuration that supplies the standard rules for TypeScript files\n` +
3636
`The base configuration for TypeScript files adds the rules from the "plugin:@typescript-eslint:all" configuration to Code Analyzer.\n` +
37-
`See https://typescript-eslint.io/rules and https://eslint.org/docs/v8.x/rules for the lists of rules.`, // TODO: This link will change when we move to v9
37+
`See https://typescript-eslint.io/rules and https://eslint.org/docs/latest/rules for the lists of rules.`,
3838

3939
ConfigFieldDescription_file_extensions:
4040
`Extensions of the files in your workspace that will be used to discover rules.\n` +
@@ -47,8 +47,8 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
4747
UnsupportedEngineName:
4848
`The ESLintEnginePlugin does not support an engine with name '%s'.`,
4949

50-
InvalidLegacyConfigFileName:
51-
`The '%s' configuration value is invalid. Expected the file name '%s' to be one of the following: %s`,
50+
InvalidESLintConfigFileName:
51+
`The '%s' configuration value is invalid. Expected either a "flat" ESLint configuration file that ends with %s or a known "legacy" ESLint configuration file name from among %s.`,
5252

5353
InvalidLegacyIgnoreFileName:
5454
`The '%s' configuration value is invalid. Expected the file name '%s' to be equal to '%s'.`,
@@ -62,7 +62,7 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
6262
ESLintWarnedWhenScanningFile:
6363
`When scanning file '%s' with the eslint engine, ESLint gave the following warning:\n%s`,
6464

65-
ESLintThrewExceptionWithPluginConflictMessage:
65+
ESLintThrewExceptionWithPluginConflictMessage: // TODO: Hopefully with W-18695515 we can manually resolve conflicts and we won't need this
6666
`The eslint engine encountered a conflict between a plugin supplied by one of your ESLint configuration ` +
6767
`files and a plugin supplied by the base configuration.\n` +
6868
`To continue to use your custom config you may need to disable one or more of the provided base ` +
@@ -79,34 +79,32 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
7979
` eslint:\n` +
8080
` eslint_config_file: null\n` +
8181
` auto_discover_eslint_config: false\n\n` +
82-
`Error thrown from %s': %s\n\n` +
82+
`Error thrown from %s':\n%s\n\n` +
8383
'ESLint options used:\n%s',
8484

8585
ESLintThrewExceptionWithUnknownMessage:
86-
`The eslint engine encountered an unexpected error thrown from '%s': %s\n\n` +
86+
`The eslint engine encountered an unexpected error thrown from '%s':\n%s\n\n` +
8787
'ESLint options used:\n%s',
8888

89-
UnusedEslintConfigFile:
89+
UnusedESLintConfigFile:
9090
`The ESLint configuration file '%s' was found but not applied.\n` +
9191
`To apply this configuration file, set it as the eslint_config_file value in your Code Analyzer configuration. For example:\n` +
9292
` engines:\n` +
9393
` eslint:\n` +
9494
` eslint_config_file: "%s"\n` +
95-
`Alternatively, to have Code Analyzer automatically discover and apply any ESLint configuration and ignore files found in your workspace, set the auto_discover_eslint_config value to true.`,
96-
97-
UnusedEslintIgnoreFile:
98-
`The ESLint ignore file '%s' was found but not applied.\n` +
99-
`To apply this ignore file, set it as the eslint_ignore_file value in your Code Analyzer configuration. For example:\n` +
100-
` engines:\n` +
101-
` eslint:\n` +
102-
` eslint_ignore_file: "%s"\n` +
103-
`Alternatively, to have Code Analyzer automatically discover and apply any ESLint configuration and ignore files found in your workspace, set the auto_discover_eslint_config value to true.`,
95+
`Alternatively, to have Code Analyzer attempt to automatically discover your ESLint configuration file in your workspace, set the auto_discover_eslint_config value to true.`,
10496

10597
IgnoringLegacyIgnoreFile:
10698
`Ignoring '%s' since ESLint v9+ does not support legacy ignore files.`,
10799

108-
IgnoringFlatConfigFile: // TODO: Remove this as soon as we support user's providing their own flat config files
109-
`Ignoring '%s' since ESLint "Flat" configuration files are not yet supported by this version of the 'eslint' engine.`,
100+
DetectedLegacyConfig:
101+
`Using ESLint v8 instead of ESLint v9 since the use of a "legacy" ESLint configuration file or ignore file was detected: %s.\n` +
102+
`Since ESLint v8 is no longer supported, Code Analyzer will be removing support for "legacy" ESLint configuration files in the coming months.\n` +
103+
`Therefore, it is highly recommended that you migrate your "legacy" configuration to the new "flat" configuration format as soon as possible.\n` +
104+
`Learn how at: https://eslint.org/docs/latest/use/configure/migration-guide`,
105+
106+
ApplyingFlatConfigFile:
107+
`Applying the ESLint "flat" configuration file: %s`,
110108

111109
UnableToCalculateBaseDirectory:
112110
`Could not calculate base directory for ESLint from the list of relevant targeted files to scan.\n` +

packages/code-analyzer-eslint-engine/src/run-eslint-worker-task.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export class RunESLintWorkerTask extends WorkerTask<RunESLintWorkerTaskInput, ES
1919
}
2020

2121
protected async exec(input: RunESLintWorkerTaskInput): Promise<ESLint.LintResult[]> {
22-
const eslint: ESLint = createESLint(input.engineConfig, input.eslintContext.baseDirectory, new Set(input.rulesToRun));
22+
const eslint: ESLint = createESLint(input.engineConfig, input.eslintContext.baseDirectory, input.eslintContext.userConfigFile, new Set(input.rulesToRun));
2323
return await eslint.lintFiles(input.eslintContext.filesToScan);
2424
}
2525
}

0 commit comments

Comments
 (0)