From 6310c9c377026d4e0598ecd213c2c9a8d299fc88 Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Mon, 14 Mar 2022 15:36:05 -0700 Subject: [PATCH 1/8] Add inline-specified inheritance type handling to heft-config-file --- common/reviews/api/heft-config-file.api.md | 3 +- .../heft-config-file/src/ConfigurationFile.ts | 385 ++++++++++++------ .../src/test/ConfigurationFile.test.ts | 273 +++++++++++++ .../ConfigurationFile.test.ts.snap | 82 +++- .../mergeBehaviorConfigFile.schema.json | 152 +++++++ .../mergeBehaviorConfigFileA.json | 34 ++ .../mergeBehaviorConfigFileB.json | 43 ++ .../badMergeBehaviorConfigFileA.json | 12 + .../badMergeBehaviorConfigFileB.json | 10 + .../badMergeBehaviorConfigFileC.json | 8 + .../badMergeBehaviorConfigFileD.json | 7 + .../badMergeBehaviorConfigFileE.json | 12 + .../simpleMergeBehaviorConfigFile.schema.json | 85 ++++ .../simpleMergeBehaviorConfigFileA.json | 23 ++ .../simpleMergeBehaviorConfigFileB.json | 24 ++ 15 files changed, 1024 insertions(+), 129 deletions(-) create mode 100644 libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json create mode 100644 libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json create mode 100644 libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json create mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json diff --git a/common/reviews/api/heft-config-file.api.md b/common/reviews/api/heft-config-file.api.md index d1321f82cf..84fe90e166 100644 --- a/common/reviews/api/heft-config-file.api.md +++ b/common/reviews/api/heft-config-file.api.md @@ -48,6 +48,7 @@ export interface IJsonPathsMetadata { export enum InheritanceType { append = "append", custom = "custom", + merge = "merge", replace = "replace" } @@ -61,7 +62,7 @@ export interface IOriginalValueOptions { // @beta (undocumented) export type IPropertiesInheritance = { - [propertyName in keyof TConfigurationFile]?: IPropertyInheritance | ICustomPropertyInheritance; + [propertyName in keyof TConfigurationFile]?: IPropertyInheritance | ICustomPropertyInheritance; }; // @beta (undocumented) diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index a1edf36693..37e6e79223 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -22,10 +22,17 @@ interface IConfigurationJson { */ export enum InheritanceType { /** - * Append additional elements after elements from the parent file's property + * Append additional elements after elements from the parent file's property. Only applicable + * for arrays. */ append = 'append', + /** + * Perform a shallow merge of additional elements after elements from the parent file's property. + * Only applicable for objects. + */ + merge = 'merge', + /** * Discard elements from the parent file's property */ @@ -63,6 +70,7 @@ export enum PathResolutionMethod { custom } +const CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX: RegExp = /^\$([^\.]+)\.mergeBehavior$/; const CONFIGURATION_FILE_FIELD_ANNOTATION: unique symbol = Symbol('configuration-file-field-annotation'); interface IAnnotatedField { @@ -125,7 +133,7 @@ export interface ICustomPropertyInheritance extends IPropertyInheritanc */ export type IPropertiesInheritance = { [propertyName in keyof TConfigurationFile]?: - | IPropertyInheritance + | IPropertyInheritance | ICustomPropertyInheritance; }; @@ -371,8 +379,6 @@ export class ConfigurationFile { throw new Error(`In config file "${resolvedConfigurationFilePathForLogging}": ${e}`); } - this._schema.validateObject(configurationJson, resolvedConfigurationFilePathForLogging); - this._annotateProperties(resolvedConfigurationFilePath, configurationJson); for (const [jsonPath, metadata] of Object.entries(this._jsonPathMetadata)) { @@ -395,7 +401,7 @@ export class ConfigurationFile { }); } - let parentConfiguration: Partial = {}; + let parentConfiguration: TConfigurationFile | undefined; if (configurationJson.extends) { try { const resolvedParentConfigPath: string = Import.resolveModule({ @@ -420,128 +426,11 @@ export class ConfigurationFile { } } - const propertyNames: Set = new Set([ - ...Object.keys(parentConfiguration), - ...Object.keys(configurationJson) - ]); - - const resultAnnotation: IConfigurationFileFieldAnnotation = { - configurationFilePath: resolvedConfigurationFilePath, - originalValues: {} as TConfigurationFile - }; - const result: TConfigurationFile = { - [CONFIGURATION_FILE_FIELD_ANNOTATION]: resultAnnotation - } as unknown as TConfigurationFile; - for (const propertyName of propertyNames) { - if (propertyName === '$schema' || propertyName === 'extends') { - continue; - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const propertyValue: unknown | undefined = (configurationJson as any)[propertyName]; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const parentPropertyValue: unknown | undefined = (parentConfiguration as any)[propertyName]; - - const bothAreArrays: boolean = Array.isArray(propertyValue) && Array.isArray(parentPropertyValue); - const defaultInheritanceType: IPropertyInheritance = bothAreArrays - ? { inheritanceType: InheritanceType.append } - : { inheritanceType: InheritanceType.replace }; - const propertyInheritance: IPropertyInheritance = - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (this._propertyInheritanceTypes as any)[propertyName] !== undefined - ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - (this._propertyInheritanceTypes as any)[propertyName] - : defaultInheritanceType; - - let newValue: unknown; - const usePropertyValue: () => void = () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ - parentObject: configurationJson, - propertyName: propertyName - }); - newValue = propertyValue; - }; - const useParentPropertyValue: () => void = () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ - parentObject: parentConfiguration, - propertyName: propertyName - }); - newValue = parentPropertyValue; - }; - - if (propertyValue !== undefined && parentPropertyValue === undefined) { - usePropertyValue(); - } else if (parentPropertyValue !== undefined && propertyValue === undefined) { - useParentPropertyValue(); - } else { - switch (propertyInheritance.inheritanceType) { - case InheritanceType.replace: { - if (propertyValue !== undefined) { - usePropertyValue(); - } else { - useParentPropertyValue(); - } - - break; - } - - case InheritanceType.append: { - if (propertyValue !== undefined && parentPropertyValue === undefined) { - usePropertyValue(); - } else if (propertyValue === undefined && parentPropertyValue !== undefined) { - useParentPropertyValue(); - } else { - if (!Array.isArray(propertyValue) || !Array.isArray(parentPropertyValue)) { - throw new Error( - `Issue in processing configuration file property "${propertyName}". ` + - `Property is not an array, but the inheritance type is set as "${InheritanceType.append}"` - ); - } - - newValue = [...parentPropertyValue, ...propertyValue]; - (newValue as unknown as IAnnotatedField)[CONFIGURATION_FILE_FIELD_ANNOTATION] = { - configurationFilePath: undefined, - originalValues: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(parentPropertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(propertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues - } - }; - } - - break; - } - - case InheritanceType.custom: { - const customInheritance: ICustomPropertyInheritance = - propertyInheritance as ICustomPropertyInheritance; - if ( - !customInheritance.inheritanceFunction || - typeof customInheritance.inheritanceFunction !== 'function' - ) { - throw new Error( - 'For property inheritance type "InheritanceType.custom", an inheritanceFunction must be provided.' - ); - } - - newValue = customInheritance.inheritanceFunction(propertyValue, parentPropertyValue); - - break; - } - - default: { - throw new Error(`Unknown inheritance type "${propertyInheritance}"`); - } - } - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (result as any)[propertyName] = newValue; - } - + const result: TConfigurationFile = this._mergeConfigurationFiles( + parentConfiguration, + configurationJson, + resolvedConfigurationFilePath + ); try { this._schema.validateObject(result, resolvedConfigurationFilePathForLogging); } catch (e) { @@ -668,6 +557,248 @@ export class ConfigurationFile { } } + private _mergeConfigurationFiles( + parentConfiguration: TConfigurationFile | undefined, + configurationJson: IConfigurationJson & TConfigurationFile, + resolvedConfigurationFilePath: string + ): TConfigurationFile { + const ignoreProperties: Set = new Set(['extends', '$schema']); + return this._mergeObjects( + parentConfiguration, + configurationJson, + resolvedConfigurationFilePath, + this._propertyInheritanceTypes, + ignoreProperties + ); + } + + private _mergeObjects( + parentObject: TField | undefined, + currentObject: TField | undefined, + resolvedConfigurationFilePath: string, + configuredPropertyInheritance?: IPropertiesInheritance, + ignoreProperties?: Set + ): TField { + const resultAnnotation: IConfigurationFileFieldAnnotation = { + configurationFilePath: resolvedConfigurationFilePath, + originalValues: {} as TField + }; + const result: TField = { + [CONFIGURATION_FILE_FIELD_ANNOTATION]: resultAnnotation + } as unknown as TField; + + // Do a first pass to gather and strip the merge behavior annotations from the merging object + const currentObjectPropertyNames: Set = new Set(Object.keys(currentObject || {})); + const filteredObjectPropertyNames: string[] = []; + const mergeBehaviorMap: Map> = new Map(); + for (const propertyName of currentObjectPropertyNames) { + if (ignoreProperties && ignoreProperties.has(propertyName)) { + continue; + } + const mergeBehaviorMatches: RegExpMatchArray | null = propertyName.match( + CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX + ); + if (mergeBehaviorMatches && mergeBehaviorMatches.length === 2) { + const mergeTargetPropertyName: string = mergeBehaviorMatches[1]; + const mergeBehaviorRaw: unknown | undefined = (currentObject || {})[propertyName]; + if (!currentObjectPropertyNames.has(mergeTargetPropertyName)) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `A merge behavior was provided but no matching property was found` + ); + } else if (typeof mergeBehaviorRaw !== 'string') { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `An unsupported merge behavior was provided: ${JSON.stringify(mergeBehaviorRaw)}` + ); + } else if (typeof (currentObject || {})[mergeTargetPropertyName] !== 'object') { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `A merge behavior was provided for a property that is not an object` + ); + } + switch (mergeBehaviorRaw.toLowerCase()) { + case 'append': + mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.append }); + break; + case 'merge': + mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.merge }); + break; + case 'replace': + mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.replace }); + break; + default: + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `An unsupported merge behavior was provided: "${mergeBehaviorRaw}"` + ); + } + } else { + filteredObjectPropertyNames.push(propertyName); + } + } + + // We only filter the currentObject because the parent object should already be filtered + const propertyNames: Set = new Set([ + ...Object.keys(parentObject || {}), + ...filteredObjectPropertyNames + ]); + + // Cycle through properties and merge them + for (const propertyName of propertyNames) { + const propertyValue: unknown | undefined = (currentObject || {})[propertyName]; + const parentPropertyValue: unknown | undefined = (parentObject || {})[propertyName]; + + // If the property is a merge behavior annotation, use it. Fallback to the configuration file inheritance + // behavior, and if one isn't specified, use the default. + let propertyInheritance: IPropertyInheritance | undefined = + mergeBehaviorMap.get(propertyName); + if (!propertyInheritance) { + const bothAreArrays: boolean = Array.isArray(propertyValue) && Array.isArray(parentPropertyValue); + const defaultInheritanceType: IPropertyInheritance = bothAreArrays + ? { inheritanceType: InheritanceType.append } + : { inheritanceType: InheritanceType.replace }; + propertyInheritance = + // eslint-disable-next-line @typescript-eslint/no-explicit-any + configuredPropertyInheritance && (configuredPropertyInheritance as any)[propertyName] !== undefined + ? // eslint-disable-next-line @typescript-eslint/no-explicit-any + (configuredPropertyInheritance as any)[propertyName] + : defaultInheritanceType; + } + + let newValue: unknown; + const usePropertyValue: () => void = () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ + parentObject: currentObject, + propertyName: propertyName + }); + newValue = propertyValue; + }; + const useParentPropertyValue: () => void = () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ + parentObject: parentObject, + propertyName: propertyName + }); + newValue = parentPropertyValue; + }; + + if (propertyValue !== undefined && parentPropertyValue === undefined) { + usePropertyValue(); + } else if (parentPropertyValue !== undefined && propertyValue === undefined) { + useParentPropertyValue(); + } else { + switch (propertyInheritance!.inheritanceType) { + case InheritanceType.replace: { + if (propertyValue !== undefined) { + usePropertyValue(); + } else { + useParentPropertyValue(); + } + + break; + } + + case InheritanceType.append: { + if (propertyValue !== undefined && parentPropertyValue === undefined) { + usePropertyValue(); + } else if (propertyValue === undefined && parentPropertyValue !== undefined) { + useParentPropertyValue(); + } else { + if (!Array.isArray(propertyValue) || !Array.isArray(parentPropertyValue)) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Property is not an array, but the inheritance type is set as "${InheritanceType.append}"` + ); + } + + newValue = [...parentPropertyValue, ...propertyValue]; + (newValue as unknown as IAnnotatedField)[CONFIGURATION_FILE_FIELD_ANNOTATION] = { + configurationFilePath: undefined, + originalValues: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(parentPropertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(propertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues + } + }; + } + + break; + } + + case InheritanceType.merge: { + if (parentPropertyValue === null || propertyValue === null) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Null values cannot be used when the inheritance type is set as "${InheritanceType.merge}"` + ); + } + + if (propertyValue !== undefined && parentPropertyValue === undefined) { + usePropertyValue(); + } else if (propertyValue === undefined && parentPropertyValue !== undefined) { + useParentPropertyValue(); + } else { + if ( + (propertyValue && typeof propertyValue !== 'object') || + (parentPropertyValue && typeof parentPropertyValue !== 'object') + ) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Primitive types cannot be provided when the inheritance type is set as "${InheritanceType.merge}"` + ); + } + if (Array.isArray(propertyValue) || Array.isArray(parentPropertyValue)) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Property is not a keyed object, but the inheritance type is set as "${InheritanceType.merge}"` + ); + } + + // Recursively merge the parent and child objects. Don't pass the configuredPropertyInheritance or + // ignoreProperties because we are no longer at the top level of the configuration file. + newValue = this._mergeObjects( + parentPropertyValue as object | undefined, + propertyValue as object | undefined, + resolvedConfigurationFilePath + ); + } + + break; + } + + case InheritanceType.custom: { + const customInheritance: ICustomPropertyInheritance = + propertyInheritance as ICustomPropertyInheritance; + if ( + !customInheritance.inheritanceFunction || + typeof customInheritance.inheritanceFunction !== 'function' + ) { + throw new Error( + 'For property inheritance type "InheritanceType.custom", an inheritanceFunction must be provided.' + ); + } + + newValue = customInheritance.inheritanceFunction(propertyValue, parentPropertyValue); + + break; + } + + default: { + throw new Error(`Unknown inheritance type "${propertyInheritance}"`); + } + } + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (result as any)[propertyName] = newValue; + } + + return result; + } + private _getConfigurationFilePathForProject(projectPath: string): string { return nodeJsPath.resolve(projectPath, this.projectRelativeFilePath); } diff --git a/libraries/heft-config-file/src/test/ConfigurationFile.test.ts b/libraries/heft-config-file/src/test/ConfigurationFile.test.ts index 76effa50d2..3e9103bbdd 100644 --- a/libraries/heft-config-file/src/test/ConfigurationFile.test.ts +++ b/libraries/heft-config-file/src/test/ConfigurationFile.test.ts @@ -421,6 +421,279 @@ describe(ConfigurationFile.name, () => { }); }); + describe('a complex file with merge behavior annotations', () => { + interface IMergeBehaviorConfigFile { + a: string; + b: { c: string }[]; + d: { + e: string; + f: string; + g: { h: string }[]; + i: { j: string }[]; + k: { + l: string; + m: { n: string }[]; + z?: string; + }; + o: { + p: { q: string }[]; + }; + r: { + s: string; + }; + y?: { + z: string; + }; + }; + y?: { + z: string; + }; + } + + interface ISimpleMergeBehaviorConfigFile { + a: { b: string }[]; + c: { + d: { e: string }[]; + }; + f: { + g: { h: string }[]; + i: { + j: { k: string }[]; + }; + }; + l: string; + } + + it('Correctly loads a complex config file with merge behavior annotations', async () => { + const projectRelativeFilePath: string = 'mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json'; + const rootConfigFilePath: string = nodeJsPath.resolve( + __dirname, + 'mergeBehaviorConfigFile', + 'mergeBehaviorConfigFileA.json' + ); + const secondConfigFilePath: string = nodeJsPath.resolve( + __dirname, + 'mergeBehaviorConfigFile', + 'mergeBehaviorConfigFileB.json' + ); + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'mergeBehaviorConfigFile', + 'mergeBehaviorConfigFile.schema.json' + ); + + const configFileLoader: ConfigurationFile = + new ConfigurationFile({ + projectRelativeFilePath: projectRelativeFilePath, + jsonSchemaPath: schemaPath + }); + const loadedConfigFile: IMergeBehaviorConfigFile = + await configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname); + const expectedConfigFile: IMergeBehaviorConfigFile = { + a: 'A', + // "$b.mergeBehavior": "append" + b: [{ c: 'A' }, { c: 'B' }], + // "$d.mergeBehavior": "merge" + d: { + e: 'A', + f: 'B', + // "$g.mergeBehavior": "append" + g: [{ h: 'A' }, { h: 'B' }], + // "$i.mergeBehavior": "replace" + i: [{ j: 'B' }], + // "$k.mergeBehavior": "merge" + k: { + l: 'A', + m: [{ n: 'A' }, { n: 'B' }], + z: 'B' + }, + // "$o.mergeBehavior": "replace" + o: { + p: [{ q: 'B' }] + }, + r: { + s: 'A' + }, + y: { + z: 'B' + } + }, + y: { + z: 'B' + } + }; + + expect(JSON.stringify(loadedConfigFile)).toEqual(JSON.stringify(expectedConfigFile)); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.b[0])).toEqual(rootConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.b[1])).toEqual(secondConfigFilePath); + + // loadedConfigFile.d source path is the second config file since it was merged into the first + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.g[0])).toEqual(rootConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.g[1])).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.i[0])).toEqual(secondConfigFilePath); + + // loadedConfigFile.d.k source path is the second config file since it was merged into the first + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.k)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.k.m[0])).toEqual(rootConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.k.m[1])).toEqual( + secondConfigFilePath + ); + + // loadedConfigFile.d.o source path is the second config file since it replaced the first + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.o)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.o.p[0])).toEqual( + secondConfigFilePath + ); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.r)).toEqual(rootConfigFilePath); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.d.y!)).toEqual(secondConfigFilePath); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.y!)).toEqual(secondConfigFilePath); + }); + + it('Correctly loads a complex config file with a single merge behavior annotation', async () => { + const projectRelativeFilePath: string = + 'simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json'; + const rootConfigFilePath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFileA.json' + ); + const secondConfigFilePath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFileB.json' + ); + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + + const configFileLoader: ConfigurationFile = + new ConfigurationFile({ + projectRelativeFilePath: projectRelativeFilePath, + jsonSchemaPath: schemaPath + }); + const loadedConfigFile: ISimpleMergeBehaviorConfigFile = + await configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname); + const expectedConfigFile: ISimpleMergeBehaviorConfigFile = { + a: [{ b: 'A' }, { b: 'B' }], + c: { + d: [{ e: 'B' }] + }, + // "$f.mergeBehavior": "merge" + f: { + g: [{ h: 'A' }, { h: 'B' }], + i: { + j: [{ k: 'B' }] + } + }, + l: 'A' + }; + + expect(JSON.stringify(loadedConfigFile)).toEqual(JSON.stringify(expectedConfigFile)); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.a[0])).toEqual(rootConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.a[1])).toEqual(secondConfigFilePath); + + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.c)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.c.d[0])).toEqual(secondConfigFilePath); + + // loadedConfigFile.f source path is the second config file since it was merged into the first + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.f)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.f.g[0])).toEqual(rootConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.f.g[1])).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.f.i)).toEqual(secondConfigFilePath); + expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.f.i.j[0])).toEqual( + secondConfigFilePath + ); + }); + + it("throws an error when an array uses the 'merge' merge behavior", async () => { + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + const configFileLoader: ConfigurationFile = new ConfigurationFile({ + projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json', + jsonSchemaPath: schemaPath + }); + + await expect( + configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + it("throws an error when a keyed object uses the 'append' merge behavior", async () => { + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + const configFileLoader: ConfigurationFile = new ConfigurationFile({ + projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json', + jsonSchemaPath: schemaPath + }); + + await expect( + configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + it('throws an error when a non-object property uses a merge behavior', async () => { + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + const configFileLoader: ConfigurationFile = new ConfigurationFile({ + projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json', + jsonSchemaPath: schemaPath + }); + + await expect( + configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + it('throws an error when a merge behavior is specified for an unspecified property', async () => { + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + const configFileLoader: ConfigurationFile = new ConfigurationFile({ + projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json', + jsonSchemaPath: schemaPath + }); + + await expect( + configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + it('throws an error when an unsupported merge behavior is specified', async () => { + const schemaPath: string = nodeJsPath.resolve( + __dirname, + 'simpleMergeBehaviorConfigFile', + 'simpleMergeBehaviorConfigFile.schema.json' + ); + const configFileLoader: ConfigurationFile = new ConfigurationFile({ + projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json', + jsonSchemaPath: schemaPath + }); + + await expect( + configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + }); + describe('loading a rig', () => { const projectFolder: string = nodeJsPath.resolve(__dirname, 'project-referencing-rig'); const rigConfig: RigConfig = RigConfig.loadForProjectFolder({ projectFolderPath: projectFolder }); 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 index 4ef175939b..8c0108a259 100644 --- a/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap +++ b/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap @@ -120,8 +120,88 @@ Object { } `; +exports[`ConfigurationFile a complex file with merge behavior annotations Correctly loads a complex config file with a single merge behavior annotation 1`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations Correctly loads a complex config file with merge behavior annotations 1`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a keyed object uses the 'append' merge behavior 1`] = `"Issue in processing configuration file property \\"c\\". Property is not an array, but the inheritance type is set as \\"append\\""`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a keyed object uses the 'append' merge behavior 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a merge behavior is specified for an unspecified property 1`] = `"Issue in processing configuration file property \\"$c.mergeBehavior\\". A merge behavior was provided but no matching property was found"`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a merge behavior is specified for an unspecified property 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a non-object property uses a merge behavior 1`] = `"Issue in processing configuration file property \\"$l.mergeBehavior\\". A merge behavior was provided for a property that is not an object"`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a non-object property uses a merge behavior 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an array uses the 'merge' merge behavior 1`] = `"Issue in processing configuration file property \\"a\\". Property is not a keyed object, but the inheritance type is set as \\"merge\\""`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an array uses the 'merge' merge behavior 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an unsupported merge behavior is specified 1`] = `"Issue in processing configuration file property \\"$a.mergeBehavior\\". An unsupported merge behavior was provided: \\"custom\\""`; + +exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an unsupported merge behavior is specified 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + exports[`ConfigurationFile error cases Throws an error for a file that doesn't match its schema 1`] = ` -"JSON validation failed: +"Resolved configuration object does not match schema: Error: JSON validation failed: /src/test/errorCases/invalidType/config.json Error: #/filePaths diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json new file mode 100644 index 0000000000..c7c3f6d04c --- /dev/null +++ b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json @@ -0,0 +1,152 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Merge Behavior Configuration", + "description": "Defines an arbitrary configration file used to test merge behavior", + "type": "object", + + "additionalProperties": false, + + "required": ["a", "b", "d"], + + "properties": { + "$schema": { + "description": "Part of the JSON Schema standard, this optional keyword declares the URL of the schema that the file conforms to. Editors may download the schema and use it to perform syntax highlighting.", + "type": "string" + }, + + "extends": { + "type": "string" + }, + + "a": { + "type": "string" + }, + + "b": { + "type": "array", + "items": { + "type": "object", + "required": ["c"], + "properties": { + "c": { + "type": "string" + } + } + } + }, + + "d": { + "type": "object", + "required": ["e", "f", "g", "i", "k", "o", "r"], + "properties": { + "e": { + "type": "string" + }, + + "f": { + "type": "string" + }, + + "g": { + "type": "array", + "items": { + "type": "object", + "required": ["h"], + "properties": { + "h": { + "type": "string" + } + } + } + }, + + "i": { + "type": "array", + "items": { + "type": "object", + "required": ["j"], + "properties": { + "j": { + "type": "string" + } + } + } + }, + + "k": { + "type": "object", + "required": ["l", "m"], + "properties": { + "l": { + "type": "string" + }, + "m": { + "type": "array", + "items": { + "type": "object", + "required": ["n"], + "properties": { + "n": { + "type": "string" + } + } + } + }, + "z": { + "type": "string" + } + } + }, + + "o": { + "type": "object", + "required": ["p"], + "properties": { + "p": { + "type": "array", + "items": { + "type": "object", + "required": ["q"], + "properties": { + "q": { + "type": "string" + } + } + } + } + } + }, + + "r": { + "type": "object", + "required": ["s"], + "properties": { + "s": { + "type": "string" + } + } + }, + + "y": { + "type": "object", + "required": ["z"], + "properties": { + "z": { + "type": "string" + } + } + } + } + }, + + "y": { + "type": "object", + "required": ["z"], + "properties": { + "z": { + "type": "string" + } + } + } + } +} diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json new file mode 100644 index 0000000000..20c60fccf6 --- /dev/null +++ b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json @@ -0,0 +1,34 @@ +{ + "$schema": "http://schema.net/", + + "a": "A", + + "b": [ + { + "c": "A" + } + ], + + "d": { + "e": "A", + + "f": "A", + + "g": [{ "h": "A" }], + + "i": [{ "j": "A" }], + + "k": { + "l": "A", + "m": [{ "n": "A" }] + }, + + "o": { + "p": [{ "q": "A" }] + }, + + "r": { + "s": "A" + } + } +} diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json new file mode 100644 index 0000000000..c5ba9dc538 --- /dev/null +++ b/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json @@ -0,0 +1,43 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./mergeBehaviorConfigFileA.json", + + "$b.mergeBehavior": "append", + "b": [ + { + "c": "B" + } + ], + + "$d.mergeBehavior": "merge", + "d": { + "f": "B", + + "$g.mergeBehavior": "append", + "g": [{ "h": "B" }], + + "$i.mergeBehavior": "replace", + "i": [{ "j": "B" }], + + "$k.mergeBehavior": "merge", + "k": { + "$m.mergeBehavior": "append", + "m": [{ "n": "B" }], + "z": "B" + }, + + "$o.mergeBehavior": "replace", + "o": { + "p": [{ "q": "B" }] + }, + + "y": { + "z": "B" + } + }, + + "y": { + "z": "B" + } +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json new file mode 100644 index 0000000000..5851c88cfa --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "$a.mergeBehavior": "merge", + "a": [ + { + "b": "B" + } + ] +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json new file mode 100644 index 0000000000..58ad4e3795 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json @@ -0,0 +1,10 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "$c.mergeBehavior": "append", + "c": { + "d": [{ "e": "B" }] + } +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json new file mode 100644 index 0000000000..c4aff0bfe0 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json @@ -0,0 +1,8 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "$l.mergeBehavior": "replace", + "l": "B" +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json new file mode 100644 index 0000000000..bb6d3c6d19 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json @@ -0,0 +1,7 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "$c.mergeBehavior": "merge" +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json new file mode 100644 index 0000000000..a6566946ea --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "$a.mergeBehavior": "custom", + "a": [ + { + "b": "B" + } + ] +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json new file mode 100644 index 0000000000..4f3ed0fff7 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json @@ -0,0 +1,85 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Merge Behavior Configuration", + "description": "Defines an arbitrary configration file used to test merge behavior", + "type": "object", + + "additionalProperties": false, + + "required": ["a", "c", "f", "l"], + + "properties": { + "a": { + "type": "array", + "items": { + "type": "object", + "required": ["b"], + "properties": { + "b": { + "type": "string" + } + } + } + }, + + "c": { + "type": "object", + "required": ["d"], + "properties": { + "d": { + "type": "array", + "items": { + "type": "object", + "required": ["e"], + "properties": { + "e": { + "type": "string" + } + } + } + } + } + }, + + "f": { + "type": "object", + "required": ["g", "i"], + "properties": { + "g": { + "type": "array", + "items": { + "type": "object", + "required": ["h"], + "properties": { + "h": { + "type": "string" + } + } + } + }, + + "i": { + "type": "object", + "properties": { + "j": { + "type": "array", + "items": { + "type": "object", + "required": ["k"], + "properties": { + "k": { + "type": "string" + } + } + } + } + } + } + } + }, + + "l": { + "type": "string" + } + } +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json new file mode 100644 index 0000000000..c005aabed7 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json @@ -0,0 +1,23 @@ +{ + "$schema": "http://schema.net/", + + "a": [ + { + "b": "A" + } + ], + + "c": { + "d": [{ "e": "A" }] + }, + + "f": { + "g": [{ "h": "A" }], + + "i": { + "j": [{ "k": "A" }] + } + }, + + "l": "A" +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json new file mode 100644 index 0000000000..91fedec9d1 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json @@ -0,0 +1,24 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleMergeBehaviorConfigFileA.json", + + "a": [ + { + "b": "B" + } + ], + + "c": { + "d": [{ "e": "B" }] + }, + + "$f.mergeBehavior": "merge", + "f": { + "g": [{ "h": "B" }], + + "i": { + "j": [{ "k": "B" }] + } + } +} From cf3ed801dd61feeb4bd9c6d18046825b3bc6e3d3 Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Mon, 14 Mar 2022 15:48:31 -0700 Subject: [PATCH 2/8] mergeBehavior -> inheritanceType --- .../heft-config-file/src/ConfigurationFile.ts | 36 +++--- .../src/test/ConfigurationFile.test.ts | 108 +++++++++--------- .../ConfigurationFile.test.ts.snap | 80 +++++++++++++ .../inheritanceTypeConfigFile.schema.json} | 4 +- .../inheritanceTypeConfigFileA.json} | 0 .../inheritanceTypeConfigFileB.json} | 16 +-- .../badInheritanceTypeConfigFileA.json | 12 ++ .../badInheritanceTypeConfigFileB.json | 10 ++ .../badInheritanceTypeConfigFileC.json | 8 ++ .../badInheritanceTypeConfigFileD.json | 7 ++ .../badInheritanceTypeConfigFileE.json | 12 ++ ...mpleInheritanceTypeConfigFile.schema.json} | 4 +- .../simpleInheritanceTypeConfigFileA.json} | 0 .../simpleInheritanceTypeConfigFileB.json} | 4 +- .../badMergeBehaviorConfigFileA.json | 12 -- .../badMergeBehaviorConfigFileB.json | 10 -- .../badMergeBehaviorConfigFileC.json | 8 -- .../badMergeBehaviorConfigFileD.json | 7 -- .../badMergeBehaviorConfigFileE.json | 12 -- 19 files changed, 215 insertions(+), 135 deletions(-) rename libraries/heft-config-file/src/test/{mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json => inheritanceTypeConfigFile/inheritanceTypeConfigFile.schema.json} (97%) rename libraries/heft-config-file/src/test/{mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json => inheritanceTypeConfigFile/inheritanceTypeConfigFileA.json} (100%) rename libraries/heft-config-file/src/test/{mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json => inheritanceTypeConfigFile/inheritanceTypeConfigFileB.json} (52%) create mode 100644 libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileA.json create mode 100644 libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileB.json create mode 100644 libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileC.json create mode 100644 libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileD.json create mode 100644 libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileE.json rename libraries/heft-config-file/src/test/{simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json => simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFile.schema.json} (96%) rename libraries/heft-config-file/src/test/{simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json => simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileA.json} (100%) rename libraries/heft-config-file/src/test/{simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json => simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileB.json} (69%) delete mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json delete mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json delete mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json delete mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json delete mode 100644 libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index 37e6e79223..43c7aacd7a 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -70,7 +70,7 @@ export enum PathResolutionMethod { custom } -const CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX: RegExp = /^\$([^\.]+)\.mergeBehavior$/; +const CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX: RegExp = /^\$([^\.]+)\.inheritanceType$/; const CONFIGURATION_FILE_FIELD_ANNOTATION: unique symbol = Symbol('configuration-file-field-annotation'); interface IAnnotatedField { @@ -587,50 +587,50 @@ export class ConfigurationFile { [CONFIGURATION_FILE_FIELD_ANNOTATION]: resultAnnotation } as unknown as TField; - // Do a first pass to gather and strip the merge behavior annotations from the merging object + // Do a first pass to gather and strip the inheritance type annotations from the merging object const currentObjectPropertyNames: Set = new Set(Object.keys(currentObject || {})); const filteredObjectPropertyNames: string[] = []; - const mergeBehaviorMap: Map> = new Map(); + const inheritanceTypeMap: Map> = new Map(); for (const propertyName of currentObjectPropertyNames) { if (ignoreProperties && ignoreProperties.has(propertyName)) { continue; } - const mergeBehaviorMatches: RegExpMatchArray | null = propertyName.match( + const inheritanceTypeMatches: RegExpMatchArray | null = propertyName.match( CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX ); - if (mergeBehaviorMatches && mergeBehaviorMatches.length === 2) { - const mergeTargetPropertyName: string = mergeBehaviorMatches[1]; - const mergeBehaviorRaw: unknown | undefined = (currentObject || {})[propertyName]; + if (inheritanceTypeMatches && inheritanceTypeMatches.length === 2) { + const mergeTargetPropertyName: string = inheritanceTypeMatches[1]; + const inheritanceTypeRaw: unknown | undefined = (currentObject || {})[propertyName]; if (!currentObjectPropertyNames.has(mergeTargetPropertyName)) { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `A merge behavior was provided but no matching property was found` + `An inheritance type was provided but no matching property was found` ); - } else if (typeof mergeBehaviorRaw !== 'string') { + } else if (typeof inheritanceTypeRaw !== 'string') { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `An unsupported merge behavior was provided: ${JSON.stringify(mergeBehaviorRaw)}` + `An unsupported inheritance type was provided: ${JSON.stringify(inheritanceTypeRaw)}` ); } else if (typeof (currentObject || {})[mergeTargetPropertyName] !== 'object') { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `A merge behavior was provided for a property that is not an object` + `An inheritance type was provided for a property that is not an object` ); } - switch (mergeBehaviorRaw.toLowerCase()) { + switch (inheritanceTypeRaw.toLowerCase()) { case 'append': - mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.append }); + inheritanceTypeMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.append }); break; case 'merge': - mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.merge }); + inheritanceTypeMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.merge }); break; case 'replace': - mergeBehaviorMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.replace }); + inheritanceTypeMap.set(mergeTargetPropertyName, { inheritanceType: InheritanceType.replace }); break; default: throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `An unsupported merge behavior was provided: "${mergeBehaviorRaw}"` + `An unsupported inheritance type was provided: "${inheritanceTypeRaw}"` ); } } else { @@ -649,10 +649,10 @@ export class ConfigurationFile { const propertyValue: unknown | undefined = (currentObject || {})[propertyName]; const parentPropertyValue: unknown | undefined = (parentObject || {})[propertyName]; - // If the property is a merge behavior annotation, use it. Fallback to the configuration file inheritance + // If the property is an inheritance type annotation, use it. Fallback to the configuration file inheritance // behavior, and if one isn't specified, use the default. let propertyInheritance: IPropertyInheritance | undefined = - mergeBehaviorMap.get(propertyName); + inheritanceTypeMap.get(propertyName); if (!propertyInheritance) { const bothAreArrays: boolean = Array.isArray(propertyValue) && Array.isArray(parentPropertyValue); const defaultInheritanceType: IPropertyInheritance = bothAreArrays diff --git a/libraries/heft-config-file/src/test/ConfigurationFile.test.ts b/libraries/heft-config-file/src/test/ConfigurationFile.test.ts index 3e9103bbdd..7510202636 100644 --- a/libraries/heft-config-file/src/test/ConfigurationFile.test.ts +++ b/libraries/heft-config-file/src/test/ConfigurationFile.test.ts @@ -421,8 +421,8 @@ describe(ConfigurationFile.name, () => { }); }); - describe('a complex file with merge behavior annotations', () => { - interface IMergeBehaviorConfigFile { + describe('a complex file with inheritance type annotations', () => { + interface IInheritanceTypeConfigFile { a: string; b: { c: string }[]; d: { @@ -450,7 +450,7 @@ describe(ConfigurationFile.name, () => { }; } - interface ISimpleMergeBehaviorConfigFile { + interface ISimpleInheritanceTypeConfigFile { a: { b: string }[]; c: { d: { e: string }[]; @@ -464,50 +464,50 @@ describe(ConfigurationFile.name, () => { l: string; } - it('Correctly loads a complex config file with merge behavior annotations', async () => { - const projectRelativeFilePath: string = 'mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json'; + it('Correctly loads a complex config file with inheritance type annotations', async () => { + const projectRelativeFilePath: string = 'inheritanceTypeConfigFile/inheritanceTypeConfigFileB.json'; const rootConfigFilePath: string = nodeJsPath.resolve( __dirname, - 'mergeBehaviorConfigFile', - 'mergeBehaviorConfigFileA.json' + 'inheritanceTypeConfigFile', + 'inheritanceTypeConfigFileA.json' ); const secondConfigFilePath: string = nodeJsPath.resolve( __dirname, - 'mergeBehaviorConfigFile', - 'mergeBehaviorConfigFileB.json' + 'inheritanceTypeConfigFile', + 'inheritanceTypeConfigFileB.json' ); const schemaPath: string = nodeJsPath.resolve( __dirname, - 'mergeBehaviorConfigFile', - 'mergeBehaviorConfigFile.schema.json' + 'inheritanceTypeConfigFile', + 'inheritanceTypeConfigFile.schema.json' ); - const configFileLoader: ConfigurationFile = - new ConfigurationFile({ + const configFileLoader: ConfigurationFile = + new ConfigurationFile({ projectRelativeFilePath: projectRelativeFilePath, jsonSchemaPath: schemaPath }); - const loadedConfigFile: IMergeBehaviorConfigFile = + const loadedConfigFile: IInheritanceTypeConfigFile = await configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname); - const expectedConfigFile: IMergeBehaviorConfigFile = { + const expectedConfigFile: IInheritanceTypeConfigFile = { a: 'A', - // "$b.mergeBehavior": "append" + // "$b.inheritanceType": "append" b: [{ c: 'A' }, { c: 'B' }], - // "$d.mergeBehavior": "merge" + // "$d.inheritanceType": "merge" d: { e: 'A', f: 'B', - // "$g.mergeBehavior": "append" + // "$g.inheritanceType": "append" g: [{ h: 'A' }, { h: 'B' }], - // "$i.mergeBehavior": "replace" + // "$i.inheritanceType": "replace" i: [{ j: 'B' }], - // "$k.mergeBehavior": "merge" + // "$k.inheritanceType": "merge" k: { l: 'A', m: [{ n: 'A' }, { n: 'B' }], z: 'B' }, - // "$o.mergeBehavior": "replace" + // "$o.inheritanceType": "replace" o: { p: [{ q: 'B' }] }, @@ -554,38 +554,38 @@ describe(ConfigurationFile.name, () => { expect(configFileLoader.getObjectSourceFilePath(loadedConfigFile.y!)).toEqual(secondConfigFilePath); }); - it('Correctly loads a complex config file with a single merge behavior annotation', async () => { + it('Correctly loads a complex config file with a single inheritance type annotation', async () => { const projectRelativeFilePath: string = - 'simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json'; + 'simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileB.json'; const rootConfigFilePath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFileA.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFileA.json' ); const secondConfigFilePath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFileB.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFileB.json' ); const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); - const configFileLoader: ConfigurationFile = - new ConfigurationFile({ + const configFileLoader: ConfigurationFile = + new ConfigurationFile({ projectRelativeFilePath: projectRelativeFilePath, jsonSchemaPath: schemaPath }); - const loadedConfigFile: ISimpleMergeBehaviorConfigFile = + const loadedConfigFile: ISimpleInheritanceTypeConfigFile = await configFileLoader.loadConfigurationFileForProjectAsync(terminal, __dirname); - const expectedConfigFile: ISimpleMergeBehaviorConfigFile = { + const expectedConfigFile: ISimpleInheritanceTypeConfigFile = { a: [{ b: 'A' }, { b: 'B' }], c: { d: [{ e: 'B' }] }, - // "$f.mergeBehavior": "merge" + // "$f.inheritanceType": "merge" f: { g: [{ h: 'A' }, { h: 'B' }], i: { @@ -613,14 +613,14 @@ describe(ConfigurationFile.name, () => { ); }); - it("throws an error when an array uses the 'merge' merge behavior", async () => { + it("throws an error when an array uses the 'merge' inheritance type", async () => { const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); const configFileLoader: ConfigurationFile = new ConfigurationFile({ - projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json', + projectRelativeFilePath: 'simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileA.json', jsonSchemaPath: schemaPath }); @@ -629,14 +629,14 @@ describe(ConfigurationFile.name, () => { ).rejects.toThrowErrorMatchingSnapshot(); }); - it("throws an error when a keyed object uses the 'append' merge behavior", async () => { + it("throws an error when a keyed object uses the 'append' inheritance type", async () => { const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); const configFileLoader: ConfigurationFile = new ConfigurationFile({ - projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json', + projectRelativeFilePath: 'simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileB.json', jsonSchemaPath: schemaPath }); @@ -645,14 +645,14 @@ describe(ConfigurationFile.name, () => { ).rejects.toThrowErrorMatchingSnapshot(); }); - it('throws an error when a non-object property uses a merge behavior', async () => { + it('throws an error when a non-object property uses an inheritance type', async () => { const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); const configFileLoader: ConfigurationFile = new ConfigurationFile({ - projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json', + projectRelativeFilePath: 'simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileC.json', jsonSchemaPath: schemaPath }); @@ -661,14 +661,14 @@ describe(ConfigurationFile.name, () => { ).rejects.toThrowErrorMatchingSnapshot(); }); - it('throws an error when a merge behavior is specified for an unspecified property', async () => { + it('throws an error when an inheritance type is specified for an unspecified property', async () => { const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); const configFileLoader: ConfigurationFile = new ConfigurationFile({ - projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json', + projectRelativeFilePath: 'simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileD.json', jsonSchemaPath: schemaPath }); @@ -677,14 +677,14 @@ describe(ConfigurationFile.name, () => { ).rejects.toThrowErrorMatchingSnapshot(); }); - it('throws an error when an unsupported merge behavior is specified', async () => { + it('throws an error when an unsupported inheritance type is specified', async () => { const schemaPath: string = nodeJsPath.resolve( __dirname, - 'simpleMergeBehaviorConfigFile', - 'simpleMergeBehaviorConfigFile.schema.json' + 'simpleInheritanceTypeConfigFile', + 'simpleInheritanceTypeConfigFile.schema.json' ); const configFileLoader: ConfigurationFile = new ConfigurationFile({ - projectRelativeFilePath: 'simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json', + projectRelativeFilePath: 'simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileE.json', jsonSchemaPath: schemaPath }); 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 index 8c0108a259..ba10202bcd 100644 --- a/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap +++ b/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap @@ -120,6 +120,86 @@ Object { } `; +exports[`ConfigurationFile a complex file with inheritance type annotations Correctly loads a complex config file with a single inheritance type annotation 1`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations Correctly loads a complex config file with inheritance type annotations 1`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a keyed object uses the 'append' inheritance type 1`] = `"Issue in processing configuration file property \\"c\\". Property is not an array, but the inheritance type is set as \\"append\\""`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a keyed object uses the 'append' inheritance type 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a non-object property uses an inheritance type 1`] = `"Issue in processing configuration file property \\"$l.inheritanceType\\". An inheritance type was provided for a property that is not an object"`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a non-object property uses an inheritance type 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an array uses the 'merge' inheritance type 1`] = `"Issue in processing configuration file property \\"a\\". Property is not a keyed object, but the inheritance type is set as \\"merge\\""`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an array uses the 'merge' inheritance type 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an inheritance type is specified for an unspecified property 1`] = `"Issue in processing configuration file property \\"$c.inheritanceType\\". An inheritance type was provided but no matching property was found"`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an inheritance type is specified for an unspecified property 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an unsupported inheritance type is specified 1`] = `"Issue in processing configuration file property \\"$a.inheritanceType\\". An unsupported inheritance type was provided: \\"custom\\""`; + +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an unsupported inheritance type is specified 2`] = ` +Object { + "debug": "", + "error": "", + "log": "", + "verbose": "", + "warning": "", +} +`; + exports[`ConfigurationFile a complex file with merge behavior annotations Correctly loads a complex config file with a single merge behavior annotation 1`] = ` Object { "debug": "", diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json b/libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFile.schema.json similarity index 97% rename from libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json rename to libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFile.schema.json index c7c3f6d04c..cf761aa0ec 100644 --- a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFile.schema.json +++ b/libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFile.schema.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-04/schema#", - "title": "Merge Behavior Configuration", - "description": "Defines an arbitrary configration file used to test merge behavior", + "title": "Inheritance Type Configuration", + "description": "Defines an arbitrary configration file used to test inheritance", "type": "object", "additionalProperties": false, diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFileA.json similarity index 100% rename from libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileA.json rename to libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFileA.json diff --git a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFileB.json similarity index 52% rename from libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json rename to libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFileB.json index c5ba9dc538..d391959250 100644 --- a/libraries/heft-config-file/src/test/mergeBehaviorConfigFile/mergeBehaviorConfigFileB.json +++ b/libraries/heft-config-file/src/test/inheritanceTypeConfigFile/inheritanceTypeConfigFileB.json @@ -1,33 +1,33 @@ { "$schema": "http://schema.net/", - "extends": "./mergeBehaviorConfigFileA.json", + "extends": "./inheritanceTypeConfigFileA.json", - "$b.mergeBehavior": "append", + "$b.inheritanceType": "append", "b": [ { "c": "B" } ], - "$d.mergeBehavior": "merge", + "$d.inheritanceType": "merge", "d": { "f": "B", - "$g.mergeBehavior": "append", + "$g.inheritanceType": "append", "g": [{ "h": "B" }], - "$i.mergeBehavior": "replace", + "$i.inheritanceType": "replace", "i": [{ "j": "B" }], - "$k.mergeBehavior": "merge", + "$k.inheritanceType": "merge", "k": { - "$m.mergeBehavior": "append", + "$m.inheritanceType": "append", "m": [{ "n": "B" }], "z": "B" }, - "$o.mergeBehavior": "replace", + "$o.inheritanceType": "replace", "o": { "p": [{ "q": "B" }] }, diff --git a/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileA.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileA.json new file mode 100644 index 0000000000..b628817309 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileA.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleInheritanceTypeConfigFileA.json", + + "$a.inheritanceType": "merge", + "a": [ + { + "b": "B" + } + ] +} diff --git a/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileB.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileB.json new file mode 100644 index 0000000000..49cd809768 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileB.json @@ -0,0 +1,10 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleInheritanceTypeConfigFileA.json", + + "$c.inheritanceType": "append", + "c": { + "d": [{ "e": "B" }] + } +} diff --git a/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileC.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileC.json new file mode 100644 index 0000000000..42f31760e8 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileC.json @@ -0,0 +1,8 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleInheritanceTypeConfigFileA.json", + + "$l.inheritanceType": "replace", + "l": "B" +} diff --git a/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileD.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileD.json new file mode 100644 index 0000000000..076557439d --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileD.json @@ -0,0 +1,7 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleInheritanceTypeConfigFileA.json", + + "$c.inheritanceType": "merge" +} diff --git a/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileE.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileE.json new file mode 100644 index 0000000000..ec3f9fe2f9 --- /dev/null +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/badInheritanceTypeConfigFileE.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://schema.net/", + + "extends": "./simpleInheritanceTypeConfigFileA.json", + + "$a.inheritanceType": "custom", + "a": [ + { + "b": "B" + } + ] +} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFile.schema.json similarity index 96% rename from libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json rename to libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFile.schema.json index 4f3ed0fff7..c96567e8d5 100644 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFile.schema.json +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFile.schema.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-04/schema#", - "title": "Merge Behavior Configuration", - "description": "Defines an arbitrary configration file used to test merge behavior", + "title": "Inheritance Type Configuration", + "description": "Defines an arbitrary configration file used to test inheritance", "type": "object", "additionalProperties": false, diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileA.json similarity index 100% rename from libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileA.json rename to libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileA.json diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileB.json similarity index 69% rename from libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json rename to libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileB.json index 91fedec9d1..2ccae63bd7 100644 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/simpleMergeBehaviorConfigFileB.json +++ b/libraries/heft-config-file/src/test/simpleInheritanceTypeConfigFile/simpleInheritanceTypeConfigFileB.json @@ -1,7 +1,7 @@ { "$schema": "http://schema.net/", - "extends": "./simpleMergeBehaviorConfigFileA.json", + "extends": "./simpleInheritanceTypeConfigFileA.json", "a": [ { @@ -13,7 +13,7 @@ "d": [{ "e": "B" }] }, - "$f.mergeBehavior": "merge", + "$f.inheritanceType": "merge", "f": { "g": [{ "h": "B" }], diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json deleted file mode 100644 index 5851c88cfa..0000000000 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileA.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "$schema": "http://schema.net/", - - "extends": "./simpleMergeBehaviorConfigFileA.json", - - "$a.mergeBehavior": "merge", - "a": [ - { - "b": "B" - } - ] -} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json deleted file mode 100644 index 58ad4e3795..0000000000 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileB.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "$schema": "http://schema.net/", - - "extends": "./simpleMergeBehaviorConfigFileA.json", - - "$c.mergeBehavior": "append", - "c": { - "d": [{ "e": "B" }] - } -} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json deleted file mode 100644 index c4aff0bfe0..0000000000 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileC.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "$schema": "http://schema.net/", - - "extends": "./simpleMergeBehaviorConfigFileA.json", - - "$l.mergeBehavior": "replace", - "l": "B" -} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json deleted file mode 100644 index bb6d3c6d19..0000000000 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileD.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "$schema": "http://schema.net/", - - "extends": "./simpleMergeBehaviorConfigFileA.json", - - "$c.mergeBehavior": "merge" -} diff --git a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json b/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json deleted file mode 100644 index a6566946ea..0000000000 --- a/libraries/heft-config-file/src/test/simpleMergeBehaviorConfigFile/badMergeBehaviorConfigFileE.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "$schema": "http://schema.net/", - - "extends": "./simpleMergeBehaviorConfigFileA.json", - - "$a.mergeBehavior": "custom", - "a": [ - { - "b": "B" - } - ] -} From 340a798b6a4d25f7de246300e43ddf07fc089a61 Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Mon, 14 Mar 2022 15:52:00 -0700 Subject: [PATCH 3/8] Rush change --- ...nade-AllowCustomMergeBehavior_2022-03-14-22-51.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json diff --git a/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json b/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json new file mode 100644 index 0000000000..ed696d8c23 --- /dev/null +++ b/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft-config-file", + "comment": "Add inline \"inheritanceType\" property support. This feature allows for configuration files to specify how object and array properties are inherited, overriding the default inheritance behavior provided by the configuration file class.", + "type": "minor" + } + ], + "packageName": "@rushstack/heft-config-file" +} \ No newline at end of file From 39d01ac80b4440c56c5670f1cc9591d7712422ed Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Tue, 15 Mar 2022 22:25:47 -0700 Subject: [PATCH 4/8] Update typings to be more strict --- common/reviews/api/heft-config-file.api.md | 2 +- .../heft-config-file/src/ConfigurationFile.ts | 215 +++++++++--------- 2 files changed, 103 insertions(+), 114 deletions(-) diff --git a/common/reviews/api/heft-config-file.api.md b/common/reviews/api/heft-config-file.api.md index 84fe90e166..c394bbf136 100644 --- a/common/reviews/api/heft-config-file.api.md +++ b/common/reviews/api/heft-config-file.api.md @@ -55,7 +55,7 @@ export enum InheritanceType { // @beta (undocumented) export interface IOriginalValueOptions { // (undocumented) - parentObject: TParentProperty; + parentObject: Partial; // (undocumented) propertyName: keyof TParentProperty; } diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index 43c7aacd7a..f667abd850 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -184,7 +184,7 @@ interface IJsonPathCallbackObject { * @beta */ export interface IOriginalValueOptions { - parentObject: TParentProperty; + parentObject: Partial; propertyName: keyof TParentProperty; } @@ -426,8 +426,8 @@ export class ConfigurationFile { } } - const result: TConfigurationFile = this._mergeConfigurationFiles( - parentConfiguration, + const result: Partial = this._mergeConfigurationFiles( + parentConfiguration || {}, configurationJson, resolvedConfigurationFilePath ); @@ -437,7 +437,8 @@ export class ConfigurationFile { throw new Error(`Resolved configuration object does not match schema: ${e}`); } - return result; + // If the schema validates, we can assume that the configuration file is complete. + return result as TConfigurationFile; } private async _tryLoadConfigurationFileInRigAsync( @@ -558,39 +559,48 @@ export class ConfigurationFile { } private _mergeConfigurationFiles( - parentConfiguration: TConfigurationFile | undefined, - configurationJson: IConfigurationJson & TConfigurationFile, + parentConfiguration: Partial, + configurationJson: Partial, resolvedConfigurationFilePath: string - ): TConfigurationFile { + ): Partial { const ignoreProperties: Set = new Set(['extends', '$schema']); + + // Need to do a dance with the casting here because while we know that JSON keys are always + // strings, TypeScript doesn't. return this._mergeObjects( - parentConfiguration, - configurationJson, + parentConfiguration as { [key: string]: unknown }, + configurationJson as { [key: string]: unknown }, resolvedConfigurationFilePath, - this._propertyInheritanceTypes, + this._propertyInheritanceTypes as IPropertiesInheritance<{ [key: string]: unknown }>, ignoreProperties - ); + ) as Partial; } - private _mergeObjects( - parentObject: TField | undefined, - currentObject: TField | undefined, + private _mergeObjects( + parentObject: Partial, + currentObject: Partial, resolvedConfigurationFilePath: string, configuredPropertyInheritance?: IPropertiesInheritance, ignoreProperties?: Set - ): TField { - const resultAnnotation: IConfigurationFileFieldAnnotation = { + ): Partial { + const resultAnnotation: IConfigurationFileFieldAnnotation> = { configurationFilePath: resolvedConfigurationFilePath, - originalValues: {} as TField + originalValues: {} as Partial }; - const result: TField = { + const result: Partial = { [CONFIGURATION_FILE_FIELD_ANNOTATION]: resultAnnotation - } as unknown as TField; - - // Do a first pass to gather and strip the inheritance type annotations from the merging object - const currentObjectPropertyNames: Set = new Set(Object.keys(currentObject || {})); - const filteredObjectPropertyNames: string[] = []; - const inheritanceTypeMap: Map> = new Map(); + } as unknown as Partial; + + // An array of property names that are on the merging object. Typed as Set since it may + // contain inheritance type annotation keys, or other built-in properties that we ignore + // (eg. "extends", "$schema"). + const currentObjectPropertyNames: Set = 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> = new Map(); + + // 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)) { continue; @@ -598,9 +608,11 @@ export class ConfigurationFile { const inheritanceTypeMatches: RegExpMatchArray | null = propertyName.match( CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX ); - if (inheritanceTypeMatches && inheritanceTypeMatches.length === 2) { + if (inheritanceTypeMatches) { + // Should always be of length 2, since the first match is the entire string and the second + // match is the capture group. const mergeTargetPropertyName: string = inheritanceTypeMatches[1]; - const inheritanceTypeRaw: unknown | undefined = (currentObject || {})[propertyName]; + const inheritanceTypeRaw: unknown | undefined = currentObject[propertyName]; if (!currentObjectPropertyNames.has(mergeTargetPropertyName)) { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + @@ -639,45 +651,26 @@ export class ConfigurationFile { } // We only filter the currentObject because the parent object should already be filtered - const propertyNames: Set = new Set([ - ...Object.keys(parentObject || {}), + const propertyNames: Set = new Set([ + ...Object.keys(parentObject), ...filteredObjectPropertyNames ]); // Cycle through properties and merge them for (const propertyName of propertyNames) { - const propertyValue: unknown | undefined = (currentObject || {})[propertyName]; - const parentPropertyValue: unknown | undefined = (parentObject || {})[propertyName]; - - // If the property is an inheritance type annotation, use it. Fallback to the configuration file inheritance - // behavior, and if one isn't specified, use the default. - let propertyInheritance: IPropertyInheritance | undefined = - inheritanceTypeMap.get(propertyName); - if (!propertyInheritance) { - const bothAreArrays: boolean = Array.isArray(propertyValue) && Array.isArray(parentPropertyValue); - const defaultInheritanceType: IPropertyInheritance = bothAreArrays - ? { inheritanceType: InheritanceType.append } - : { inheritanceType: InheritanceType.replace }; - propertyInheritance = - // eslint-disable-next-line @typescript-eslint/no-explicit-any - configuredPropertyInheritance && (configuredPropertyInheritance as any)[propertyName] !== undefined - ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - (configuredPropertyInheritance as any)[propertyName] - : defaultInheritanceType; - } + const propertyValue: TField[keyof TField] | undefined = currentObject[propertyName]; + const parentPropertyValue: TField[keyof TField] | undefined = parentObject[propertyName]; - let newValue: unknown; + let newValue: TField[keyof TField] | undefined; const usePropertyValue: () => void = () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ + resultAnnotation.originalValues[propertyName] = this.getPropertyOriginalValue({ parentObject: currentObject, propertyName: propertyName }); newValue = propertyValue; }; const useParentPropertyValue: () => void = () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (resultAnnotation.originalValues as any)[propertyName] = this.getPropertyOriginalValue({ + resultAnnotation.originalValues[propertyName] = this.getPropertyOriginalValue({ parentObject: parentObject, propertyName: propertyName }); @@ -688,43 +681,46 @@ export class ConfigurationFile { usePropertyValue(); } else if (parentPropertyValue !== undefined && propertyValue === undefined) { useParentPropertyValue(); - } else { - switch (propertyInheritance!.inheritanceType) { + } else if (propertyValue !== undefined && parentPropertyValue !== undefined) { + // If the property is an inheritance type annotation, use it. Fallback to the configuration file inheritance + // behavior, and if one isn't specified, use the default. + let propertyInheritance: IPropertyInheritance | undefined = + inheritanceTypeMap.get(propertyName); + if (!propertyInheritance) { + const bothAreArrays: boolean = Array.isArray(propertyValue) && Array.isArray(parentPropertyValue); + propertyInheritance = + configuredPropertyInheritance?.[propertyName] ?? + (bothAreArrays + ? { inheritanceType: InheritanceType.append } + : { inheritanceType: InheritanceType.replace }); + } + + switch (propertyInheritance.inheritanceType) { case InheritanceType.replace: { - if (propertyValue !== undefined) { - usePropertyValue(); - } else { - useParentPropertyValue(); - } + usePropertyValue(); break; } case InheritanceType.append: { - if (propertyValue !== undefined && parentPropertyValue === undefined) { - usePropertyValue(); - } else if (propertyValue === undefined && parentPropertyValue !== undefined) { - useParentPropertyValue(); - } else { - if (!Array.isArray(propertyValue) || !Array.isArray(parentPropertyValue)) { - throw new Error( - `Issue in processing configuration file property "${propertyName}". ` + - `Property is not an array, but the inheritance type is set as "${InheritanceType.append}"` - ); - } - - newValue = [...parentPropertyValue, ...propertyValue]; - (newValue as unknown as IAnnotatedField)[CONFIGURATION_FILE_FIELD_ANNOTATION] = { - configurationFilePath: undefined, - originalValues: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(parentPropertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(propertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues - } - }; + if (!Array.isArray(propertyValue) || !Array.isArray(parentPropertyValue)) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Property is not an array, but the inheritance type is set as "${InheritanceType.append}"` + ); } + newValue = [...parentPropertyValue, ...propertyValue] as TField[keyof TField]; + (newValue as unknown as IAnnotatedField)[CONFIGURATION_FILE_FIELD_ANNOTATION] = { + configurationFilePath: undefined, + originalValues: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(parentPropertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(propertyValue as any)[CONFIGURATION_FILE_FIELD_ANNOTATION].originalValues + } + }; + break; } @@ -735,43 +731,37 @@ export class ConfigurationFile { `Null values cannot be used when the inheritance type is set as "${InheritanceType.merge}"` ); } - - if (propertyValue !== undefined && parentPropertyValue === undefined) { - usePropertyValue(); - } else if (propertyValue === undefined && parentPropertyValue !== undefined) { - useParentPropertyValue(); - } else { - if ( - (propertyValue && typeof propertyValue !== 'object') || - (parentPropertyValue && typeof parentPropertyValue !== 'object') - ) { - throw new Error( - `Issue in processing configuration file property "${propertyName}". ` + - `Primitive types cannot be provided when the inheritance type is set as "${InheritanceType.merge}"` - ); - } - if (Array.isArray(propertyValue) || Array.isArray(parentPropertyValue)) { - throw new Error( - `Issue in processing configuration file property "${propertyName}". ` + - `Property is not a keyed object, but the inheritance type is set as "${InheritanceType.merge}"` - ); - } - - // Recursively merge the parent and child objects. Don't pass the configuredPropertyInheritance or - // ignoreProperties because we are no longer at the top level of the configuration file. - newValue = this._mergeObjects( - parentPropertyValue as object | undefined, - propertyValue as object | undefined, - resolvedConfigurationFilePath + if ( + (propertyValue && typeof propertyValue !== 'object') || + (parentPropertyValue && typeof parentPropertyValue !== 'object') + ) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Primitive types cannot be provided when the inheritance type is set as "${InheritanceType.merge}"` + ); + } + if (Array.isArray(propertyValue) || Array.isArray(parentPropertyValue)) { + throw new Error( + `Issue in processing configuration file property "${propertyName}". ` + + `Property is not a keyed object, but the inheritance type is set as "${InheritanceType.merge}"` ); } + // Recursively merge the parent and child objects. Don't pass the configuredPropertyInheritance or + // ignoreProperties because we are no longer at the top level of the configuration file. We also know + // that it must be a string-keyed object, since the JSON spec requires it. + newValue = this._mergeObjects( + parentPropertyValue as { [key: string]: unknown }, + propertyValue as { [key: string]: unknown }, + resolvedConfigurationFilePath + ) as TField[keyof TField]; + break; } case InheritanceType.custom: { - const customInheritance: ICustomPropertyInheritance = - propertyInheritance as ICustomPropertyInheritance; + const customInheritance: ICustomPropertyInheritance = + propertyInheritance as ICustomPropertyInheritance; if ( !customInheritance.inheritanceFunction || typeof customInheritance.inheritanceFunction !== 'function' @@ -792,8 +782,7 @@ export class ConfigurationFile { } } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (result as any)[propertyName] = newValue; + result[propertyName] = newValue; } return result; From 7209b87d4638c5225c81f78700064fd1068bf79f Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Tue, 15 Mar 2022 22:32:43 -0700 Subject: [PATCH 5/8] Add clarifying comment about possible future usage of magic properties --- libraries/heft-config-file/src/ConfigurationFile.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index f667abd850..5946a49494 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -605,6 +605,13 @@ export class ConfigurationFile { if (ignoreProperties && ignoreProperties.has(propertyName)) { continue; } + + // Try to get the inheritance type annotation from the merging object using the regex. + // Note: since this regex matches a specific style of property name, we should not need to + // allow for any escaping of $-prefixed properties. If this ever changes (eg. to allow for + // `"$propertyName": { ... }` options), then we'll likely need to handle that error case, + // as well as allow escaping $-prefixed properties that developers want to be serialized, + // possibly by using the form `$$propertyName` to escape `$propertyName`. const inheritanceTypeMatches: RegExpMatchArray | null = propertyName.match( CONFIGURATION_FILE_MERGE_BEHAVIOR_FIELD_REGEX ); From 211cd7d0774bc45e36cfa1c12660b986cf65878e Mon Sep 17 00:00:00 2001 From: Daniel <3473356+D4N14L@users.noreply.github.com> Date: Tue, 15 Mar 2022 22:34:17 -0700 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Ian Clanton-Thuon --- ...user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json | 2 +- libraries/heft-config-file/src/ConfigurationFile.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json b/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json index ed696d8c23..04c0814bab 100644 --- a/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json +++ b/common/changes/@rushstack/heft-config-file/user-danade-AllowCustomMergeBehavior_2022-03-14-22-51.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/heft-config-file", - "comment": "Add inline \"inheritanceType\" property support. This feature allows for configuration files to specify how object and array properties are inherited, overriding the default inheritance behavior provided by the configuration file class.", + "comment": "Add support for an inline \"$.inheritanceType\" property. This feature allows for configuration files to specify how object and array properties are inherited, overriding the default inheritance behavior provided by the configuration file class.", "type": "minor" } ], diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index 5946a49494..aa1d69cbbe 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -623,7 +623,7 @@ export class ConfigurationFile { if (!currentObjectPropertyNames.has(mergeTargetPropertyName)) { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `An inheritance type was provided but no matching property was found` + `An inheritance type was provided but no matching property was found in the parent.` ); } else if (typeof inheritanceTypeRaw !== 'string') { throw new Error( @@ -633,7 +633,7 @@ export class ConfigurationFile { } else if (typeof (currentObject || {})[mergeTargetPropertyName] !== 'object') { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `An inheritance type was provided for a property that is not an object` + `An inheritance type was provided for a property that is not an object or array.` ); } switch (inheritanceTypeRaw.toLowerCase()) { From 8b4f109b6ff67e8dc4c8ce5ab86642ebd6ccf03b Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Wed, 16 Mar 2022 10:27:19 -0700 Subject: [PATCH 7/8] Cleanup --- libraries/heft-config-file/src/ConfigurationFile.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/heft-config-file/src/ConfigurationFile.ts b/libraries/heft-config-file/src/ConfigurationFile.ts index aa1d69cbbe..977a603711 100644 --- a/libraries/heft-config-file/src/ConfigurationFile.ts +++ b/libraries/heft-config-file/src/ConfigurationFile.ts @@ -630,10 +630,10 @@ export class ConfigurationFile { `Issue in processing configuration file property "${propertyName}". ` + `An unsupported inheritance type was provided: ${JSON.stringify(inheritanceTypeRaw)}` ); - } else if (typeof (currentObject || {})[mergeTargetPropertyName] !== 'object') { + } else if (typeof currentObject[mergeTargetPropertyName] !== 'object') { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + - `An inheritance type was provided for a property that is not an object or array.` + `An inheritance type was provided for a property that is not a keyed object or array.` ); } switch (inheritanceTypeRaw.toLowerCase()) { @@ -737,8 +737,7 @@ export class ConfigurationFile { `Issue in processing configuration file property "${propertyName}". ` + `Null values cannot be used when the inheritance type is set as "${InheritanceType.merge}"` ); - } - if ( + } else if ( (propertyValue && typeof propertyValue !== 'object') || (parentPropertyValue && typeof parentPropertyValue !== 'object') ) { @@ -746,8 +745,7 @@ export class ConfigurationFile { `Issue in processing configuration file property "${propertyName}". ` + `Primitive types cannot be provided when the inheritance type is set as "${InheritanceType.merge}"` ); - } - if (Array.isArray(propertyValue) || Array.isArray(parentPropertyValue)) { + } else if (Array.isArray(propertyValue) || Array.isArray(parentPropertyValue)) { throw new Error( `Issue in processing configuration file property "${propertyName}". ` + `Property is not a keyed object, but the inheritance type is set as "${InheritanceType.merge}"` From 175475b172dbd21ed7b6323d0afd32dcdc927c40 Mon Sep 17 00:00:00 2001 From: Daniel Nadeau <3473356+D4N14L@users.noreply.github.com> Date: Wed, 16 Mar 2022 12:00:58 -0700 Subject: [PATCH 8/8] Update snapshots --- .../ConfigurationFile.test.ts.snap | 84 +------------------ 1 file changed, 2 insertions(+), 82 deletions(-) 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 index ba10202bcd..22283591d2 100644 --- a/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap +++ b/libraries/heft-config-file/src/test/__snapshots__/ConfigurationFile.test.ts.snap @@ -152,7 +152,7 @@ Object { } `; -exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a non-object property uses an inheritance type 1`] = `"Issue in processing configuration file property \\"$l.inheritanceType\\". An inheritance type was provided for a property that is not an object"`; +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a non-object property uses an inheritance type 1`] = `"Issue in processing configuration file property \\"$l.inheritanceType\\". An inheritance type was provided for a property that is not a keyed object or array."`; exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when a non-object property uses an inheritance type 2`] = ` Object { @@ -176,7 +176,7 @@ Object { } `; -exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an inheritance type is specified for an unspecified property 1`] = `"Issue in processing configuration file property \\"$c.inheritanceType\\". An inheritance type was provided but no matching property was found"`; +exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an inheritance type is specified for an unspecified property 1`] = `"Issue in processing configuration file property \\"$c.inheritanceType\\". An inheritance type was provided but no matching property was found in the parent."`; exports[`ConfigurationFile a complex file with inheritance type annotations throws an error when an inheritance type is specified for an unspecified property 2`] = ` Object { @@ -200,86 +200,6 @@ Object { } `; -exports[`ConfigurationFile a complex file with merge behavior annotations Correctly loads a complex config file with a single merge behavior annotation 1`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations Correctly loads a complex config file with merge behavior annotations 1`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a keyed object uses the 'append' merge behavior 1`] = `"Issue in processing configuration file property \\"c\\". Property is not an array, but the inheritance type is set as \\"append\\""`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a keyed object uses the 'append' merge behavior 2`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a merge behavior is specified for an unspecified property 1`] = `"Issue in processing configuration file property \\"$c.mergeBehavior\\". A merge behavior was provided but no matching property was found"`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a merge behavior is specified for an unspecified property 2`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a non-object property uses a merge behavior 1`] = `"Issue in processing configuration file property \\"$l.mergeBehavior\\". A merge behavior was provided for a property that is not an object"`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when a non-object property uses a merge behavior 2`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an array uses the 'merge' merge behavior 1`] = `"Issue in processing configuration file property \\"a\\". Property is not a keyed object, but the inheritance type is set as \\"merge\\""`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an array uses the 'merge' merge behavior 2`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an unsupported merge behavior is specified 1`] = `"Issue in processing configuration file property \\"$a.mergeBehavior\\". An unsupported merge behavior was provided: \\"custom\\""`; - -exports[`ConfigurationFile a complex file with merge behavior annotations throws an error when an unsupported merge behavior is specified 2`] = ` -Object { - "debug": "", - "error": "", - "log": "", - "verbose": "", - "warning": "", -} -`; - exports[`ConfigurationFile error cases Throws an error for a file that doesn't match its schema 1`] = ` "Resolved configuration object does not match schema: Error: JSON validation failed: /src/test/errorCases/invalidType/config.json