Skip to content

Commit

Permalink
Fix for issue 14059
Browse files Browse the repository at this point in the history
  • Loading branch information
anthony-c-martin committed May 9, 2024
1 parent 44e0c7d commit 22f030b
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 24 deletions.
19 changes: 19 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6053,4 +6053,23 @@ func genModuleTags(moduleName string) moduleTags => {
["name name"] = "name",
});
}

// https://github.com/Azure/bicep/issues/14059
[TestMethod]
public void Test_Issue14059()
{
var result = CompilationHelper.Compile("""
param foo string
param index int
var test = [
{ foo: foo }
{ }
][index]
output val string = test.foo
""");

result.Should().NotHaveAnyDiagnostics();
}
}
84 changes: 60 additions & 24 deletions src/Bicep.Core/TypeSystem/TypeAssignmentVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1785,36 +1785,72 @@ private TypeSymbol GetAccessedType(AccessExpressionSyntax syntax, IDiagnosticWri
: baseType;
}

private static TypeSymbol GetNamedPropertyType(PropertyAccessSyntax syntax, TypeSymbol baseType, IDiagnosticWriter diagnostics) => UnwrapType(baseType) switch
private static TypeSymbol? TryGetReadablePropertyType(ObjectType objectType, string propertyName)
{
ErrorType error => error,
TypeSymbol withErrors when withErrors.GetDiagnostics().Any() => ErrorType.Create(withErrors.GetDiagnostics()),

TypeSymbol original when TypeHelper.TryRemoveNullability(original) is TypeSymbol nonNullable => EmitNullablePropertyAccessDiagnosticAndEraseNullability(syntax, original, nonNullable, diagnostics),

// the property is not valid
// there's already a parse error for it, so we don't need to add a type error as well
ObjectType when !syntax.PropertyName.IsValid => ErrorType.Empty(),
if (objectType.Properties.TryGetValue(propertyName) is {} property && !property.Flags.HasFlag(TypePropertyFlags.WriteOnly))
{
return property.TypeReference.Type;
}

ObjectType objectType => TypeHelper.GetNamedPropertyType(objectType,
syntax.PropertyName,
syntax.PropertyName.IdentifierName,
syntax.IsSafeAccess || TypeValidator.ShouldWarnForPropertyMismatch(objectType),
diagnostics),
if (objectType.AdditionalPropertiesType is {} additionalPropertiesType && !objectType.AdditionalPropertiesFlags.HasFlag(TypePropertyFlags.WriteOnly))
{
return additionalPropertiesType.Type;
}

UnionType unionType when unionType.Members.All(x => x is ObjectType)
=> TypeHelper.CreateTypeUnion(unionType.Members.Select(member => GetNamedPropertyType(syntax, member.Type, diagnostics))),
// TODO could consider returning the null type here, but need to be careful about downstream impact.
// See https://github.com/Azure/bicep/issues/14059 for example
return null;
}

// TODO: We might be able use the declared type here to resolve discriminator to improve the assigned type
DiscriminatedObjectType => LanguageConstants.Any,
private static TypeSymbol GetNamedPropertyType(PropertyAccessSyntax syntax, TypeSymbol baseType, IDiagnosticWriter diagnostics)
{
var unwrapped = UnwrapType(baseType);
switch (unwrapped)
{
case ErrorType error:
return error;
case TypeSymbol withErrors when withErrors.GetDiagnostics().Any():
return ErrorType.Create(withErrors.GetDiagnostics());
case TypeSymbol original when TypeHelper.TryRemoveNullability(original) is TypeSymbol nonNullable:
return EmitNullablePropertyAccessDiagnosticAndEraseNullability(syntax, original, nonNullable, diagnostics);
case ObjectType when !syntax.PropertyName.IsValid:
// the property is not valid
// there's already a parse error for it, so we don't need to add a type error as well
return ErrorType.Empty();
case ObjectType objectType:
return TypeHelper.GetNamedPropertyType(objectType,
syntax.PropertyName,
syntax.PropertyName.IdentifierName,
syntax.IsSafeAccess || TypeValidator.ShouldWarnForPropertyMismatch(objectType),
diagnostics);
case UnionType unionType when syntax.PropertyName.IsValid:
var propertyName = syntax.PropertyName.IdentifierName;
var members = new List<TypeSymbol>();
foreach (var member in unionType.Members)
{
if (member is not ObjectType objectType ||
TryGetReadablePropertyType(objectType, propertyName) is not {} propertyType)
{
// fall back to any if we can't definitively obtain the property type.
// this may give some false positives - we can further refine this if desired.
return LanguageConstants.Any;
}

// We can assign to an object, but we don't have a type for that object.
// The best we can do is allow it and return the 'any' type.
TypeSymbol maybeObject when TypeValidator.AreTypesAssignable(maybeObject, LanguageConstants.Object) => LanguageConstants.Any,
members.Add(propertyType);
}

// can only access properties of objects
TypeSymbol otherwise => ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.PropertyName).ObjectRequiredForPropertyAccess(otherwise)),
};
return TypeHelper.CreateTypeUnion(members);
case DiscriminatedObjectType:
// TODO: We might be able use the declared type here to resolve discriminator to improve the assigned type
return LanguageConstants.Any;
case TypeSymbol maybeObject when TypeValidator.AreTypesAssignable(maybeObject, LanguageConstants.Object):
// We can assign to an object, but we don't have a type for that object.
// The best we can do is allow it and return the 'any' type.
return LanguageConstants.Any;
default:
return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.PropertyName).ObjectRequiredForPropertyAccess(unwrapped));
}
}

private static TypeSymbol EmitNullablePropertyAccessDiagnosticAndEraseNullability(PropertyAccessSyntax syntax, TypeSymbol originalBaseType, TypeSymbol nonNullableBaseType, IDiagnosticWriter diagnostics)
{
Expand Down

0 comments on commit 22f030b

Please sign in to comment.