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

User-defined types - Conflicting types #10237

Closed
slavizh opened this issue Mar 28, 2023 · 3 comments
Closed

User-defined types - Conflicting types #10237

slavizh opened this issue Mar 28, 2023 · 3 comments

Comments

@slavizh
Copy link
Contributor

slavizh commented Mar 28, 2023

Bicep version
Bicep CLI version 0.15.31 (3ba6e06)

Describe the bug
I have the following code:

// Does not allow properties that are not defined to be added
@sealed()
type resourceGroupsType = {
  @description('The name of the resource group. Test link in intellisense. https://www.google.com/')
  name: string
  location: string?
  create: bool?
  tags: tagsType?
}
type tagsType = {}

var defaultResourceGroup = {
  create: true
  tags: {}
}

resource resourceGroupsRes 'Microsoft.Resources/resourceGroups@2021-04-01' = [for resourceGroup in resourceGroups: if (union(defaultResourceGroup, resourceGroup).create) {
  name: resourceGroup.name
  location: resourceGroup.location
  tags: union(allTags, union(defaultResourceGroup, resourceGroup).tags)
  properties: {}
}]

On the location value of the for the resourceGroupsRes I get warning like:

Expected a value of type "string" but the provided value is of type "null | string".bicep(BCP321)

I am not sure how this can be resolved exactly but it seems cases like this could be quite common where user-defined types conflict with types on actual resources. As you can see location is not mandatory in my defined type as basically it is required only when create is set to true otherwise the resource is not deployed at all.

To Reproduce
Steps to reproduce the behavior:
shown

Additional context
Add any other context about the problem here.

@ghost ghost added the Needs: Triage 🔍 label Mar 28, 2023
@jeskew
Copy link
Member

jeskew commented Mar 28, 2023

@slavizh Correct me if I'm wrong, but I think what you need here is a way to make the type of location dependent on the value of create? The location property on a 'Microsoft.Resources/resourceGroups@2021-04-01' resource is required, so the type check warning being surfaced is correct, since resourceGroup.location is typed as a nullable string and is being assigned to a location that requires a non-nullable string.

You can suppress the warning with ! (i.e., location: resourceGroup.location! -- this will be proposed as an automated fix in VS Code), but there's a chance the deployment will fail at runtime, since { name: 'foo', create: true } would pass type validation for resourceGroupsType.

If #9230 were in place, you could use a discriminated union to capture both states. Something along the lines of:

type existingResourceGroupsType = {
  variant: 'existing'
  name: string
}

type newResourceGroupsType = {
  variant: 'new'
  name: string
  location: string
  tags: object
}

type resourceGroupsType = existingResourceGroupsType | newResourceGroupsType

But that feature and the syntax that would be used have not been started.

@slavizh
Copy link
Contributor Author

slavizh commented Mar 29, 2023

Yes, correct that is what I want ideally is to require certain parameters based on the value of another parameter.
Yes ! only suppresses the warning and deployment fails if not provided. BTW is there documentation for ! on its usage as I think I have missed that one.
I think the mentioned future syntax will be able to solve that case. I would assume you will be able to define multiple types in case you have more than two.

@jeskew
Copy link
Member

jeskew commented Mar 29, 2023

BTW is there documentation for ! on its usage as I think I have missed that one.

Not yet, but I believe it should be up soon since #9832 is tagged for the 0.16 release. The short version is that the postfix ! operator asserts that a given value is not null, which transforms the assigned type of the value from null | <type> to just <type>. If the transformed type is not nullable, then the ! operator has no effect.

Upvotes and discussion for conditionally requiring properties is tracked in #9641, so I'm going to close this as a duplicate. The discriminated union feature (#9230) doesn't have finalized syntax yet but is being actively worked on.

@jeskew jeskew closed this as completed Mar 29, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants