Refactor merge file discovery in the Outline view (#182)

* Refactor merge file discovery in the Outline view

* Update unit tests

* Fix failing tests

* Fix lint issue

* Update missing tests

* Refactor merge-command utility functions

* Update DbgConfigNode

* Update utility functions and update tests

Author: mguzmanm

src/utils/fs-utils.ts

@@ -93,6 +93,18 @@ export function deleteFileIfExists(filePath?: string) {
     }
 }
 
+export function copyFile(source: string, destination: string) {
+    fs.copyFileSync(source, destination);
+}
+
+export function renameFile(source: string, destination: string) {
+    fs.renameSync(source, destination);
+}
+
+export function readDirectory(directoryPath: string): string[] {
+    return fs.readdirSync(directoryPath);
+}
+
 export function getFileModificationTime(filePath: string): number {
     if (fileExists(filePath)) {
         const stat = fs.statSync(filePath);

src/views/solution-outline/commands/merge-command.test.ts

@@ -21,12 +21,14 @@ import { exec, ExecException, execSync } from 'child_process';
 import { COutlineItem } from '../tree-structure/solution-outline-item';
 import path from 'path';
 import * as os from 'os';
-import * as fs from 'fs';
 import { ActiveSolutionTracker } from '../../../solutions/active-solution-tracker';
+import semver from 'semver';
+import { extractVersion } from '../../../utils/string-utils';
+import * as fsUtils from '../../../utils/fs-utils';
 
 export class MergeCommand {
     public static readonly mergeFile = `${PACKAGE_NAME}.mergeFile`;
-    private static readonly disallowedCmdChars  = /[\r\n&|<>^%"']/;
+    private static readonly disallowedCmdChars = /[\r\n&|<>^%"']/;
 
     constructor(
         private readonly commandsProvider: CommandsProvider,
@@ -47,23 +49,12 @@ export class MergeCommand {
             return;
         }
 
-        let local = node.getAttribute('local');
-        if (!local) {
-            vscode.window.showErrorMessage('Required local file is missing to perform merge.');
+        const mergeFiles = this.discoverMergeFiles(node);
+        if (!mergeFiles) {
             return;
         }
 
-        let update = node.getAttribute('update');
-        if (!update) {
-            vscode.window.showErrorMessage('Required update file is missing to perform merge.');
-            return;
-        }
-
-        let base = node.getAttribute('base');
-        if (!base) {
-            vscode.window.showErrorMessage('Required base file is missing to perform merge.');
-            return;
-        }
+        let { local, update, base } = mergeFiles;
 
         const codePath = this.getVSCodeExecutablePath();
         if (!codePath) {
@@ -75,7 +66,7 @@ export class MergeCommand {
         let merged = local + '.merged';
 
         // make a copy of local to create merged file
-        fs.copyFileSync(local, merged);
+        fsUtils.copyFile(local, merged);
         node.setAttribute('merged', merged);
 
         // ensure all paths are absolute
@@ -85,53 +76,22 @@ export class MergeCommand {
         merged = path.resolve(merged);
 
         // get the modification time of the merged file before merge
-        let mergedMTimeBefore = 0;
-        if (fs.existsSync(merged)) {
-            mergedMTimeBefore = fs.statSync(merged).mtimeMs;
-        }
+        const mergedMTimeBefore = fsUtils.getFileModificationTime(merged);
 
         try {
             const command = this.buildMergeCommand(codePath, local, update, base, merged);
             const exitCode = await this.doOpen3WayMerge(command);
 
             // get the modification time after merge
-            let mergedMTimeAfter = 0;
-            if (fs.existsSync(merged)) {
-                mergedMTimeAfter = fs.statSync(merged).mtimeMs;
-            }
+            const mergedMTimeAfter = fsUtils.getFileModificationTime(merged);
 
             if (exitCode !== 0) {
                 console.warn(`Merge exited with code ${exitCode}. Conflicts may exist.`);
                 return;
             }
 
-            // perform post-merge file operations
             if (exitCode === 0 && mergedMTimeAfter > mergedMTimeBefore) {
-                // create .bak file of local file
-                const backupPath = `${local}.bak`;
-                fs.copyFileSync(local, backupPath);
-
-                // delete local file
-                if (fs.existsSync(local)) {
-                    fs.unlinkSync(local);
-                }
-
-                // delete base file
-                if (fs.existsSync(base)) {
-                    fs.unlinkSync(base);
-                }
-
-                // rename update file to base file
-                const newBaseFileName = path.basename(update).replaceAll('update', 'base');
-                const baseDirPath = path.dirname(update);
-                const newBase = path.join(baseDirPath, newBaseFileName);
-                fs.renameSync(update, newBase);
-
-                // rename merged file to local file
-                fs.renameSync(merged, local);
-
-                // refresh tree view to update file status
-                this.activeSolutionTracker.triggerReload();
+                this.performPostMergeOperations(local, update, base, merged);
             }
 
         } catch (err) {
@@ -141,6 +101,53 @@ export class MergeCommand {
         }
     }
 
+    private discoverMergeFiles(node: COutlineItem): { local: string; update: string; base: string } | undefined {
+        const local = node.getAttribute('local');
+        if (!local) {
+            vscode.window.showErrorMessage('Required local file is missing to perform merge.');
+            return undefined;
+        }
+
+        const discovered = this.findNewestMergeFiles(local);
+        const update = discovered.update;
+        if (!update) {
+            vscode.window.showErrorMessage('Required update file is missing to perform merge.');
+            return undefined;
+        }
+
+        const base = discovered.base;
+        if (!base) {
+            vscode.window.showErrorMessage('Required base file is missing to perform merge.');
+            return undefined;
+        }
+
+        return { local, update, base };
+    }
+
+    private performPostMergeOperations(local: string, update: string, base: string, merged: string): void {
+        // create .bak file of local file
+        const backupPath = `${local}.bak`;
+        fsUtils.copyFile(local, backupPath);
+
+        // delete local file
+        fsUtils.deleteFileIfExists(local);
+
+        // delete base file
+        fsUtils.deleteFileIfExists(base);
+
+        // rename update file to base file
+        const newBaseFileName = path.basename(update).replaceAll('update', 'base');
+        const baseDirPath = path.dirname(update);
+        const newBase = path.join(baseDirPath, newBaseFileName);
+        fsUtils.renameFile(update, newBase);
+
+        // rename merged file to local file
+        fsUtils.renameFile(merged, local);
+
+        // refresh tree view to update file status
+        this.activeSolutionTracker.triggerReload();
+    }
+
     private getVSCodeExecutablePath(): string | undefined {
         const platform = os.platform();
 
@@ -154,7 +161,7 @@ export class MergeCommand {
             ];
 
             for (const p of possiblePaths) {
-                if (fs.existsSync(p)) {
+                if (fsUtils.fileExists(p)) {
                     return p;
                 }
             }
@@ -168,15 +175,15 @@ export class MergeCommand {
             ];
 
             for (const p of possiblePaths) {
-                if (fs.existsSync(p)) {
+                if (fsUtils.fileExists(p)) {
                     return p;
                 }
             }
 
             // fallback to using 'which code'
             try {
                 const resolved = execSync('which code', { stdio: ['pipe', 'pipe', 'ignore'] }).toString().trim();
-                if (resolved && fs.existsSync(resolved)) return resolved;
+                if (resolved && fsUtils.fileExists(resolved)) return resolved;
             } catch (err) {
                 vscode.window.showWarningMessage(`Could not resolve 'code' binary via 'which': ${err instanceof Error ? err.message : String(err)}`);
                 return undefined;
@@ -230,4 +237,51 @@ export class MergeCommand {
             });
         });
     }
+
+    private findNewestMergeFiles(localPath: string): { update: string | undefined; base: string | undefined } {
+        const dir = path.dirname(localPath);
+        const fileName = path.basename(localPath);
+
+        let fileNames: string[];
+        try {
+            fileNames = fsUtils.readDirectory(dir);
+        } catch {
+            return { update: undefined, base: undefined };
+        }
+        const updatePrefix = `${fileName}.update@`;
+        const basePrefix = `${fileName}.base@`;
+
+        const update = this.resolveNewestMergeSibling(fileNames, updatePrefix);
+        const base = this.resolveNewestMergeSibling(fileNames, basePrefix);
+
+        if (!update || !base) {
+            return { update: undefined, base: undefined };
+        }
+
+        return {
+            update: update ? path.join(dir, update) : undefined,
+            base: base ? path.join(dir, base) : undefined,
+        };
+    }
+
+    private resolveNewestMergeSibling(fileNames: string[], prefix: string): string | undefined {
+        const matches = fileNames.filter(fileName => fileName.startsWith(prefix));
+        return this.selectNewestMergeSibling(matches);
+    }
+
+    private selectNewestMergeSibling(fileNames: string[]): string | undefined {
+        if (fileNames.length === 0) {
+            return undefined;
+        }
+
+        return fileNames
+            .map(fileName => {
+                const version = semver.valid(extractVersion(fileName));
+                return version ? { fileName, version } : undefined;
+            })
+            .filter((candidate): candidate is { fileName: string; version: string } => candidate !== undefined)
+            .sort((left, right) => semver.rcompare(left.version, right.version))[0]?.fileName;
+    }
+
 }
+

src/views/solution-outline/commands/merge-command.ts

@@ -16,21 +16,23 @@
 
 import { FileItemBuilder } from './solution-outline-file-item';
 import { parseYamlToCTreeItem } from '../../../generic/tree-item-yaml-parser';
-import fs from 'fs';
-import os from 'os';
+import * as manifest from '../../../manifest';
 import path from 'path';
 import { COutlineItem } from './solution-outline-item';
+import { TestDataHandler } from '../../../__test__/test-data';
+import * as fsUtils from '../../../utils/fs-utils';
 
 
 describe('FileItem', () => {
+    const testDataHandler = new TestDataHandler();
     let fileItem: FileItemBuilder;
     let projectDir: string;
     let cSolFile: string;
     let componentNode: COutlineItem;
 
     beforeEach(async () => {
-        const tmpDir = os.tmpdir();
-        projectDir = fs.mkdtempSync(path.join(tmpDir, 'myProject'));
+        testDataHandler.rmTmpDir();
+        projectDir = path.join(testDataHandler.tmpDir, 'myProject');
         cSolFile = `${projectDir}/Blinky.csolution.yml`;
 
         fileItem = new FileItemBuilder();
@@ -39,16 +41,20 @@ describe('FileItem', () => {
         componentNode.setTag('component');
         componentNode.setAttribute('label', 'Component X');
         componentNode.setAttribute('local', 'localPath');
-        componentNode.setAttribute('update', 'updatePath');
-        componentNode.setAttribute('base', 'basePath');
-
     });
 
-    afterEach(() => {
-        fs.rmSync(projectDir, { recursive: true, force: true });
+    afterAll(() => {
+        testDataHandler.dispose();
     });
 
     it('creates file node with merge feature', async () => {
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.c'));
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.c.base@4.0.0'));
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.c.update@5.1.1'));
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.h'));
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.h.base@5.5.1'));
+        fsUtils.writeTextFile(path.join(projectDir, 'RTE', 'CMSIS', 'RTX_Config.h.update@5.5.2'));
+
         const cbuildContent = `
         build:
             components:
@@ -98,9 +104,9 @@ describe('FileItem', () => {
         createdChildren.forEach((child, idx) => {
             expect(child.getTag()).toBe('file');
             expect(child.getAttribute('label')).toContain(fileLabels[idx]);
-            expect(child.getAttribute('update')).toContain('update');
-            expect(child.getAttribute('base')).toContain('base');
             expect(child.getAttribute('description')).toBeUndefined();
+            expect(path.basename(child.getAttribute('resourcePath') ?? '')).toBe(fileLabels[idx]);
+            expect(child.getAttribute('features')).toContain(manifest.MERGE_FILE_CONTEXT);
         });
 
     });

src/views/solution-outline/tree-structure/solution-outline-file-item.test.ts

@@ -93,46 +93,16 @@ export class FileItemBuilder extends SolutionOutlineItemBuilder {
         return item as COutlineItem;
     }
 
-    public addMergeFeature(f: ITreeItem<CTreeItem>, cfileItem: COutlineItem, options?: { skipValidation?: boolean; localPathOverride?: string }) {
-        if (!options?.skipValidation) {
-            const attr = f.getValue('attr');
-            if (attr !== 'config') {
-                return;
-            }
-            const localPath = cfileItem.getValue('resourcePath');
-            if (!localPath) {
-                return;
-            }
-        }
-
-        // use override if provided, else use resourcePath
-        const localPath = options?.localPathOverride ?? cfileItem.getValue('resourcePath');
-
-        const update = f.getValue('update');
-        if (!update) {
-            return;
-        }
-
-        const base = f.getValue('base');
-        if (!base) {
+    public addMergeFeature(f: ITreeItem<CTreeItem>, cfileItem: COutlineItem) {
+        // Only config files with a resolvable resource path are eligible.
+        const attr = f.getValue('attr');
+        if (attr !== 'config') {
             return;
         }
-
         const fileStatus = f.getValue('status');
         if (!fileStatus) {
             return;
         }
-
-        // resolve paths
-        const rootFileName = f.rootFileName;
-        const dir = path.dirname(rootFileName);
-        const updatePath = path.join(dir, update);
-        const basePath = path.join(dir, base);
-
-        // assign merge context
         setMergeFileContext(cfileItem);
-        cfileItem.setAttribute('local', localPath);
-        cfileItem.setAttribute('update', updatePath);
-        cfileItem.setAttribute('base', basePath);
     }
 }