Repository: lukeautry/tsoa (ts)

Issue:
Empty string in Enum got converted to '0'
When using empty string `''` in string Enum used a input parameter, it got converted as `'0'`.

So imagine an enum like:

```ts
export enum MyThings {
    Undefined = '',
    Something = 'Something',
    Otherwise = 'Another Something',
```

## Sorting

* **I'm submitting a ...**
  - [X] bug report

* I confirm that I
  - [X] used the [search](https://github.com/lukeautry/tsoa/search?type=Issues) to make sure that a similar issue hasn't already been submit

## Expected Behavior

I would expect that the empty string is returned in the swagger doc.

```yaml
    "MyThings ": {
        "enum": [
            "",
            "Something",
            "Another Something"
        ],
        "type": "string"
    },
```

And the '' value should be accepted during data validation.

## Current Behavior
But I got that in swagger config and for validation:

```yaml
    "MyThings ": {
        "enum": [
            "0",
            "Something",
            "Another Something"
        ],
        "type": "string"
    },
```

Well it's 0 because it's in first position, but moving it elsewhere, it will take the index of the position:

```yaml
    "MyThings ": {
        "enum": [
            "Something",
            "Another Something",
            "2"
        ],
        "type": "string"
    },
```

Trying to pass empty string as the value to end point, I got the error "Should be one of [0, Something, Another Something]"

## Workaround
You can change the empty string for something else, like a single space, dash or the word 'Undefined'. But this break Boolean conversion and seems counter-intuitive.

## Context (Environment)
Version of the library: 2.3.8 & 2.4.11
Version of NodeJS: 10.14.1
* Confirm you were using yarn not npm: [X]

## Detailed Description
<!--- Provide a detailed description of the change or addition you are proposing -->

## Breaking change?
This change the current behaviour, so it's a breaking change in principle. But this seems unlikely that anyone have used empty string in enum as it is.

Pull request:
Close #460 empty string enum
### All Submissions:

* [X] Have you followed the guidelines in our [Contributing](https://github.com/lukeautry/tsoa/tree/master/docs/CONTRIBUTING.md) document?
* [X] Have you checked to ensure there aren't other open [Pull Requests](https://github.com/lukeautry/tsoa/pulls) for the same update/change?
* [x] Have you written unit tests?
* [x] Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
* [X] This PR is associated with an existing issue?

**Closing issues**

This closes #460 by removing the confusion between falsy and incorrect value. On validation we check that the value in enum is found instead of thruty, and we construct the accept list of values correctly.

Allowing empty enum is close to optional enum parameter, but it enforce the user of the API to knowingly pass the empty value instead of simply ignoring it. Moreover, the lack of support is a bug.

**Potential Problems With The Approach**

Don't see one, but I have very little knowledge of the code base. At least, this didn't break any unit test.

**Test plan**

None added. Probably should as the code base seems to confuse falsy/invalid, but don't know how.

1. `Adding VALUE_0 = '' as any` to testModel.ts > EnumStringValue
2. Then? I though about checking propertySchema.enum in definition.spec.ts line 263, but the value is undefined.


Patch:
diff --git a/src/metadataGeneration/typeResolver.ts b/src/metadataGeneration/typeResolver.ts
index 6a4a87b4..3c22ea9a 100644
--- a/src/metadataGeneration/typeResolver.ts
+++ b/src/metadataGeneration/typeResolver.ts
@@ -298,7 +298,8 @@ export class TypeResolver {
 
     if (extractEnum) {
       const enums = enumDeclaration.members.map((member: any, index) => {
-        return getEnumValue(member) || String(index);
+        const enumValue = getEnumValue(member);
+        return !!enumValue || enumValue === '' ? enumValue : String(index);
       });
       return {
         dataType: 'refEnum',
diff --git a/src/routeGeneration/templateHelpers.ts b/src/routeGeneration/templateHelpers.ts
index 43716343..9f660e91 100644
--- a/src/routeGeneration/templateHelpers.ts
+++ b/src/routeGeneration/templateHelpers.ts
@@ -237,7 +237,7 @@ export class ValidationService {
     const enumValue = members.find(member => {
       return member === String(value);
     });
-    if (!enumValue) {
+    if (enumValue === undefined) {
       fieldErrors[parent + name] = {
         message: `should be one of the following; ['${members.join(`', '`)}']`,
         value,


Test patch:
diff --git a/tests/fixtures/testModel.ts b/tests/fixtures/testModel.ts
index a1f79f43..883c4775 100644
--- a/tests/fixtures/testModel.ts
+++ b/tests/fixtures/testModel.ts
@@ -120,6 +120,7 @@ export enum EnumIndexValue {
  * EnumNumberValue.
  */
 export enum EnumNumberValue {
+  VALUE_0 = 0,
   VALUE_1 = 2,
   VALUE_2 = 5,
 }
@@ -128,6 +129,7 @@ export enum EnumNumberValue {
  * EnumStringValue.
  */
 export enum EnumStringValue {
+  EMPTY = '' as any,
   VALUE_1 = 'VALUE_1' as any,
   VALUE_2 = 'VALUE_2' as any,
 }
diff --git a/tests/unit/swagger/definitionsGeneration/definitions.spec.ts b/tests/unit/swagger/definitionsGeneration/definitions.spec.ts
index 527bbb42..8ebf7fe2 100644
--- a/tests/unit/swagger/definitionsGeneration/definitions.spec.ts
+++ b/tests/unit/swagger/definitionsGeneration/definitions.spec.ts
@@ -249,6 +249,10 @@ describe('Definition generation', () => {
             expect(propertySchema.$ref).to.eq('#/definitions/EnumNumberValue', `for property ${propertyName}.$ref`);
             expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`);
             expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
+
+            const validatedDefinition = getValidatedDefinition('EnumNumberValue', currentSpec);
+            const expectedEnumValues = ['0', '2', '5'];
+            expect(validatedDefinition.enum).to.eql(expectedEnumValues, `for property ${propertyName}[enum]`);
           },
           enumNumberArray: (propertyName, propertySchema) => {
             expect(propertySchema.type).to.eq('array', `for property ${propertyName}.type`);
@@ -265,6 +269,10 @@ describe('Definition generation', () => {
             expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
             expect(propertySchema.$ref).to.eq('#/definitions/EnumStringValue', `for property ${propertyName}.$ref`);
             expect(propertySchema['x-nullable']).to.eq(true, `for property ${propertyName}[x-nullable]`);
+
+            const validatedDefinition = getValidatedDefinition('EnumStringValue', currentSpec);
+            const expectedEnumValues = ['', 'VALUE_1', 'VALUE_2'];
+            expect(validatedDefinition.enum).to.eql(expectedEnumValues, `for property ${propertyName}[enum]`);
           },
           enumStringArray: (propertyName, propertySchema) => {
             expect(propertySchema.type).to.eq('array', `for property ${propertyName}.type`);
diff --git a/tests/unit/swagger/schemaDetails3.spec.ts b/tests/unit/swagger/schemaDetails3.spec.ts
index b0e110d9..50cd9430 100644
--- a/tests/unit/swagger/schemaDetails3.spec.ts
+++ b/tests/unit/swagger/schemaDetails3.spec.ts
@@ -32,6 +32,20 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
     specName: 'specWithNoImplicitExtras',
   };
 
+  const getComponentSchema = (name: string, chosenSpec: SpecAndName) => {
+    if (!chosenSpec.spec.components.schemas) {
+      throw new Error(`No schemas were generated for ${chosenSpec.specName}.`);
+    }
+
+    const schema = chosenSpec.spec.components.schemas[name];
+
+    if (!schema) {
+      throw new Error(`${name} should have been automatically generated in ${chosenSpec.specName}.`);
+    }
+
+    return schema;
+  };
+
   /**
    * This allows us to iterate over specs that have different options to ensure that certain behavior is consistent
    */
@@ -289,6 +303,9 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
             expect(propertySchema.$ref).to.eq('#/components/schemas/EnumNumberValue', `for property ${propertyName}.$ref`);
             expect(propertySchema.nullable).to.eq(true, `for property ${propertyName}.nullable`);
             expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
+
+            const schema = getComponentSchema('EnumNumberValue', currentSpec);
+            expect(schema.enum).to.eql(['0', '2', '5']);
           },
           enumNumberArray: (propertyName, propertySchema) => {
             expect(propertySchema.type).to.eq('array', `for property ${propertyName}.type`);
@@ -305,6 +322,9 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
             expect(propertySchema).to.not.haveOwnProperty('additionalProperties', `for property ${propertyName}`);
             expect(propertySchema.$ref).to.eq('#/components/schemas/EnumStringValue', `for property ${propertyName}.$ref`);
             expect(propertySchema.nullable).to.eq(true, `for property ${propertyName}.nullable`);
+
+            const schema = getComponentSchema('EnumStringValue', currentSpec);
+            expect(schema.enum).to.eql(['', 'VALUE_1', 'VALUE_2']);
           },
           enumStringArray: (propertyName, propertySchema) => {
             expect(propertySchema.type).to.eq('array', `for property ${propertyName}.type`);
diff --git a/tests/unit/swagger/templateHelpers.spec.ts b/tests/unit/swagger/templateHelpers.spec.ts
index 4669045b..dfbf001a 100644
--- a/tests/unit/swagger/templateHelpers.spec.ts
+++ b/tests/unit/swagger/templateHelpers.spec.ts
@@ -363,6 +363,21 @@ describe('ValidationService', () => {
       expect(result).to.equal(value);
     });
 
+    it('should enum empty string value', () => {
+      const value = '';
+      const result = new ValidationService({}).validateEnum('name', value, {}, [''] as any);
+      expect(result).to.equal(value);
+    });
+
+    it('should enum null is not empty string value', () => {
+      const value = null;
+      const error: any = {};
+      const name = 'name';
+      const result = new ValidationService({}).validateEnum(name, value, error, [''] as any);
+      expect(result).to.equal(undefined);
+      expect(error[name].message).to.equal(`should be one of the following; ['']`);
+    });
+
     it('should enum string value', () => {
       const value = 'HELLO';
       const result = new ValidationService({}).validateEnum('name', value, {}, ['HELLO'] as any);