Skip to content

Commit

Permalink
NEW: @W-16263070@: Exclude from the workspace any dot files/folders u…
Browse files Browse the repository at this point in the history
…nless explicitly added (#63)
  • Loading branch information
stephen-carter-at-sf authored Aug 7, 2024
1 parent e893ec6 commit dec54fd
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 82 deletions.
61 changes: 32 additions & 29 deletions packages/code-analyzer-core/test/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,12 @@ describe("Tests for the run method of CodeAnalyzer", () => {
const expectedEngineRunOptions: engApi.RunOptions = {
workspace: new engApi.Workspace([path.resolve('src')], "FixedId")
};
expect(stubEngine1.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine1RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine2.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine2RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine1.runRulesCallHistory).toHaveLength(1);
expect(stubEngine1.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine1RuleNames);
expectEquivalentRunOptions(stubEngine1.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
expect(stubEngine2.runRulesCallHistory).toHaveLength(1);
expect(stubEngine2.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine2RuleNames);
expectEquivalentRunOptions(stubEngine2.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
});

it("When specifying path start points as files and subfolders, then they are passed to each engine successfully", async () => {
Expand All @@ -150,14 +148,12 @@ describe("Tests for the run method of CodeAnalyzer", () => {
{ file: path.resolve("test", "run.test.ts")}
]
};
expect(stubEngine1.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine1RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine2.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine2RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine1.runRulesCallHistory).toHaveLength(1);
expect(stubEngine1.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine1RuleNames);
expectEquivalentRunOptions(stubEngine1.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
expect(stubEngine2.runRulesCallHistory).toHaveLength(1);
expect(stubEngine2.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine2RuleNames);
expectEquivalentRunOptions(stubEngine2.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
});

it("When specifying path start points individual methods, then they are passed to each engine successfully", async () => {
Expand Down Expand Up @@ -195,14 +191,12 @@ describe("Tests for the run method of CodeAnalyzer", () => {
}
]
};
expect(stubEngine1.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine1RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine2.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine2RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine1.runRulesCallHistory).toHaveLength(1);
expect(stubEngine1.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine1RuleNames);
expectEquivalentRunOptions(stubEngine1.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
expect(stubEngine2.runRulesCallHistory).toHaveLength(1);
expect(stubEngine2.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine2RuleNames);
expectEquivalentRunOptions(stubEngine2.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
});

it("When the workspace provided is one that is not constructed from CodeAnalyzer's createWorkspace method, then it should still work", async () => {
Expand All @@ -226,11 +220,10 @@ describe("Tests for the run method of CodeAnalyzer", () => {
const expectedEngineRunOptions: engApi.RunOptions = {
workspace: new engApi.Workspace([__dirname], "FixedId")
};
expect(stubEngine1.runRulesCallHistory).toEqual([{
ruleNames: expectedStubEngine1RuleNames,
runOptions: expectedEngineRunOptions
}]);
expect(stubEngine2.runRulesCallHistory).toEqual([]);
expect(stubEngine1.runRulesCallHistory).toHaveLength(1);
expect(stubEngine1.runRulesCallHistory[0].ruleNames).toEqual(expectedStubEngine1RuleNames);
expectEquivalentRunOptions(stubEngine1.runRulesCallHistory[0].runOptions, expectedEngineRunOptions);
expect(stubEngine2.runRulesCallHistory).toHaveLength(0);
});

it("When zero rules are selected, then all engines should be skipped and returned results contain no violations", async () => {
Expand Down Expand Up @@ -632,4 +625,14 @@ class DummyWorkspace implements Workspace {
getFilesAndFolders(): string[] {
return [__dirname];
}
}

function expectEquivalentRunOptions(actual: engApi.RunOptions, expected: engApi.RunOptions): void {
expectEquivalentWorkspaces(actual.workspace, expected.workspace);
expect(actual.pathStartPoints).toEqual(expected.pathStartPoints);
}

function expectEquivalentWorkspaces(actual: engApi.Workspace, expected: engApi.Workspace): void {
expect(actual.getWorkspaceId()).toEqual(expected.getWorkspaceId());
expect(actual.getFilesAndFolders()).toEqual(expected.getFilesAndFolders());
}
10 changes: 7 additions & 3 deletions packages/code-analyzer-core/test/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@ describe("Tests for the createWorkspace method of CodeAnalyzer", () => {
expect(workspace.getFilesAndFolders()).toEqual([path.resolve('test')]);
});

it("When including files we care not to process like .gitignore file or a node_modules folder, then they are removed", async () => {
it("When explicitly including files that we normally would ignore, then they are still included since they were explicitely added", async () => {
const workspace: Workspace = await codeAnalyzer.createWorkspace([
'test/test-data/sampleWorkspace/node_modules/place_holder.txt',
'test/test-data/sampleWorkspace/someFile.txt',
'test/test-data/sampleWorkspace/.gitignore',]);
expect(workspace.getFilesAndFolders()).toEqual([path.resolve('test', 'test-data', 'sampleWorkspace', 'someFile.txt')]);
'test/test-data/sampleWorkspace/.gitignore']);
expect(workspace.getFilesAndFolders()).toEqual([
path.resolve('test', 'test-data', 'sampleWorkspace', 'node_modules', 'place_holder.txt'),
path.resolve('test', 'test-data', 'sampleWorkspace', 'someFile.txt'),
path.resolve('test', 'test-data', 'sampleWorkspace', '.gitignore')
].sort());
});
});
121 changes: 81 additions & 40 deletions packages/code-analyzer-engine-api/src/workspace.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import fs from "node:fs";
import path from "node:path";

const UNWANTED_FOLDERS: string[] = ['node_modules', '.git', '.github'];
const UNWANTED_FILES: string[] = ['code_analyzer_config.yml', 'code_analyzer_config.yaml', '.gitignore'];
const NON_DOT_FOLDERS_TO_EXCLUDE: string[] = ['node_modules'];
const NON_DOT_FILES_TO_EXCLUDE: string[] = ['code_analyzer_config.yml', 'code_analyzer_config.yaml'];

export class Workspace {
private static nextId: number = 0;
private readonly workspaceId: string;
private readonly filesAndFolders: string[];
private readonly rawFilesAndFolders: string[];

private filesAndFolders?: string[];
private expandedFiles?: string[];
private workspaceRoot?: string | null;

Expand All @@ -19,9 +21,7 @@ export class Workspace {
*/
constructor(absoluteFileAndFolderPaths: string[], workspaceId?: string) {
this.workspaceId = workspaceId || `workspace${++Workspace.nextId}`;
this.filesAndFolders = removeRedundantPaths(absoluteFileAndFolderPaths)
.filter(isWantedPath)
.map(removeTrailingPathSep);
this.rawFilesAndFolders = absoluteFileAndFolderPaths;
}

/**
Expand All @@ -33,10 +33,13 @@ export class Workspace {

/**
* Returns the unique list of files and folders that were used to construct the workspace.
* Any files or folders that Code Analyzer chooses to ignore (like .gitignore files, node_modules folders, etc.)
* are excluded from the list.
* Note that besides removing redundant paths, no other filtering is done. For example, if a user explicitly
* provided to the Workspace constructor a .dotFile then we will not exclude this file.
*/
getFilesAndFolders(): string[] {
if (!this.filesAndFolders) {
this.filesAndFolders = this.removeRedundantPaths(this.rawFilesAndFolders).map(removeTrailingPathSep);
}
return this.filesAndFolders;
}

Expand All @@ -49,7 +52,7 @@ export class Workspace {
*/
getWorkspaceRoot(): string | null {
if (this.workspaceRoot === undefined) {
this.workspaceRoot = calculateLongestCommonParentFolderOf(this.getFilesAndFolders());
this.workspaceRoot = calculateLongestCommonParentFolderOf(this.rawFilesAndFolders);
}
return this.workspaceRoot;
}
Expand All @@ -59,34 +62,81 @@ export class Workspace {
* This list is composed of the files that getFilesAndFolders() returns plus any files found recursively inside
* any of the folders that getFilesAndFolders() returns. That is, the folders are expanded so that the resulting
* list only contains file paths.
* Any files that Code Analyzer chooses to ignore (like .gitignore files, files in node_modules folders, etc.)
* are excluded from the list.
* Any files underneath the workspace root that Code Analyzer chooses to ignore (like .gitignore files,
* files in node_modules folders, etc.) are automatically excluded unless they were explicitly provided when
* constructing the workspace.
*/
async getExpandedFiles(): Promise<string[]> {
if (!this.expandedFiles) {
this.expandedFiles = (await expandToListAllFiles(this.filesAndFolders)).filter(isWantedPath);
this.expandedFiles = (await expandToListAllFiles(this.getFilesAndFolders())).filter(f => !this.shouldExclude(f));
}
return this.expandedFiles as string[];
}
}

/**
* Removes redundant paths.
* If a user supplies a parent folder and subfolder of file underneath the parent folder, then we can safely
* remove that subfolder or file. Also, if we find duplicate entries, we remove those as well.
*/
function removeRedundantPaths(absolutePaths: string[]): string[] {
const pathsSortedByLength: string[] = absolutePaths.sort((a, b) => a.length - b.length);
const filteredPaths: string[] = [];
for (const currentPath of pathsSortedByLength) {
const isAlreadyContained = filteredPaths.some(existingPath =>
currentPath.startsWith(existingPath + path.sep) || existingPath === currentPath
);
if (!isAlreadyContained) {
filteredPaths.push(currentPath);
/**
* Returns whether a path should be excluded or not (like paths under a "node_modules" folder should be excluded)
* Idea: In the future, we might consider having a .code_analyzer_ignore file or something that users can create
* which could help the user have better control over what files are excluded.
*
* Note: When determining whether to exclude a file or not, we base it entirely off looking at the
* portion of the path underneath the workspace root. This allows us to not accidentally remove all files if the
* workspace happens to live under a dot folder for example. Additionally, we do not remove any files that have
* been explicitly provided to the Workspace constructor, which allows users to target .dot folders and files if
* they choose to do so.
*/
private shouldExclude(fileOrFolder: string): boolean {
return this.isExcludeCandidate(fileOrFolder) && !this.excludeCandidateWasExplicitlyProvided(fileOrFolder);
}
private isExcludeCandidate(fileOrFolder: string): boolean {
const relativeFileOrFolder: string = this.makeRelativeToWorkspaceRoot(fileOrFolder);
if (relativeFileOrFolder.length === 0) { // folder is equal to the workspace root
return false;
}
return NON_DOT_FOLDERS_TO_EXCLUDE.some(f => relativeFileOrFolder.includes(`${path.sep}${f}${path.sep}`)) ||
NON_DOT_FILES_TO_EXCLUDE.includes(path.basename(relativeFileOrFolder)) ||
relativeFileOrFolder.includes(`${path.sep}.`);
}
private excludeCandidateWasExplicitlyProvided(fileOrFolder: string): boolean {
if (this.rawFilesAndFolders.includes(fileOrFolder)) {
return true;
}
const parentFolder: string = path.dirname(fileOrFolder);
if (this.isExcludeCandidate(parentFolder)) {
return this.excludeCandidateWasExplicitlyProvided(parentFolder);
}
return false;
}

/**
* Returns the file or folder as a relative path, relative to the workspace root
*/
private makeRelativeToWorkspaceRoot(fileOrFolder: string): string {
if(this.getWorkspaceRoot()) {
return fileOrFolder.slice(this.getWorkspaceRoot()!.length);
}
/* istanbul ignore next */
return fileOrFolder;
}

/**
* Removes redundant paths.
* If a user supplies a parent folder and subfolder of file underneath the parent folder, then we can safely
* remove that subfolder or file (unless it is an excludeCandidate that has been explicitly requested to keep).
* Also, if we find duplicate entries, we remove those as well.
*/
private removeRedundantPaths(absolutePaths: string[]): string[] {
const pathsSortedByLength: string[] = [...absolutePaths].sort((a, b) => a.length - b.length);
const filteredPaths: string[] = [];
for (const currentPath of pathsSortedByLength) {
const isAlreadyContained = filteredPaths.some(existingPath =>
currentPath.startsWith(existingPath + path.sep) || existingPath === currentPath
);
if (!isAlreadyContained || this.isExcludeCandidate(currentPath)) {
filteredPaths.push(currentPath);
}
}
return filteredPaths.sort(); // sort alphabetically
}
return filteredPaths.sort(); // sort alphabetically
}

/**
Expand All @@ -97,31 +147,22 @@ function removeTrailingPathSep(absolutePath: string): string {
absolutePath.slice(0, absolutePath.length - path.sep.length) : absolutePath;
}

/**
* Returns whether a path should be included or not (like those that live under a "node_modules" folder should not be)
* Idea: in the future, we might consider having a .code_analyzer_ignore file or something that users can create
*/
function isWantedPath(absolutePath: string): boolean {
return !UNWANTED_FOLDERS.some(f => absolutePath.includes(`${path.sep}${f}${path.sep}`)) &&
!UNWANTED_FILES.includes(path.basename(absolutePath));
}

/**
* Expands a list of files and/or folders to be a list of all contained files, including the files found in subfolders
*/
export async function expandToListAllFiles(absoluteFileOrFolderPaths: string[]): Promise<string[]> {
const allFiles: string[] = [];
const allFiles: Set<string> = new Set(); // Using a set to guarantee uniqueness
async function processPath(currentPath: string): Promise<void> {
if ((await fs.promises.stat(currentPath)).isDirectory()) {
const subPaths: string[] = await fs.promises.readdir(currentPath);
const absSubPaths: string[] = subPaths.map(f => path.join(currentPath, f));
await Promise.all(absSubPaths.map(processPath)); // Process subdirectories recursively
} else {
allFiles.push(currentPath);
allFiles.add(currentPath);
}
}
await Promise.all(absoluteFileOrFolderPaths.map(processPath));
return allFiles.sort();
return [... allFiles].sort();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!node_modules/
Loading

0 comments on commit dec54fd

Please sign in to comment.