Skip to content

Commit

Permalink
Adding fix to process 64 bit integers correctly. (#5791)
Browse files Browse the repository at this point in the history
* Adding fix to process 64 bit integers correctly.

* Adding fix to process 64 bit integers correctly.

* Adding fix to process 64 bit integers correctly.

* Adding baseline tests update.

* Added changes

* Fixed test for positive 64 bit integer.
  • Loading branch information
davidcho23 committed Feb 4, 2022
1 parent b777a99 commit 1faac71
Show file tree
Hide file tree
Showing 16 changed files with 192 additions and 25 deletions.
61 changes: 61 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2813,6 +2813,67 @@ public void Test_Issue5099()
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();
result.Template.Should().HaveValueAtPath("$.variables.myValue", "[json('-9223372036854775808')]");
}

[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();
result.Template.Should().HaveValueAtPath("$.variables.myValue", 9223372036854775807);
}

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

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
result.Template.Should().HaveValueAtPath("$.variables.myValue", -2147483648);
}

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

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
result.Template.Should().HaveValueAtPath("$.variables.myValue", 2147483647);
}

[DataTestMethod]
[DataRow("var myValue = -9223372036854775809")]
[DataRow("var myValue = 9223372036854775808")]
// https://github.com/Azure/bicep/issues/5371
public void Test_Issue5371_negative_tests(string fileContents)
{
var result = CompilationHelper.Compile(fileContents);

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

/// <summary>
/// https://github.com/Azure/bicep/issues/5456
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var bad = 3 * 4 /
var bad = 222222222222222222222222222222222222222222 * 4
//@[4:7) [BCP028 (Error)] Identifier "bad" is declared multiple times. Remove or rename the duplicates. (CodeDescription: none) |bad|
//@[4:7) [no-unused-vars (Warning)] Variable "bad" is declared but never used. (CodeDescription: bicep core(https://aka.ms/bicep/linter/no-unused-vars)) |bad|
//@[10:52) [BCP010 (Error)] Expected a valid 32-bit signed integer. (CodeDescription: none) |222222222222222222222222222222222222222222|
//@[10:52) [BCP010 (Error)] Expected a valid 64-bit signed integer. (CodeDescription: none) |222222222222222222222222222222222222222222|
var bad = (null) ?
//@[4:7) [BCP028 (Error)] Identifier "bad" is declared multiple times. Remove or rename the duplicates. (CodeDescription: none) |bad|
//@[4:7) [no-unused-vars (Warning)] Variable "bad" is declared but never used. (CodeDescription: bicep core(https://aka.ms/bicep/linter/no-unused-vars)) |bad|
Expand Down
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.SourceFile.ProgramSyntax, diagnosticWriter);

DetectDuplicateNames(model, diagnosticWriter, resourceScopeData, moduleScopeData);
DetectIncorrectlyFormattedNames(model, diagnosticWriter);
Expand Down
50 changes: 42 additions & 8 deletions src/Bicep.Core/Emit/ExpressionConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using Azure.Deployments.Core.Extensions;
using Azure.Deployments.Expression.Expressions;
using Bicep.Core.Extensions;
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 +54,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 +812,63 @@ 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);
// shortcutting the integer parsing logic here because we need to return either the literal 32 bit integer or the FunctionExpression of an integer outside the 32 bit range
return ConvertInteger(integerLiteral, true);
}

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

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

// the deployment engine can only handle 32 bit integers expressed as literal values, so for 32 bit integers, we return the literal integer value
// for values outside that signed 32 bit integer range, we return the FunctionExpression
private LanguageExpression ConvertInteger(IntegerLiteralSyntax integerSyntax, bool minus)
{
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)
{
return CreateFunction("json", new JTokenExpression($"-{integerSyntax.Value.ToString(CultureInfo.InvariantCulture)}"));
}
else
{
// the integerSyntax.Value is a valid negative 32 bit integer.
// because integerSyntax.Value is a ulong type, it is always positive. we need to first cast it to a long in order to negate it.
// after negating, cast it to a int type because that is what represents a signed 32 bit integer.
var longValue = -(long)integerSyntax.Value;
return new JTokenExpression((int)longValue);
}
}
else
{
if (integerSyntax.Value > int.MaxValue)
{
return CreateFunction("json", new JTokenExpression(integerSyntax.Value.ToString(CultureInfo.InvariantCulture)));
}
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
52 changes: 52 additions & 0 deletions src/Bicep.Core/Emit/IntegerValidatorVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

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

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

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

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

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 && syntax.Expression is IntegerLiteralSyntax integerLiteral)
{
if (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 (ulong.TryParse(literal.Text, NumberStyles.None, CultureInfo.InvariantCulture, out ulong value))
{
return new IntegerLiteralSyntax(literal, value);
}
Expand Down
14 changes: 12 additions & 2 deletions src/Bicep.Core/Semantics/Namespaces/SystemNamespaceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -685,9 +685,19 @@ static DecoratorEvaluator MergeToTargetObject(string propertyName, Func<Decorato

static long? TryGetIntegerLiteralValue(SyntaxBase syntax) => syntax switch
{
// if integerLiteralSyntax.Value is within the 64 bit integer range, negate it after casting to a long type
// long.MaxValue + 1 (9,223,372,036,854,775,808) is the only invalid 64 bit integer value that may be passed. we avoid casting to a long because this causes overflow. we need to just return long.MinValue (-9,223,372,036,854,775,808)
// if integerLiteralSyntax.Value is outside the range, return null. it should have already been caught by a different validation
UnaryOperationSyntax { Operator: UnaryOperator.Minus } unaryOperatorSyntax
when unaryOperatorSyntax.Expression is IntegerLiteralSyntax integerLiteralSyntax => -1 * integerLiteralSyntax.Value,
IntegerLiteralSyntax integerLiteralSyntax => integerLiteralSyntax.Value,
when unaryOperatorSyntax.Expression is IntegerLiteralSyntax integerLiteralSyntax => integerLiteralSyntax.Value switch {
<= long.MaxValue => -(long)integerLiteralSyntax.Value,
(ulong)long.MaxValue + 1 => long.MinValue,
_ => null
},

// this ternary check is to make sure that the integer value is within the range of a signed 64 bit integer before casting to a long type
// if not, it would have been caught already by a different validation
IntegerLiteralSyntax integerLiteralSyntax => integerLiteralSyntax.Value <= long.MaxValue ? (long)integerLiteralSyntax.Value : null,
_ => 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, ulong.Parse(literal.Text));
}
void ISyntaxVisitor.VisitIntegerLiteralSyntax(IntegerLiteralSyntax syntax) => ReplaceCurrent(syntax, ReplaceIntegerLiteralSyntax);

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 => expression.Value.Value<long>() is long value && value >= 0 ? ParseIntegerJToken((JValue)value) : ParseIntegerJToken((JValue)(-value)),
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
11 changes: 9 additions & 2 deletions src/Bicep.LangServer/Handlers/InsertResourceHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,16 @@ 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

0 comments on commit 1faac71

Please sign in to comment.