[v6] dedupe parameter mappers

src/generators/modelsGenerator.ts

@@ -36,6 +36,7 @@ import {
   getResponseTypeName,
   getAllModelsNames
 } from "./utils/responseTypeUtils";
+import { getParameterDescription } from "../utils/getParameterDescription";
 
 export function generateModels(clientDetails: ClientDetails, project: Project) {
   const modelsIndexFile = project.createSourceFile(
@@ -127,7 +128,10 @@ function writeOptionsParameter(
     operationName,
     optionalParams,
     sourceFile,
-    { mediaTypes: operationRequestMediaTypes }
+    {
+      mediaTypes: operationRequestMediaTypes,
+      operationFullName: operation.fullName
+    }
   );
 }
 
@@ -423,6 +427,11 @@ function writeUniontypes({ objects }: ClientDetails, modelsFile: SourceFile) {
 interface WriteOptionalParametersOptions {
   baseClass?: string;
   mediaTypes?: Set<KnownMediaType>;
+
+  /**
+   * Useful for getting parameter description.
+   */
+  operationFullName?: string;
 }
 
 function getOptionalGroups(
@@ -463,7 +472,7 @@ function writeOptionalParameters(
   operationName: string,
   optionalParams: ParameterDetails[],
   modelsIndexFile: SourceFile,
-  { baseClass, mediaTypes }: WriteOptionalParametersOptions
+  { baseClass, mediaTypes, operationFullName }: WriteOptionalParametersOptions
 ) {
   if (!optionalParams || !optionalParams.length) {
     return;
@@ -487,13 +496,16 @@ function writeOptionalParameters(
           ...optionalGroupDeclarations,
           ...optionalParams
             .filter(p => !p.targetMediaType || p.targetMediaType === mediaType)
-            .map<PropertySignatureStructure>(p => ({
-              name: p.name,
-              hasQuestionToken: true,
-              type: p.typeDetails.typeName,
-              docs: p.description ? [p.description] : undefined,
-              kind: StructureKind.PropertySignature
-            }))
+            .map<PropertySignatureStructure>(p => {
+              const description = getParameterDescription(p, operationFullName);
+              return {
+                name: p.name,
+                hasQuestionToken: true,
+                type: p.typeDetails.typeName,
+                docs: description ? [description] : undefined,
+                kind: StructureKind.PropertySignature
+              };
+            })
         ]
       });
     }
@@ -505,13 +517,16 @@ function writeOptionalParameters(
       extends: [baseClass || "coreHttp.OperationOptions"],
       properties: [
         ...optionalGroupDeclarations,
-        ...optionalParams.map<PropertySignatureStructure>(p => ({
-          name: p.name,
-          hasQuestionToken: true,
-          type: p.typeDetails.typeName,
-          docs: p.description ? [p.description] : undefined,
-          kind: StructureKind.PropertySignature
-        }))
+        ...optionalParams.map<PropertySignatureStructure>(p => {
+          const description = getParameterDescription(p, operationFullName);
+          return {
+            name: p.name,
+            hasQuestionToken: true,
+            type: p.typeDetails.typeName,
+            docs: description ? [description] : undefined,
+            kind: StructureKind.PropertySignature
+          };
+        })
       ]
     });
   }

src/generators/operationGenerator.ts

@@ -45,6 +45,7 @@ import {
   getAllModelsNames,
   getResponseTypeName
 } from "./utils/responseTypeUtils";
+import { getParameterDescription } from "../utils/getParameterDescription";
 
 /**
  * Function that writes the code for all the operations.
@@ -520,7 +521,7 @@ function getOperationParameterSignatures(
 
       const newParameter = {
         name: param.name,
-        description: param.description,
+        description: getParameterDescription(param, operation.fullName),
         type: typeName,
         hasQuestionToken: !param.required,
         isContentType: Boolean(

src/generators/utils/parameterUtils.ts

@@ -60,7 +60,7 @@ export function filterOperationParameters(
     !!(includeGlobalParameters || !param.isGlobal);
 
   const isInOperation = (param: ParameterDetails) =>
-    !!(param.operationsIn && param.operationsIn.includes(operation.fullName));
+    !!(param.operationsIn && param.operationsIn[operation.fullName]);
 
   // We may want to filter out any parameter that is grouped by here.
   // This is so that we can place the group parameter instead.

src/models/parameterDetails.ts

@@ -14,7 +14,7 @@ import { KnownMediaType } from "@azure-tools/codegen";
 export interface ParameterDetails {
   nameRef: string;
   defaultValue?: any;
-  description: string;
+  description?: string;
   name: string;
   serializedName: string;
   location: ParameterLocation;
@@ -23,7 +23,7 @@ export interface ParameterDetails {
   mapper: string | Mapper;
   isGlobal: boolean;
   parameter: Parameter;
-  operationsIn?: string[];
+  operationsIn?: { [operationName: string]: { description: string } };
   collectionFormat?: string;
   schemaType: AllSchemaTypes;
   implementationLocation?: ImplementationLocation;

src/transforms/extensions.ts

@@ -7,7 +7,8 @@ import {
   Parameter,
   StringSchema,
   Protocol,
-  ParameterLocation
+  ParameterLocation,
+  ImplementationLocation
 } from "@azure-tools/codemodel";
 import { cloneOperation } from "../utils/cloneOperation";
 import { extractPaginationDetails } from "../utils/extractPaginationDetails";
@@ -157,10 +158,17 @@ function addPageableMethods(codeModel: CodeModel) {
           },
           protocol: {
             http: httpProtocol
-          }
+          },
+          implementation: ImplementationLocation.Method
         }
       );
-      nextLinkMethod.requests?.[0].addParameter(nextLinkParameter);
+
+      // Ensure all overloads support the nextLink parameter.
+      for (const request of nextLinkMethod.requests ?? []) {
+        const parameters = request.parameters ?? [];
+        parameters.push(nextLinkParameter);
+        request.parameters = parameters;
+      }
 
       operationGroup.addOperation(nextLinkMethod);
     }

src/transforms/operationTransforms.ts

@@ -456,7 +456,7 @@ function getGroupedParameters(
   const operationParams = parameters.filter(p => {
     // Ensure parameters are specific to the operation.
     const matchesOperation =
-      p.operationsIn && p.operationsIn.indexOf(operationFullname) > -1;
+      p.operationsIn && p.operationsIn[operationFullname];
     // Consider the media type as a match if none was provided, or they actually match.
     // This is important when an operation supports multiple media types.
     const matchesMediaType =

src/transforms/parameterTransforms.ts

@@ -216,17 +216,25 @@ export function populateOperationParameters(
 
   const sameNameParams = operationParameters.filter(p => p.name === name);
   description += getSchemaTypeDocumentation(parameter.schema);
+  const isRequired = getParameterRequired(parameter);
 
   const collectionFormat = getCollectionFormat(parameter);
   const typeDetails = getTypeForSchema(parameter.schema);
   const paramDetails: ParameterDetails = {
     nameRef: name,
-    description,
+    description:
+      isRequired && parameter.implementation === ImplementationLocation.Method
+        ? undefined
+        : description,
     name,
     serializedName,
-    operationsIn: [operationName],
+    operationsIn: {
+      [operationName]: {
+        description
+      }
+    },
     location: getParameterLocation(parameter),
-    required: getParameterRequired(parameter),
+    required: isRequired,
     schemaType: parameter.schema.type,
     parameterPath: getParameterPath(parameter),
     mapper: getMapperOrRef(
@@ -255,7 +263,8 @@ export function populateOperationParameters(
     paramDetails,
     operationParameters,
     sameNameParams,
-    operationName
+    operationName,
+    description
   );
 }
 
@@ -388,7 +397,6 @@ function getParameterName(parameter: Parameter) {
  */
 function getComparableParameter({
   name,
-  description,
   serializedName,
   location,
   required,
@@ -404,7 +412,6 @@ function getComparableParameter({
 }: ParameterDetails) {
   return {
     name,
-    description,
     serializedName,
     location,
     required,
@@ -434,7 +441,8 @@ export function disambiguateParameter(
   parameter: ParameterDetails,
   operationParameters: ParameterDetails[],
   sameNameParams: ParameterDetails[],
-  operationName: string
+  operationName: string,
+  description: string
 ) {
   const param = getComparableParameter(parameter);
   const existingParam = sameNameParams.find(p =>
@@ -443,9 +451,9 @@ export function disambiguateParameter(
 
   if (existingParam) {
     if (existingParam.operationsIn) {
-      existingParam.operationsIn.push(operationName);
+      existingParam.operationsIn[operationName] = { description };
     } else {
-      existingParam.operationsIn = [operationName];
+      existingParam.operationsIn = { [operationName]: { description } };
     }
     return;
   } else {

src/utils/getParameterDescription.ts

@@ -0,0 +1,15 @@
+import { ParameterDetails } from "../models/parameterDetails";
+
+export function getParameterDescription(
+  parameter: ParameterDetails,
+  operationName?: string
+): string {
+  let description: string | undefined;
+  if (operationName) {
+    description = parameter.operationsIn?.[operationName].description;
+  }
+  if (!description) {
+    description = parameter.description ?? "";
+  }
+  return description;
+}

test/unit/generators/utils/parameterUtils.spec.ts

@@ -24,7 +24,7 @@ describe("parameterUtils", () => {
     });
 
     it("should include optional parameters", () => {
-      const operationsIn = [operation.fullName];
+      const operationsIn = { [operation.fullName]: { description: "" } };
       const parameters = [
         getParameter({ name: "requiredParam", operationsIn, required: true }),
         getParameter({
@@ -42,7 +42,7 @@ describe("parameterUtils", () => {
     });
 
     it("should not include optional parameters", () => {
-      const operationsIn = [operation.fullName];
+      const operationsIn = { [operation.fullName]: { description: "" } };
       const parameters = [
         getParameter({ name: "requiredParam", operationsIn, required: true }),
         getParameter({
@@ -61,7 +61,7 @@ describe("parameterUtils", () => {
     });
 
     it("should honor includeConstant option", () => {
-      const operationsIn = [operation.fullName];
+      const operationsIn = { [operation.fullName]: { description: "" } };
       const parameters = [
         getParameter({
           name: "constantParameter",
@@ -84,7 +84,7 @@ describe("parameterUtils", () => {
       assert.equal(result.length, 0);
     });
     it("should include honor incluideClientParmeter options", () => {
-      const operationsIn = [operation.fullName];
+      const operationsIn = { [operation.fullName]: { description: "" } };
       const parameters = [
         getParameter({
           name: "clientParameter",
@@ -108,7 +108,7 @@ describe("parameterUtils", () => {
     });
 
     it("should honor includeGlobalParameters", () => {
-      const operationsIn = [operation.fullName];
+      const operationsIn = { [operation.fullName]: { description: "" } };
       const parameters = [
         getParameter({
           name: "globalParameter",
@@ -134,11 +134,11 @@ describe("parameterUtils", () => {
       const parameters = [
         getParameter({
           name: "foreignParameter",
-          operationsIn: ["some_other_operation"]
+          operationsIn: { ["some_other_operation"]: { description: "" } }
         }),
         getParameter({
           name: "goodParameter",
-          operationsIn: [operation.fullName]
+          operationsIn: { [operation.fullName]: { description: "" } }
         })
       ];
 
@@ -168,7 +168,7 @@ const getParameter = ({
     "",
     new StringSchema("mock_string", "")
   ),
-  operationsIn = [],
+  operationsIn = {},
   implementationLocation = ImplementationLocation.Method
 }: Partial<ParameterDetails & { sufix: string }>): ParameterDetails => ({
   nameRef: nameRef || `MockParameter${sufix}`,

test/unit/transforms/operationTransforms.spec.ts

@@ -214,7 +214,9 @@ describe("OperationTransforms", () => {
         const operationSpec = transformOperationSpec(operationDetails, [
           {
             nameRef: "MockOperation",
-            operationsIn: ["MockOperationGroup_getNull"],
+            operationsIn: {
+              ["MockOperationGroup_getNull"]: { description: "" }
+            },
             parameterPath: "mockOperation",
             isGlobal: false,
             mapper: "",

test/unit/transforms/parameterTransforms.spec.ts

@@ -171,8 +171,12 @@ describe("parameterTransforms", () => {
         p => p.nameRef === "mockParam1"
       )!;
 
-      assert.deepEqual(p1.operationsIn, ["OperationGroup1_operation1"]);
-      assert.deepEqual(p2.operationsIn, ["OperationGroup2_operation2"]);
+      assert.deepEqual(p1.operationsIn, {
+        OperationGroup1_operation1: { description: "" }
+      });
+      assert.deepEqual(p2.operationsIn, {
+        OperationGroup2_operation2: { description: "" }
+      });
       assert.equal(p1.parameter, param1);
       assert.equal(p2.parameter, param2);
     });
@@ -245,10 +249,10 @@ describe("parameterTransforms", () => {
         p => p.nameRef === "mockParam1"
       )!;
 
-      assert.deepEqual(p1.operationsIn, [
-        "OperationGroup1_operation1",
-        "OperationGroup2_operation2"
-      ]);
+      assert.deepEqual(p1.operationsIn, {
+        OperationGroup1_operation1: { description: "" },
+        OperationGroup2_operation2: { description: "" }
+      });
       assert.equal(p1.parameter, param1);
     });
 
@@ -321,8 +325,12 @@ describe("parameterTransforms", () => {
         p => p.nameRef === "mockParam2"
       )!;
 
-      assert.deepEqual(p1.operationsIn, ["OperationGroup1_operation1"]);
-      assert.deepEqual(p2.operationsIn, ["OperationGroup1_operation1"]);
+      assert.deepEqual(p1.operationsIn, {
+        OperationGroup1_operation1: { description: "" }
+      });
+      assert.deepEqual(p2.operationsIn, {
+        OperationGroup1_operation1: { description: "" }
+      });
 
       assert.equal(p1.parameter, param1);
       assert.equal(p2.parameter, param2);