FIX: @W-18403733@: Pass in raw selection to be scanned instead of expanding… (#235)

Author: stephen-carter-at-sf

src/extension.ts

@@ -33,6 +33,7 @@ import {CliCommandExecutor, CliCommandExecutorImpl} from "./lib/cli-commands";
 import {getErrorMessage} from "./lib/utils";
 import {FileHandler, FileHandlerImpl} from "./lib/fs-utils";
 import {VscodeWorkspace, VscodeWorkspaceImpl, WindowManager, WindowManagerImpl} from "./lib/vscode-api";
+import {Workspace} from "./lib/workspace";
 
 
 // Object to hold the state of our extension for a specific activation context, to be returned by our activate function
@@ -85,13 +86,14 @@ export async function activate(context: vscode.ExtensionContext): Promise<SFCAEx
     context.subscriptions.push(scanManager);
 
     const windowManager: WindowManager = new WindowManagerImpl(outputChannel);
+    const vscodeWorkspace: VscodeWorkspace = new VscodeWorkspaceImpl();
 
     const taskWithProgressRunner: TaskWithProgressRunner = new TaskWithProgressRunnerImpl();
 
+
     const cliCommandExecutor: CliCommandExecutor = new CliCommandExecutorImpl(logger);
-    const vscodeWorkspace: VscodeWorkspace = new VscodeWorkspaceImpl();
     const fileHandler: FileHandler = new FileHandlerImpl();
-    const codeAnalyzer: CodeAnalyzer = new CodeAnalyzerImpl(cliCommandExecutor, settingsManager, display, vscodeWorkspace, fileHandler);
+    const codeAnalyzer: CodeAnalyzer = new CodeAnalyzerImpl(cliCommandExecutor, settingsManager, display, fileHandler);
     const dfaRunner: DfaRunner = new DfaRunner(context, codeAnalyzer, telemetryService, logger); // This thing is really old and clunky. It'll go away when we remove v4 stuff. But if we don't want to wait we could move all this into the v4-scanner.ts file
     context.subscriptions.push(dfaRunner);
 
@@ -126,7 +128,8 @@ export async function activate(context: vscode.ExtensionContext): Promise<SFCAEx
             vscode.window.showWarningMessage(messages.noActiveEditor);
             return;
         }
-        return codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, [document.fileName]);
+        const workspace: Workspace = await Workspace.fromTargetPaths([document.fileName], vscodeWorkspace, fileHandler);
+        return codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, workspace);
     });
 
     // "Analyze On Open" and "Analyze on Save" functionality:
@@ -139,7 +142,8 @@ export async function activate(context: vscode.ExtensionContext): Promise<SFCAEx
         const isValidFileThatHasNotBeenScannedYet = isValidFile && !scanManager.haveAlreadyScannedFile(editor.document.fileName);
         if (isValidFileThatHasNotBeenScannedYet) {
             scanManager.addFileToAlreadyScannedFiles(editor.document.fileName);
-            await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, [editor.document.fileName]);
+            const workspace: Workspace = await Workspace.fromTargetPaths([editor.document.fileName], vscodeWorkspace, fileHandler);
+            await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, workspace);
         }
     });
     onDidSaveTextDocument(async (document: vscode.TextDocument) => {
@@ -154,22 +158,20 @@ export async function activate(context: vscode.ExtensionContext): Promise<SFCAEx
 
         if (settingsManager.getAnalyzeOnSave()) {
             scanManager.addFileToAlreadyScannedFiles(document.fileName);
-            await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, [document.fileName]);
+            const workspace: Workspace = await Workspace.fromTargetPaths([document.fileName], vscodeWorkspace, fileHandler);
+            await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_ACTIVE_FILE, workspace);
         }
     });
 
     // COMMAND_RUN_ON_SELECTED: Invokable by 'explorer/context' menu always. Uses v4 instead of v5 when 'sfca.codeAnalyzerV4Enabled'.
     registerCommand(Constants.COMMAND_RUN_ON_SELECTED, async (singleSelection: vscode.Uri, multiSelection?: vscode.Uri[]) => {
         const selection: vscode.Uri[] = (multiSelection && multiSelection.length > 0) ? multiSelection : [singleSelection];
-        // TODO: We may wish to consider moving away from this target resolution, and just passing in files and folders
-        //       as given to us. It's possible the current style could lead to overflowing the CLI when a folder has
-        //       many files.
-        const selectedFiles: string[] = await targeting.getFilesFromSelection(selection);
-        if (selectedFiles.length == 0) { // I have not found a way to hit this, but we should check just in case
+        const workspace: Workspace = await Workspace.fromTargetPaths(selection.map(uri => uri.fsPath), vscodeWorkspace, fileHandler);
+        if (workspace.getRawTargetPaths().length == 0) { // I have not found a way to hit this, but we should check just in case
             vscode.window.showWarningMessage(messages.targeting.error.noFileSelected);
             return;
         }
-        await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_SELECTED, selectedFiles);
+        await codeAnalyzerRunAction.run(Constants.COMMAND_RUN_ON_SELECTED, workspace);
     });
 
 

src/lib/cli-commands.ts

@@ -96,20 +96,30 @@ export class CliCommandExecutorImpl implements CliCommandExecutor {
 
     async exec(command: string, args: string[], options: ExecOptions = {}): Promise<CommandOutput> {
         return new Promise((resolve) => {
-            const childProcess: cp.ChildProcessWithoutNullStreams  = cp.spawn(command, args, {
-                shell: process.platform.startsWith('win'), // Use shell on Windows machines
-            });
-
-            if (options.pidHandler) {
-                options.pidHandler(childProcess.pid);
-            }
-            const logLevel: vscode.LogLevel = options.logLevel === undefined ? vscode.LogLevel.Trace : options.logLevel;
-
             const output: CommandOutput = {
                 stdout: '',
                 stderr: '',
                 exitCode: 0
             };
+
+            let childProcess: cp.ChildProcessWithoutNullStreams;
+            try {
+                childProcess = cp.spawn(command, args, {
+                    shell: process.platform.startsWith('win'), // Use shell on Windows machines
+                });
+            } catch (err) {
+                this.logger.logAtLevel(vscode.LogLevel.Error, `Failed to execute the following command:\n` +
+                    indent(`${command} ${args.map(arg => arg.includes(' ') ? `"${arg}"` : arg).join(' ')}`) + `\n\n` +
+                    'Error Thrown:\n' + indent(getErrorMessageWithStack(err)));
+                output.stderr = getErrorMessageWithStack(err);
+                output.exitCode = 127;
+                resolve(output);
+            }
+
+            if (options.pidHandler) {
+                options.pidHandler(childProcess.pid);
+            }
+            const logLevel: vscode.LogLevel = options.logLevel === undefined ? vscode.LogLevel.Trace : options.logLevel;
             let combinedOut: string = '';
 
             this.logger.logAtLevel(logLevel, `Executing with background process (${childProcess.pid}):\n` +

src/lib/code-analyzer-run-action.ts

@@ -9,6 +9,7 @@ import {Display} from "./display";
 import {getErrorMessage, getErrorMessageWithStack} from "./utils";
 import {ProgressReporter, TaskWithProgressRunner} from "./progress";
 import {WindowManager} from "./vscode-api";
+import {Workspace} from "./workspace";
 
 export const UNINSTANTIABLE_ENGINE_RULE = 'UninstantiableEngineError';
 
@@ -35,9 +36,9 @@ export class CodeAnalyzerRunAction {
     /**
      * Runs the scanner against the specified file and displays the results.
      * @param commandName The command being run
-     * @param filesToScan The files to run against
+     * @param workspace The workspace to run against
      */
-    run(commandName: string, filesToScan: string[]): Promise<void> {
+    run(commandName: string, workspace: Workspace): Promise<void> {
         return this.taskWithProgressRunner.runTask(async (progressReporter: ProgressReporter) => {
             const startTime: number = Date.now();
 
@@ -59,7 +60,7 @@ export class CodeAnalyzerRunAction {
                     increment: 20
                 });
                 this.logger.log(messages.info.scanningWith(await this.codeAnalyzer.getScannerName()));
-                const violations: Violation[] = await this.codeAnalyzer.scan(filesToScan);
+                const violations: Violation[] = await this.codeAnalyzer.scan(workspace);
 
                 progressReporter.reportProgress({
                     message: messages.scanProgressReport.processingResults,
@@ -78,9 +79,10 @@ export class CodeAnalyzerRunAction {
                 });
 
                 const diagnostics: CodeAnalyzerDiagnostic[] = violationsWithFileLocation.map(v => CodeAnalyzerDiagnostic.fromViolation(v));
-                this.diagnosticManager.clearDiagnosticsForFiles(filesToScan.map(f => vscode.Uri.file(f)));
+                const targetedFiles: string[] = await workspace.getTargetedFiles();
+                this.diagnosticManager.clearDiagnosticsForFiles(targetedFiles.map(f => vscode.Uri.file(f)));
                 this.diagnosticManager.addDiagnostics(diagnostics);
-                void this.displayResults(filesToScan.length, violationsWithFileLocation);
+                void this.displayResults(targetedFiles.length, violationsWithFileLocation);
 
                 this.telemetryService.sendCommandEvent(Constants.TELEM_SUCCESSFUL_STATIC_ANALYSIS, {
                     commandName: commandName,

src/lib/code-analyzer.ts

@@ -11,8 +11,8 @@ import {
     RECOMMENDED_MINIMUM_REQUIRED_CODE_ANALYZER_CLI_PLUGIN_VERSION
 } from "./constants";
 import {CliScannerStrategy} from "./scanner-strategies/scanner-strategy";
-import {VscodeWorkspace, VscodeWorkspaceImpl} from "./vscode-api";
 import {FileHandler, FileHandlerImpl} from "./fs-utils";
+import {Workspace} from "./workspace";
 
 export interface CodeAnalyzer extends CliScannerStrategy {
     validateEnvironment(): Promise<void>;
@@ -22,7 +22,6 @@ export class CodeAnalyzerImpl implements CodeAnalyzer {
     private readonly cliCommandExecutor: CliCommandExecutor;
     private readonly settingsManager: SettingsManager;
     private readonly display: Display;
-    private readonly vscodeWorkspace: VscodeWorkspace;
     private readonly fileHandler: FileHandler
 
     private cliIsInstalled: boolean = false;
@@ -31,11 +30,10 @@ export class CodeAnalyzerImpl implements CodeAnalyzer {
     private codeAnalyzerV5?: CliScannerV5Strategy;
 
     constructor(cliCommandExecutor: CliCommandExecutor, settingsManager: SettingsManager, display: Display,
-                vscodeWorkspace: VscodeWorkspace = new VscodeWorkspaceImpl(), fileHandler: FileHandler = new FileHandlerImpl()) {
+                fileHandler: FileHandler = new FileHandlerImpl()) {
         this.cliCommandExecutor = cliCommandExecutor;
         this.settingsManager = settingsManager;
         this.display = display;
-        this.vscodeWorkspace = vscodeWorkspace;
         this.fileHandler = fileHandler;
     }
 
@@ -87,11 +85,11 @@ export class CodeAnalyzerImpl implements CodeAnalyzer {
             this.display.displayWarning(messages.codeAnalyzer.usingOlderVersion(installedVersion.toString(), recommendedMinVersion.toString()) + '\n'
                 + messages.codeAnalyzer.installLatestVersion);
         }
-        this.codeAnalyzerV5 = new CliScannerV5Strategy(installedVersion, this.cliCommandExecutor, this.settingsManager, this.vscodeWorkspace, this.fileHandler);
+        this.codeAnalyzerV5 = new CliScannerV5Strategy(installedVersion, this.cliCommandExecutor, this.settingsManager, this.fileHandler);
     }
 
-    async scan(filesToScan: string[]): Promise<Violation[]> {
-        return (await this.getDelegate()).scan(filesToScan);
+    async scan(workspace: Workspace): Promise<Violation[]> {
+        return (await this.getDelegate()).scan(workspace);
     }
 
     async getScannerName(): Promise<string> {

src/lib/scanner-strategies/scanner-strategy.ts

@@ -1,7 +1,8 @@
 import {Violation} from '../diagnostics';
+import {Workspace} from "../workspace";
 
 export interface CliScannerStrategy {
-    scan(filesToScan: string[]): Promise<Violation[]>;
+    scan(workspace: Workspace): Promise<Violation[]>;
 
     getScannerName(): Promise<string>;
 

src/lib/scanner-strategies/v4-scanner.ts

@@ -6,6 +6,7 @@ import {SettingsManager} from "../settings";
 import * as semver from 'semver';
 import {CliCommandExecutor, CommandOutput} from "../cli-commands";
 import {FileHandler} from "../fs-utils";
+import {Workspace} from "../workspace";
 
 export type BaseV4Violation = {
     ruleName: string;
@@ -66,9 +67,9 @@ export class CliScannerV4Strategy implements CliScannerStrategy {
         return Promise.resolve(`@salesforce/sfdx-scanner@${this.version.toString()} via CLI`);
     }
 
-    public async scan(filesToScan: string[]): Promise<Violation[]> {
+    public async scan(workspace: Workspace): Promise<Violation[]> {
         // Create the arg array.
-        const args: string[] = await this.createArgArray(filesToScan);
+        const args: string[] = await this.createArgArray(workspace);
 
         // Invoke the scanner.
         const executionResult: V4ExecutionResult = await this.invokeAnalyzer(args);
@@ -77,7 +78,7 @@ export class CliScannerV4Strategy implements CliScannerStrategy {
         return this.processResults(executionResult);
     }
 
-    private async createArgArray(targets: string[]): Promise<string[]> {
+    private async createArgArray(workspace: Workspace): Promise<string[]> {
         const engines: string = this.settingsManager.getEnginesToRun();
         const pmdCustomConfigFile: string | undefined = this.settingsManager.getPmdCustomConfigFile();
         const rulesCategory: string | undefined = this.settingsManager.getRulesCategory();
@@ -89,7 +90,7 @@ export class CliScannerV4Strategy implements CliScannerStrategy {
 
         const args: string[] = [
             'scanner', 'run',
-            '--target', `${targets.join(',')}`,
+            '--target', `${workspace.getRawTargetPaths().join(',')}`,
             `--engine`, engines,
             `--json`
         ];

src/lib/scanner-strategies/v5-scanner.ts

@@ -7,7 +7,7 @@ import * as path from 'node:path';
 import * as semver from 'semver';
 import {SettingsManager} from "../settings";
 import {CliCommandExecutor, CommandOutput} from "../cli-commands";
-import {VscodeWorkspace} from "../vscode-api";
+import {Workspace} from "../workspace";
 
 type ResultsJson = {
     runDir: string;
@@ -31,16 +31,14 @@ export class CliScannerV5Strategy implements CliScannerStrategy {
     private readonly version: semver.SemVer;
     private readonly cliCommandExecutor: CliCommandExecutor;
     private readonly settingsManager: SettingsManager;
-    private readonly vscodeWorkspace: VscodeWorkspace;
     private readonly fileHandler: FileHandler;
 
     private ruleDescriptionMap?: Map<string, string>;
 
-    public constructor(version: semver.SemVer, cliCommandExecutor: CliCommandExecutor, settingsManager: SettingsManager, vscodeWorkspace: VscodeWorkspace, fileHandler: FileHandler) {
+    public constructor(version: semver.SemVer, cliCommandExecutor: CliCommandExecutor, settingsManager: SettingsManager, fileHandler: FileHandler) {
         this.version = version;
         this.cliCommandExecutor = cliCommandExecutor;
         this.settingsManager = settingsManager;
-        this.vscodeWorkspace = vscodeWorkspace;
         this.fileHandler = fileHandler;
     }
 
@@ -63,25 +61,18 @@ export class CliScannerV5Strategy implements CliScannerStrategy {
         return this.ruleDescriptionMap;
     }
 
-    public async scan(filesToScan: string[]): Promise<Violation[]> {
+    public async scan(workspace: Workspace): Promise<Violation[]> {
         const ruleSelector: string = this.settingsManager.getCodeAnalyzerRuleSelectors();
         const configFile: string = this.settingsManager.getCodeAnalyzerConfigFile();
 
         const args: string[] = ['code-analyzer', 'run'];
 
         if (semver.gte(this.version, '5.0.0')) {
-            // Just in case a file is open in the editor that does not live in the current workspace, or if there
-            // is no workspace open at all, we still want to be able to run code analyzer without error, so we
-            // include the files to scan always along with any workspace folders.
-            const workspacePaths: string[] = [
-                ...this.vscodeWorkspace.getWorkspaceFolders(),
-                ...filesToScan
-            ]
-            workspacePaths.forEach(p => args.push('-w', p));
-            filesToScan.forEach(p => args.push('-t', p));
+            workspace.getRawWorkspacePaths().forEach(p => args.push('-w', p));
+            workspace.getRawTargetPaths().forEach(p => args.push('-t', p));
         } else {
-            // Before 5.0.0 the --target flag did not exist, so we just make the workspace equal to the files to scan
-            filesToScan.forEach(p => args.push('-w', p));
+            // Before 5.0.0 the --target flag did not exist, so we just make the workspace equal to the target paths
+            workspace.getRawTargetPaths().forEach(p => args.push('-w', p));
         }
 
         if (ruleSelector) {

src/lib/workspace.ts

@@ -0,0 +1,72 @@
+import * as vscode from "vscode";
+import {FileHandler} from "./fs-utils";
+import {messages} from "./messages";
+import {glob} from "glob";
+import {VscodeWorkspace} from "./vscode-api";
+
+// Note, calling this Workspace since the future we might make this close and closer like what we have in Core and
+// eventually replace it when we no longer depend on the CLI.
+export class Workspace {
+    private readonly rawTargets: string[];
+    private readonly vscodeWorkspace: VscodeWorkspace;
+    private readonly fileHandler: FileHandler;
+
+    private constructor(rawTargets: string[], vscodeWorkspace: VscodeWorkspace, fileHandler: FileHandler) {
+        this.rawTargets = rawTargets;
+        this.vscodeWorkspace = vscodeWorkspace;
+        this.fileHandler = fileHandler;
+    }
+
+    static async fromTargetPaths(targetedPaths: string[], vscodeWorkspace: VscodeWorkspace, fileHandler: FileHandler): Promise<Workspace> {
+        const uniqueTargetPaths: Set<string> = new Set();
+        for (const target of targetedPaths) {
+            if (!(await fileHandler.exists(target))) {
+                // This should never happen, but we should handle it gracefully regardless.
+                throw new Error(messages.targeting.error.nonexistentSelectedFileGenerator(target));
+            }
+            uniqueTargetPaths.add(target);
+        }
+        return new Workspace([...uniqueTargetPaths], vscodeWorkspace, fileHandler);
+    }
+
+    /**
+     * Unique string array of targeted files and folders as they were selected by the user
+     */
+    getRawTargetPaths(): string[] {
+        return this.rawTargets;
+    }
+
+    /**
+     * Unique array of files and folders that make up the workspace.
+     *
+     * Just in case a file is open in the editor that does not live in the current workspace, or if there
+     * is no workspace open at all, we still want to be able to run code analyzer without error, so we
+     * include the raw targeted files and folders always along with any vscode workspace folders.
+     */
+    getRawWorkspacePaths(): string[] {
+        return [... new Set([
+            ...this.vscodeWorkspace.getWorkspaceFolders(),
+            ...this.getRawTargetPaths()
+        ])];
+    }
+
+    /**
+     * String array of expanded files that make up the targeted files
+     * This array is derived by expanding the targeted folders recursively into their children files.
+     */
+    async getTargetedFiles(): Promise<string[]> {
+        const workspaceFiles: string[] = [];
+        for (const fileOrFolder of this.getRawTargetPaths()) {
+            if (await this.fileHandler.isDir(fileOrFolder)) {
+                // Globby wants forward-slashes, but Windows uses back-slashes, so always convert to forward slashes
+                const globbablePath: string = fileOrFolder.replace(/\\/g, '/');
+                const globOut: string[] = await glob(`${globbablePath}/**/*`, {nodir: true});
+                // Globby's results are Unix-formatted. Do a Uri.file round-trip to return the path to its expected form.
+                globOut.forEach(o => workspaceFiles.push(vscode.Uri.file(o).fsPath));
+            } else {
+                workspaceFiles.push(fileOrFolder);
+            }
+        }
+        return workspaceFiles;
+    }
+}