Using a conditional in a module or resource scope produces error

[miqm]

Adding one case that I think it's related with this - using a conditional in a module scope:

Following code:

module mod 'mod.bicep' = if (!empty(otherResourceGroup)) {
  name: '${_deployment}-mod'
  scope: !empty(otherResourceGroup) ? resourceGroup(otherResourceGroup) : resourceGroup()

results in error:

The property "scope" expected a value of type "resourceGroup" but the provided value is of type "resourceGroup | resourceGroup".

Originally posted by @miqm in #449 (comment)

[miqm]

@anthony-c-martin - there's an even bigger bug with this - when I use any() in the scope, it doesn't produce the scope json code - it just ignores it without any warning.

[majastrz]

'@anthony-c-martin - there's an even bigger bug with this - when I use any() in the scope, it doesn't produce the scope json code - it just ignores it without any warning.

Something similar happens with any() in dependsOn as well, but it's not as bad.

[anthony-c-martin]

@anthony-c-martin - there's an even bigger bug with this - when I use any() in the scope, it doesn't produce the scope json code - it just ignores it without any warning.

Yeah, we started off with the premise that any should be assignable to anything (similar to TypeScript), but I think we're now realizing that there are places (e.g. where Bicep does some semantic analysis rather than just translating the properties directly to JSON) where we must tighten this up. Sounds like we're going to need a strict type flag for these properties.

[majastrz]

The any() issue mentioned and linked in the comments has already been fixed.

For the issue originally reported here, I think we need a mechanism to simplify redundant union types in the type system. For this specific issue, removing duplicate types should be sufficient. But there are other cases. For example, string | 'a' | 'b' is equivalent to string. Similarly array | string[] is the same as the array type.

[majastrz]

In addition, we're not handling union types correctly when type checking the value of the scope property. A type like resourcegroup | resourcegroup should be legitimately assignable to that property even if it's redundant.

[majastrz]

I looked into this and this is not as simple as just fixing the type checks. To emit the JSON correctly, we need the following:

1. The type of scope (tenant, mg, sub, rg, etc.). This must be known at compile time and cannot be dynamic. Currently, this is established by the use of scope functions.
2. The relationship of the target scope with the scope of the deployment must be known at compile time. For example, we must know if a specific module is attempting a cross-resource group deployment vs deploying to the RG of the deployment at compile time.
3. The scope identifiers. These do not need to be known at compile time. We use the expressions from positional scope function arguments and transform them into whatever is needed in the JSON.

Number 1 prevents us from ever supporting free-form scope expressions like <exp> ? subscription() : resourceGroup() until this is implemented in the runtime. We could special case <exp> ? resourceGroup() : resourceGroup(<exp2>), but it would mean having special handling for TernaryOperatorSyntax in scope expressions and we could never support all possible expressions.

Another option that occurs to me is to support a scope expression like resourceGroup(<exp> ? null : <exp2>). However number 2 above precludes this option as well.

[alex-frankel]

Because this is not a straightforward fix, we'd recommend using a ternary expression inside the resourceGroup() function like so:

scope: resourceGroup(!empty(otherResourceGroup) ? otherResourceGroup : resourceGroup().name) 

We'll revisit this once we support the scope property in the intermediate language layer for all resource types.

[majastrz]

@alex-frankel and I also discussed an alternative to special case expressions like scope: !empty(otherResourceGroup) ? resourceGroup(otherResourceGroup) : resourceGroup()) and transform them into scope: resourceGroup(!empty(otherResourceGroup) ? otherResourceGroup : resourceGroup().name) inside the compiler, but decided not to add extra complexity at this time.