Merge pull request #233 from MarkusAmshove/natparse/check-decide-on-d…

…atatypes

Validate data type of DECIDE ON branches

libs/natparse/src/main/java/org/amshove/natparse/natural/IDataType.java

@@ -1,5 +1,7 @@
 package org.amshove.natparse.natural;
 
+import static org.amshove.natparse.natural.DataFormat.*;
+
 public interface IDataType
 {
 	int ONE_GIGABYTE = 1073741824;
@@ -13,18 +15,50 @@ public interface IDataType
 
 	boolean hasDynamicLength();
 
+	/**
+	 * Determines if this type fits into the given type. Implicit conversion is taken into account.
+	 */
 	default boolean fitsInto(IDataType target)
 	{
 		var bytesFit = this.byteSize() <= target.byteSize();
-		var targetFormat = target.format();
-		var ourFormat = format();
-		var formatIsCompatible = ourFormat == targetFormat || switch (ourFormat)
+		var formatIsCompatible = hasCompatibleFormat(target);
+
+		return bytesFit && formatIsCompatible;
+	}
+
+	/**
+	 * Determines if both types have the same family, e.g. N, I, P are all numeric.
+	 */
+	default boolean hasSameFamily(IDataType other)
+	{
+		var targetFormat = other.format();
+		return format() == targetFormat || switch (format())
 		{
-			case PACKED, FLOAT, INTEGER, NUMERIC -> targetFormat == DataFormat.ALPHANUMERIC || targetFormat == DataFormat.UNICODE;
-			default -> false; // we don't know whats implicitly compatible yet
+			case PACKED, FLOAT, INTEGER, NUMERIC -> targetFormat == PACKED
+				|| targetFormat == FLOAT
+				|| targetFormat == INTEGER
+				|| targetFormat == NUMERIC
+				|| targetFormat == BINARY;
+			case ALPHANUMERIC, UNICODE, BINARY -> targetFormat == ALPHANUMERIC
+				|| targetFormat == UNICODE
+				|| targetFormat == BINARY;
+			default -> false;
 		};
+	}
 
-		return bytesFit && formatIsCompatible;
+	/**
+	 * Takes implicit conversion into account, e.g. N -> A
+	 */
+	default boolean hasCompatibleFormat(IDataType other)
+	{
+		var targetFormat = other.format();
+		return hasSameFamily(other) || switch (format())
+		{
+			case PACKED, FLOAT, INTEGER, NUMERIC -> targetFormat == ALPHANUMERIC
+				|| targetFormat == UNICODE
+				|| targetFormat == BINARY;
+			default -> false; // we don't know whats implicitly compatible yet
+		};
 	}
 
 	default String toShortString()
@@ -34,7 +68,7 @@ default String toShortString()
 		details += "(%s".formatted(format().identifier());
 		if (length() > 0.0)
 		{
-			details += "%s".formatted(DataFormat.formatLength(length()));
+			details += "%s".formatted(formatLength(length()));
 		}
 		details += ")";
 

libs/natparse/src/main/java/org/amshove/natparse/parsing/LiteralNode.java

@@ -16,14 +16,30 @@ public LiteralNode(SyntaxToken token)
 		dataType = switch (token.kind())
 		{
 			case STRING_LITERAL -> new LiteralType(DataFormat.ALPHANUMERIC, token.stringValue().length());
-			case NUMBER_LITERAL -> new LiteralType(DataFormat.NUMERIC, Double.parseDouble(token.source().replace(",", ".")));
+			case NUMBER_LITERAL -> new LiteralType(DataFormat.NUMERIC, getNumericLiteralLength(token.source()));
 			case TRUE, FALSE -> new LiteralType(DataFormat.LOGIC, 1);
 			case ASTERISK -> new LiteralType(DataFormat.NONE, 0);
 
 			default -> throw new IllegalStateException("Invalid literal kind: " + token.kind());
 		};
 	}
 
+	private double getNumericLiteralLength(String source)
+	{
+		if (!source.contains(".") && !source.contains(","))
+		{
+			return source.length();
+		}
+
+		var normalized = source.replace(',', '.');
+		var split = normalized.split("\\.");
+		if (split.length == 1)
+		{
+			return split[0].length();
+		}
+		return Double.parseDouble("%s.%s".formatted(split[0].length(), split[1].length())); // there must be a smarter way
+	}
+
 	@Override
 	public IDataType dataType()
 	{

libs/natparse/src/main/java/org/amshove/natparse/parsing/TypeChecker.java

@@ -70,12 +70,44 @@ private void checkNode(ISyntaxNode node)
 			checkAlphanumericInitLength(typedVariableNode);
 		}
 
+		if (node instanceof IDecideOnNode decideOn)
+		{
+			checkDecideOnBranches(decideOn);
+		}
+
 		if (node instanceof IVariableReferenceNode variableReference)
 		{
 			checkVariableReference(variableReference);
 		}
 	}
 
+	private void checkDecideOnBranches(IDecideOnNode decideOn)
+	{
+		if (!(decideOn.operand()instanceof IVariableReferenceNode target)
+			|| !(target.reference()instanceof ITypedVariableNode typedTarget)
+			|| typedTarget.type() == null)
+		{
+			return;
+		}
+
+		for (var branch : decideOn.branches())
+		{
+			for (var value : branch.values())
+			{
+				var inferredType = inferDataType(value);
+				if (inferredType.format() != DataFormat.NONE && !inferredType.hasSameFamily(typedTarget.type()))
+				{
+					report(
+						ParserErrors.typeMismatch(
+							"Inferred format %s is not compatible with %s (%s)".formatted(inferredType.format(), typedTarget.declaration().symbolName(), typedTarget.type().format()),
+							value
+						)
+					);
+				}
+			}
+		}
+	}
+
 	private void checkVariableReference(IVariableReferenceNode variableReference)
 	{
 		if (!(variableReference.reference()instanceof IVariableNode target))
@@ -340,4 +372,30 @@ private boolean containsDynamicDimension(IRangedArrayAccessNode ranged)
 			&& (lowerLiteral.token().kind() == SyntaxKind.ASTERISK
 				|| upperLiteral.token().kind() == SyntaxKind.ASTERISK);
 	}
+
+	private IDataType inferDataType(IOperandNode operand)
+	{
+		if (operand instanceof IVariableReferenceNode variable && variable.reference()instanceof ITypedVariableNode typedRef)
+		{
+			return typedRef.type();
+		}
+
+		if (operand instanceof ILiteralNode literal)
+		{
+			return literal.dataType();
+		}
+
+		if (operand instanceof ISystemFunctionNode sysFunction)
+		{
+			return BuiltInFunctionTable.getDefinition(sysFunction.systemFunction()).type();
+		}
+
+		if (operand instanceof ISystemVariableNode sysVar)
+		{
+			return BuiltInFunctionTable.getDefinition(sysVar.systemVariable()).type();
+		}
+
+		return new DataType(DataFormat.NONE, IDataType.ONE_GIGABYTE); // couldn't infer, don't raise something yet
+	}
+
 }

libs/natparse/src/test/java/org/amshove/natparse/parsing/DataTypeCheckingShould.java

@@ -3,6 +3,7 @@
 import org.amshove.natparse.natural.DataFormat;
 import org.amshove.natparse.natural.IDataType;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
@@ -113,6 +114,48 @@ void seeImplicitlyCompatibleDataTypesAsIncompatibleWithBiggerLength(String numer
 		);
 	}
 
+	@ParameterizedTest
+	@CsvSource(
+		{
+			"N,N", "N,I", "N,F", "N,P", "N,B",
+			"A,U", "A,B", "A,A"
+		}
+	)
+	void recognizeDataFormatsAsTheSameFamily(String firstFormat, String secondFormat)
+	{
+		assertSameFamily(firstFormat, secondFormat);
+	}
+
+	@ParameterizedTest
+	@CsvSource(
+		{
+			"N,A", "N,U", "N,L", "N,C", "N,D", "N,T",
+			"A,L", "A,C", "A,D", "A,F", "A,I", "A,P", "A,T",
+			"I,A", "I,U", "I,L", "I,C", "I,D", "I,T",
+			"P,A", "P,U", "P,L", "P,C", "P,D", "P,T",
+			"F,A", "F,U", "F,L", "F,C", "F,D", "F,T",
+			"L,B", "L,D", "L,T", "L,U", "L,A", "L,P"
+		}
+	)
+	void recognizeDataFormatsAsDifferentFamily(String firstFormat, String secondFormat)
+	{
+		assertDifferentFamily(firstFormat, secondFormat);
+	}
+
+	private void assertSameFamily(String firstFormat, String secondFormat)
+	{
+		assertThat(type(DataFormat.fromSource(firstFormat), IDataType.ONE_GIGABYTE).hasSameFamily(type(DataFormat.fromSource(secondFormat), IDataType.ONE_GIGABYTE)))
+			.as("Expected %s and %s to be the same format family".formatted(firstFormat, secondFormat))
+			.isTrue();
+	}
+
+	private void assertDifferentFamily(String firstFormat, String secondFormat)
+	{
+		assertThat(type(DataFormat.fromSource(firstFormat), IDataType.ONE_GIGABYTE).hasSameFamily(type(DataFormat.fromSource(secondFormat), IDataType.ONE_GIGABYTE)))
+			.as("Expected %s and %s to not be the same format family".formatted(firstFormat, secondFormat))
+			.isFalse();
+	}
+
 	private void assertCompatible(IDataType firstType, IDataType targetType)
 	{
 		assertThat(firstType.fitsInto(targetType))

libs/natparse/src/test/resources/org/amshove/natparse/parsing/typing/decideOnTyping

@@ -0,0 +1,52 @@
+DEFINE DATA LOCAL
+1 #B-10 (B10)
+1 #A-10 (A10)
+1 #N-10 (N10)
+END-DEFINE
+
+DECIDE ON FIRST VALUE OF #A-10
+
+    VALUE 5 /* !{D:ERROR:NPP037}
+    IGNORE
+
+    VALUE TRUE /* !{D:ERROR:NPP037}
+    IGNORE
+
+    VALUE FALSE /* !{D:ERROR:NPP037}
+    IGNORE
+
+    NONE
+    IGNORE
+END-DECIDE
+
+DECIDE ON FIRST VALUE OF #N-10
+
+    VALUE 5
+    IGNORE
+
+    VALUE 500
+    IGNORE
+
+    VALUE 'Hi' /* !{D:ERROR:NPP037}
+    IGNORE
+
+    VALUE TRUE /* !{D:ERROR:NPP037}
+    IGNORE
+
+    VALUE FALSE /* !{D:ERROR:NPP037}
+    IGNORE
+
+    NONE
+    IGNORE
+END-DECIDE
+
+DECIDE ON FIRST VALUE OF #B-10
+    VALUE 5
+    IGNORE
+    VALUE 'Hi'
+    IGNORE
+    NONE
+    IGNORE
+END-DECIDE
+
+END