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.

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

Adding fix to process 64 bit integers correctly. #5791

Merged
merged 8 commits into from
Feb 4, 2022
48 changes: 48 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2812,5 +2812,53 @@ public void Test_Issue5099()
result.Template!.Should().HaveValueAtPath("$.resources[?(@.name == '[format(\\'foo{0}\\', parameters(\\'rgNames\\')[copyIndex()])]')].metadata.description", "module loop");
result.Template!.Should().HaveValueAtPath("$.outputs.productGroupsResourceIds.metadata.description", "The Resources Ids of the API management service product groups");
}

[TestMethod]
// https://github.com/Azure/bicep/issues/5371
public void Test_Issue5371_positive_test()
{
var result = CompilationHelper.Compile(@"
var myValue = -9223372036854775808
");

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

[TestMethod]
// https://github.com/Azure/bicep/issues/5371
public void Test_Issue5371_positive_test_2()
{
var result = CompilationHelper.Compile(@"
var myValue = 9223372036854775807
");

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

[TestMethod]
// https://github.com/Azure/bicep/issues/5371
public void Test_Issue5371_negative_test()
{
var result = CompilationHelper.Compile(@"
var myValue = -9223372036854775809
");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[] {
("BCP010", DiagnosticLevel.Error, "Expected a valid 64-bit signed integer.")
});
}

[TestMethod]
// https://github.com/Azure/bicep/issues/5371
public void Test_Issue5371_negative_test_2()
{
var result = CompilationHelper.Compile(@"
var myValue = 9223372036854775808
");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[] {
("BCP010", DiagnosticLevel.Error, "Expected a valid 64-bit signed integer.")
});
}
}
}
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Utils/TestSyntaxFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static class TestSyntaxFactory

public static StringSyntax CreateString(string value) => SyntaxFactory.CreateStringLiteral(value);

public static IntegerLiteralSyntax CreateInt(long value) => new(CreateToken(TokenType.Integer), value);
public static IntegerLiteralSyntax CreateInt(ulong value) => new(CreateToken(TokenType.Integer), value);

public static BooleanLiteralSyntax CreateBool(bool value) => new BooleanLiteralSyntax(value ? CreateToken(TokenType.TrueKeyword) : CreateToken(TokenType.FalseKeyword), value);

Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private static string BuildBicepConfigurationClause(string? configurationPath) =
public ErrorDiagnostic InvalidInteger() => new(
TextSpan,
"BCP010",
"Expected a valid 32-bit signed integer.");
"Expected a valid 64-bit signed integer.");

public ErrorDiagnostic InvalidType() => new(
TextSpan,
Expand Down
1 change: 1 addition & 0 deletions src/Bicep.Core/Emit/EmitLimitationCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public static EmitLimitationInfo Calculate(SemanticModel model)
DeployTimeConstantValidator.Validate(model, diagnosticWriter);
ForSyntaxValidatorVisitor.Validate(model, diagnosticWriter);
FunctionPlacementValidatorVisitor.Validate(model, diagnosticWriter);
IntegerValidatorVisitor.Validate(model, diagnosticWriter);

DetectDuplicateNames(model, diagnosticWriter, resourceScopeData, moduleScopeData);
DetectIncorrectlyFormattedNames(model, diagnosticWriter);
Expand Down
48 changes: 40 additions & 8 deletions src/Bicep.Core/Emit/ExpressionConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Bicep.Core.Semantics;
using Bicep.Core.Semantics.Metadata;
using Bicep.Core.Syntax;
using JetBrains.Annotations;
using Newtonsoft.Json.Linq;

namespace Bicep.Core.Emit
Expand Down Expand Up @@ -52,7 +53,7 @@ public LanguageExpression ConvertExpression(SyntaxBase expression)
return CreateFunction(boolSyntax.Value ? "true" : "false");

case IntegerLiteralSyntax integerSyntax:
return integerSyntax.Value > int.MaxValue || integerSyntax.Value < int.MinValue ? CreateFunction("json", new JTokenExpression(integerSyntax.Value.ToInvariantString())) : new JTokenExpression((int)integerSyntax.Value);
return ConvertInteger(integerSyntax, false);

case StringSyntax stringSyntax:
// using the throwing method to get semantic value of the string because
Expand Down Expand Up @@ -810,31 +811,62 @@ private LanguageExpression ConvertBinary(BinaryOperationSyntax syntax)

private LanguageExpression ConvertUnary(UnaryOperationSyntax syntax)
{
LanguageExpression convertedOperand = ConvertExpression(syntax.Expression);

switch (syntax.Operator)
{
case UnaryOperator.Not:
LanguageExpression convertedOperand = ConvertExpression(syntax.Expression);
return CreateFunction("not", convertedOperand);

case UnaryOperator.Minus:
if (convertedOperand is JTokenExpression literal && literal.Value.Type == JTokenType.Integer)
if (syntax.Expression is IntegerLiteralSyntax integerLiteral)
{
// invert the integer literal
int literalValue = literal.Value.Value<int>();
return new JTokenExpression(-literalValue);
//do codegen logic
LanguageExpression integerExpression = ConvertInteger(integerLiteral, true);
return integerExpression;
}

return CreateFunction(
"sub",
new JTokenExpression(0),
convertedOperand);
ConvertExpression(syntax.Expression));

default:
throw new NotImplementedException($"Cannot emit unexpected unary operator '{syntax.Operator}.");
}
}

// for 32 bit integers, we return the integer value
// for values outside that range, return the FunctionExpression
private LanguageExpression ConvertInteger(IntegerLiteralSyntax integerSyntax, bool minus)
{
long integerValue = (long)integerSyntax.Value;

if (minus)
{
// integerSyntax.Value is always positive, so for the most negative signed 32 bit integer -2,147,483,648
// we would compare its positive token (2,147,483,648) to int.MaxValue (2,147,483,647) + 1
if (integerSyntax.Value > (ulong)int.MaxValue + 1)
{
long negativeIntegerValue = -1 * integerValue;
return CreateFunction("json", new JTokenExpression(negativeIntegerValue.ToInvariantString()));
}
else
{
return new JTokenExpression((int)integerSyntax.Value);
}
} else
{
if (integerSyntax.Value > int.MaxValue)
{
return CreateFunction("json", new JTokenExpression(integerValue.ToInvariantString()));
}
else
{
return new JTokenExpression((int)integerSyntax.Value);
}
}
}

public LanguageExpression GenerateSymbolicReference(string symbolName, SyntaxBase? indexExpression)
{
if (indexExpression is null)
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/ExpressionEmitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ symbol is FunctionSymbol functionSymbol &&

writer.WriteValue(serialized);
}
public void EmitCopyObject(string? name, ForSyntax syntax, SyntaxBase? input, string? copyIndexOverride = null, long? batchSize = null)
public void EmitCopyObject(string? name, ForSyntax syntax, SyntaxBase? input, string? copyIndexOverride = null, ulong? batchSize = null)
{
// local function
static bool CanEmitAsInputDirectly(SyntaxBase input)
Expand Down
62 changes: 62 additions & 0 deletions src/Bicep.Core/Emit/IntegerValidatorVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Bicep.Core.Diagnostics;
using Bicep.Core.Semantics;
using Bicep.Core.Syntax;

namespace Bicep.Core.Emit
{
public class IntegerValidatorVisitor : SyntaxVisitor
{
private readonly SemanticModel semanticModel;
private readonly IDiagnosticWriter diagnosticWriter;

private IntegerValidatorVisitor(SemanticModel semanticModel, IDiagnosticWriter diagnosticWriter)
{
this.semanticModel = semanticModel;
this.diagnosticWriter = diagnosticWriter;
}

public static void Validate(SemanticModel semanticModel, IDiagnosticWriter diagnosticWriter)
{
var visitor = new IntegerValidatorVisitor(semanticModel, diagnosticWriter);
// visiting writes diagnostics in some cases
visitor.Visit(semanticModel.SourceFile.ProgramSyntax);
}

protected override void VisitInternal(SyntaxBase node)
{
base.VisitInternal(node);
}

public override void VisitIntegerLiteralSyntax(IntegerLiteralSyntax syntax)
{
// syntax.Value is always positive and can't be greater than the greatest 64 bit integer
if (syntax.Value > long.MaxValue)
{
diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax).InvalidInteger());
}
base.VisitIntegerLiteralSyntax(syntax);
}

public override void VisitUnaryOperationSyntax(UnaryOperationSyntax syntax)
{
// a negative integer is parsed into a minus token and an integer token which is always positive
// so for the most negative valid signed 64 bit integer -9,223,372,036,854,775,808, we need to compare its positive integer token (9,223,372,036,854,775,808) to long.MaxValue (9,223,372,036,854,775,807) + 1
if (syntax.Operator == UnaryOperator.Minus)
{
if (syntax.Expression is IntegerLiteralSyntax integerLiteral && integerLiteral.Value > (ulong)long.MaxValue + 1)
{
diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax).InvalidInteger());
}

} else
{
base.VisitUnaryOperationSyntax(syntax);
}
}


}
}
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/TemplateWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ private void EmitResources(JsonTextWriter jsonWriter, ExpressionEmitter emitter)
}
}

private long? GetBatchSize(StatementSyntax statement)
private ulong? GetBatchSize(StatementSyntax statement)
{
var decorator = SemanticModelHelper.TryGetDecoratorInNamespace(
context.SemanticModel,
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Parsing/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ private IntegerLiteralSyntax NumericLiteral()
{
var literal = Expect(TokenType.Integer, b => b.ExpectedNumericLiteral());

if (long.TryParse(literal.Text, NumberStyles.None, CultureInfo.InvariantCulture, out long value))
if (UInt64.TryParse(literal.Text, NumberStyles.None, CultureInfo.InvariantCulture, out ulong value))
{
return new IntegerLiteralSyntax(literal, value);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Semantics/Namespaces/SystemNamespaceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ static DecoratorEvaluator MergeToTargetObject(string propertyName, Func<Decorato
static long? TryGetIntegerLiteralValue(SyntaxBase syntax) => syntax switch
{
UnaryOperationSyntax { Operator: UnaryOperator.Minus } unaryOperatorSyntax
when unaryOperatorSyntax.Expression is IntegerLiteralSyntax integerLiteralSyntax => -1 * integerLiteralSyntax.Value,
IntegerLiteralSyntax integerLiteralSyntax => integerLiteralSyntax.Value,
when unaryOperatorSyntax.Expression is IntegerLiteralSyntax integerLiteralSyntax => -1 * (long)integerLiteralSyntax.Value,
IntegerLiteralSyntax integerLiteralSyntax => (long)integerLiteralSyntax.Value,
_ => null,
};

Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Syntax/IntegerLiteralSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ namespace Bicep.Core.Syntax
{
public class IntegerLiteralSyntax : ExpressionSyntax
{
public IntegerLiteralSyntax(Token literal, long value)
public IntegerLiteralSyntax(Token literal, ulong value)
{
Literal = literal;
Value = value;
}

public Token Literal { get; }

public long Value { get; }
public ulong Value { get; }

public override void Accept(ISyntaxVisitor visitor)
=> visitor.VisitIntegerLiteralSyntax(this);
Expand Down
4 changes: 3 additions & 1 deletion src/Bicep.Core/Syntax/SyntaxFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ public static SyntaxBase CreateObjectPropertyKey(string text)
return CreateStringLiteral(text);
}

public static IntegerLiteralSyntax CreateIntegerLiteral(long value) => new(CreateToken(TokenType.Integer, value.ToString()), value);
public static IntegerLiteralSyntax CreateIntegerLiteral(ulong value) => new(CreateToken(TokenType.Integer, value.ToString()), value);

public static UnaryOperationSyntax CreateNegativeIntegerLiteral(ulong value) => new(MinusToken, CreateIntegerLiteral(value));

public static StringSyntax CreateStringLiteral(string value) => CreateString(value.AsEnumerable(), Enumerable.Empty<SyntaxBase>());

Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Syntax/SyntaxRewriteVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ protected virtual SyntaxBase ReplaceIntegerLiteralSyntax(IntegerLiteralSyntax sy
return syntax;
}

return new IntegerLiteralSyntax(literal, long.Parse(literal.Text));
return new IntegerLiteralSyntax(literal, UInt64.Parse(literal.Text));
}
void ISyntaxVisitor.VisitIntegerLiteralSyntax(IntegerLiteralSyntax syntax) => ReplaceCurrent(syntax, ReplaceIntegerLiteralSyntax);

Expand Down
19 changes: 18 additions & 1 deletion src/Bicep.Core/TypeSystem/TypeAssignmentVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Bicep.Core.Syntax;
using Bicep.Core.Syntax.Visitors;
using Bicep.Core.Text;
using Microsoft.Identity.Client;

namespace Bicep.Core.TypeSystem
{
Expand Down Expand Up @@ -514,7 +515,17 @@ public override void VisitStringSyntax(StringSyntax syntax)
});

public override void VisitIntegerLiteralSyntax(IntegerLiteralSyntax syntax)
=> AssignType(syntax, () => LanguageConstants.Int);
=> AssignType(syntax, () =>
{
//if (syntax.Literal)

if (syntax.Value > long.MaxValue)
{
return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).InvalidInteger());
}

return LanguageConstants.Int;
});

public override void VisitNullLiteralSyntax(NullLiteralSyntax syntax)
=> AssignType(syntax, () => LanguageConstants.Null);
Expand Down Expand Up @@ -717,6 +728,12 @@ public override void VisitUnaryOperationSyntax(UnaryOperationSyntax syntax)
_ => throw new NotImplementedException()
};

//if minus and value > 8, (ex: -9 with integerValue of 9), then this is invalid 64 bit int
if (syntax.Operator == UnaryOperator.Minus && syntax.Expression is IntegerLiteralSyntax integerLiteral && integerLiteral.Value > (ulong)long.MaxValue + 1)
{
return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.Expression).InvalidInteger());
}

var operandType = typeManager.GetTypeInfo(syntax.Expression);
CollectErrors(errors, operandType);

Expand Down
6 changes: 3 additions & 3 deletions src/Bicep.Decompiler/TemplateConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private SyntaxBase ParseJTokenExpression(JTokenExpression expression)
=> expression.Value.Type switch
{
JTokenType.String => SyntaxFactory.CreateStringLiteral(expression.Value.Value<string>()!),
JTokenType.Integer => new IntegerLiteralSyntax(SyntaxFactory.CreateToken(TokenType.Integer, expression.Value.ToString()), expression.Value.Value<long>()),
JTokenType.Integer => new IntegerLiteralSyntax(SyntaxFactory.CreateToken(TokenType.Integer, expression.Value.ToString()), expression.Value.Value<ulong>()),
JTokenType.Boolean => expression.Value.Value<bool>() ?
new BooleanLiteralSyntax(SyntaxFactory.TrueKeywordToken, true) :
new BooleanLiteralSyntax(SyntaxFactory.FalseKeywordToken, false),
Expand Down Expand Up @@ -592,8 +592,8 @@ private SyntaxBase ParseString(string? value, IJsonLineInfo lineInfo)

private static IntegerLiteralSyntax ParseIntegerJToken(JValue value)
{
var longValue = value.Value<long>();
return SyntaxFactory.CreateIntegerLiteral(longValue);
var ulongValue = value.Value<ulong>();
return SyntaxFactory.CreateIntegerLiteral(ulongValue);
}

private SyntaxBase ParseJValue(JValue value)
Expand Down
10 changes: 8 additions & 2 deletions src/Bicep.LangServer/Handlers/InsertResourceHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,15 @@ private static SyntaxBase ConvertJsonElement(JsonElement element)
case JsonValueKind.String:
return SyntaxFactory.CreateStringLiteral(element.GetString()!);
case JsonValueKind.Number:
if (element.TryGetInt32(out var intValue))
if (element.TryGetInt64(out var intValue))
{
return SyntaxFactory.CreateIntegerLiteral(element.GetInt32()!);
if (intValue >= 0)
{
return SyntaxFactory.CreateIntegerLiteral((ulong)intValue);
} else
{
return SyntaxFactory.CreateNegativeIntegerLiteral((ulong)-intValue);
}
}
return SyntaxFactory.CreateStringLiteral(element.ToString()!);
case JsonValueKind.True:
Expand Down