Skip to content

Commit 6346e61

Browse files
stephen-carter-at-sfstephen-carter-at-sf
committed
NEW(eslint): @W-18495555@: Allow users to supply their own custom "flat" ESLint configuration file
1 parent cf29fdb commit 6346e61

29 files changed

Lines changed: 921 additions & 226 deletions

‎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: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export class ESLintEngine extends Engine {
3434
private readonly delegateV8Engine: Engine;
3535
private userConfigInfoCache: Map<string, UserConfigInfo> = new Map();
3636
private eslintContextCache: Map<string, ESLintContext> = new Map();
37+
private haveWarnedAboutESLint8: boolean = false;
3738

3839
constructor(engineConfig: ESLintEngineConfig, delegateV8Engine: Engine) {
3940
super();
@@ -60,14 +61,20 @@ export class ESLintEngine extends Engine {
6061
const userConfigInfo: UserConfigInfo = this.getUserConfigInfo(describeOptions.workspace);
6162
this.emitLogEvent(LogLevel.Fine, `Detected the following state regarding the user's ESLint configuration: ${userConfigInfo}`);
6263

63-
if (this.shouldDelegateToV8(userConfigInfo)) {
64+
if (userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG) {
65+
this.warnAboutUsingESLint8IfNeeded(userConfigInfo);
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,8 @@ 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) {
108+
this.warnAboutUsingESLint8IfNeeded(userConfigInfo);
101109
return this.delegateV8Engine.runRules(ruleNames, runOptions);
102110
}
103111

@@ -129,6 +137,14 @@ export class ESLintEngine extends Engine {
129137
return engineResults;
130138
}
131139

140+
private warnAboutUsingESLint8IfNeeded(userConfigInfo: UserConfigInfo): void {
141+
if (userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG && !this.haveWarnedAboutESLint8) {
142+
this.emitLogEvent(LogLevel.Warn, getMessage('DetectedLegacyConfig',
143+
userConfigInfo.getChosenUserConfigFile() ?? userConfigInfo.getChosenUserIgnoreFile()!));
144+
this.haveWarnedAboutESLint8 = true;
145+
}
146+
}
147+
132148
private toViolations(eslintResults: ESLint.LintResult[]): Violation[] {
133149
const violations: Violation[] = [];
134150
for (const eslintResult of eslintResults) {
@@ -161,17 +177,14 @@ export class ESLintEngine extends Engine {
161177
return this.userConfigInfoCache.get(cacheKey)!;
162178
}
163179

164-
private shouldDelegateToV8(userConfigInfo: UserConfigInfo): boolean {
165-
return userConfigInfo.getState() === UserConfigState.LEGACY_USER_CONFIG;
166-
}
167-
168180
private async getESLintContext(workspace?: Workspace): Promise<ESLintContext> {
169181
const cacheKey: string = workspace?.getWorkspaceId() ?? process.cwd();
170182
if (!this.eslintContextCache.has(cacheKey)) {
171183
const userConfigInfo: UserConfigInfo = this.getUserConfigInfo(workspace);
172184
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);
185+
this.engineConfig.config_root, this.engineConfig.file_extensions, userConfigInfo.getChosenUserConfigFile());
186+
const context: ESLintContext = await calculateESLintContext(this.engineConfig, eslintWorkspace,
187+
userConfigInfo.getChosenUserConfigFile());
175188
this.eslintContextCache.set(cacheKey, context);
176189
}
177190
return this.eslintContextCache.get(cacheKey)!;
@@ -267,3 +280,11 @@ function normalizeEndValue(endValue: number | undefined): number | undefined {
267280
/* istanbul ignore next */
268281
return endValue && endValue > 0 ? endValue : undefined;
269282
}
283+
284+
285+
function makeRelativeTo(absFolderPath: string, absFilePath: string): string {
286+
absFolderPath = absFolderPath.endsWith(path.sep) ?
287+
/* istanbul ignore next */ absFolderPath : absFolderPath + path.sep;
288+
return absFilePath.startsWith(absFolderPath) ?
289+
absFilePath.slice(absFolderPath.length) : /* istanbul ignore next */ absFilePath;
290+
}

‎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

0 commit comments

Comments
 (0)