fixup config file

dmichon-msft · dmichon-msft · commit 233e6570e4e4 · 2025-03-05T23:18:04.000Z
diff --git a/common/reviews/api/heft-config-file.api.md b/common/reviews/api/heft-config-file.api.md
@@ -20,12 +20,12 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     static _formatPathForLogging: (path: string) => string;
     getObjectSourceFilePath<TObject extends object>(obj: TObject): string | undefined;
     getPropertyOriginalValue<TParentProperty extends object, TValue>(options: IOriginalValueOptions<TParentProperty>): TValue | undefined;
-    // Warning: (ae-forgotten-export) The symbol "IOnFileNotFoundCallback" needs to be exported by the entry point index.d.ts
+    // Warning: (ae-forgotten-export) The symbol "IOnConfigurationFileNotFoundCallback" needs to be exported by the entry point index.d.ts
     //
     // (undocumented)
-    protected _loadConfigurationFileInnerWithCache(terminal: ITerminal, resolvedConfigurationFilePath: string, projectFolderPath: string | undefined, onFileNotFound?: IOnFileNotFoundCallback): TConfigurationFile;
+    protected _loadConfigurationFileInnerWithCache(terminal: ITerminal, resolvedConfigurationFilePath: string, projectFolderPath: string | undefined, onConfigurationFileNotFound?: IOnConfigurationFileNotFoundCallback): TConfigurationFile;
     // (undocumented)
-    protected _loadConfigurationFileInnerWithCacheAsync(terminal: ITerminal, resolvedConfigurationFilePath: string, projectFolderPath: string | undefined, onFileNotFound?: IOnFileNotFoundCallback): Promise<TConfigurationFile>;
+    protected _loadConfigurationFileInnerWithCacheAsync(terminal: ITerminal, resolvedConfigurationFilePath: string, projectFolderPath: string | undefined, onFileNotFound?: IOnConfigurationFileNotFoundCallback): Promise<TConfigurationFile>;
 }
 
 // @beta
diff --git a/libraries/heft-config-file/src/ConfigurationFileBase.ts b/libraries/heft-config-file/src/ConfigurationFileBase.ts
@@ -289,8 +289,10 @@ export interface IJsonPathsMetadata<TConfigurationFile> {
 
 /**
  * A function to invoke after schema validation to validate the configuration file.
- * This function log errors using the terminal and return false if the configuration file
- * is invalid.
+ * If this function returns any value other than `true`, the configuration file API
+ * will throw an error indicating that custom validation failed. If the function wishes
+ * to provide its own error message, it may use any combination of the terminal and throwing
+ * its own error.
  * @beta
  */
 export type CustomValidationFunction<TConfigurationFile> = (
@@ -324,7 +326,8 @@ export interface IConfigurationFileOptionsBase<TConfigurationFile> {
    * Use this property if you need to validate the configuration file in ways beyond what JSON schema can handle.
    * This function will be invoked after JSON schema validation.
    *
-   * The function should throw if the configuration file is invalid.
+   * If the file is valid, this function should return `true`, otherwise `ConfigurationFile` will throw an error
+   * indicating that custom validation failed. To suppress this error, the function may itself choose to throw.
    */
   customValidationFunction?: CustomValidationFunction<TConfigurationFile>;
 }
@@ -388,7 +391,12 @@ interface IConfigurationFileCacheEntry<TConfigFile> {
   configurationFile: TConfigFile & IConfigurationJson;
 }
 
-export type IOnFileNotFoundCallback = (resolvedConfigurationFilePathForLogging: string) => string | undefined;
+/**
+ * Callback that returns a fallback configuration file path if the original configuration file was not found.
+ */
+export type IOnConfigurationFileNotFoundCallback = (
+  resolvedConfigurationFilePathForLogging: string
+) => string | undefined;
 
 /**
  * @beta
@@ -469,15 +477,15 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     projectFolderPath: string | undefined,
-    onFileNotFound?: IOnFileNotFoundCallback
+    onConfigurationFileNotFound?: IOnConfigurationFileNotFoundCallback
   ): TConfigurationFile {
     const visitedConfigurationFilePaths: Set<string> = new Set<string>();
     const cacheEntry: IConfigurationFileCacheEntry<TConfigurationFile> =
       this._loadConfigurationFileEntryWithCache(
         terminal,
         resolvedConfigurationFilePath,
         visitedConfigurationFilePaths,
-        onFileNotFound
+        onConfigurationFileNotFound
       );
 
     const result: TConfigurationFile = this._finalizeConfigurationFile(
@@ -493,7 +501,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     projectFolderPath: string | undefined,
-    onFileNotFound?: IOnFileNotFoundCallback
+    onFileNotFound?: IOnConfigurationFileNotFoundCallback
   ): Promise<TConfigurationFile> {
     const visitedConfigurationFilePaths: Set<string> = new Set<string>();
     const cacheEntry: IConfigurationFileCacheEntry<TConfigurationFile> =
@@ -517,7 +525,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     visitedConfigurationFilePaths: Set<string>,
-    onFileNotFound?: IOnFileNotFoundCallback
+    onFileNotFound?: IOnConfigurationFileNotFoundCallback
   ): IConfigurationFileCacheEntry<TConfigurationFile> {
     if (visitedConfigurationFilePaths.has(resolvedConfigurationFilePath)) {
       const resolvedConfigurationFilePathForLogging: string = ConfigurationFileBase._formatPathForLogging(
@@ -550,7 +558,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     visitedConfigurationFilePaths: Set<string>,
-    onFileNotFound?: IOnFileNotFoundCallback
+    onConfigurationFileNotFound?: IOnConfigurationFileNotFoundCallback
   ): Promise<IConfigurationFileCacheEntry<TConfigurationFile>> {
     if (visitedConfigurationFilePaths.has(resolvedConfigurationFilePath)) {
       const resolvedConfigurationFilePathForLogging: string = ConfigurationFileBase._formatPathForLogging(
@@ -570,7 +578,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
         terminal,
         resolvedConfigurationFilePath,
         visitedConfigurationFilePaths,
-        onFileNotFound
+        onConfigurationFileNotFound
       ).then((value: IConfigurationFileCacheEntry<TConfigurationFile>) => {
         this._configCache.set(resolvedConfigurationFilePath, value);
         return value;
@@ -595,7 +603,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     try {
       configurationJson = JsonFile.parseString(fileText);
     } catch (e) {
-      throw new Error(`In config file "${resolvedConfigurationFilePathForLogging}": ${e}`);
+      throw new Error(`In configuration file "${resolvedConfigurationFilePathForLogging}": ${e}`);
     }
 
     return configurationJson;
@@ -699,12 +707,15 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     }
 
     if (
-      this._customValidationFunction?.(
+      this._customValidationFunction &&
+      !this._customValidationFunction(
         result as TConfigurationFile,
         resolvedConfigurationFilePathForLogging,
         terminal
-      ) === false
+      )
     ) {
+      // To suppress this error, the function may throw its own error, such as an AlreadyReportedError if it already
+      // logged to the terminal.
       throw new Error(
         `Resolved configuration file at "${resolvedConfigurationFilePathForLogging}" failed custom validation.`
       );
@@ -721,7 +732,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     visitedConfigurationFilePaths: Set<string>,
-    fileNotFoundFallback?: IOnFileNotFoundCallback
+    fileNotFoundFallback?: IOnConfigurationFileNotFoundCallback
   ): IConfigurationFileCacheEntry<TConfigurationFile> {
     const resolvedConfigurationFilePathForLogging: string = ConfigurationFileBase._formatPathForLogging(
       resolvedConfigurationFilePath
@@ -736,11 +747,18 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
           resolvedConfigurationFilePathForLogging
         );
         if (fallbackPath) {
-          return this._loadConfigurationFileEntryWithCache(
-            terminal,
-            fallbackPath,
-            visitedConfigurationFilePaths
-          );
+          try {
+            return this._loadConfigurationFileEntryWithCache(
+              terminal,
+              fallbackPath,
+              visitedConfigurationFilePaths
+            );
+          } catch (fallbackError) {
+            if (!FileSystem.isNotExistError(fallbackError as Error)) {
+              throw fallbackError;
+            }
+            // Otherwise report the missing original file.
+          }
         }
 
         terminal.writeDebugLine(`Configuration file "${resolvedConfigurationFilePathForLogging}" not found.`);
@@ -794,26 +812,33 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     terminal: ITerminal,
     resolvedConfigurationFilePath: string,
     visitedConfigurationFilePaths: Set<string>,
-    fileNotFoundFallback?: IOnFileNotFoundCallback
+    fileNotFoundFallback?: IOnConfigurationFileNotFoundCallback
   ): Promise<IConfigurationFileCacheEntry<TConfigurationFile>> {
     const resolvedConfigurationFilePathForLogging: string = ConfigurationFileBase._formatPathForLogging(
       resolvedConfigurationFilePath
     );
 
     let fileText: string;
     try {
-      fileText = FileSystem.readFile(resolvedConfigurationFilePath);
+      fileText = await FileSystem.readFileAsync(resolvedConfigurationFilePath);
     } catch (e) {
       if (FileSystem.isNotExistError(e as Error)) {
         const fallbackPath: string | undefined = fileNotFoundFallback?.(
           resolvedConfigurationFilePathForLogging
         );
         if (fallbackPath) {
-          return this._loadConfigurationFileEntryWithCacheAsync(
-            terminal,
-            fallbackPath,
-            visitedConfigurationFilePaths
-          );
+          try {
+            return await this._loadConfigurationFileEntryWithCacheAsync(
+              terminal,
+              fallbackPath,
+              visitedConfigurationFilePaths
+            );
+          } catch (fallbackError) {
+            if (!FileSystem.isNotExistError(fallbackError as Error)) {
+              throw fallbackError;
+            }
+            // Otherwise report the missing original file.
+          }
         }
 
         terminal.writeDebugLine(`Configuration file "${resolvedConfigurationFilePathForLogging}" not found.`);
@@ -830,7 +855,7 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     let parentConfiguration: IConfigurationFileCacheEntry<TConfigurationFile> | undefined;
     if (configurationJson.extends) {
       try {
-        const resolvedParentConfigPath: string = Import.resolveModule({
+        const resolvedParentConfigPath: string = await Import.resolveModuleAsync({
           modulePath: configurationJson.extends,
           baseFolderPath: nodeJsPath.dirname(resolvedConfigurationFilePath)
         });
@@ -974,11 +999,13 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
     // contain inheritance type annotation keys, or other built-in properties that we ignore
     // (eg. "extends", "$schema").
     const currentObjectPropertyNames: Set<string> = new Set(Object.keys(currentObject));
-    // An array of property names that should be included in the resulting object.
-    const filteredObjectPropertyNames: (keyof TField)[] = [];
     // A map of property names to their inheritance type.
     const inheritanceTypeMap: Map<keyof TField, IPropertyInheritance<InheritanceType>> = new Map();
 
+    // The set of property names that should be included in the resulting object
+    // All names from the parent are assumed to already be filtered.
+    const mergedPropertyNames: Set<keyof TField> = new Set(Object.keys(parentObject));
+
     // Do a first pass to gather and strip the inheritance type annotations from the merging object.
     for (const propertyName of currentObjectPropertyNames) {
       if (ignoreProperties && ignoreProperties.has(propertyName)) {
@@ -1032,18 +1059,12 @@ export abstract class ConfigurationFileBase<TConfigurationFile, TExtraOptions ex
             );
         }
       } else {
-        filteredObjectPropertyNames.push(propertyName);
+        mergedPropertyNames.add(propertyName);
       }
     }
 
-    // We only filter the currentObject because the parent object should already be filtered
-    const propertyNames: Set<keyof TField> = new Set([
-      ...Object.keys(parentObject),
-      ...filteredObjectPropertyNames
-    ]);
-
     // Cycle through properties and merge them
-    for (const propertyName of propertyNames) {
+    for (const propertyName of mergedPropertyNames) {
       const propertyValue: TField[keyof TField] | undefined = currentObject[propertyName];
       const parentPropertyValue: TField[keyof TField] | undefined = parentObject[propertyName];
 
diff --git a/libraries/heft-config-file/src/ProjectConfigurationFile.ts b/libraries/heft-config-file/src/ProjectConfigurationFile.ts
@@ -8,7 +8,7 @@ import type { IRigConfig } from '@rushstack/rig-package';
 
 import {
   ConfigurationFileBase,
-  type IOnFileNotFoundCallback,
+  type IOnConfigurationFileNotFoundCallback,
   type IConfigurationFileOptions
 } from './ConfigurationFileBase';
 
@@ -129,13 +129,13 @@ export class ProjectConfigurationFile<TConfigurationFile> extends ConfigurationF
   private _getRigConfigFallback(
     terminal: ITerminal,
     rigConfig: IRigConfig | undefined
-  ): IOnFileNotFoundCallback | undefined {
+  ): IOnConfigurationFileNotFoundCallback | undefined {
     return rigConfig
       ? (resolvedConfigurationFilePathForLogging: string) => {
           if (rigConfig.rigFound) {
             const rigProfileFolder: string = rigConfig.getResolvedProfileFolder();
             terminal.writeDebugLine(
-              `Config file "${resolvedConfigurationFilePathForLogging}" does not exist. Attempting to load via rig ("${ConfigurationFileBase._formatPathForLogging(rigProfileFolder)}").`
+              `Configuration file "${resolvedConfigurationFilePathForLogging}" does not exist. Attempting to load via rig ("${ConfigurationFileBase._formatPathForLogging(rigProfileFolder)}").`
             );
             return nodeJsPath.resolve(rigProfileFolder, this.projectRelativeFilePath);
           } else {
diff --git a/libraries/heft-config-file/src/test/ConfigurationFile.test.ts b/libraries/heft-config-file/src/test/ConfigurationFile.test.ts
@@ -1826,7 +1826,7 @@ describe('ConfigurationFile', () => {
       // a newline on Windows, and a curly brace on other platforms, even though the location is
       // accurate in both cases. Use a regex to match either.
       expect(() => configFileLoader.loadConfigurationFileForProject(terminal, __dirname)).toThrowError(
-        /In config file "<project root>\/lib\/test\/errorCases\/invalidJson\/config.json": SyntaxError: Unexpected token '(}|\\n)' at 2:19/
+        /In configuration file "<project root>\/lib\/test\/errorCases\/invalidJson\/config.json": SyntaxError: Unexpected token '(}|\\n)' at 2:19/
       );
 
       jest.restoreAllMocks();
@@ -1861,7 +1861,7 @@ describe('ConfigurationFile', () => {
       await expect(
         configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname)
       ).rejects.toThrowError(
-        /In config file "<project root>\/lib\/test\/errorCases\/invalidJson\/config.json": SyntaxError: Unexpected token '(}|\\n)' at 2:19/
+        /In configuration file "<project root>\/lib\/test\/errorCases\/invalidJson\/config.json": SyntaxError: Unexpected token '(}|\\n)' at 2:19/
       );
 
       jest.restoreAllMocks();
diff --git a/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap b/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap
