allow string concat in enum member declaration #21476

Kingwl · 2018-01-30T11:33:27Z

Fixes #20784

mhegazy · 2018-01-30T21:31:49Z

if the first entry in the enum is a string literal you will still get the error for string enums not allowing expressions. you need to revisit the implementation of getEnumKind.

also @RyanCavanaugh, did we decide to reflect the concatenation in the type, i.e. is enum E { a = "value" + "one" } the same type as the literal const v: "valueone" = E.a ?

mhegazy · 2018-01-30T21:32:23Z

src/compiler/checker.ts

                            }
                        }
+                        if (typeof left === "string" && typeof right === "string") {
+                            switch ((<BinaryExpression>expr).operatorToken.kind) {


i would make this a simple if statement.. and consider merging it with typeof left === "string" && typeof right === "string.

mhegazy · 2018-01-30T21:32:33Z

src/compiler/checker.ts

                                case SyntaxKind.AsteriskAsteriskToken: return left ** right;
                            }
                        }
+                        if (typeof left === "string" && typeof right === "string") {


mhegazy · 2018-01-30T21:34:00Z

src/compiler/checker.ts

                        break;
                    case SyntaxKind.StringLiteral:
                        return (<StringLiteral>expr).text;
+                    case SyntaxKind.NoSubstitutionTemplateLiteral:


make sure getEnumKind handles NoSubstitutionTemplateLiteral correctly, and add a test for an literal enum with only template literals. I would recommend doing this in a separate PR instead, since this is not really the change originally intended.

RyanCavanaugh · 2018-01-30T21:36:31Z

also @RyanCavanaugh, did we decide to reflect the concatenation in the type, i.e. is enum E { a = "value" + "one" } the same type as the literal const v: "valueone" = E.a ?

@mhegazy Seems right. I don't see what the alternative would even be

mhegazy · 2018-01-30T21:51:45Z

I don't see what the alternative would even be

well, it will be a non-union (non-literal type) enum.. I am asking cause we do not do this literal type generation from parts any where else

Kingwl · 2018-01-31T01:49:33Z

EnumKind only have Numeric and Literal kind, may i append a String Kind?

const enum EnumKind {
    Numeric,                            // Numeric enum (each member has a TypeFlags.Enum type)
    Literal                             // Literal enum (each member has a TypeFlags.EnumLiteral type)
}

mhegazy · 2018-01-31T06:41:30Z

Well.. if we do what @RyanCavanaugh was suggestion it should be a Literal enum, just the type will be the concatenation of all the parts.

Kingwl · 2018-02-22T08:03:16Z

ping @mhegazy
still need review 😅

joseSantacruz · 2018-04-15T22:54:33Z

+1 to this new feature is useful when you want to restrict something that changes base on env variables for example database tables names on dynamoDB where you don't have the database context only a full list of tables

DanielRosenwasser

Almost good to go!

DanielRosenwasser · 2018-04-16T05:00:53Z

src/compiler/checker.ts

+                return true;
+            }
+            else if (expr.kind === SyntaxKind.BinaryExpression) {
+                return isStringConcatExpression((<BinaryExpression>expr).left) && isStringConcatExpression((<BinaryExpression>expr).right);


I'll be pedantic even though this is only part of what determines whether an enum type is a literal enum type: but think you should also check whether the token is a PlusToken

Kingwl · 2018-04-16T06:21:13Z

seems this future can be merged with #6805

mhegazy · 2018-04-18T00:17:35Z

Can u add a test for declaration emit.

mhegazy · 2018-04-18T00:19:02Z

src/compiler/checker.ts

+            }
+            else if (expr.kind === SyntaxKind.BinaryExpression) {
+                const binaryExpression = <BinaryExpression>expr;
+                return binaryExpression.operatorToken.kind === SyntaxKind.PlusToken && isStringConcatExpression(binaryExpression.left) && isStringConcatExpression(binaryExpression.right);


use isBinaryExpression and avoid the cast.

Kingwl · 2018-04-18T02:50:21Z

Can u add a test for declaration emit.

could you take a example? i'm not sure that i catch your point

mhegazy · 2018-04-18T02:52:37Z

A test with // @declaration: true
This will generate the .d.ts file for the enum, which should use the constant value.

mhegazy · 2018-04-18T16:10:50Z

One failing test that needs attention.

Kingwl · 2018-04-19T01:23:37Z

Everything is ok in my pc... i'm trying to fix that

Kingwl · 2018-04-19T04:17:34Z

what ~~the fuck~~ is happened 😂

mhegazy · 2018-04-20T16:59:52Z

i think you just undid your change to add // @declaration: true to the test. is that intentional? anything i can help with there?

RyanCavanaugh approved these changes Jan 30, 2018

View reviewed changes

mhegazy reviewed Jan 30, 2018

View reviewed changes

Kingwl force-pushed the concat-string-in-enum-member branch from 6b9bf89 to aa10919 Compare January 31, 2018 01:48

Kingwl force-pushed the concat-string-in-enum-member branch 2 times, most recently from 03d9f72 to 6a2f996 Compare January 31, 2018 10:49

DanielRosenwasser requested changes Apr 16, 2018

View reviewed changes

mhegazy reviewed Apr 18, 2018

View reviewed changes

Kingwl force-pushed the concat-string-in-enum-member branch from 6764b30 to 3190b57 Compare April 18, 2018 10:37

Kingwl force-pushed the concat-string-in-enum-member branch from 3190b57 to 2689a99 Compare April 19, 2018 01:19

Kingwl force-pushed the concat-string-in-enum-member branch from 2689a99 to 3371f3d Compare April 19, 2018 01:41

Kingwl force-pushed the concat-string-in-enum-member branch from 3371f3d to f658dbd Compare April 19, 2018 10:53

allow string concat in enum member declaration

2455405

Kingwl force-pushed the concat-string-in-enum-member branch from f658dbd to 2455405 Compare April 19, 2018 13:49

accept baseline

1e1b728

mhegazy merged commit 7f34340 into microsoft:master Apr 21, 2018

microsoft locked and limited conversation to collaborators Jul 31, 2018

Kingwl deleted the concat-string-in-enum-member branch April 28, 2019 15:43

allow string concat in enum member declaration #21476

allow string concat in enum member declaration #21476

Uh oh!

Conversation

Kingwl commented Jan 30, 2018

Uh oh!

mhegazy commented Jan 30, 2018

Uh oh!

mhegazy Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

mhegazy Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

mhegazy Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

RyanCavanaugh commented Jan 30, 2018

Uh oh!

mhegazy commented Jan 30, 2018

Uh oh!

Kingwl commented Jan 31, 2018

Uh oh!

mhegazy commented Jan 31, 2018

Uh oh!

Kingwl commented Feb 22, 2018

Uh oh!

joseSantacruz commented Apr 15, 2018

Uh oh!

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Uh oh!

DanielRosenwasser Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kingwl commented Apr 16, 2018

Uh oh!

mhegazy commented Apr 18, 2018

Uh oh!

mhegazy Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

Kingwl commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhegazy commented Apr 18, 2018

Uh oh!

mhegazy commented Apr 18, 2018

Uh oh!

Kingwl commented Apr 19, 2018

Uh oh!

Kingwl commented Apr 19, 2018

Uh oh!

mhegazy commented Apr 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DanielRosenwasser Apr 16, 2018 •

edited

Loading

Kingwl commented Apr 18, 2018 •

edited

Loading