-
-
Notifications
You must be signed in to change notification settings - Fork 677
feat: support element access of enum #2950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: support element access of enum #2950
Conversation
Concept ACK but I'll have to review later |
ping @CountBleck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems decent
for (let _keys = Map_keys(members), _values = Map_values(members), i = 1, k = _keys.length; i <= k; ++i) { | ||
let enumValueName = unchecked(_keys[k - i]); | ||
let member = unchecked(_values[k - i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (let _keys = Map_keys(members), _values = Map_values(members), i = 1, k = _keys.length; i <= k; ++i) { | |
let enumValueName = unchecked(_keys[k - i]); | |
let member = unchecked(_values[k - i]); | |
for (let _keys = Map_keys(members), _values = Map_values(members), i = _keys.length - 1; i >= 0; --i) { | |
let enumValueName = unchecked(_keys[i]); | |
let member = unchecked(_values[i]); |
Doesn't this work?
return true; | ||
} | ||
|
||
private ensureEnumToString(enumElement: Enum, reportNode: Node): string | null { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a couple blank lines in here? It's a little hard to read
let resolver = this.resolver; | ||
let targetElement = resolver.lookupExpression(targetExpression, this.currentFlow, Type.auto, ReportMode.Swallow); | ||
if (targetElement && targetElement.kind == ElementKind.Enum) { | ||
const elementExpr = this.compileExpression(expression.elementExpression, Type.i32, Constraints.ConvImplicit); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we're allowing Enum[Enum.X]
but not Enum["X"]
(only Enum.X
, the property access expression, is allowed). This is likely a good thing to have, unless users really want to get enum members with computed string values (which should probably be discouraged regardless).
(It also seems like we don't have a good error for someObject["property"]
and we use TS2329 instead, so a good error for Enum["X"]
isn't necessary for now.)
No description provided.