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

Generate type definitions for 'list<action>' functions #667

Closed
anthony-c-martin opened this issue Oct 16, 2020 · 8 comments
Closed

Generate type definitions for 'list<action>' functions #667

anthony-c-martin opened this issue Oct 16, 2020 · 8 comments
Labels
enhancement New feature or request types

Comments

@anthony-c-martin
Copy link
Member

Support one of the following syntaxes:

var keys = storageAccount.listKeys() // uses the api version of the storageAccount resource (if possible)
var keys = storageAccount.listKeys('2020-01-01') // specific api version

or

var keys = listKeys(storageAccount)
var keys = listKeys(storageAccount, '2020-01-01')
@anthony-c-martin
Copy link
Member Author

I was having a think about what the implementation would look like for a syntax like account.listKeys(). That'll require a few changes to consume the work that @jorgecotillo did for namespaces, and if possible, I'd like to share logic with namespace function resolution.

Some things I noticed which I think may need to change to support this:

  1. Functions are currently symbols rather than type symbols.
  2. Namespaces are currently symbols rather than type symbols.

My thoughts for changes required to Bicep:

  1. Convert functions & function overloads to being explicit types rather than symbols.
  2. Add dictionary of 'member functions' to the ObjectTypeSymbol definition (similar to 'member properties').
  3. Create a TypeSymbolValidationFlags flag which blocks assignment (for the namespace)
  4. Convert NamespaceSymbol into a type symbol which extends NamedObjectType, and set the unnassignable type flag on it.
  5. Move instance function selection logic from NameBinding to TypeAssignment
  6. Add special logic to handle emission of member functions on a resource (e.g. myres.listKeys() -> [listKeys(<id expression)]

@majastrz
Copy link
Collaborator

I'm not sure I understand why functions and overloads must be modeled as types.

@anthony-c-martin
Copy link
Member Author

anthony-c-martin commented Oct 19, 2020

I'm not sure I understand why functions and overloads must be modeled as types.

Function resolution will depend on the type of the parent, so the function resolution logic can't happen at name binding time (where it is today). This only works for namespaces because we have pre-existing mapping from name -> namespace.

@majastrz
Copy link
Collaborator

I understand that we need to have object types that have functions and functions have return types, but functions themselves aren't types. It seems that after the name binding visitor is done, the type assignment visitor could add additional bindings for instance functions. (Technically the whole name binding logic could probably be moved into the type assignment visitor to make it deferred, but that's probably not required.)

@jorgecotillo
Copy link
Contributor

Maybe I am not understanding something but in TypeAssignmentVisitor FunctionSymbols return a TypeSymbol (return type of the function), which I think makes sense because a function itself is not a type is a symbol that returns some type. Is the proposal to defer the lookup of functions to TypeAssignmentVisitor?

@anthony-c-martin
Copy link
Member Author

anthony-c-martin commented Oct 20, 2020

Maybe I am not understanding something but in TypeAssignmentVisitor FunctionSymbols return a TypeSymbol (return type of the function), which I think makes sense because a function itself is not a type is a symbol that returns some type. Is the proposal to defer the lookup of functions to TypeAssignmentVisitor?

Yup, exactly this. Functions will need to be defined in the type system (although not directly as types to Marcin's point), rather than assigned directly to symbols.

@jorgecotillo
Copy link
Contributor

Makes sense

@alex-frankel
Copy link
Collaborator

This is done

@Azure Azure locked as resolved and limited conversation to collaborators May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request types
Projects
None yet
Development

No branches or pull requests

4 participants