Merge pull request #3497 from iclanton/fileerror-relative-path

[node-core-library] Remove the ./ from the relative FileError.
microsoft · Jun 27, 2022 · 4b759f4 · 4b759f4
2 parents fd44f1c + 7e932cf
commit 4b759f4
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 30 deletions.
diff --git a/common/changes/@rushstack/node-core-library/fileerror-relative-path_2022-06-27-00-33.json b/common/changes/@rushstack/node-core-library/fileerror-relative-path_2022-06-27-00-33.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/node-core-library",
+      "comment": "Add a \"trimLeadingDotSlash\" option to the Path.formatConcisely function to not include the leading \"./\" in paths under the baseFolder.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/node-core-library"
+}
diff --git a/common/changes/@rushstack/node-core-library/fileerror-relative-path_2022-06-27-00-34.json b/common/changes/@rushstack/node-core-library/fileerror-relative-path_2022-06-27-00-34.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/node-core-library",
+      "comment": "Change the FileError relative path output to not include the leading \"./\"",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/node-core-library"
+}
diff --git a/common/reviews/api/node-core-library.api.md b/common/reviews/api/node-core-library.api.md
@@ -574,6 +574,7 @@ export interface IParsedPackageNameOrError extends IParsedPackageName {
 export interface IPathFormatConciselyOptions {
     baseFolder: string;
     pathToConvert: string;
+    trimLeadingDotSlash?: boolean;
 }
 
 // @public

diff --git a/libraries/node-core-library/src/Path.ts b/libraries/node-core-library/src/Path.ts
@@ -55,10 +55,16 @@ export interface IPathFormatConciselyOptions {
    * The path to be converted.
    */
   pathToConvert: string;
+
   /**
    * The base path to use when converting `pathToConvert` to a relative path.
    */
   baseFolder: string;
+
+  /**
+   * If set to true, don't include the leading `./` if the path is under the base folder.
+   */
+  trimLeadingDotSlash?: boolean;
 }
 
 /**
@@ -122,7 +128,8 @@ export class Path {
   /**
    * Formats a path to look nice for reporting purposes.
    * @remarks
-   * If `pathToConvert` is under the `baseFolder`, then it will be converted to a relative with the `./` prefix.
+   * If `pathToConvert` is under the `baseFolder`, then it will be converted to a relative with the `./` prefix
+   * unless the {@link IPathFormatConciselyOptions.trimLeadingDotSlash} option is set to `true`.
    * Otherwise, it will be converted to an absolute path.
    *
    * Backslashes will be converted to slashes, unless the path starts with an OS-specific string like `C:\`.
@@ -134,7 +141,14 @@ export class Path {
 
     if (isUnderOrEqual) {
       // Note that isUnderOrEqual()'s relativePath is the reverse direction
-      return './' + Path.convertToSlashes(path.relative(options.baseFolder, options.pathToConvert));
+      const convertedPath: string = Path.convertToSlashes(
+        path.relative(options.baseFolder, options.pathToConvert)
+      );
+      if (options.trimLeadingDotSlash) {
+        return convertedPath;
+      } else {
+        return `./${convertedPath}`;
+      }
     }
 
     const absolutePath: string = path.resolve(options.pathToConvert);
@@ -157,7 +171,8 @@ export class Path {
     const filePath: string = baseFolder
       ? Path.formatConcisely({
           pathToConvert: pathToFormat,
-          baseFolder
+          baseFolder,
+          trimLeadingDotSlash: true
         })
       : path.resolve(pathToFormat);
 

diff --git a/libraries/node-core-library/src/test/FileError.test.ts b/libraries/node-core-library/src/test/FileError.test.ts
@@ -28,7 +28,7 @@ describe(FileError.name, () => {
       absolutePath: `/path/to/project/path/to/file`,
       projectFolder: '/path/to/project'
     });
-    expect(error1.toString()).toEqual('./path/to/file - message');
+    expect(error1.toString()).toMatchInlineSnapshot(`"path/to/file - message"`);
   });
 
   it('correctly performs Unix-style relative file path formatting', () => {
@@ -38,28 +38,36 @@ describe(FileError.name, () => {
       line: 5,
       column: 12
     });
-    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toEqual('./path/to/file:5:12 - message');
+    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"path/to/file:5:12 - message"`
+    );
 
     const error2: FileError = new FileError('message', {
       absolutePath: '/path/to/project/path/to/file',
       projectFolder: '/path/to/project',
       line: 5
     });
-    expect(error2.getFormattedErrorMessage({ format: 'Unix' })).toEqual('./path/to/file:5 - message');
+    expect(error2.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"path/to/file:5 - message"`
+    );
 
     const error3: FileError = new FileError('message', {
       absolutePath: '/path/to/project/path/to/file',
       projectFolder: '/path/to/project',
       line: undefined,
       column: 12
     });
-    expect(error3.getFormattedErrorMessage({ format: 'Unix' })).toEqual('./path/to/file - message');
+    expect(error3.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"path/to/file - message"`
+    );
 
     const error4: FileError = new FileError('message', {
       absolutePath: '/path/to/project/path/to/file',
       projectFolder: '/path/to/project'
     });
-    expect(error4.getFormattedErrorMessage({ format: 'Unix' })).toEqual('./path/to/file - message');
+    expect(error4.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"path/to/file - message"`
+    );
   });
 
   it('correctly performs Unix-style file absolute path formatting', () => {
@@ -120,17 +128,17 @@ describe(FileError.name, () => {
       line: 5,
       column: 12
     });
-    expect(error1.getFormattedErrorMessage({ format: 'VisualStudio' })).toEqual(
-      './path/to/file(5,12) - message'
+    expect(error1.getFormattedErrorMessage({ format: 'VisualStudio' })).toMatchInlineSnapshot(
+      `"path/to/file(5,12) - message"`
     );
 
     const error2: FileError = new FileError('message', {
       absolutePath: '/path/to/project/path/to/file',
       projectFolder: '/path/to/project',
       line: 5
     });
-    expect(error2.getFormattedErrorMessage({ format: 'VisualStudio' })).toEqual(
-      './path/to/file(5) - message'
+    expect(error2.getFormattedErrorMessage({ format: 'VisualStudio' })).toMatchInlineSnapshot(
+      `"path/to/file(5) - message"`
     );
 
     const error3: FileError = new FileError('message', {
@@ -139,13 +147,17 @@ describe(FileError.name, () => {
       line: undefined,
       column: 12
     });
-    expect(error3.getFormattedErrorMessage({ format: 'VisualStudio' })).toEqual('./path/to/file - message');
+    expect(error3.getFormattedErrorMessage({ format: 'VisualStudio' })).toMatchInlineSnapshot(
+      `"path/to/file - message"`
+    );
 
     const error4: FileError = new FileError('message', {
       absolutePath: '/path/to/project/path/to/file',
       projectFolder: '/path/to/project'
     });
-    expect(error4.getFormattedErrorMessage({ format: 'VisualStudio' })).toEqual('./path/to/file - message');
+    expect(error4.getFormattedErrorMessage({ format: 'VisualStudio' })).toMatchInlineSnapshot(
+      `"path/to/file - message"`
+    );
   });
 
   it('correctly performs Visual Studio-style absolute file path formatting', () => {
@@ -225,8 +237,8 @@ describe(`${FileError.name} using arbitrary base folder`, () => {
       line: 5,
       column: 12
     });
-    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toEqual(
-      './to/project/path/to/file:5:12 - message'
+    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"to/project/path/to/file:5:12 - message"`
     );
   });
 });
@@ -256,7 +268,9 @@ describe(`${FileError.name} using PROJECT_FOLDER base folder`, () => {
       line: 5,
       column: 12
     });
-    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toEqual('./path/to/file:5:12 - message');
+    expect(error1.getFormattedErrorMessage({ format: 'Unix' })).toMatchInlineSnapshot(
+      `"path/to/file:5:12 - message"`
+    );
   });
 });
 

diff --git a/libraries/node-core-library/src/test/Path.test.ts b/libraries/node-core-library/src/test/Path.test.ts
@@ -89,21 +89,67 @@ describe(Path.name, () => {
     });
   });
   describe(Path.formatConcisely.name, () => {
-    test('tests', () => {
-      expect(
-        Path.formatConcisely({ pathToConvert: '/folder1/folder2/folder3', baseFolder: '/folder1' })
-      ).toEqual('./folder2/folder3');
-      expect(
-        path.isAbsolute(
-          Path.formatConcisely({ pathToConvert: '/folder1/folder2/folder3', baseFolder: '/folder4' })
-        )
-      ).toBe(true);
-      expect(
-        Path.formatConcisely({
+    describe('With trimLeadingDotSlash unset', () => {
+      it('Formats a path under a base folder', () => {
+        const result: string = Path.formatConcisely({
+          pathToConvert: '/folder1/folder2/folder3',
+          baseFolder: '/folder1'
+        });
+        expect(result).toMatchInlineSnapshot(`"./folder2/folder3"`);
+        expect(path.isAbsolute(result)).toBe(false);
+      });
+
+      it('Formats a path not under the base folder', () => {
+        const result: string = Path.formatConcisely({
+          pathToConvert: '/folder1/folder2/folder3',
+          baseFolder: '/folder4'
+        });
+        // We can't do a snapshot test here because the result is OS-specific
+        // expect(result).toMatchInlineSnapshot();
+        expect(path.isAbsolute(result)).toBe(true);
+      });
+
+      it('Formats a path containing a ".." under a base folder', () => {
+        const result: string = Path.formatConcisely({
           pathToConvert: '/folder1/folder2/folder3/folder4/../file.txt',
           baseFolder: '/folder1/folder2/folder3'
-        })
-      ).toEqual('./file.txt');
+        });
+        expect(result).toMatchInlineSnapshot(`"./file.txt"`);
+        expect(path.isAbsolute(result)).toBe(false);
+      });
+    });
+
+    describe('With trimLeadingDotSlash set to true', () => {
+      it('Formats a path under a base folder', () => {
+        const result: string = Path.formatConcisely({
+          pathToConvert: '/folder1/folder2/folder3',
+          baseFolder: '/folder1',
+          trimLeadingDotSlash: true
+        });
+        expect(result).toMatchInlineSnapshot(`"folder2/folder3"`);
+        expect(path.isAbsolute(result)).toBe(false);
+      });
+
+      it('Formats a path not under the base folder', () => {
+        const result: string = Path.formatConcisely({
+          pathToConvert: '/folder1/folder2/folder3',
+          baseFolder: '/folder4',
+          trimLeadingDotSlash: true
+        });
+        // We can't do a snapshot test here because the result is OS-specific
+        // expect(result).toMatchInlineSnapshot();
+        expect(path.isAbsolute(result)).toBe(true);
+      });
+
+      it('Formats a path containing a ".." under a base folder', () => {
+        const result: string = Path.formatConcisely({
+          pathToConvert: '/folder1/folder2/folder3/folder4/../file.txt',
+          baseFolder: '/folder1/folder2/folder3',
+          trimLeadingDotSlash: true
+        });
+        expect(result).toMatchInlineSnapshot(`"file.txt"`);
+        expect(path.isAbsolute(result)).toBe(false);
+      });
     });
   });
 });