From a612becb7a553d9f1b13d3176581f7e51c518e60 Mon Sep 17 00:00:00 2001 From: Markus Amshove Date: Sun, 7 May 2023 20:25:48 +0200 Subject: [PATCH] Validate data type of DECIDE ON branches --- .../amshove/natparse/natural/IDataType.java | 48 ++++++++++++--- .../amshove/natparse/parsing/LiteralNode.java | 18 +++++- .../amshove/natparse/parsing/TypeChecker.java | 58 +++++++++++++++++++ .../parsing/DataTypeCheckingShould.java | 43 ++++++++++++++ .../natparse/parsing/typing/decideOnTyping | 52 +++++++++++++++++ 5 files changed, 211 insertions(+), 8 deletions(-) create mode 100644 libs/natparse/src/test/resources/org/amshove/natparse/parsing/typing/decideOnTyping diff --git a/libs/natparse/src/main/java/org/amshove/natparse/natural/IDataType.java b/libs/natparse/src/main/java/org/amshove/natparse/natural/IDataType.java index c26c76801..1a6978fa8 100644 --- a/libs/natparse/src/main/java/org/amshove/natparse/natural/IDataType.java +++ b/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 += ")"; diff --git a/libs/natparse/src/main/java/org/amshove/natparse/parsing/LiteralNode.java b/libs/natparse/src/main/java/org/amshove/natparse/parsing/LiteralNode.java index 3193742dd..8d91c99a4 100644 --- a/libs/natparse/src/main/java/org/amshove/natparse/parsing/LiteralNode.java +++ b/libs/natparse/src/main/java/org/amshove/natparse/parsing/LiteralNode.java @@ -16,7 +16,7 @@ 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); @@ -24,6 +24,22 @@ public LiteralNode(SyntaxToken token) }; } + 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() { diff --git a/libs/natparse/src/main/java/org/amshove/natparse/parsing/TypeChecker.java b/libs/natparse/src/main/java/org/amshove/natparse/parsing/TypeChecker.java index ce1bf78bb..4bad07699 100644 --- a/libs/natparse/src/main/java/org/amshove/natparse/parsing/TypeChecker.java +++ b/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 + } + } diff --git a/libs/natparse/src/test/java/org/amshove/natparse/parsing/DataTypeCheckingShould.java b/libs/natparse/src/test/java/org/amshove/natparse/parsing/DataTypeCheckingShould.java index f95e13f7a..d949b56bd 100644 --- a/libs/natparse/src/test/java/org/amshove/natparse/parsing/DataTypeCheckingShould.java +++ b/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)) diff --git a/libs/natparse/src/test/resources/org/amshove/natparse/parsing/typing/decideOnTyping b/libs/natparse/src/test/resources/org/amshove/natparse/parsing/typing/decideOnTyping new file mode 100644 index 000000000..4d8593b80 --- /dev/null +++ b/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