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 @@ -2813,6 +2813,54 @@ 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();
}

[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();
}
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved

[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.")
});
}

/// <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, diagnosticWriter);

DetectDuplicateNames(model, diagnosticWriter, resourceScopeData, moduleScopeData);
DetectIncorrectlyFormattedNames(model, diagnosticWriter);
Expand Down
49 changes: 41 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,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);
// 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
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
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
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
{
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
54 changes: 54 additions & 0 deletions src/Bicep.Core/Emit/IntegerValidatorVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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;
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
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);
}

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)
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
{
if (syntax.Expression is IntegerLiteralSyntax integerLiteral && integerLiteral.Value > (ulong)long.MaxValue + 1)
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
{
diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax).InvalidInteger());
}
} else
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
{
base.VisitUnaryOperationSyntax(syntax);
}
}
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
}
}
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 {
_ when integerLiteralSyntax.Value <= long.MaxValue => -(long)integerLiteralSyntax.Value,
_ when integerLiteralSyntax.Value == (ulong)long.MaxValue + 1 => long.MinValue,
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
_ => 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
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
davidcho23 marked this conversation as resolved.
Show resolved Hide resolved
{
return SyntaxFactory.CreateNegativeIntegerLiteral((ulong)-intValue);
}
}
return SyntaxFactory.CreateStringLiteral(element.ToString()!);
anthony-c-martin marked this conversation as resolved.
Show resolved Hide resolved
case JsonValueKind.True:
Expand Down