Change: Single value enum default to be extensible (#4184)

* Single value enum extensible by default

* Changes

* Wi

* Fix

* Fix

common/changes/@autorest/modelerfour/feature-single-value-enum-sealed_2021-06-24-18-09.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/modelerfour",
+      "comment": "**Change** Single-value enums are extensible by default and will not generate a constant ",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@autorest/modelerfour",
+  "email": "tiguerin@microsoft.com"
+}

packages/extensions/modelerfour/readme.md

@@ -107,6 +107,10 @@ modelerfour:
   # This is a temporary flag to smooth transition. It WILL be removed in a future version.
   treat-type-object-as-anything: false|true
 
+  # **TEMPORARY FLAG DO NOT DEPEND ON IT**
+  # Enable older inconsistent behavior that an enum with a single value would become a constant by default.
+  seal-single-value-enum-by-default: false|true
+
   # customization of the identifier normalization and naming provided by the prenamer.
   # pascal|pascalcase - MultiWordIdentifier
   # camel|camelcase - multiWordIdentifier

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

@@ -49,6 +49,11 @@ export interface ModelerFourOptions {
    */
   "treat-type-object-as-anything"?: boolean;
 
+  /**
+   * Enable older inconsistent behavior that an enum with a single value would become a constant by default.
+   */
+  "seal-single-value-enum-by-default"?: boolean;
+
   /**
    * List of header names that shouldn't be included in the codemodel.
    * Those header would already be handled by the generator.

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

@@ -713,8 +713,19 @@ export class ModelerFour {
     const type = this.getPrimitiveSchemaForEnum(schema);
     const choices = [...parentChoices, ...this.interpret.getEnumChoices(schema)];
 
+    if (this.options["seal-single-value-enum-by-default"]) {
+      this.session.warning(
+        "`seal-single-value-enum-by-default` is a temporary flag that **WILL** be removed in the future. Please change the spec to add x-ms-enum.modelAsString=false for enums with this issue.",
+        ["Deprecated"],
+      );
+    }
+
+    const singleValueEnumSealed = this.options["seal-single-value-enum-by-default"]
+      ? !alwaysSeal && xmse?.modelAsString !== true
+      : !alwaysSeal && sealed;
+
     // model as string forces it to be a choice/enum.
-    if (!alwaysSeal && xmse?.modelAsString !== true && choices.length === 1) {
+    if (singleValueEnumSealed && choices.length === 1) {
       const constVal = choices[0].value;
 
       return this.codeModel.schemas.add(

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

@@ -1,6 +1,7 @@
 import { JsonType } from "@azure-tools/openapi";
 import assert from "assert";
-import { addSchema, createTestSpec, findByName } from "../utils";
+import { ChoiceSchema, ConstantSchema, SealedChoiceSchema } from "../../../../libs/codemodel/dist/exports";
+import { addSchema, assertSchema, createTestSpec, findByName } from "../utils";
 import { runModeler } from "./modelerfour-utils";
 
 describe("Modelerfour.Schemas", () => {
@@ -214,6 +215,131 @@ describe("Modelerfour.Schemas", () => {
       expect(foo?.choiceType.type).toEqual("string");
       expect(foo?.choices.map((x) => x.value)).toEqual(["one", "two"]);
     });
+
+    it("creates an ChoiceSchema by default for single value enums", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "Foo", {
+        enum: ["one"],
+      });
+
+      const codeModel = await runModeler(spec);
+      const foo = findByName("Foo", codeModel.schemas.choices);
+      expect(foo).toBeInstanceOf(ChoiceSchema);
+    });
+
+    it("creates an Constant by default for single value enums if `seal-single-value-enum-by-default` is ON", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "Foo", {
+        enum: ["one"],
+      });
+
+      const codeModel = await runModeler(spec, {
+        modelerfour: {
+          "seal-single-value-enum-by-default": true,
+        },
+      });
+      expect(findByName("Foo", codeModel.schemas.choices)).toBeUndefined();
+      const foo = findByName("Foo", codeModel.schemas.constants);
+      expect(foo).toBeInstanceOf(ConstantSchema);
+    });
+
+    it("creates an Constant if enum is marked modelAsString=false for single value enums", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "Foo", {
+        "enum": ["one"],
+        "x-ms-enum": {
+          modelAsString: false,
+        },
+      });
+
+      const codeModel = await runModeler(spec);
+      expect(findByName("Foo", codeModel.schemas.choices)).toBeUndefined();
+      const foo = findByName("Foo", codeModel.schemas.constants);
+      expect(foo).toBeInstanceOf(ConstantSchema);
+    });
+
+    it("creates an ChoiceSchema by default for multi value enums", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "Foo", {
+        enum: ["one", "two"],
+      });
+
+      const codeModel = await runModeler(spec);
+      const foo = findByName("Foo", codeModel.schemas.choices);
+      expect(foo).toBeInstanceOf(ChoiceSchema);
+    });
+
+    it("creates an SealedChoice if enum is marked modelAsString=false for multu value enums", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "Foo", {
+        "enum": ["one", "two"],
+        "x-ms-enum": {
+          modelAsString: false,
+        },
+      });
+
+      const codeModel = await runModeler(spec);
+      expect(findByName("Foo", codeModel.schemas.choices)).toBeUndefined();
+      const foo = findByName("Foo", codeModel.schemas.sealedChoices);
+      expect(foo).toBeInstanceOf(SealedChoiceSchema);
+    });
+
+    it("always-seal-x-ms-enum configuration produces SealedChoiceSchema for all x-ms-enums", async () => {
+      const spec = createTestSpec();
+
+      addSchema(spec, "ModelAsString", {
+        "type": "string",
+        "enum": ["Apple", "Orange"],
+        "x-ms-enum": {
+          modelAsString: true,
+        },
+      });
+
+      addSchema(spec, "ShouldBeSealed", {
+        "type": "string",
+        "enum": ["Apple", "Orange"],
+        "x-ms-enum": {
+          modelAsString: false,
+        },
+      });
+
+      addSchema(spec, "SingleValueEnum", {
+        "type": "string",
+        "enum": ["Apple"],
+        "x-ms-enum": {
+          modelAsString: false,
+        },
+      });
+
+      const codeModelWithoutSetting = await runModeler(spec, {
+        modelerfour: {
+          "always-seal-x-ms-enums": false,
+        },
+      });
+
+      assertSchema("ModelAsString", codeModelWithoutSetting.schemas.choices, (s) => s.choiceType.type, "string");
+
+      assertSchema("ShouldBeSealed", codeModelWithoutSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
+
+      assertSchema("SingleValueEnum", codeModelWithoutSetting.schemas.constants, (s) => s.valueType.type, "string");
+
+      const codeModelWithSetting = await runModeler(spec, {
+        modelerfour: {
+          "always-seal-x-ms-enums": true,
+        },
+      });
+
+      assertSchema("ModelAsString", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
+
+      assertSchema("ShouldBeSealed", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
+
+      assertSchema("SingleValueEnum", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
+    });
   });
 
   describe("Deprecation", () => {

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

@@ -4,25 +4,18 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 import { Parameter, SchemaResponse, ConstantSchema, SealedChoiceSchema } from "@autorest/codemodel";
-import { addOperation, addSchema, createTestSpec, findByName, InitialTestSpec, response, responses } from "../utils";
+import {
+  addOperation,
+  addSchema,
+  assertSchema,
+  createTestSpec,
+  findByName,
+  InitialTestSpec,
+  response,
+  responses,
+} from "../utils";
 import { runModeler } from "./modelerfour-utils";
 
-function assertSchema(
-  schemaName: string,
-  schemaList: Array<any> | undefined,
-  accessor: (schema: any) => any,
-  expected: any,
-) {
-  expect(schemaList).not.toBeFalsy();
-
-  // We've already asserted, but make the compiler happy
-  if (schemaList) {
-    const schema = findByName(schemaName, schemaList);
-    expect(schema).not.toBeFalsy();
-    expect(accessor(schema)).toEqual(expected);
-  }
-}
-
 describe("Modeler", () => {
   it("preserves 'info' metadata", async () => {
     const spec = createTestSpec();
@@ -183,29 +176,6 @@ describe("Modeler", () => {
     assertSchema("Int64", codeModel.schemas.numbers, (s) => s.precision, 64);
   });
 
-  it("modelAsString=true creates ChoiceSchema for single-value enum", async () => {
-    const spec = createTestSpec();
-
-    addSchema(spec, "ShouldBeConstant", {
-      type: "string",
-      enum: ["html_strip"],
-    });
-
-    addSchema(spec, "ShouldBeChoice", {
-      "type": "string",
-      "enum": ["html_strip"],
-      "x-ms-enum": {
-        modelAsString: true,
-      },
-    });
-
-    const codeModel = await runModeler(spec);
-
-    assertSchema("ShouldBeConstant", codeModel.schemas.constants, (s) => s.value.value, "html_strip");
-
-    assertSchema("ShouldBeChoice", codeModel.schemas.choices, (s) => s.choices[0].value, "html_strip");
-  });
-
   it("propagates 'nullable' to properties, parameters, collections, and responses", async () => {
     const spec = createTestSpec();
 
@@ -682,58 +652,6 @@ describe("Modeler", () => {
     expect(existingAcceptParam!.origin).toEqual(undefined);
   });
 
-  it("always-seal-x-ms-enum configuration produces SealedChoiceSchema for all x-ms-enums", async () => {
-    const spec = createTestSpec();
-
-    addSchema(spec, "ModelAsString", {
-      "type": "string",
-      "enum": ["Apple", "Orange"],
-      "x-ms-enum": {
-        modelAsString: true,
-      },
-    });
-
-    addSchema(spec, "ShouldBeSealed", {
-      "type": "string",
-      "enum": ["Apple", "Orange"],
-      "x-ms-enum": {
-        modelAsString: false,
-      },
-    });
-
-    addSchema(spec, "SingleValueEnum", {
-      "type": "string",
-      "enum": ["Apple"],
-      "x-ms-enum": {
-        modelAsString: false,
-      },
-    });
-
-    const codeModelWithoutSetting = await runModeler(spec, {
-      modelerfour: {
-        "always-seal-x-ms-enums": false,
-      },
-    });
-
-    assertSchema("ModelAsString", codeModelWithoutSetting.schemas.choices, (s) => s.choiceType.type, "string");
-
-    assertSchema("ShouldBeSealed", codeModelWithoutSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
-
-    assertSchema("SingleValueEnum", codeModelWithoutSetting.schemas.constants, (s) => s.valueType.type, "string");
-
-    const codeModelWithSetting = await runModeler(spec, {
-      modelerfour: {
-        "always-seal-x-ms-enums": true,
-      },
-    });
-
-    assertSchema("ModelAsString", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
-
-    assertSchema("ShouldBeSealed", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
-
-    assertSchema("SingleValueEnum", codeModelWithSetting.schemas.sealedChoices, (s) => s.choiceType.type, "string");
-  });
-
   it("allows header parameters with 'x-ms-api-version: true' to become full api-version parameters", async () => {
     const spec = createTestSpec();
 

packages/extensions/modelerfour/test/utils/codemodel-utils.ts

@@ -9,3 +9,19 @@ export interface NamedItem {
 export function findByName<T extends NamedItem>(name: string, items: T[] | undefined): T | undefined {
   return items && items.find((x) => x.language.default.name === name);
 }
+
+export function assertSchema(
+  schemaName: string,
+  schemaList: Array<any> | undefined,
+  accessor: (schema: any) => any,
+  expected: any,
+) {
+  expect(schemaList).not.toBeFalsy();
+
+  // We've already asserted, but make the compiler happy
+  if (schemaList) {
+    const schema = findByName(schemaName, schemaList);
+    expect(schema).not.toBeFalsy();
+    expect(accessor(schema)).toEqual(expected);
+  }
+}

regression-tests/regression-compare.yaml

@@ -71,6 +71,7 @@ languages:
       - --use:../packages/extensions/modelerfour
       - --use:@autorest/python@5.7.0
       - --modelerfour.treat-type-object-as-anything
+      - --modelerfour.seal-single-value-enum-by-default
 
   - language: typescript
     outputPath: ./output/typescript
@@ -89,4 +90,6 @@ languages:
       - --modelerfour.treat-type-object-as-anything
       - --package-name:test-package
       - --title:TestClient
+      - --modelerfour.seal-single-value-enum-by-default
       - --use:@autorest/typescript@6.0.0-beta.5
+