fix: sketch Save As errors on name collision

The Save As operation is halted and the problem clearly communicated to
the user when there is a collision between the target primary source
file name and existing secondary source files of the sketch.

Closes #827

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

Diff for: arduino-ide-extension/src/browser/contributions/save-as-sketch.ts

@@ -3,10 +3,12 @@ import { NavigatableWidget } from '@theia/core/lib/browser/navigatable';
 import { Saveable } from '@theia/core/lib/browser/saveable';
 import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
 import { WindowService } from '@theia/core/lib/browser/window/window-service';
+import { ApplicationError } from '@theia/core/lib/common/application-error';
 import { nls } from '@theia/core/lib/common/nls';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { EditorManager } from '@theia/editor/lib/browser/editor-manager';
 import { WorkspaceInput } from '@theia/workspace/lib/browser/workspace-service';
+import { SketchesError } from '../../common/protocol';
 import { StartupTasks } from '../../electron-common/startup-task';
 import { ArduinoMenus } from '../menu/arduino-menus';
 import { CurrentSketch } from '../sketches-service-client-impl';
@@ -35,7 +37,29 @@ export class SaveAsSketch extends CloudSketchContribution {
 
   override registerCommands(registry: CommandRegistry): void {
     registry.registerCommand(SaveAsSketch.Commands.SAVE_AS_SKETCH, {
-      execute: (args) => this.saveAs(args),
+      execute: async (args) => {
+        try {
+          return await this.saveAs(args);
+        } catch (err) {
+          let message = String(err);
+          if (ApplicationError.is(err)) {
+            if (SketchesError.SketchAlreadyContainsThisFile.is(err)) {
+              message = nls.localize(
+                'arduino/sketch/sketchAlreadyContainsThisFileMessage',
+                'Failed to save sketch "{0}" as "{1}". {2}',
+                err.data.sourceSketchName,
+                err.data.targetSketchName,
+                err.message
+              );
+            } else {
+              message = err.message;
+            }
+          } else if (err instanceof Error) {
+            message = err.message;
+          }
+          this.messageService.error(message);
+        }
+      },
     });
   }
 
@@ -58,13 +82,14 @@ export class SaveAsSketch extends CloudSketchContribution {
    * Resolves `true` if the sketch was successfully saved as something.
    */
   private async saveAs(
-    {
+    params = SaveAsSketch.Options.DEFAULT
+  ): Promise<boolean> {
+    const {
       execOnlyIfTemp,
       openAfterMove,
       wipeOriginal,
       markAsRecentlyOpened,
-    }: SaveAsSketch.Options = SaveAsSketch.Options.DEFAULT
-  ): Promise<boolean> {
+    } = params;
     assertConnectedToBackend({
       connectionStatusService: this.connectionStatusService,
       messageService: this.messageService,

Diff for: arduino-ide-extension/src/common/protocol/sketches-service.ts

@@ -9,6 +9,7 @@ export namespace SketchesError {
     NotFound: 5001,
     InvalidName: 5002,
     InvalidFolderName: 5003,
+    SketchAlreadyContainsThisFile: 5004,
   };
   export const NotFound = ApplicationError.declare(
     Codes.NotFound,
@@ -37,6 +38,21 @@ export namespace SketchesError {
       };
     }
   );
+  // https://github.com/arduino/arduino-ide/issues/827
+  export const SketchAlreadyContainsThisFile = ApplicationError.declare(
+    Codes.SketchAlreadyContainsThisFile,
+    (
+      message: string,
+      sourceSketchName: string,
+      targetSketchName: string,
+      existingSketchFilename: string
+    ) => {
+      return {
+        message,
+        data: { sourceSketchName, targetSketchName, existingSketchFilename },
+      };
+    }
+  );
 }
 
 export const SketchesServicePath = '/services/sketches-service';

Diff for: arduino-ide-extension/src/node/sketches-service-impl.ts

@@ -530,6 +530,35 @@ export class SketchesServiceImpl
       throw SketchesError.InvalidFolderName(message, destinationFolderBasename);
     }
 
+    // verify a possible name collision with existing ino files
+    if (sourceFolderBasename !== destinationFolderBasename) {
+      try {
+        const otherInoBasename = `${destinationFolderBasename}.ino`;
+        const otherInoPath = join(source, otherInoBasename);
+        const stat = await fs.stat(otherInoPath);
+        if (stat.isFile()) {
+          const message = nls.localize(
+            'arduino/sketch/sketchAlreadyContainsThisFileError',
+            "The sketch already contains a file named '{0}'",
+            otherInoBasename
+          );
+          // if can read the file, it will be a collision
+          throw SketchesError.SketchAlreadyContainsThisFile(
+            message,
+            sourceFolderBasename,
+            destinationFolderBasename,
+            otherInoBasename
+          );
+        }
+        // Otherwise, it's OK, if it is an existing directory
+      } catch (err) {
+        // It's OK if the file is missing.
+        if (!ErrnoException.isENOENT(err)) {
+          throw err;
+        }
+      }
+    }
+
     let filter: CopyOptions['filter'];
     if (onlySketchFiles) {
       // The Windows paths, can be a trash (see below). Hence, it must be resolved with Node.js.

Diff for: arduino-ide-extension/src/test/node/sketches-service-impl.slow-test.ts

@@ -158,6 +158,37 @@ describe('sketches-service-impl', () => {
       );
     });
 
+    it('should error when the destination sketch folder name collides with an existing sketch file name', async () => {
+      const sketchesService =
+        container.get<SketchesServiceImpl>(SketchesService);
+      const tempDirPath = await sketchesService['createTempFolder']();
+      const destinationPath = join(tempDirPath, 'ExistingSketchFile');
+      let sketch = await sketchesService.createNewSketch();
+      toDispose.push(disposeSketch(sketch));
+      const sourcePath = FileUri.fsPath(sketch.uri);
+      const otherInoBasename = 'ExistingSketchFile.ino';
+      const otherInoPath = join(sourcePath, otherInoBasename);
+      await fs.writeFile(otherInoPath, '// a comment', { encoding: 'utf8' });
+
+      sketch = await sketchesService.loadSketch(sketch.uri);
+      expect(Sketch.isInSketch(FileUri.create(otherInoPath), sketch)).to.be
+        .true;
+
+      await rejects(
+        sketchesService.copy(sketch, {
+          destinationUri: FileUri.create(destinationPath).toString(),
+        }),
+        (reason) => {
+          return (
+            typeof reason === 'object' &&
+            reason !== null &&
+            SketchesError.SketchAlreadyContainsThisFile.is(reason) &&
+            reason.data.existingSketchFilename === otherInoBasename
+          );
+        }
+      );
+    });
+
     it('should copy a sketch when the destination does not exist', async () => {
       const sketchesService =
         container.get<SketchesServiceImpl>(SketchesService);

Diff for: i18n/en.json

@@ -460,6 +460,8 @@
       "saveSketchAs": "Save sketch folder as...",
       "showFolder": "Show Sketch Folder",
       "sketch": "Sketch",
+      "sketchAlreadyContainsThisFileError": "The sketch already contains a file named '{0}'",
+      "sketchAlreadyContainsThisFileMessage": "Failed to save sketch \"{0}\" as \"{1}\". {2}",
       "sketchbook": "Sketchbook",
       "titleLocalSketchbook": "Local Sketchbook",
       "titleSketchbook": "Sketchbook",