Skip to content

Commit

Permalink
Don't raise missing properties diagnostic if assigned object value ha…
Browse files Browse the repository at this point in the history
…s an additional properties type (#13272)

Resolves #13247 

The type narrowing on objects introduced in #12798 will erroneously
raise a missing property diagnostic if the assigned value does not have
an explicit property defined for each of the target's required
properties, even if the assigned value allows additional properties.
This PR updates the type narrowing to consider required properties not
to be missing if the assigned value allows arbitrary additional
properties.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/13272)
  • Loading branch information
jeskew committed Feb 9, 2024
1 parent 804565c commit 396ffee
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 6 deletions.
43 changes: 43 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5676,4 +5676,47 @@ public void Test_Issue12347()

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
}

// https://github.com/Azure/bicep/issues/13427
[TestMethod]
public void Test_Issue13427()
{
var result = CompilationHelper.Compile("""
@export()
type naming = {
@description('Override the abbreviation of this resource with this parameter')
abbreviation: string?
@description('The resource environment (for example: dev, tst, acc, prd)')
environment: string?
@description('The resource location (for example: weu, we, westeurope)')
location: string?
@description('The name of the customer')
customer: string?
@description('The delimiter between resources (default: -)')
delimiter: string?
@description('The order of the array defines the order of elements in the naming scheme')
nameFormat: ('abbreviation' | 'function' | 'environment' | 'location' | 'customer' | 'param1' | 'param2' | 'param3')[]?
@description('Extra parameter self defined')
param1: string?
@description('Extra parameter self defined')
param2: string?
@description('Extra parameter self defined')
param3: string?
@description('Full name of the resource overwrites the combinated name')
overrideName: string?
@description('Function of the resource [can be app, db, security,...]')
function: string
@description('Suffix for the resource, if empty non will be appended, otherwise will be added to the end [can be index, ...]')
suffix: string?
}

param defaultNaming naming

param resourceNaming naming

param test naming = union(defaultNaming, resourceNaming)
""");

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
}
}
16 changes: 10 additions & 6 deletions src/Bicep.Core/TypeSystem/TypeValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,12 +1057,16 @@ static TypeSymbol RemoveImplicitNull(TypeSymbol type, bool typeWasPreserved)

if (expressionType is ObjectType expressionObjectType)
{
var missingRequiredProperties = targetType.Properties.Values
.Where(p => p.Flags.HasFlag(TypePropertyFlags.Required) &&
!AreTypesAssignable(LanguageConstants.Null, p.TypeReference.Type) &&
!expressionObjectType.Properties.ContainsKey(p.Name))
.OrderBy(p => p.Name)
.ToImmutableArray();
var missingRequiredProperties = expressionObjectType.AdditionalPropertiesType is not null
// if the assigned value allows additional properties, we can't know if it's missing any
? ImmutableArray<TypeProperty>.Empty
// otherwise, look for required properties on the target for which there is no declared counterpart on the assigned value
: targetType.Properties.Values
.Where(p => p.Flags.HasFlag(TypePropertyFlags.Required) &&
!AreTypesAssignable(LanguageConstants.Null, p.TypeReference.Type) &&
!expressionObjectType.Properties.ContainsKey(p.Name))
.OrderBy(p => p.Name)
.ToImmutableArray();

if (missingRequiredProperties.Length > 0)
{
Expand Down

0 comments on commit 396ffee

Please sign in to comment.