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.

Sign up for GitHub

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

Jump to bottom

use-safe-access linter rule: Expand to check for nullable properties #15838

Merged
merged 1 commit into from
Dec 12, 2024
Merged

use-safe-access linter rule: Expand to check for nullable properties #15838

merged 1 commit into from
Dec 12, 2024

Conversation

anthony-c-martin
Copy link
Member

@anthony-c-martin anthony-c-martin commented Dec 12, 2024 •
edited
Loading

Some examples where a replacement will be suggested:

Example 1

Input

param foo {
  bar: string?
}
output bar string? = foo.bar

Replacement

param foo {
  bar: string?
}
output bar string? = foo.?bar

Example 2

module.bicep has declared the bar output as a nullable string.

Input

module foo 'module.bicep' = {
  name: 'foo'
}
output bar string? = foo.outputs.bar

Replacement

module foo 'module.bicep' = {
  name: 'foo'
}
output bar string? = foo.outputs.?bar

Closes #15823

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 12, 2024 •
edited
Loading

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

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

@github-actions GitHub Actions
Copy link
Contributor

Dotnet Test Results

    77 files   -     40      77 suites   - 40   32m 28s ⏱️ - 14m 9s
11 532 tests  -     19  11 532 ✅  -     19  0 💤 ±0  0 ❌ ±0 
26 795 runs   - 13 372  26 795 ✅  - 13 372  0 💤 ±0  0 ❌ ±0 

Results for commit 34f1f10. ± Comparison against base commit c404992.

This pull request removes 1846 and adds 637 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
�ӽ
�0\u0010\u0007��>E� ͝�\u001c\u000e�\u001d}��\u0005?h*m�\u0005��M\u0007��ť������\.�U�sݶp��n\u0014\u0012R���td�\u0006�{@V\u0000�h��\u0018\u0010Bv�O2�m���(s�����\u000b��ȁפ���"C̛U,��J �.\u001c��_|u���T��/G��~e2��\u0007���O��\u001a\u0005\u0010��`�0�\u001f�n9K&�<����\u0013$I�$Kx\u0002�l��\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
�Խ
�0\u0010\u0000��>E�\u0003�IzI��Ap�"U\u0010\%؂\u0015Z��P��M\u0007qiq�`�1w���\u001dq��^':N��p��\u0012�7jH���\u0006\u0003��z��*!8B�\u0016��҅)e��~\u0010�c]�Y\u00120�\u0001��$'�3�=93�sW�����K��qR�ky˛��+}�cZ\u001d>�7�\u0015}���W��\u0001\J&\u0014pD���\u001f�(=����t��܆��氋�\u00056\u0013x�,˲��\u0000V\u0008ch\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
�Խ
�0\u0010\u0007��>E�\u0003�IzIT��"��\u0000�=Q���\u0015
⻛\u000e����\u000f����A.	��pg�
�\u0014ˊI�\u0010224�h����\u0000C�y�\u001b�$!�\u0019|�\u000e����\u001be��~�\R[�3����K\u0010�3�T\u0004 \u0017����Ċ\u0016�>��<ņ]�[޶g}�s\u001f���y���[�y\u0004��\u0001Rk��K>�J���$����\u0007
�96\u0005&5�[�\u000eX\u0006kz��
�s��y���\u0005�hh\u001e\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�ӽ
�0\u0010\u0000��>E� �K�����W\u0008Z���Ҧ� ��� 8���V0ߘ��.$Dz�\u001dv�=�M��6�\u0019#s�@	1z\u001e�D��\u0005\u0008h)C6\u001df�dD�zۄQ������֟��@�\u000b�\u000242��(��\u0010;LƐެ?\u0015����w���.\u0016TS\u0005k_4\u0019��~���i�\u00018A�y�&R�\u000e��1�.��������\u0013$I�$kx\u0002(LA�\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��K
�0\u0010\u0006�=E�\u0001b�i���\u0008n<@lG��Z�
\u0005��\u000bq��\u000f�|���L\u0008��zo�-�\u0014���H�5Ac������äA̼QD��\u000c!܎�I�{��ʭ2Ǭ\u001f�Cl�,��\u0019!)��p"�\u000e\u0005U+WK�j,¥m�qV�ВK}+��|�}�kz\u0003>�7݌o��B"�\u0007p��\u000b�˿VJ��ϒ�?��\u0003\u0007�\u0002�\u0012�\u0006�\u001d�G��
>�k
ϥw�<���\u0002�-��\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��M\u000b�0\u0018\u0007��\u0014�\u0007���K\u0006\u001e�\u000e\u0019aA�5F\u000e2�B
��|���t�%h�����?��u�6:1E�Y�\u000b\u000f��\u0011 9o]oP�\u0010Uo\u0004\u0011%$Eȩ{��ţ�t\u0001����\u0007����43!U>'�\u0013�0g�\\u0011>�ڹ�F\u0003箫K�扩�����k��Ǵ:|�o�\u001e��\u000fo\u0005�\u001f���\u0010|ȿ\u0014�A�G�����F�q��V��a\u0017�\u000b\u0007~�g�,˲��\u0002EU�^\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 ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = mcr.microsoft.com, RepositoryPath = unknown/path/az, ExtensionVersion = 0.0.0-placeholder },Azure.RequestFailedException: The artifact does not exist in the registry.
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138,[(BCP192, Error, Unable to restore the artifact with reference "br:mcr.microsoft.com/unknown/path/az:0.0.0-placeholder": The artifact does not exist in the registry.)])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = mcr.microsoft.com, RepositoryPath = unknown/path/az, ExtensionVersion = 0.0.0-placeholder },Azure.RequestFailedException: The artifact does not exist in the registry.
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138,[(BCP192, Error, Unable to restore the artifact with reference "br:mcr.microsoft.com/unknown/path/az:0.0.0-placeholder": The artifact does not exist in the registry.)])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = unknown.registry.azurecr.io, RepositoryPath = bicep/extensions/az, ExtensionVersion = 0.0.0-placeholder },System.AggregateException: Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443))
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 51
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 63
   at Bicep.Core.Registry.OciArtifactRegistry.TryRestoreArtifactAsync(RootConfiguration configuration, OciArtifactReference reference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/OciArtifactRegistry.cs:line 495,[(BCP192, Error, Unable to restore the artifact with reference "br:unknown.registry.azurecr.io/bicep/extensions/az:0.0.0-placeholder": Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)))])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = unknown.registry.azurecr.io, RepositoryPath = bicep/extensions/az, ExtensionVersion = 0.0.0-placeholder },System.AggregateException: Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443))
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 51
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 63
   at Bicep.Core.Registry.OciArtifactRegistry.TryRestoreArtifactAsync(RootConfiguration configuration, OciArtifactReference reference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\OciArtifactRegistry.cs:line 495,[(BCP192, Error, Unable to restore the artifact with reference "br:unknown.registry.azurecr.io/bicep/extensions/az:0.0.0-placeholder": Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)))])
…

jeskew
jeskew reviewed Dec 12, 2024
View reviewed changes
src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/UseSafeAccessRuleTests.cs
public void Rule_ignores_array_access_on_array() => AssertNoDiagnostics("""
param foo (string | null)[]

output bar string? = foo[0]
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but in an ideal world, should we be suggesting foo[?0]? Out of bounds array accesses will throw, but we don't always have min/max length metadata on arrays (and we don't in this case).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. Might be worth creating an issue foi it?

@anthony-c-martin anthony-c-martin merged commit 1ae3143 into main Dec 12, 2024
47 checks passed
@anthony-c-martin anthony-c-martin deleted the ant/safe_access branch December 12, 2024 21:32
This was referenced Jan 8, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeskew jeskew jeskew approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Linter rule to propose usage of .? operator for outputs
2 participants
@anthony-c-martin @jeskew