fix: aligned sketch folder validation based on the review request

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

Author: Akos Kitta

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

@@ -182,12 +182,11 @@ export namespace Sketch {
 
   /**
    * `undefined` if the candidate cloud sketch folder name is valid. Otherwise, the validation error message.
-   * Based on how https://create.arduino.cc/editor/ works.
    */
   export function validateCloudSketchFolderName(
     candidate: string
   ): string | undefined {
-    return /^[0-9a-zA-Z_]{1,36}$/.test(candidate)
+    return /^[0-9a-zA-Z]{1}[0-9a-zA-Z_\.-]{0,35}$/.test(candidate)
       ? undefined
       : invalidCloudSketchFolderNameMessage;
   }
@@ -232,7 +231,10 @@ export namespace Sketch {
    */
   export function toValidCloudSketchFolderName(candidate: string): string {
     return candidate
-      ? candidate.replace(/[^0-9a-zA-Z_]/g, defaultFallbackChar).slice(0, 36)
+      ? candidate
+          .replace(/^[^0-9a-zA-Z]{1}/g, defaultFallbackFirstChar)
+          .replace(/[^0-9a-zA-Z_\.-]/g, defaultFallbackChar)
+          .slice(0, 36)
       : defaultSketchFolderName;
   }
 

Diff for: arduino-ide-extension/src/test/browser/create-api.test.ts

@@ -222,6 +222,35 @@ describe('create-api', () => {
     expect(findByName(name, sketches)).to.be.not.undefined;
     expect(findByName(otherName, sketches)).to.be.not.undefined;
   });
+
+  ['.', '-', '_'].map((char) => {
+    it(`should create a new sketch with '${char}' in the sketch folder name although it's disallowed from the Create Editor`, async () => {
+      const name = `sketch${char}`;
+      const posixPath = toPosix(name);
+      const newSketch = await createApi.createSketch(
+        posixPath,
+        'void setup(){} void loop(){}'
+      );
+
+      expect(newSketch).to.be.not.undefined;
+      expect(newSketch.name).to.be.equal(name);
+
+      let sketches = await createApi.sketches();
+      let sketch = findByName(name, sketches);
+      expect(sketch).to.be.not.undefined;
+      // TODO: Cannot do deep equals because the Create API responses with different objects on POST and GET
+      // `"libraries": [null]` vs `"libraries": []`
+      // `"created_at": "2023-02-08T09:39:32.16994555Z"` vs `"created_at": "2023-02-08T09:39:32.169946Z"`
+      // expect(newSketch).to.be.deep.equal(sketch);
+      expect(newSketch.path).to.be.equal(sketch?.path);
+      expect(newSketch.name).to.be.equal(sketch?.name);
+
+      await createApi.deleteSketch(sketch?.path!);
+      sketches = await createApi.sketches();
+      sketch = findByName(name, sketches);
+      expect(sketch).to.be.undefined;
+    });
+  });
 });
 
 // Using environment variables is recommended for testing but you can modify the module too.

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

@@ -43,10 +43,12 @@ describe('sketch', () => {
     (
       [
         ['sketch', true],
-        ['no-dashes', false],
-        ['no-dots', false],
+        ['can-contain-dashes', true],
+        ['can.contain.dots', true],
+        ['-cannot-start-with-dash', false],
+        ['.cannot.start.with.dash', false],
+        ['_cannot_start_with_underscore', false],
         ['No Spaces', false],
-        ['_canStartWithUnderscore', true],
         ['Invalid+Char.ino', false],
         ['', false],
         ['/', false],
@@ -79,6 +81,7 @@ describe('sketch', () => {
         '012345678901234567890123456789012345678901234567890123456789012',
       ],
       ['foo bar', 'foo_bar'],
+      ['_foobar', '0foobar'],
       ['vAlid', 'vAlid'],
     ].map(([input, expected]) =>
       toMapIt(input, expected, Sketch.toValidSketchFolderName)
@@ -102,6 +105,8 @@ describe('sketch', () => {
         '0123456789012345678901234567890123456789012' + epochSuffix,
       ],
       ['foo bar', 'foo_bar' + epochSuffix],
+      ['.foobar', '0foobar' + epochSuffix],
+      ['-fooBar', '0fooBar' + epochSuffix],
       ['vAlid', 'vAlid' + epochSuffix],
     ].map(([input, expected]) =>
       toMapIt(input, expected, (input: string) =>
@@ -113,15 +118,25 @@ describe('sketch', () => {
   describe('toValidCloudSketchFolderName', () => {
     [
       ['sketch', 'sketch'],
-      ['can-contain-slash-and-dot.ino', 'can_contain_slash_and_dot_ino'],
+      [
+        'slash-and-dot-is-ok+but+no+plus.ino',
+        'slash-and-dot-is-ok_but_no_plus.ino',
+      ],
       ['regex++', 'regex__'],
-      ['dots...', 'dots___'],
+      ['dots...', 'dots...'],
+      ['.dots...', '0dots...'],
+      ['-dashes---', '0dashes---'],
+      ['_underscore___', '0underscore___'],
       ['No Spaces', 'No_Spaces'],
-      ['_startsWithUnderscore', '_startsWithUnderscore'],
-      ['Invalid+Char.ino', 'Invalid_Char_ino'],
+      ['_startsWithUnderscore', '0startsWithUnderscore'],
+      ['Invalid+Char.ino', 'Invalid_Char.ino'],
       ['', 'sketch'],
-      ['/', '_'],
-      ['//trash/', '__trash_'],
+      ['/', '0'],
+      [
+        '//////////////////////////////////////-/',
+        '0___________________________________',
+      ],
+      ['//trash/', '0_trash_'],
       [
         '63Length_012345678901234567890123456789012345678901234567890123',
         '63Length_012345678901234567890123456',
@@ -140,6 +155,15 @@ function toMapIt(
 ): Mocha.Test {
   return it(`should map the '${input}' ${
     cloud ? 'cloud ' : ''
-  }sketch folder name to '${expected}'`, () =>
-    expect(testMe(input)).to.be.equal(expected));
+  }sketch folder name to '${expected}'`, () => {
+    const actual = testMe(input);
+    expect(actual).to.be.equal(expected);
+    const errorMessage = Sketch.validateSketchFolderName(actual);
+    try {
+      expect(errorMessage).to.be.undefined;
+    } catch (err) {
+      console.log('HELLO', actual, errorMessage);
+      throw err;
+    }
+  });
 }