Skip to content

Commit

Permalink
[#64] Split type Number
Browse files Browse the repository at this point in the history
To be more robust and keep a logic in our code, split the type `Number`.

This change imply to correct lot of tests to consider the new types.
However, the production code was not a big change.

This commit does the following:

* Correct antlr definition to identify both integers and numbers
* Create type `IntegerValue` to recognize an integer instead of a number
* Rename controls `CalculatorOperandsAreNumber` and `OrderOperandsAreNumber` to be more expressive
* Calculator and orders must be on the same numeric type to avoid risks in target languages
* Correct tests to use the right numeric type when there was a confusion between the two
* Add test ensuring that the same numeric type is used in calculator and order expressions
  • Loading branch information
Gaëtan Rizio committed Aug 11, 2018
1 parent f205a7b commit f3e2435
Show file tree
Hide file tree
Showing 51 changed files with 366 additions and 294 deletions.
2 changes: 1 addition & 1 deletion build.sbt
Expand Up @@ -17,7 +17,7 @@ libraryDependencies += "org.typelevel" %% "cats-core" % "1.2.0"

libraryDependencies += "org.scalatest" %% "scalatest" % "3.0.5" % "test"
libraryDependencies += "org.scalacheck" %% "scalacheck" % "1.14.0" % "test"
libraryDependencies += "io.github.definiti" % "api" % "0.3.0" % "test"
libraryDependencies += "io.github.definiti" % "api" % "0.3.1-SNAPSHOT" % "test"

scalacOptions ++= Seq("-unchecked", "-deprecation", "-language:implicitConversions", "-feature")

Expand Down
4 changes: 3 additions & 1 deletion src/main/antlr/Definiti.g4
Expand Up @@ -17,7 +17,8 @@ KO : 'ko';
AS : 'as';

BOOLEAN : 'true' | 'false';
NUMBER : [0-9]+('.'[0-9]+)?;
NUMBER : [0-9]+'.'([0-9]+)?;
INTEGER : [0-9]+;
STRING : '"' ( '\\"' | . )*? '"';
IDENTIFIER : [a-zA-Z0-9]+;
CALCULATOR_OPERATOR_LEVEL_1 : ('*' | '/' | '%');
Expand Down Expand Up @@ -71,6 +72,7 @@ expression
atomicExpression
: booleanExpression=BOOLEAN
| numberExpression=NUMBER
| integerExpression=INTEGER
| stringExpression=STRING
| referenceExpression=IDENTIFIER
;
Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/definiti/common/ast/Expression.scala
Expand Up @@ -28,6 +28,8 @@ case class Not(inner: Expression, returnType: AbstractTypeReference, location: L

case class BooleanValue(value: Boolean, returnType: AbstractTypeReference, location: Location) extends AtomicExpression

case class IntegerValue(value: BigInt, returnType: AbstractTypeReference, location: Location) extends AtomicExpression

case class NumberValue(value: BigDecimal, returnType: AbstractTypeReference, location: Location) extends AtomicExpression

case class QuotedStringValue(value: String, returnType: AbstractTypeReference, location: Location) extends AtomicExpression
Expand Down
Expand Up @@ -100,8 +100,8 @@ private[core] class DefinitiFileASTParser(
private def processAttributeDefinition(context: AttributeDefinitionContext): AttributeDefinition = {
val attributeName = context.attributeName.getText
val typeDeclaration = Option(context.typeDeclaration)
.map(processTypeDeclaration)
.getOrElse(TypeDeclaration(attributeName.capitalize, Seq.empty, Seq.empty, getLocationFromToken(context.attributeName)))
.map(processTypeDeclaration)
.getOrElse(TypeDeclaration(attributeName.capitalize, Seq.empty, Seq.empty, getLocationFromToken(context.attributeName)))
AttributeDefinition(
name = attributeName,
typeDeclaration = typeDeclaration,
Expand Down Expand Up @@ -389,6 +389,8 @@ private[core] class DefinitiFileASTParser(
processBooleanExpression(context)
} else if (context.numberExpression != null) {
processNumberExpression(context)
} else if (context.integerExpression != null) {
processIntegerExpression(context)
} else if (context.stringExpression != null) {
processStringExpression(context)
} else if (context.referenceExpression != null) {
Expand All @@ -415,6 +417,14 @@ private[core] class DefinitiFileASTParser(
)
}

private def processIntegerExpression(context: AtomicExpressionContext): AtomicExpression = {
IntegerValue(
value = BigInt(context.integerExpression.getText),
returnType = Unset,
location = getLocationFromContext(context)
)
}

private def processStringExpression(context: AtomicExpressionContext): AtomicExpression = {
QuotedStringValue(
value = extractStringContent(context.stringExpression.getText),
Expand Down
4 changes: 3 additions & 1 deletion src/main/scala/definiti/core/typing/ExpressionTyping.scala
Expand Up @@ -33,6 +33,7 @@ private[core] class ExpressionTyping(context: Context) {
def addTypeIntoAtomicExpression(expression: AtomicExpression): Validated[AtomicExpression] = {
expression match {
case booleanValue: BooleanValue => Valid(booleanValue.copy(returnType = boolean))
case integerValue: IntegerValue => Valid(integerValue.copy(returnType = integer))
case numberValue: NumberValue => Valid(numberValue.copy(returnType = number))
case quotedStringValue: QuotedStringValue => Valid(quotedStringValue.copy(returnType = string))
case reference: Reference => addTypesIntoReference(reference)
Expand All @@ -56,7 +57,7 @@ private[core] class ExpressionTyping(context: Context) {
calculatorExpression.copy(
left = typedLeft,
right = typedRight,
returnType = number
returnType = typedLeft.returnType
)
}
}
Expand Down Expand Up @@ -341,6 +342,7 @@ object ExpressionTyping {
type LeftRightExpressionConstructor = (Expression, Expression, TypeReference, Range) => Expression

val boolean = TypeReference("Boolean", Seq.empty)
val integer = TypeReference("Integer", Seq.empty)
val number = TypeReference("Number", Seq.empty)
val string = TypeReference("String", Seq.empty)
val okko = TypeReference("OkKo", Seq.empty)
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/definiti/core/validation/Controls.scala
Expand Up @@ -21,7 +21,7 @@ object Controls {
AliasTypeTypeControl,
AttributeTypeControl,
AttributeTypeUniquenessControl,
CalculatorOperandsAreNumberControl,
CalculatorOperandsAreSameNumericControl,
EnumerationUniquenessControl,
ComparisonOnSameTypeControl,
FunctionParametersControl,
Expand All @@ -30,7 +30,7 @@ object Controls {
NamedFunctionParameterTypeControl,
NamedFunctionTypeControl,
NotExpressionIsBooleanControl,
OrderOperandsAreNumberControl,
OrderOperandsAreSameNumericControl,
VerificationParameterUsableControl,
TopLevelFullNameUniquenessControl,
TypeNameFormatControl,
Expand Down

This file was deleted.

@@ -0,0 +1,56 @@
package definiti.core.validation.controls

import definiti.common.ast._
import definiti.common.control.{Control, ControlLevel, ControlResult}
import definiti.common.validation.Alert
import definiti.core.validation.helpers.{ExpressionControlHelper, TypeReferenceControlHelper}

private[core] object CalculatorOperandsAreSameNumericControl extends Control[Root] with ExpressionControlHelper with TypeReferenceControlHelper {
override val description: String = "Check if all operands of calculator expression are number expressions"
override val defaultLevel: ControlLevel.Value = ControlLevel.error

override def control(root: Root, library: Library): ControlResult = {
testAllExpressions(library) { expression =>
deepControl(expression) {
case calculatorExpression: CalculatorExpression =>
controlCalculatorExpression(calculatorExpression, library)
}
}
}

private def controlCalculatorExpression(expression: CalculatorExpression, library: Library): ControlResult = {
val leftIsNumber = isNumber(expression.left.returnType, library)
val rightIsNumber = isNumber(expression.right.returnType, library)

val leftIsInteger = isInteger(expression.left.returnType, library)
val rightIsInteger = isInteger(expression.right.returnType, library)

val leftIsNumeric = leftIsNumber || leftIsInteger
val rightIsNumeric = rightIsNumber || rightIsInteger

if (leftIsNumeric && rightIsNumeric) {
if (leftIsNumber && rightIsNumber || leftIsInteger && rightIsInteger) {
OK
} else {
ControlResult(differentNumeric(expression.left.returnType, expression.right.returnType, expression.location))
}
} else if (leftIsNumeric) {
ControlResult(errorNotNumeric(expression.right.returnType, expression.right.location))
} else if (rightIsNumeric) {
ControlResult(errorNotNumeric(expression.left.returnType, expression.left.location))
} else {
ControlResult(
errorNotNumeric(expression.left.returnType, expression.left.location),
errorNotNumeric(expression.right.returnType, expression.right.location)
)
}
}

def errorNotNumeric(returnType: AbstractTypeReference, location: Location): Alert = {
alert(s"Numeric expected, got: ${returnType.readableString}", location)
}

def differentNumeric(left: AbstractTypeReference, right: AbstractTypeReference, location: Location): Alert = {
alert(s"Numeric types are different: ${left.readableString} and ${right.readableString}", location)
}
}

This file was deleted.

@@ -0,0 +1,63 @@
package definiti.core.validation.controls

import definiti.common.ast._
import definiti.common.control.{Control, ControlLevel, ControlResult}
import definiti.common.validation.Alert
import definiti.core.validation.helpers.{ExpressionControlHelper, TypeReferenceControlHelper}

private[core] object OrderOperandsAreSameNumericControl extends Control[Root] with ExpressionControlHelper with TypeReferenceControlHelper {
override val description: String = "Check if all operands of logical expression are number expressions for >, <, >= and <="
override val defaultLevel: ControlLevel.Value = ControlLevel.error

override def control(root: Root, library: Library): ControlResult = {
testAllExpressions(library) { expression =>
deepControl(expression) {
case logicalExpression: LogicalExpression if shouldBeControlled(logicalExpression) =>
controlLogicalExpression(logicalExpression, library)
}
}
}

private def shouldBeControlled(logicalExpression: LogicalExpression): Boolean = {
logicalExpression.operator == LogicalOperator.Lower ||
logicalExpression.operator == LogicalOperator.Upper ||
logicalExpression.operator == LogicalOperator.LowerOrEqual ||
logicalExpression.operator == LogicalOperator.UpperOrEqual
}

private def controlLogicalExpression(expression: LogicalExpression, library: Library): ControlResult = {
val leftIsNumber = isNumber(expression.left.returnType, library)
val rightIsNumber = isNumber(expression.right.returnType, library)

val leftIsInteger = isInteger(expression.left.returnType, library)
val rightIsInteger = isInteger(expression.right.returnType, library)

val leftIsNumeric = leftIsNumber || leftIsInteger
val rightIsNumeric = rightIsNumber || rightIsInteger

if (leftIsNumeric && rightIsNumeric) {
if (leftIsNumber && rightIsNumber || leftIsInteger && rightIsInteger) {
OK
} else {
ControlResult(differentNumeric(expression.left.returnType, expression.right.returnType, expression.location))
}
} else if (leftIsNumeric) {
ControlResult(errorNotNumeric(expression.right.returnType, expression.right.location))
} else if (rightIsNumeric) {
ControlResult(errorNotNumeric(expression.left.returnType, expression.left.location))
} else {
ControlResult(
errorNotNumeric(expression.left.returnType, expression.left.location),
errorNotNumeric(expression.right.returnType, expression.right.location)
)
}
}

def errorNotNumeric(returnType: AbstractTypeReference, location: Location): Alert = {
alert(s"Number expected, got: ${returnType.readableString}", location)
}

def differentNumeric(left: AbstractTypeReference, right: AbstractTypeReference, location: Location): Alert = {
alert(s"Numeric types are different: ${left.readableString} and ${right.readableString}", location)
}
}
Expand Up @@ -35,23 +35,23 @@ private[core] trait TypeReferenceControlHelper {
}

def isBoolean(typeReference: AbstractTypeReference, library: Library): Boolean = {
typeReference match {
case typeReference: TypeReference =>
library.typesMap.get(typeReference.typeName) match {
case Some(native: NativeClassDefinition) => native.name == "Boolean"
case Some(alias: AliasType) => isBoolean(alias.alias, library)
case _ => false
}
case _ => false
}
isTypeDeeply("Boolean", typeReference, library)
}

def isInteger(typeReference: AbstractTypeReference, library: Library): Boolean = {
isTypeDeeply("Integer", typeReference, library)
}

def isNumber(typeReference: AbstractTypeReference, library: Library): Boolean = {
isTypeDeeply("Number", typeReference, library)
}

private def isTypeDeeply(typeName: String, typeReference: AbstractTypeReference, library: Library): Boolean = {
typeReference match {
case typeReference: TypeReference =>
library.typesMap.get(typeReference.typeName) match {
case Some(native: NativeClassDefinition) => native.name == "Number"
case Some(alias: AliasType) => isNumber(alias.alias, library)
case Some(native: NativeClassDefinition) => native.name == typeName
case Some(alias: AliasType) => isTypeDeeply(typeName, alias.alias, library)
case _ => false
}
case _ => false
Expand Down
@@ -0,0 +1,3 @@
def compare(number: Number, integer: Integer): Boolean => {
number / integer
}
Expand Up @@ -9,12 +9,12 @@ type MyType {
verify {
"Some test"
(value) => {
value.min - value.max + 1
value.min - value.max + 1.8
}
}
}

type MyAlias = Number {
type MyAlias = Integer {
verify {
"Some test"
(value) => {
Expand All @@ -25,7 +25,7 @@ type MyAlias = Number {

verification MyVerification {
"Some test"
(value: Number) => {
(value: Integer) => {
value + 15 * 100
}
}

0 comments on commit f3e2435

Please sign in to comment.