Skip to content
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

Feature: Default for enums as well #128

Closed

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Dec 13, 2021

Seperating this pr from the unions(include the union change) as it require a change to the binding and reference resolving

@bterlson if you have time to take a look and confirm this is how I should be using the binders and locals.

Azure/cadl-azure#1041

@@ -267,6 +267,14 @@ export function createBinder(program: Program, options: BinderOptions = {}): Bin

function bindEnumStatement(node: EnumStatementNode) {
declareSymbol(getContainingSymbolTable(), node, node.id.sv);
node.locals = createSymbolTable();
for (const enumMember of node.members) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good first step but if we add enum members dynamically we won't get a symbol for it. You could imagine doing late-bound symbols for this, initializing the bindings in the symbol table when the enum type is instantiated. Think that's worth doing here?

@bterlson
Copy link
Member

I think this is good. But one issue gives me a bit of pause - in projections, I am currently using . to index into operations on the enum type, so e.g. E.name is valid and returns the name of the enum. This is a conflict we need to resolve. I considered classes and thought it was ok to use special syntax to access the member of a model (e.g. SomeModel["someProperty"] in the current prototype) but this feels much less good for enums.

There's another thing to think about too - given an enum like enum Foo { x = "hi"; y = "bye" }, does Foo.x give you the enum member itself (x = "hi") or the enum member's type ("hi")? For models, I think we want the simple syntax to give you the type of the member so I can do something like op get(id: Model.id): Model, but to support openapi links the type would carry some additional state to say it was peeled off of Model. I wonder if we want to do a similar thing to enums?

I think we should wait on this and make sure we consider the enum and model use cases together and come to an understanding on what . means for enums and models in and out of projections and how to refer to a member vs. the member's type vs. projection operations on the base type.

@timotheeguerin
Copy link
Member Author

@bterlson yeah that make sense, should wait until we figure everything out. Created an issue for this so we don't forget.
For the projection maybe we can have a different character for properties of the type Enum#name for example.

The way I was thinking about it was to point to the member itself not the value but I guess this is also a question for the subtype/supertype relation.
If we just get the value should then this be allowed as well:

foo?: MyEnum = "foo"

enum MyEnum {foo,bar}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants