Skip to content

Raise error diagnostic on ambiguous scope #17202

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

Merged
merged 3 commits into from
May 27, 2025
Merged

Raise error diagnostic on ambiguous scope #17202

merged 3 commits into from
May 27, 2025

Conversation

jeskew
Copy link
Member

@jeskew jeskew commented May 23, 2025

Description

Resolves #17157

If a template uses an expression that passes type checking but cannot be understood by ScopeHelper, the scope is currently silently dropped. This would happen if the expression would evaluate to a union type where each member of the union is a valid scope.

Cases where this could happen:

param condition bool

resource ... = {
  scope: condition ? scopeA : scopeB
param index int
var scopeTuple = [scopeA, scopeB]

resource ... = {
  scope: scopeTuple[index]

Normally, this would cause a deploy-time error, but some resource (like role assignments) are deployable at all scopes, so rather than having the deployment fail, the resource would deploy at the wrong scope.

Checklist

Microsoft Reviewers: Open in CodeFlow

@jeskew jeskew requested review from a team and Copilot May 23, 2025 14:32
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error handling for ambiguous scope expressions by ensuring that an appropriate diagnostic is raised when a scope cannot be resolved at compile time. Key changes include:

  • Adjusting delegate signatures to allow a nullable supplied scope.
  • Explicitly returning null after emitting a diagnostic for union types with ambiguous scope.
  • Adding a new diagnostic (BCP420) with a corresponding integration test for ambiguous scoping expressions.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Bicep.Core/Emit/ScopeHelper.cs Updated delegate signatures and error handling for ambiguous scopes
src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs Added new diagnostic for unresolvable scope expressions (BCP420)
src/Bicep.Core.IntegrationTests/ScenarioTests.cs Added integration test validating the new ambiguous scope diagnostic

Copy link
Contributor

github-actions bot commented May 23, 2025

Test this change out locally with the following install scripts (Action run 15276789134)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 15276789134
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 15276789134"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 15276789134
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 15276789134"

Copy link
Contributor

github-actions bot commented May 23, 2025

Dotnet Test Results

    78 files   -     39      78 suites   - 39   35m 40s ⏱️ - 19m 41s
12 157 tests  -     12  12 157 ✅  -     12  0 💤 ±0  0 ❌ ±0 
28 076 runs   - 14 030  28 076 ✅  - 14 030  0 💤 ±0  0 ❌ ±0 

Results for commit 9abcfae. ± Comparison against base commit f2217b9.

This pull request removes 1904 and adds 642 tests. Note that renamed tests count towards both.

		nestedProp1: 1
		nestedProp2: 2
		prop1: true
		prop2: false
	1
	2
	\$'")
	prop1: true
	prop2: false
…
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
�ӽ\u000e�0\u0010\u0007��>E�\u0003Ԗr-������\u0000\u0015Έ\u0011$�	���-�q���ab�c�^����޴[4)V5��\u000f֌��[*\u0008z�;�ք~�k\u0000E\u0008mG��ǽnLeG��\u001f�kj�,�X� � "	LH\u0015�peK�@ID�4�9Ί\u0014[v�oEם\u000ft/�Fg��\u0007��ҷ�si�\u000f\����&\\u0001h��Y2����P�P`[b�`�������'s���l��8�t^���\u0000\u000c\u0000\u0000,"Value cannot be null. (Parameter 'source')")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
�Խ
�0\u0010\u0007��>E�\u0001b.�5��ApP�*\u0008�\u0012l�
Vi+\u0014|y� .-.~�����\\u0012�a��m&�f��\u0018j!\u0007��\u001a�b)[�o@*\u0002�\u0013�0\u0006Bh��IZ��ږ~�O�������\u000f.\u0001%u�`�a�\u0018ɵ��ڶ�\u0006��l�K�"s
�W���~�j��1�\u000e��{�\u0015=�?\u0017�\u0000r�(�X\u0001�1��#�H&�<���4]����f�Z��!�?�g
� \u0008��
��L�\u0000\u000c\u0000\u0000,"'7' is an invalid end of a number. Expected a delimiter. Path: $.INVALID_JSON | LineNumber: 0 | BytePositionInLine: 20.")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
�Խ\u000e� \u0010\u0007pf��O@9�
\u001d�;�
D���R�������`�@�R[\u0013����\u0003��;3nks�����2�dn�+���\u0007�@@�0�\u0014�_���w\u00121��t��%��A\gƝ��\u0002%�@(yI�B�u��s��9Ȯ�\u001d��^l{��ܷ6\u00144S\u0005k\u001f4�z��o]ѧ�3�	��\u0006��B��?���E2����?�� I�$Y�\u0013�I�\u0004\u0000\u000c\u0000\u0000,"The path: index.json was not found in artifact contents")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003�ӽ
�0\u0010\u0007��>E�\u0003Ĥ�5���Ep�\u0001b{b���T(��n;�K�K?\u0004�\u001bs\u0007w�����4[4)V�I_�kF��[�R��\u001d�4\u0011��\u0013�\u0001\u0002Bh3�&=�6U��\u001c�~�\u001fRSg9�B�P�\u0008}�t�d\u0004z�֒���hi�s�\u0015)6�boEמ\u000f�/�Mg��~�����.\u0015\u0011�\u0005H	 \u0014�\u0001(h�?K&�<�\u000f�\u001d
lJLjLw�\u001f��6�d�\u0016�K��8��L�\u0005���\\u0000\u000c\u0000\u0000,"Value cannot be null. (Parameter 'source')")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003��K
�0\u0010\u0006�=EN���a��K�\u0010����ҦX\u0010��t!�hqS[�|�0!̄�Gd;�\u0017�?�M+�T.\u0013ln@�Z��\u0007h��}\u0001\u0006�P��~�IFtm�
��D�\u001f$\u001d��T�9Z�A'AX	�jC{��=�W\u001f�y\u0017.��\u0005qn�0\u001c��\u000e�}�d���}�ǧ�\u0003H���\u0001�(�����\u0017�����X{�$I�d
O\u0012��r\u0000\u000c\u0000\u0000,"The path: index.json was not found in artifact contents")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003���\u000b�0\u0014\u0007��+����ܞ��C�!#,\u0008���A\u0006Z���?�<D\u0017��?��9�=xo�W�Z�D\u0017%�\u001e\u000f\��F
_���\u000635&�(�\u0012�#���7i�(+U�UƘ��<�U�f:dR\u0004\u001cX�Q"y |��ҹ������K�扮ɵ��Mw��=�\u001d�n�\u0017\u001cnƷ�S��\u001f(\u0003�\u0001�@ԇ�\u001b���?��Ӊ��r\u001b�N��.v\u0016���S�dY�e
�\u0005H4)P\u0000\u000c\u0000\u0000,"'7' is an invalid end of a number. Expected a delimiter. Path: $.INVALID_JSON | LineNumber: 0 | BytePositionInLine: 20.")
Bicep.Core.IntegrationTests.DirectResourceCollectionTests ‑ DirectResourceCollectionAccess_NotAllowedWithinLoops ("output loopOutput array = [for i in range(0, 2): {
  prop: map(containerWorkers, (w) => w.properties.ipAddress.ip)
}]")
Bicep.Core.IntegrationTests.DirectResourceCollectionTests ‑ DirectResourceCollectionAccess_NotAllowedWithinLoops ("resource propertyLoop 'Microsoft.ContainerInstance/containerGroups@2022-09-01' = {
  name: 'gh9440-loop'
  location: 'westus'
  properties: {
    containers: [for i in range(0, 2): {
      name: 'gh9440-w1c-${i}'
      properties: {
        command: [
          'echo "${join(map(containerWorkers, (w) => w.properties.ipAddress.ip), ',')}"'
        ]
      }
    }]
  }
}")
Bicep.Core.IntegrationTests.DirectResourceCollectionTests ‑ DirectResourceCollectionAccess_NotAllowedWithinLoops ("var loopVar = [for i in range(0, 2): {
  prop: map(containerWorkers, (w) => w.properties.ipAddress.ip)
}]")
Bicep.Core.IntegrationTests.Emit.ParamsFileWriterTests ‑ Params_file_with_no_errors_should_compile_correctly ("
using 'main.bicep'

// involves all syntax
param myParam = {
  arr: [
    {
      a : 'b'
    }
    {
      c : true
    }
  ]
  name: 'complex object!'
  priority: 3
  val: null
  obj: {
      a: 'b'
      c: [
          'd'
           1
      ]
  }
}","
{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentParameters.json#",
  "contentVersion": "1.0.0.0",
  "parameters": {
    "myParam": {
      "value": {
        "arr" : [
          {
            "a" : "b"
          },
          {
            "c" : true
          }
        ],
        "name" : "complex object!",
        "priority" : 3,
        "val" : null,
        "obj" : {
          "a" : "b",
          "c" : [
            "d",
            1
          ]
        }
      }
    }
  }
}","
param myParam object
")
…

♻️ This comment has been updated with latest results.

jeskew added 2 commits May 23, 2025 12:08
# Conflicts:
#	src/Bicep.Core.IntegrationTests/ScenarioTests.cs
@jeskew jeskew merged commit 912809e into main May 27, 2025
44 checks passed
@jeskew jeskew deleted the jeskew/17157 branch May 27, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Role assignment scope not set in ARM template when Bicep resource scope set via ternary
2 participants