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!: add method getConfigStatus, update isFileIgnored #7

Merged
merged 6 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 88 additions & 1 deletion packages/config-array/src/config-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,67 @@ function assertExtraConfigTypes(extraConfigTypes) {
}
}

/**
* Calculates whether a file has a config or why it doesn't.
* @param {ConfigArray} configArray A normalized config array.
* @param {string} filePath The path of the file to check.
* @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the constants returned by
* {@linkcode ConfigArray.getConfigStatus}.
*/
function calculateConfigStatus(configArray, filePath) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of repeated logic from getConfig, and a separate cache.

Maybe we could rename the current getConfig to getConfigWithStatus, and modify it to cache and return { config, status }? Then, the new getConfig would be just return this.getConfigWithStatus(filePath).config;, and getConfigStatus would be just return this.getConfigWithStatus(filePath).status;.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I also thought that. Would like @nzakas' perspective on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, done in 5ce6b18. Please, have a look.

// check if the file is outside the base path
const relativeFilePath = path.relative(configArray.basePath, filePath);
if (relativeFilePath.startsWith("..")) return "external";

// next check to see if the file should be ignored
if (
configArray.isDirectoryIgnored(path.dirname(filePath)) ||
shouldIgnorePath(configArray.ignores, filePath, relativeFilePath)
) {
return "ignored";
}

// filePath isn't automatically ignored, so try to find a matching config
const universalPattern = /\/\*{1,2}$/;

for (const config of configArray) {
if (!config.files) continue;

/*
* If a config has a files pattern ending in /** or /*, and the
* filePath only matches those patterns, then the config is only
* applied if there is another config where the filePath matches
* a file with a specific extensions such as *.js.
*/

const universalFiles = config.files.filter(pattern =>
universalPattern.test(pattern),
);

let filteredConfig;

// universal patterns were found so we need to filter the files
if (universalFiles.length) {
const nonUniversalFiles = config.files.filter(
pattern => !universalPattern.test(pattern),
);

filteredConfig = {
files: nonUniversalFiles,
ignores: config.ignores,
};
} else {
filteredConfig = config;
}

if (pathMatches(filePath, configArray.basePath, filteredConfig)) {
return "matched";
}
}

return "unconfigured";
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -582,6 +643,7 @@ export class ConfigArray extends Array {
directoryMatches: new Map(),
files: undefined,
ignores: undefined,
configStatus: new Map(),
});

// load the configs into this array
Expand Down Expand Up @@ -985,6 +1047,31 @@ export class ConfigArray extends Array {
return finalConfig;
}

/**
* Determines whether a file has a config or why it doesn't.
* @param {string} filePath The path of the file to check.
* @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the following values:
* * `"ignored"`: the file is ignored
* * `"external"`: the file is outside the base path
* * `"unconfigured"`: the file is not matched by any config
* * `"matched"`: the file has a matching config
*/
getConfigStatus(filePath) {
assertNormalized(this);

// first check the cache for a filename match to avoid duplicate work
const cache = dataCache.get(this).configStatus;
let configStatus = cache.get(filePath);

if (!configStatus) {
// no cached value found, so calculate
configStatus = calculateConfigStatus(this, filePath);
cache.set(configStatus);
}

return configStatus;
}

/**
* Determines if the given filepath is ignored based on the configs.
* @param {string} filePath The complete path of a file to check.
Expand All @@ -1001,7 +1088,7 @@ export class ConfigArray extends Array {
* @returns {boolean} True if the path is ignored, false if not.
*/
isFileIgnored(filePath) {
return this.getConfig(filePath) === undefined;
return this.getConfigStatus(filePath) === "ignored";
}
Comment on lines 1048 to 1050
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ESLint, we'll need to update all uses of isFileIgnored() to getConfig() or getConfigStatus() in order to retain current functionalities, which I think raises the question of whether isFileIgnored is needed anymore. I think we should just remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still like it as a shorthand for getConfigStatus() === "ignored".


/**
Expand Down
Loading