Improve name is empty error and source maps (#4555)

common/changes/@autorest/core/improve-name-empty-error_2022-06-09-19-03.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/core",
+      "comment": "Enable identity sourcemap for extensions",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@autorest/core"
+}

common/changes/@autorest/extension-base/improve-name-empty-error_2022-06-09-19-03.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/extension-base",
+      "comment": "Enable identity sourcemap for extensions",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@autorest/extension-base"
+}

common/changes/@autorest/modelerfour/improve-name-empty-error_2022-06-09-19-03.json

@@ -0,0 +1,15 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/modelerfour",
+      "comment": "Errors emitted in 'modelerfour' step  will have sourcemap.",
+      "type": "minor"
+    },
+    {
+      "packageName": "@autorest/modelerfour",
+      "comment": "Improved 'name is empty' error to contain more information.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@autorest/modelerfour"
+}

packages/extensions/core/src/lib/pipeline/plugin-endpoint.ts

@@ -6,7 +6,15 @@
 import { ChildProcess } from "child_process";
 import { Readable, Writable } from "stream";
 import { Exception } from "@autorest/common";
-import { DataHandle, DataSink, DataSource, LazyPromise, Mapping, PathPosition } from "@azure-tools/datastore";
+import {
+  DataHandle,
+  DataSink,
+  DataSource,
+  IdentityPathMappings,
+  LazyPromise,
+  Mapping,
+  PathPosition,
+} from "@azure-tools/datastore";
 import { ensureIsFolderUri } from "@azure-tools/uri";
 import { RawSourceMap } from "source-map";
 import { CancellationToken, createMessageConnection } from "vscode-jsonrpc";
@@ -270,11 +278,12 @@ export class AutoRestExtension extends EventEmitter {
       filename: string,
       content: string,
       artifactType?: string,
-      sourceMap?: Array<Mapping> | RawSourceMap,
+      sourceMap?: Mapping[] | RawSourceMap | { type: "identity"; source: string },
     ): Promise<Artifact> => {
       if (!sourceMap) {
         sourceMap = [];
       }
+
       // TODO: transform mappings so friendly names are replaced by internals
       let handle: DataHandle;
       if (typeof (<any>sourceMap).mappings === "string") {
@@ -290,7 +299,10 @@ export class AutoRestExtension extends EventEmitter {
       } else {
         onFile(
           (handle = await sink.writeData(filename, content, ["fix-me-here2"], artifactType, {
-            pathMappings: sourceMap as any,
+            pathMappings:
+              "type" in sourceMap && sourceMap.type === "identity"
+                ? new IdentityPathMappings((await friendly2internal(sourceMap.source)) ?? sourceMap.source)
+                : <any>sourceMap,
           })),
         );
       }

packages/extensions/modelerfour/src/modeler/modelerfour.ts

@@ -59,7 +59,7 @@ import {
   AnyObjectSchema,
 } from "@autorest/codemodel";
 import { Session, Channel } from "@autorest/extension-base";
-import { fail, minimum, pascalCase, KnownMediaType } from "@azure-tools/codegen";
+import { fail, minimum, pascalCase, KnownMediaType, shadowPosition } from "@azure-tools/codegen";
 import {
   Model as oai3,
   Dereferenced,
@@ -71,6 +71,7 @@ import {
   NumberFormat,
   MediaType,
   omitXDashProperties,
+  OpenAPI3Document,
 } from "@azure-tools/openapi";
 import * as OpenAPI from "@azure-tools/openapi";
 import { uniq, every } from "lodash";
@@ -147,7 +148,6 @@ export class ModelerFour {
   private apiVersionParameter!: "choice" | "constant" | undefined;
   private useModelNamespace!: boolean | undefined;
   private profileFilter!: Array<string>;
-  private apiVersionFilter!: Array<string>;
   private schemaCache = new ProcessingCache((schema: OpenAPI.Schema, name: string) =>
     this.processSchemaImpl(schema, name),
   );
@@ -158,8 +158,8 @@ export class ModelerFour {
   private ignoreHeaders: Set<string> = new Set();
   private specialHeaders: Set<string> = new Set();
 
-  constructor(protected session: Session<oai3>) {
-    this.input = session.model; //shadowPosition(session.model);
+  constructor(protected session: Session<OpenAPI3Document>) {
+    this.input = session.model; // shadowPosition(session.model);
 
     const i = this.input.info;
 
@@ -268,7 +268,6 @@ export class ModelerFour {
     }
 
     this.profileFilter = await this.session.getValue("profile", []);
-    this.apiVersionFilter = await this.session.getValue("api-version", []);
     this.ignoreHeaders = new Set(this.options["ignore-headers"] ?? []);
     this.specialHeaders = new Set(
       KnownSpecialHeaders.filter((x) => this.options["skip-special-headers"]?.map((x) => x.toLowerCase())?.includes(x)),

packages/extensions/modelerfour/src/prenamer/naming-utils.ts

@@ -35,41 +35,51 @@ interface SetNameOptions {
   nameEmptyErrorMessage?: string;
 }
 
-const setNameDefaultOptions: SetNameOptions = Object.freeze({
+const setNameDefaultOptions = Object.freeze({
   removeDuplicates: true,
-  nameEmptyErrorMessage: `Name cannot be empty.`,
 });
 
 export interface Nameable {
   language: Languages;
 }
 
-export function setName(
-  thing: Nameable,
-  styler: Styler,
-  defaultValue: string,
-  overrides: Record<string, string>,
-  options?: SetNameOptions,
-) {
-  setNameAllowEmpty(thing, styler, defaultValue, overrides, options);
-  if (!thing.language.default.name) {
-    throw new Error(options?.nameEmptyErrorMessage ?? setNameDefaultOptions.nameEmptyErrorMessage);
+function getNameEmptyError(thing: Nameable): string {
+  if (thing.language.default.serializedName) {
+    return `Name for '${thing.constructor.name}' with serializedName '${thing.language.default.serializedName}' cannot be empty.`;
   }
+  return `Name for '${thing.constructor.name}' cannot be empty.`;
 }
 
-export function setNameAllowEmpty(
-  thing: Nameable,
-  styler: Styler,
-  defaultValue: string,
-  overrides: Record<string, string>,
-  options?: SetNameOptions,
-) {
-  options = { ...setNameDefaultOptions, ...options };
-  thing.language.default.name = styler(
-    defaultValue && isUnassigned(thing.language.default.name) ? defaultValue : thing.language.default.name,
-    options.removeDuplicates,
-    overrides,
-  );
+export class NamingService {
+  public constructor(private session: Session<unknown>) {}
+
+  public setName(
+    thing: Nameable,
+    styler: Styler,
+    defaultValue: string,
+    overrides: Record<string, string>,
+    options?: SetNameOptions,
+  ) {
+    this.setNameAllowEmpty(thing, styler, defaultValue, overrides, options);
+    if (!thing.language.default.name) {
+      this.session.error(options?.nameEmptyErrorMessage ?? getNameEmptyError(thing), ["Prenamer", "NameEmpty"], thing);
+    }
+  }
+
+  public setNameAllowEmpty(
+    thing: Nameable,
+    styler: Styler,
+    defaultValue: string,
+    overrides: Record<string, string>,
+    options?: SetNameOptions,
+  ) {
+    options = { ...setNameDefaultOptions, ...options };
+    thing.language.default.name = styler(
+      defaultValue && isUnassigned(thing.language.default.name) ? defaultValue : thing.language.default.name,
+      options.removeDuplicates,
+      overrides,
+    );
+  }
 }
 
 export function isUnassigned(value: string) {

packages/extensions/modelerfour/src/prenamer/plugin-prenamer.ts

@@ -21,6 +21,9 @@ export async function processRequest(host: AutorestExtensionHost) {
     // go!
     const result = plugin.process();
 
+    // throw on errors.
+    session.checkpoint();
+
     // output the model to the pipeline
     if (options["emit-yaml-tags"] !== false) {
       host.writeFile({

packages/extensions/modelerfour/src/prenamer/prenamer.ts

@@ -16,10 +16,10 @@ import {
   SealedChoiceSchema,
 } from "@autorest/codemodel";
 import { Session } from "@autorest/extension-base";
-import { selectName, Style } from "@azure-tools/codegen";
+import { selectName, shadowPosition, Style } from "@azure-tools/codegen";
 import { partition } from "lodash";
 import { ModelerFourOptions } from "../modeler/modelerfour-options";
-import { getNameOptions, isUnassigned, ScopeNamer, setName, setNameAllowEmpty } from "./naming-utils";
+import { getNameOptions, isUnassigned, ScopeNamer, NamingService } from "./naming-utils";
 
 export class PreNamer {
   codeModel: CodeModel;
@@ -43,8 +43,12 @@ export class PreNamer {
 
   enum = 0;
   constant = 0;
+
+  private namingService: NamingService;
+
   constructor(protected session: Session<CodeModel>) {
-    this.codeModel = session.model; // shadow(session.model, filename);
+    this.codeModel = shadowPosition(session.model);
+    this.namingService = new NamingService(session);
   }
 
   async init() {
@@ -92,38 +96,38 @@ export class PreNamer {
 
     // constant
     for (const schema of values(this.codeModel.schemas.constants)) {
-      setName(schema, this.format.constant, `Constant${this.enum++}`, this.format.override);
+      this.namingService.setName(schema, this.format.constant, `Constant${this.enum++}`, this.format.override);
     }
 
     // strings
     for (const schema of values(this.codeModel.schemas.strings)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     // number
     for (const schema of values(this.codeModel.schemas.numbers)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.dates)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
     for (const schema of values(this.codeModel.schemas.dateTimes)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
     for (const schema of values(this.codeModel.schemas.durations)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
     for (const schema of values(this.codeModel.schemas.uuids)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.uris)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.unixtimes)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
 
       if (isUnassigned(schema.language.default.description)) {
         schema.language.default.description = "date in seconds since 1970-01-01T00:00:00Z.";
@@ -131,24 +135,24 @@ export class PreNamer {
     }
 
     for (const schema of values(this.codeModel.schemas.byteArrays)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.chars)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.booleans)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     for (const schema of values(this.codeModel.schemas.flags)) {
-      setName(schema, this.format.type, schema.type, this.format.override);
+      this.namingService.setName(schema, this.format.type, schema.type, this.format.override);
     }
 
     // dictionary
     for (const schema of values(this.codeModel.schemas.dictionaries)) {
-      setName(
+      this.namingService.setName(
         schema,
         this.format.type,
         `DictionaryOf${schema.elementType.language.default.name}`,
@@ -160,7 +164,12 @@ export class PreNamer {
     }
 
     for (const schema of values(this.codeModel.schemas.arrays)) {
-      setName(schema, this.format.type, `ArrayOf${schema.elementType.language.default.name}`, this.format.override);
+      this.namingService.setName(
+        schema,
+        this.format.type,
+        `ArrayOf${schema.elementType.language.default.name}`,
+        this.format.override,
+      );
       if (isUnassigned(schema.language.default.description)) {
         schema.language.default.description = `Array of ${schema.elementType.language.default.name}`;
       }
@@ -184,22 +193,28 @@ export class PreNamer {
       scopeNamer.add(schema, this.format.type, "");
 
       for (const property of values(schema.properties)) {
-        setName(property, this.format.property, "", this.format.override);
+        this.namingService.setName(property, this.format.property, "", this.format.override);
       }
     }
 
     for (const parameter of values(this.codeModel.globalParameters)) {
       if (parameter.schema.type === SchemaType.Constant) {
-        setName(parameter, this.format.constantParameter, "", this.format.override);
+        this.namingService.setName(parameter, this.format.constantParameter, "", this.format.override);
       } else {
-        setName(parameter, this.format.parameter, "", this.format.override);
+        this.namingService.setName(parameter, this.format.parameter, "", this.format.override);
       }
     }
 
     for (const operationGroup of this.codeModel.operationGroups) {
-      setNameAllowEmpty(operationGroup, this.format.operationGroup, operationGroup.$key, this.format.override, {
-        removeDuplicates: false,
-      });
+      this.namingService.setNameAllowEmpty(
+        operationGroup,
+        this.format.operationGroup,
+        operationGroup.$key,
+        this.format.override,
+        {
+          removeDuplicates: false,
+        },
+      );
       const operationScopeNamer = new ScopeNamer(this.session, {
         overrides: this.format.override,
       });
@@ -231,7 +246,7 @@ export class PreNamer {
     scopeNamer.process();
 
     // set a styled client name
-    setName(this.codeModel, this.format.client, this.codeModel.info.title, this.format.override);
+    this.namingService.setName(this.codeModel, this.format.client, this.codeModel.info.title, this.format.override);
 
     // fix collisions from flattening on ObjectSchemas
     this.fixPropertyCollisions();
@@ -247,7 +262,7 @@ export class PreNamer {
       scopeNamer.add(schema, this.format.choice, `Enum${this.enum++}`);
 
       for (const choice of values(schema.choices)) {
-        setName(choice, this.format.choiceValue, "", this.format.override, {
+        this.namingService.setName(choice, this.format.choiceValue, "", this.format.override, {
           removeDuplicates: false,
           nameEmptyErrorMessage: `Enum '${schema.language.default.name}' cannot have a value '${choice.value}' that result in an empty name. Use x-ms-enum.values to specify the name of the values.`,
         });
@@ -258,20 +273,20 @@ export class PreNamer {
   private setParameterNames(parameterContainer: Operation | Request) {
     for (const parameter of values(parameterContainer.signatureParameters)) {
       if (parameter.schema.type === SchemaType.Constant) {
-        setName(parameter, this.format.constantParameter, "", this.format.override);
+        this.namingService.setName(parameter, this.format.constantParameter, "", this.format.override);
       } else {
-        setName(parameter, this.format.parameter, "", this.format.override);
+        this.namingService.setName(parameter, this.format.parameter, "", this.format.override);
       }
     }
     for (const parameter of values(parameterContainer.parameters)) {
       if ((parameterContainer.signatureParameters ?? []).indexOf(parameter) === -1) {
         if (parameter.schema.type === SchemaType.Constant) {
-          setName(parameter, this.format.constantParameter, "", this.format.override);
+          this.namingService.setName(parameter, this.format.constantParameter, "", this.format.override);
         } else {
           if (parameter.implementation === ImplementationLocation.Client) {
-            setName(parameter, this.format.global, "", this.format.override);
+            this.namingService.setName(parameter, this.format.global, "", this.format.override);
           } else {
-            setName(parameter, this.format.local, "", this.format.override);
+            this.namingService.setName(parameter, this.format.local, "", this.format.override);
           }
         }
       }
@@ -281,7 +296,12 @@ export class PreNamer {
   private setResponseHeaderNames(response: Response) {
     if (response.protocol.http) {
       for (const header of Object.values(response.protocol.http.headers ?? {})) {
-        setName(header as { language: Languages }, this.format.responseHeader, "", this.format.override);
+        this.namingService.setName(
+          header as { language: Languages },
+          this.format.responseHeader,
+          "",
+          this.format.override,
+        );
       }
     }
   }

packages/extensions/modelerfour/src/quality-precheck/prechecker.ts

@@ -37,11 +37,13 @@ export async function processRequest(host: AutorestExtensionHost) {
       filename: "prechecked-openapi-document.yaml",
       content: JSON.stringify(result, null, 2),
       artifactType: "prechecked-openapi-document",
+      sourceMap: { type: "identity", source: session.filename },
     });
     host.writeFile({
       filename: "original-openapi-document.yaml",
       content: JSON.stringify(input, null, 2),
       artifactType: "openapi-document",
+      sourceMap: { type: "identity", source: session.filename },
     });
   } catch (error: any) {
     if (debug) {

packages/extensions/modelerfour/test/modeler/modelerfour.test.ts

@@ -30,16 +30,6 @@ describe("Modeler", () => {
   });
 
   it("tracks schema usage", async () => {
-    const testSchema = {
-      type: "object",
-      properties: {
-        "prop-one": {
-          type: "integer",
-          format: "int32",
-        },
-      },
-    };
-
     const spec = createTestSpec();
 
     addSchema(spec, "Input", {

packages/libs/codegen/src/source-track.ts

@@ -99,8 +99,16 @@ export interface ProxyPosition {
 
 export type ShadowedObject<T> = T & ProxyPosition;
 
-export function shadowPosition<T extends object>(source: T, path: Array<string | number> = []): ShadowedObject<T> {
-  return new Proxy<ShadowedObject<T>>(source as any, {
+export function shadowPosition<T extends object>(
+  source: T,
+  cache: WeakMap<any, any> = new WeakMap(),
+  path: Array<string | number> = [],
+): ShadowedObject<T> {
+  const cached = cache.get(source);
+  if (cached) {
+    return cached;
+  }
+  const proxy = new Proxy<ShadowedObject<T>>(source as any, {
     get(target: any, p: PropertyKey) {
       if (p === ShadowedNodePath) {
         // they want the source location for this node.
@@ -118,15 +126,18 @@ export function shadowPosition<T extends object>(source: T, path: Array<string |
         case "number":
           return value;
         case "array":
-          return shadowPosition(value, [...path, key]);
+          return shadowPosition(value, cache, [...path, key]);
         case "object":
-          return shadowPosition(value, [...path, key]);
+          return shadowPosition(value, cache, [...path, key]);
 
         default:
           throw new Error(`Unhandled shadow of type '${typeOf(value)}' `);
       }
     },
   });
+
+  cache.set(source, proxy);
+  return proxy;
 }
 
 function getKey(p: PropertyKey) {

packages/libs/extension-base/src/extension-host.ts

@@ -18,6 +18,18 @@ namespace IAutoRestPluginInitiatorTypes {
   export const Message = new NotificationType2<string, Message, void>("Message");
 }
 
+/**
+ * Sepecify that the source file and current file is a 1:1 mapping.
+ */
+export interface IdentitySourceMap {
+  type: "identity";
+
+  /**1
+   * Name of the file that this file is mapping to.
+   */
+  source: string;
+}
+
 export interface WriteFileOptions {
   /**
    * @param filename Name of the file.
@@ -32,7 +44,7 @@ export interface WriteFileOptions {
   /**
    * @param sourceMap Source map that can be used to trace back source position in case of error.
    */
-  sourceMap?: Mapping[] | RawSourceMap;
+  sourceMap?: Mapping[] | RawSourceMap | IdentitySourceMap;
 
   /**
    * @param artifactType Artifact type