From 01653d7c03284f9335edaf719442d30850f40325 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 20 Apr 2021 10:10:58 -0700 Subject: [PATCH] Interpolate generator with plugin name when using default license header (#4073) --- ...ature-generator-name_2021-04-14-21-41.json | 11 ++++ ...ature-generator-name_2021-04-14-21-41.json | 11 ++++ .../extensions/core/src/lib/autorest-core.ts | 2 +- .../lib/context/autorest-context-loader.ts | 8 ++- .../core/src/lib/context/autorest-context.ts | 45 +++++++++++--- .../core/src/lib/pipeline/pipeline.ts | 44 ++++++-------- .../core/src/lib/pipeline/plugin-endpoint.ts | 13 +++-- .../core/src/lib/pipeline/plugin-loader.ts | 58 +++++++++++++++++++ .../extensions/core/src/lib/plugins/index.ts | 3 +- .../resources/default-configuration.md | 17 +++--- 10 files changed, 161 insertions(+), 51 deletions(-) create mode 100644 common/changes/@autorest/configuration/feature-generator-name_2021-04-14-21-41.json create mode 100644 common/changes/@autorest/core/feature-generator-name_2021-04-14-21-41.json create mode 100644 packages/extensions/core/src/lib/pipeline/plugin-loader.ts diff --git a/common/changes/@autorest/configuration/feature-generator-name_2021-04-14-21-41.json b/common/changes/@autorest/configuration/feature-generator-name_2021-04-14-21-41.json new file mode 100644 index 000000000..cd8d671d1 --- /dev/null +++ b/common/changes/@autorest/configuration/feature-generator-name_2021-04-14-21-41.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@autorest/configuration", + "comment": "", + "type": "none" + } + ], + "packageName": "@autorest/configuration", + "email": "tiguerin@microsoft.com" +} \ No newline at end of file diff --git a/common/changes/@autorest/core/feature-generator-name_2021-04-14-21-41.json b/common/changes/@autorest/core/feature-generator-name_2021-04-14-21-41.json new file mode 100644 index 000000000..95a29ed7b --- /dev/null +++ b/common/changes/@autorest/core/feature-generator-name_2021-04-14-21-41.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@autorest/core", + "comment": "**Fix** Default license header containing uninterpolated {generator}", + "type": "patch" + } + ], + "packageName": "@autorest/core", + "email": "tiguerin@microsoft.com" +} \ No newline at end of file diff --git a/packages/extensions/core/src/lib/autorest-core.ts b/packages/extensions/core/src/lib/autorest-core.ts index 695c00ca6..422472fb9 100644 --- a/packages/extensions/core/src/lib/autorest-core.ts +++ b/packages/extensions/core/src/lib/autorest-core.ts @@ -80,7 +80,7 @@ export class AutoRest extends EventEmitter { messageEmitter.Message.Subscribe((cfg, message) => this.Message.Dispatch(message)); const stats = new StatsCollector(); - return (this._view = await new AutorestContextLoader(this.fileSystem, stats, this.configFileOrFolderUri).CreateView( + return (this._view = await new AutorestContextLoader(this.fileSystem, stats, this.configFileOrFolderUri).createView( messageEmitter, includeDefault, ...this._configurations, diff --git a/packages/extensions/core/src/lib/context/autorest-context-loader.ts b/packages/extensions/core/src/lib/context/autorest-context-loader.ts index 93e80ac64..1bcdb606b 100644 --- a/packages/extensions/core/src/lib/context/autorest-context-loader.ts +++ b/packages/extensions/core/src/lib/context/autorest-context-loader.ts @@ -95,7 +95,7 @@ export class AutorestContextLoader { } } - public async CreateView( + public async createView( messageEmitter: MessageEmitter, includeDefault: boolean, ...configs: AutorestRawConfiguration[] @@ -127,7 +127,11 @@ export class AutorestContextLoader { loadedExtensions[definition.fullyQualified] = { extension, autorestExtension: new LazyPromise(async () => - AutoRestExtension.FromChildProcess(definition.name, await extension.start(enableDebugger)), + AutoRestExtension.fromChildProcess( + definition.name, + extension.version, + await extension.start(enableDebugger), + ), ), }; } diff --git a/packages/extensions/core/src/lib/context/autorest-context.ts b/packages/extensions/core/src/lib/context/autorest-context.ts index 114cb8213..f9c8ff820 100644 --- a/packages/extensions/core/src/lib/context/autorest-context.ts +++ b/packages/extensions/core/src/lib/context/autorest-context.ts @@ -19,6 +19,7 @@ import { AutorestCoreLogger } from "./logger"; import { VERSION } from "../constants"; import { StatsCollector } from "../stats"; import { LoggingSession } from "./logging-session"; +import { PipelinePluginDefinition } from "../pipeline/plugin-loader"; export class AutorestContext implements AutorestLogger { public config: AutorestConfiguration; @@ -31,6 +32,7 @@ export class AutorestContext implements AutorestLogger { public messageEmitter: MessageEmitter, public stats: StatsCollector, public asyncLogManager: LoggingSession, + private plugin?: PipelinePluginDefinition, ) { this.config = config; this.logger = new AutorestCoreLogger(config, messageEmitter, asyncLogManager); @@ -115,12 +117,14 @@ export class AutorestContext implements AutorestLogger { public get HeaderText(): string { const h = this.config["header-definitions"]; + + const defaultText = this.getDefaultHeaderText(); switch (this.config["license-header"]?.toLowerCase()) { case "microsoft_mit": - return `${h.microsoft}\n${h.mit}\n${h.default.replace("{core}", VERSION)}\n${h.warning}`; + return `${h.microsoft}\n${h.mit}\n${defaultText}\n${h.warning}`; case "microsoft_apache": - return `${h.microsoft}\n${h.apache}\n${h.default.replace("{core}", VERSION)}\n${h.warning}`; + return `${h.microsoft}\n${h.apache}\n${defaultText}\n${h.warning}`; case "microsoft_mit_no_version": return `${h.microsoft}\n${h.mit}\n${h["no-version"]}\n${h.warning}`; @@ -135,20 +139,26 @@ export class AutorestContext implements AutorestLogger { return ""; case "microsoft_mit_small": - return `${h.microsoft}\n${h["mit-small"]}\n${h.default.replace("{core}", VERSION)}\n${h.warning}`; + return `${h.microsoft}\n${h["mit-small"]}\n${defaultText}\n${h.warning}`; case "microsoft_mit_small_no_codegen": return `${h.microsoft}\n${h["mit-small"]}\n${h["no-version"]}`; case null: case undefined: - return `${h.default.replace("{core}", VERSION)}\n${h.warning}`; + return `${defaultText}\n${h.warning}`; default: return `${this.config["license-header"]}`; } } + private getDefaultHeaderText() { + const extension = this.plugin?.extension; + const generator = extension ? `${extension?.extensionName}@${extension?.extensionVersion}` : ""; + return this.config["header-definitions"].default.replace("{core}", VERSION).replace("{generator}", generator); + } + public IsOutputArtifactRequested(artifact: string): boolean { return From(arrayOf(this.config["output-artifact"])).Contains(artifact); } @@ -178,9 +188,21 @@ export class AutorestContext implements AutorestLogger { return result; } - public *getNestedConfiguration(pluginName: string): Iterable { - for (const nestedConfig of getNestedConfiguration(this.config, pluginName)) { - yield new AutorestContext(nestedConfig, this.fileSystem, this.messageEmitter, this.stats, this.asyncLogManager); + /** + * Get a new configuration that is extended with the properties under the given scope. + * @param scope Name of the nested property to flatten. + * @param plugin Optional plugin requesting this configuration. + */ + public *getNestedConfiguration(scope: string, plugin?: PipelinePluginDefinition): Iterable { + for (const nestedConfig of getNestedConfiguration(this.config, scope)) { + yield new AutorestContext( + nestedConfig, + this.fileSystem, + this.messageEmitter, + this.stats, + this.asyncLogManager, + plugin, + ); } } @@ -190,6 +212,13 @@ export class AutorestContext implements AutorestLogger { */ public extendWith(...overrides: AutorestNormalizedConfiguration[]): AutorestContext { const nestedConfig = extendAutorestConfiguration(this.config, overrides); - return new AutorestContext(nestedConfig, this.fileSystem, this.messageEmitter, this.stats, this.asyncLogManager); + return new AutorestContext( + nestedConfig, + this.fileSystem, + this.messageEmitter, + this.stats, + this.asyncLogManager, + this.plugin, + ); } } diff --git a/packages/extensions/core/src/lib/pipeline/pipeline.ts b/packages/extensions/core/src/lib/pipeline/pipeline.ts index 329c9ab47..c992a2cfc 100644 --- a/packages/extensions/core/src/lib/pipeline/pipeline.ts +++ b/packages/extensions/core/src/lib/pipeline/pipeline.ts @@ -16,17 +16,17 @@ import { PipeState, mergePipeStates, } from "@azure-tools/datastore"; -import { AutorestContext, getExtension } from "../context"; +import { AutorestContext } from "../context"; import { Channel } from "../message"; import { OutstandingTaskAwaiter } from "../outstanding-task-awaiter"; -import { AutoRestExtension } from "./plugin-endpoint"; import { createArtifactEmitterPlugin } from "../plugins/emitter"; -import { createExternalPlugin } from "../plugins/external"; import { createHash } from "crypto"; import { isCached, readCache, writeCache } from "./pipeline-cache"; import { values } from "@azure-tools/linq"; -import { PLUGIN_MAP } from "../plugins"; +import { CORE_PLUGIN_MAP } from "../plugins"; +import { loadPlugins, PipelinePluginDefinition } from "./plugin-loader"; +import { mapValues, omitBy } from "lodash"; const safeEval = createSandbox(); @@ -44,6 +44,7 @@ interface PipelineNode { function buildPipeline( context: AutorestContext, + plugins: { [key: string]: PipelinePluginDefinition }, ): { pipeline: { [name: string]: PipelineNode }; configs: { [jsonPath: string]: AutorestContext } } { const cfgPipeline = context.GetEntry("pipeline"); const pipeline: { [name: string]: PipelineNode } = {}; @@ -89,7 +90,8 @@ function buildPipeline( } // derive information about given pipeline stage - const plugin = cfg.plugin || stageName.split("/").reverse()[0]; + const pluginName = cfg.plugin || stageName.split("/").reverse()[0]; + const plugin = plugins[pluginName]; const outputArtifact = cfg["output-artifact"]; let scope = cfg.scope; if (!cfg.scope) { @@ -120,7 +122,7 @@ function buildPipeline( ) => { if (inputNodes.length === 0) { const config = configCache[stringify(configScope)]; - const configs = scope ? [...config.getNestedConfiguration(scope)] : [config]; + const configs = scope ? [...config.getNestedConfiguration(scope, plugin)] : [config]; for (let i = 0; i < configs.length; ++i) { const newSuffix = configs.length === 1 ? "" : "/" + i; suffixes.push(suffix + newSuffix); @@ -133,7 +135,7 @@ function buildPipeline( } configCache[stringify(path)] = configs[i]; pipeline[stageName + suffix + newSuffix] = { - pluginName: plugin, + pluginName: pluginName, outputArtifact, configScope: path, inputs, @@ -185,19 +187,11 @@ function isDrainRequired(p: PipelineNode) { } export async function runPipeline(configView: AutorestContext, fileSystem: IFileSystem): Promise { - // built-in plugins - - // dynamically loaded, auto-discovered plugins - const __extensionExtension: { [pluginName: string]: AutoRestExtension } = {}; - for (const useExtensionQualifiedName of configView.GetEntry("used-extension") || []) { - const extension = await getExtension(useExtensionQualifiedName); - for (const plugin of await extension.GetPluginNames(configView.CancellationToken)) { - if (!PLUGIN_MAP[plugin]) { - PLUGIN_MAP[plugin] = createExternalPlugin(extension, plugin); - __extensionExtension[plugin] = extension; - } - } - } + const plugins = await loadPlugins(configView); + const __extensionExtension = mapValues( + omitBy(plugins, (x) => x.builtIn), + (x) => x.extension, + ); // __status scope const startTime = Date.now(); @@ -232,8 +226,8 @@ export async function runPipeline(configView: AutorestContext, fileSystem: IFile // TODO: think about adding "number of files in scope" kind of validation in between pipeline steps - const fsInput = configView.DataStore.GetReadThroughScope(fileSystem); - const pipeline = buildPipeline(configView); + const fsInput = configView.DataStore.getReadThroughScope(fileSystem); + const pipeline = buildPipeline(configView, plugins); const times = !!configView.config["timestamp"]; const tasks: { [name: string]: Promise } = {}; @@ -292,12 +286,12 @@ export async function runPipeline(configView: AutorestContext, fileSystem: IFile configEntry?.["null"] === true || values(configView.GetEntry("null")).any((each) => each === pluginName); const plugin = usenull - ? PLUGIN_MAP.null + ? CORE_PLUGIN_MAP.null : passthru - ? PLUGIN_MAP.identity + ? CORE_PLUGIN_MAP.identity : pluginName === "pipeline-emitter" ? pipelineEmitterPlugin - : PLUGIN_MAP[pluginName]; + : plugins[pluginName]?.plugin; if (!plugin) { throw new Error(`Plugin '${pluginName}' not found.`); diff --git a/packages/extensions/core/src/lib/pipeline/plugin-endpoint.ts b/packages/extensions/core/src/lib/pipeline/plugin-endpoint.ts index dac0412e2..64deede5a 100644 --- a/packages/extensions/core/src/lib/pipeline/plugin-endpoint.ts +++ b/packages/extensions/core/src/lib/pipeline/plugin-endpoint.ts @@ -62,17 +62,21 @@ export class AutoRestExtension extends EventEmitter { public static async FromModule(modulePath: string): Promise { const childProc = fork(modulePath, [], { silent: true }); - return AutoRestExtension.FromChildProcess(modulePath, childProc); + return AutoRestExtension.fromChildProcess(modulePath, "", childProc); } - public static async FromChildProcess(extensionName: string, childProc: ChildProcess): Promise { + public static async fromChildProcess( + extensionName: string, + version: string, + childProc: ChildProcess, + ): Promise { if (childProc.stdout === null) { throw new Error("Child Process has no stdout pipe."); } if (childProc.stdin === null) { throw new Error("Child Process has no stdin pipe."); } - const plugin = new AutoRestExtension(extensionName, childProc.stdout, childProc.stdin, childProc); + const plugin = new AutoRestExtension(extensionName, version, childProc.stdout, childProc.stdin, childProc); if (childProc.stderr !== null) { childProc.stderr.pipe(process.stderr); } @@ -91,7 +95,8 @@ export class AutoRestExtension extends EventEmitter { private __inspectTraffic: Array<[number, boolean /*outgoing (core => ext)*/, string]> = []; public constructor( - private extensionName: string, + public extensionName: string, + public extensionVersion: string, reader: Readable, writer: Writable, private childProcess: ChildProcess, diff --git a/packages/extensions/core/src/lib/pipeline/plugin-loader.ts b/packages/extensions/core/src/lib/pipeline/plugin-loader.ts new file mode 100644 index 000000000..b289b1eb4 --- /dev/null +++ b/packages/extensions/core/src/lib/pipeline/plugin-loader.ts @@ -0,0 +1,58 @@ +import { AutorestContext, getExtension } from "../context"; +import { CORE_PLUGIN_MAP } from "../plugins"; +import { createExternalPlugin } from "../plugins/external"; +import { PipelinePlugin } from "./common"; +import { AutoRestExtension } from "./plugin-endpoint"; + +export interface PipelinePluginDefinition { + /** + * Name of the plugin. + */ + readonly name: string; + + /** + * Plugin function. + */ + readonly plugin: PipelinePlugin; + + /** + * Extension defining plugin or undefined if built-in. + */ + readonly extension: AutoRestExtension | undefined; + + /** + * If this plugin is built-in inside of @autorest/core. + */ + readonly builtIn: boolean; +} + +/** + * Resolve all the plugins defined in core and loaded extensions. + * @param context AutorestContext + * @returns Map of plugin name to plugin definition. + */ +export async function loadPlugins( + context: AutorestContext, +): Promise<{ [pluginName: string]: PipelinePluginDefinition }> { + const plugins: { [pluginName: string]: PipelinePluginDefinition } = {}; + + for (const [name, plugin] of Object.entries(CORE_PLUGIN_MAP)) { + plugins[name] = { name, plugin, builtIn: true, extension: undefined }; + } + + for (const useExtensionQualifiedName of context.config["used-extension"] || []) { + const extension = await getExtension(useExtensionQualifiedName); + for (const name of await extension.GetPluginNames(context.CancellationToken)) { + if (!plugins[name]) { + plugins[name] = { + name, + plugin: createExternalPlugin(extension, name), + builtIn: false, + extension: extension, + }; + } + } + } + + return plugins; +} diff --git a/packages/extensions/core/src/lib/plugins/index.ts b/packages/extensions/core/src/lib/plugins/index.ts index a3ed4181e..823537482 100644 --- a/packages/extensions/core/src/lib/plugins/index.ts +++ b/packages/extensions/core/src/lib/plugins/index.ts @@ -32,13 +32,12 @@ import { createApiVersionParameterHandlerPlugin } from "./version-param-handler" import { createJsonToYamlPlugin, createYamlToJsonPlugin } from "./yaml-and-json"; import { createOpenApiSchemaValidatorPlugin, createSwaggerSchemaValidatorPlugin } from "./schema-validation"; import { createOpenAPIStatsCollectorPlugin } from "./openapi-stats-collector"; -import { PipelinePlugin } from "../pipeline/common"; import { QuickDataSource } from "@azure-tools/datastore"; import { createCSharpReflectApiVersionPlugin } from "./metadata-generation"; import { createComponentModifierPlugin } from "./component-modifier"; import { createSemanticValidationPlugin } from "./semantics-validation"; -export const PLUGIN_MAP: { [name: string]: PipelinePlugin } = { +export const CORE_PLUGIN_MAP = { "help": createHelpPlugin(), "identity": createIdentityPlugin(), "null": createNullPlugin(), diff --git a/packages/libs/configuration/resources/default-configuration.md b/packages/libs/configuration/resources/default-configuration.md index d78338929..b8883789f 100644 --- a/packages/libs/configuration/resources/default-configuration.md +++ b/packages/libs/configuration/resources/default-configuration.md @@ -4,7 +4,7 @@ This configuration applies to every run of AutoRest, but with less priority than ## Basic Settings -``` yaml +```yaml azure-arm: false output-folder: generated openapi-type: arm @@ -40,7 +40,7 @@ require: These extra settings are applied when `azure-arm: true`: -``` yaml $(azure-arm) +```yaml $(azure-arm) head-as-boolean: true ``` @@ -49,7 +49,7 @@ head-as-boolean: true If `--legacy` isn't provided, we set the `track2` option to `true` to indicate to V3 generators that the user wants to generate a Track 2 library. -``` yaml !$(legacy) +```yaml !$(legacy) track2: true ``` @@ -57,11 +57,11 @@ track2: true If we don't specify `--help`, we will trigger the setting to load files -``` yaml !$(help) +```yaml !$(help) perform-load: true # kick off loading ``` -``` yaml enableAllVersionsMode() +```yaml enableAllVersionsMode() # when an autorest-v3 generator is loading, and a profile or api-verison is specified, # we need to force the tag: all-api-versions so that it loads the whole api set. # but not TOO high, as then it'll be evaluated before $(pipeline-model) @@ -69,11 +69,11 @@ tag: all-api-versions load-priority: 500 ``` -``` yaml +```yaml header-definitions: warning: Changes may cause incorrect behavior and will be lost if the code is regenerated. - default: 'Code generated by Microsoft (R) AutoRest Code Generator (autorest: {core}, generator: {generator})' + default: "Code generated by Microsoft (R) AutoRest Code Generator (autorest: {core}, generator: {generator})" no-version: Code generated by Microsoft (R) AutoRest Code Generator. @@ -93,5 +93,4 @@ header-definitions: mit-small: Licensed under the MIT License. microsoft: Copyright (c) Microsoft Corporation. All rights reserved. - - +```