From 7947fabed6d44485444612ab06dbf3690c9a413e Mon Sep 17 00:00:00 2001 From: Daniel Trierweiler Date: Thu, 23 Jul 2026 14:40:24 +0200 Subject: [PATCH 1/2] #407 - Add support for inline if expressions --- CHANGELOG.md | 4 + .../au/com/integradev/delphi/antlr/Delphi.g | 6 + .../antlr/ast/node/IfExpressionNodeImpl.java | 75 +++++++++++++ .../ast/visitors/DelphiParserVisitor.java | 5 + .../delphi/cfg/ControlFlowGraphVisitor.java | 8 ++ .../resolve/ExpressionTypeResolver.java | 42 +++++++ .../api/ast/IfExpressionNode.java | 43 ++++++++ .../integradev/delphi/antlr/GrammarTest.java | 5 + .../ast/node/IfExpressionNodeImplTest.java | 104 ++++++++++++++++++ .../resolve/ExpressionTypeResolverTest.java | 91 +++++++++++++++ .../delphi/grammar/IfExpression.pas | 31 ++++++ 11 files changed, 414 insertions(+) create mode 100644 delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java create mode 100644 delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java create mode 100644 delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java create mode 100644 delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java create mode 100644 delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas diff --git a/CHANGELOG.md b/CHANGELOG.md index 278025ce9..1c61f68bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Support for `if` conditional expressions, introduced in Delphi 13. + ## [1.19.0] - 2026-06-09 ### Added diff --git a/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g b/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g index 0b3599e56..335ac389b 100644 --- a/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g +++ b/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g @@ -922,6 +922,7 @@ attribute : (ASSEMBLY ':')? expression (':' expression)* //---------------------------------------------------------------------------- expression : relationalExpression | anonymousMethod + | ifExpression ; // ANTLR sets the begin and end tokens for nested binary expression nodes // in relationalOperator, not relationalExpression, meaning that their @@ -984,6 +985,11 @@ anonymousMethodHeading : PROCEDURE routineParameters? ((';')? interfaceDir | FUNCTION routineParameters? routineReturnType ((';')? interfaceDirective)* -> ^(TkAnonymousMethodHeading FUNCTION routineParameters? routineReturnType ((';')? interfaceDirective)*) ; +// Conditional expression ("inline if"), introduced in Delphi 13. +// Unlike ifStatement, both branches are required since the construct must always yield a value. +// See: https://docwiki.embarcadero.com/RADStudio/Florence/en/Conditional_Operators_(Delphi) +ifExpression : IF^ expression THEN expression ELSE expression + ; expressionOrRange : expression ('..'^ expression)? ; expressionList : (expression (','!)?)+ diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java new file mode 100644 index 000000000..aeb3df405 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java @@ -0,0 +1,75 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2019 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.antlr.ast.node; + +import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor; +import au.com.integradev.delphi.symbol.resolve.ExpressionTypeResolver; +import javax.annotation.Nonnull; +import org.antlr.runtime.Token; +import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; +import org.sonar.plugins.communitydelphi.api.type.Type; + +public final class IfExpressionNodeImpl extends ExpressionNodeImpl implements IfExpressionNode { + private String image; + + public IfExpressionNodeImpl(Token token) { + super(token); + } + + @Override + public T accept(DelphiParserVisitor visitor, T data) { + return visitor.visit(this, data); + } + + @Override + public ExpressionNode getConditionExpression() { + return (ExpressionNode) getChild(0); + } + + @Override + public ExpressionNode getThenExpression() { + return (ExpressionNode) getChild(2); + } + + @Override + public ExpressionNode getElseExpression() { + return (ExpressionNode) getChild(4); + } + + @Override + public String getImage() { + if (image == null) { + image = + "if " + + getConditionExpression().getImage() + + " then " + + getThenExpression().getImage() + + " else " + + getElseExpression().getImage(); + } + return image; + } + + @Override + @Nonnull + protected Type createType() { + return new ExpressionTypeResolver(getTypeFactory()).resolve(this); + } +} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DelphiParserVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DelphiParserVisitor.java index 2809a1a1a..2800be80b 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DelphiParserVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DelphiParserVisitor.java @@ -81,6 +81,7 @@ import org.sonar.plugins.communitydelphi.api.ast.GotoStatementNode; import org.sonar.plugins.communitydelphi.api.ast.HelperTypeNode; import org.sonar.plugins.communitydelphi.api.ast.IdentifierNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ImplementationSectionNode; import org.sonar.plugins.communitydelphi.api.ast.ImportClauseNode; @@ -636,6 +637,10 @@ default T visit(BinaryExpressionNode node, T data) { return visit((ExpressionNode) node, data); } + default T visit(IfExpressionNode node, T data) { + return visit((ExpressionNode) node, data); + } + default T visit(ParenthesizedExpressionNode node, T data) { return visit((ExpressionNode) node, data); } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java index bf8240419..7d2105b55 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java @@ -50,6 +50,7 @@ import org.sonar.plugins.communitydelphi.api.ast.ForStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForToStatementNode; import org.sonar.plugins.communitydelphi.api.ast.GotoStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.IntegerLiteralNode; import org.sonar.plugins.communitydelphi.api.ast.LabelStatementNode; @@ -123,6 +124,13 @@ public ControlFlowGraphBuilder visit(RangeExpressionNode node, ControlFlowGraphB return build(node.getLowExpression(), builder); } + @Override + public ControlFlowGraphBuilder visit(IfExpressionNode node, ControlFlowGraphBuilder builder) { + build(node.getElseExpression(), builder); + build(node.getThenExpression(), builder); + return build(node.getConditionExpression(), builder); + } + @Override public ControlFlowGraphBuilder visit(ArrayConstructorNode node, ControlFlowGraphBuilder builder) { return build(node.getElements(), builder); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java index 8c12323e9..f2d3df288 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java @@ -36,6 +36,7 @@ import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; import org.sonar.plugins.communitydelphi.api.ast.Node; import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode; @@ -93,6 +94,47 @@ public Type resolve(UnaryExpressionNode expression) { } } + public Type resolve(IfExpressionNode expression) { + Type thenType = expression.getThenExpression().getType(); + Type elseType = expression.getElseExpression().getType(); + return findLeastUpperBoundType(thenType, elseType); + } + + /** + * Determines the "least upper bound" (LUB) type of an {@link IfExpressionNode}'s branches - the + * common type that both the {@code then} and {@code else} expressions' types can be implicitly + * converted to. If neither type can be converted to the other, the types are incompatible and the + * resulting type is unknown. + */ + private static Type findLeastUpperBoundType(Type thenType, Type elseType) { + if (thenType.isUnknown() || elseType.isUnknown()) { + return unknownType(); + } + + if (thenType.is(elseType)) { + return thenType; + } + + EqualityType thenToElse = TypeComparer.compare(thenType, elseType); + EqualityType elseToThen = TypeComparer.compare(elseType, thenType); + + if (thenToElse == EqualityType.INCOMPATIBLE_TYPES + && elseToThen == EqualityType.INCOMPATIBLE_TYPES) { + return unknownType(); + } + + if (thenToElse.ordinal() > elseToThen.ordinal()) { + // "then" converts more cleanly onto "else" than vice versa, so "else" is the wider type. + return elseType; + } else if (elseToThen.ordinal() > thenToElse.ordinal()) { + return thenType; + } else { + // Conversion quality is a tie (e.g. equally-sized, unrelated types) - fall back to picking + // the larger type, mirroring the widening behaviour used for array constructor elements. + return thenType.size() >= elseType.size() ? thenType : elseType; + } + } + public Type resolve(PrimaryExpressionNode expression) { Type type = unknownType(); boolean regularArrayProperty = false; diff --git a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java new file mode 100644 index 000000000..0e3b62e13 --- /dev/null +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java @@ -0,0 +1,43 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2019 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.communitydelphi.api.ast; + +/** + * A conditional expression ("inline if"), introduced in Delphi 13. + * + *

Unlike {@link IfStatementNode}, both the {@code then} and {@code else} branches are mandatory, + * as the expression must always produce a value. The resulting {@link + * org.sonar.plugins.communitydelphi.api.type.Type} is the least upper bound (i.e. common type) of + * the {@code then} and {@code else} expressions' types. + * + * 

+ *   Result := if Condition then ThenExpression else ElseExpression;
+ *
+ * + * @see + * Conditional Operators (Delphi) + */ +public interface IfExpressionNode extends ExpressionNode { + ExpressionNode getConditionExpression(); + + ExpressionNode getThenExpression(); + + ExpressionNode getElseExpression(); +} diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/GrammarTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/GrammarTest.java index 1ec18c6eb..6e0646b04 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/GrammarTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/GrammarTest.java @@ -371,4 +371,9 @@ void testQualifiedExports() { void testGenericConstraints() { assertParsed("GenericConstraints.pas"); } + + @Test + void testIfExpression() { + assertParsed("IfExpression.pas"); + } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java new file mode 100644 index 000000000..c92a931e5 --- /dev/null +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java @@ -0,0 +1,104 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2019 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.antlr.ast.node; + +import static org.assertj.core.api.Assertions.assertThat; + +import au.com.integradev.delphi.utils.files.DelphiFileUtils; +import org.junit.jupiter.api.Test; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; + +class IfExpressionNodeImplTest { + @Test + void testBranchesShouldBeFound() { + IfExpressionNode node = + parse( + "procedure Test;", // + "begin", + " Result := if True then 1 else 2;", + "end;"); + + assertThat(node.getConditionExpression().getImage()).isEqualTo("True"); + assertThat(node.getThenExpression().getImage()).isEqualTo("1"); + assertThat(node.getElseExpression().getImage()).isEqualTo("2"); + } + + @Test + void testImageShouldReconstructExpression() { + IfExpressionNode node = + parse( + "procedure Test;", // + "begin", + " Result := if True then 1 else 2;", + "end;"); + + assertThat(node.getImage()).isEqualTo("if True then 1 else 2"); + } + + @Test + void testNestedElseBranchShouldParseAsChain() { + IfExpressionNode node = + parse( + "procedure Test;", // + "begin", + " Result := if True then 1 else if False then 2 else 3;", + "end;"); + + assertThat(node.getElseExpression()).isInstanceOf(IfExpressionNode.class); + } + + @Test + void testIntegerAndRealBranchesShouldResolveToRealType() { + IfExpressionNode node = + parse( + "procedure Test;", // + "begin", + " Result := if True then 1 else 2.0;", + "end;"); + + assertThat(node.getType().isReal()).isTrue(); + } + + @Test + void testIncompatibleBranchesShouldResolveToUnknownType() { + IfExpressionNode node = + parse( + "procedure Test;", // + "begin", + " Result := if True then 1 else 'Two';", + "end;"); + + assertThat(node.getType().isUnknown()).isTrue(); + } + + private static IfExpressionNode parse(String... lines) { + return DelphiFileUtils.parse( + "unit Test;", + "", + "interface", + "", + "implementation", + "", + lines.length == 0 ? "" : String.join("\n", lines), + "", + "end.") + .getAst() + .getFirstDescendantOfType(IfExpressionNode.class); + } +} diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java new file mode 100644 index 000000000..b286771ff --- /dev/null +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java @@ -0,0 +1,91 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2019 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.symbol.resolve; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import au.com.integradev.delphi.DelphiProperties; +import au.com.integradev.delphi.compiler.Toolchain; +import au.com.integradev.delphi.type.factory.TypeFactoryImpl; +import org.junit.jupiter.api.Test; +import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; +import org.sonar.plugins.communitydelphi.api.type.IntrinsicType; +import org.sonar.plugins.communitydelphi.api.type.Type; +import org.sonar.plugins.communitydelphi.api.type.TypeFactory; + +class ExpressionTypeResolverTest { + private static final TypeFactory TYPE_FACTORY = + new TypeFactoryImpl(Toolchain.DCC64, DelphiProperties.COMPILER_VERSION_DEFAULT); + private static final ExpressionTypeResolver RESOLVER = new ExpressionTypeResolver(TYPE_FACTORY); + + @Test + void testIdenticalTypesResolveToThatType() { + Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolveIfExpression(integer, integer)).isEqualTo(integer); + } + + @Test + void testIntegerAndRealResolveToReal() { + Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); + Type extended = TYPE_FACTORY.getIntrinsic(IntrinsicType.EXTENDED); + assertThat(resolveIfExpression(integer, extended)).isEqualTo(extended); + assertThat(resolveIfExpression(extended, integer)).isEqualTo(extended); + } + + @Test + void testNarrowerIntegerWidensToWiderIntegerRegardlessOfBranchOrder() { + Type shortInt = TYPE_FACTORY.getIntrinsic(IntrinsicType.SHORTINT); + Type int64 = TYPE_FACTORY.getIntrinsic(IntrinsicType.INT64); + assertThat(resolveIfExpression(shortInt, int64)).isEqualTo(int64); + assertThat(resolveIfExpression(int64, shortInt)).isEqualTo(int64); + } + + @Test + void testIncompatibleTypesResolveToUnknown() { + Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); + Type string = TYPE_FACTORY.getIntrinsic(IntrinsicType.UNICODESTRING); + assertThat(resolveIfExpression(integer, string).isUnknown()).isTrue(); + } + + @Test + void testUnknownBranchTypeResolvesToUnknown() { + Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolveIfExpression(integer, TypeFactory.unknownType()).isUnknown()).isTrue(); + assertThat(resolveIfExpression(TypeFactory.unknownType(), integer).isUnknown()).isTrue(); + } + + private static Type resolveIfExpression(Type thenType, Type elseType) { + ExpressionNode thenExpression = mockExpressionNode(thenType); + ExpressionNode elseExpression = mockExpressionNode(elseType); + + IfExpressionNode ifExpression = mock(IfExpressionNode.class); + when(ifExpression.getThenExpression()).thenReturn(thenExpression); + when(ifExpression.getElseExpression()).thenReturn(elseExpression); + return RESOLVER.resolve(ifExpression); + } + + private static ExpressionNode mockExpressionNode(Type type) { + ExpressionNode result = mock(); + when(result.getType()).thenReturn(type); + return result; + } +} diff --git a/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas new file mode 100644 index 000000000..0bd1b9be1 --- /dev/null +++ b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas @@ -0,0 +1,31 @@ +unit IfExpression; + +interface + +implementation + +function Max(A, B: Integer): Integer; +begin + Result := if A > B then A else B; +end; + +procedure Test; +var + X: Integer; + S: string; +begin + X := if X > 100 then 22 else 45; + + ShowMessage(if X < 100 then 'Small' else 'Big'); + + // Nested if-expressions chain like Pascal's elseif. + S := if X = 1 then 'One' else if X = 2 then 'Two' else 'Many'; + + // Parentheses control how the low-priority `if` interacts with other operators. + ShowMessage((if X < 100 then 'Small' else 'Large') + '!'); + + // The condition and both branches may themselves be parenthesized or nested expressions. + X := if (X > 0) then (X + 1) else (X - 1); +end; + +end. From 18d6b50ccef24a007941d95afb19bd6caee68705 Mon Sep 17 00:00:00 2001 From: Daniel Trierweiler Date: Thu, 23 Jul 2026 16:03:31 +0200 Subject: [PATCH 2/2] #407 - Fix `if` expression type resolution and control flow modeling The LUB computation picked whichever branch converted more cleanly to the other, which can only ever return one of the two branch types. The Delphi compiler's actual promotion rules sometimes resolve to a type that is neither branch's type (e.g. Byte/Word -> Integer), and never derive a common ancestor for sibling class types. Replace it with a table-driven implementation matching the compiler's documented behavior, verified against dcc64 output. Also model the expression as a real CFG branch (only one side is actually evaluated at runtime) instead of a flat both-sides-evaluated node, count it in cyclomatic/cognitive complexity like an if statement, and rename getConditionExpression to getGuardExpression for consistency with IfStatementNode. --- .../antlr/ast/node/IfExpressionNodeImpl.java | 4 +- .../visitors/CognitiveComplexityVisitor.java | 14 ++ .../visitors/CyclomaticComplexityVisitor.java | 7 + .../delphi/cfg/ControlFlowGraphVisitor.java | 19 +- .../resolve/ExpressionTypeResolver.java | 141 ++++++++++-- .../api/ast/IfExpressionNode.java | 2 +- .../ast/node/IfExpressionNodeImplTest.java | 2 +- .../delphi/cfg/ControlFlowGraphTest.java | 15 ++ .../resolve/ExpressionTypeResolverTest.java | 208 ++++++++++++++---- .../delphi/grammar/IfExpression.pas | 11 + 10 files changed, 356 insertions(+), 67 deletions(-) diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java index aeb3df405..588eb8ee5 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java @@ -39,7 +39,7 @@ public T accept(DelphiParserVisitor visitor, T data) { } @Override - public ExpressionNode getConditionExpression() { + public ExpressionNode getGuardExpression() { return (ExpressionNode) getChild(0); } @@ -58,7 +58,7 @@ public String getImage() { if (image == null) { image = "if " - + getConditionExpression().getImage() + + getGuardExpression().getImage() + " then " + getThenExpression().getImage() + " else " diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CognitiveComplexityVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CognitiveComplexityVisitor.java index 32ccaade4..1c4e4e574 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CognitiveComplexityVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CognitiveComplexityVisitor.java @@ -31,6 +31,7 @@ import org.sonar.plugins.communitydelphi.api.ast.ExceptBlockNode; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ForStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode; import org.sonar.plugins.communitydelphi.api.ast.StatementNode; @@ -90,6 +91,19 @@ public Data visit(IfStatementNode statement, Data data) { return data; } + @Override + public Data visit(IfExpressionNode expression, Data data) { + data.increaseComplexityByNesting(); + expression.getGuardExpression().accept(this, data); + + ++data.nesting; + expression.getThenExpression().accept(this, data); + expression.getElseExpression().accept(this, data); + --data.nesting; + + return data; + } + @Override public Data visit(ExceptBlockNode exceptBlock, Data data) { if (exceptBlock.hasHandlers()) { diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CyclomaticComplexityVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CyclomaticComplexityVisitor.java index e4d832883..93bd73f84 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CyclomaticComplexityVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CyclomaticComplexityVisitor.java @@ -23,6 +23,7 @@ import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode; @@ -80,6 +81,12 @@ public Data visit(IfStatementNode statement, Data data) { return DelphiParserVisitor.super.visit(statement, data); } + @Override + public Data visit(IfExpressionNode expression, Data data) { + ++data.complexity; + return DelphiParserVisitor.super.visit(expression, data); + } + @Override public Data visit(BinaryExpressionNode expression, Data data) { BinaryOperator operator = expression.getOperator(); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java index 7d2105b55..13fa760e1 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java @@ -124,11 +124,28 @@ public ControlFlowGraphBuilder visit(RangeExpressionNode node, ControlFlowGraphB return build(node.getLowExpression(), builder); } + /* + * Modeled the same way as an `if` statement: only one branch is actually evaluated at runtime, + * so the guard's two outcomes are mutually-exclusive paths rather than a single linear block. + * + * ┌─> true ──> `ThenBlock` ─┐ + * `Guard` ─────┤ ├─> after + * └─> false ─> `ElseBlock` ─┘ + */ @Override public ControlFlowGraphBuilder visit(IfExpressionNode node, ControlFlowGraphBuilder builder) { + ProtoBlock after = builder.getCurrentBlock(); + + builder.addBlockBefore(after); build(node.getElseExpression(), builder); + ProtoBlock elseBlock = builder.getCurrentBlock(); + + builder.addBlockBefore(after); build(node.getThenExpression(), builder); - return build(node.getConditionExpression(), builder); + ProtoBlock thenBlock = builder.getCurrentBlock(); + + builder.addBlock(ProtoBlockFactory.branch(node, thenBlock, elseBlock)); + return buildCondition(builder, node.getGuardExpression(), thenBlock, elseBlock); } @Override diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java index f2d3df288..b10f44613 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolver.java @@ -58,6 +58,7 @@ import org.sonar.plugins.communitydelphi.api.type.Type; import org.sonar.plugins.communitydelphi.api.type.Type.ClassReferenceType; import org.sonar.plugins.communitydelphi.api.type.Type.CollectionType; +import org.sonar.plugins.communitydelphi.api.type.Type.IntegerType; import org.sonar.plugins.communitydelphi.api.type.Type.PointerType; import org.sonar.plugins.communitydelphi.api.type.Type.ProceduralType; import org.sonar.plugins.communitydelphi.api.type.TypeFactory; @@ -95,44 +96,138 @@ public Type resolve(UnaryExpressionNode expression) { } public Type resolve(IfExpressionNode expression) { - Type thenType = expression.getThenExpression().getType(); - Type elseType = expression.getElseExpression().getType(); - return findLeastUpperBoundType(thenType, elseType); + return commonType( + expression.getThenExpression().getType(), expression.getElseExpression().getType()); } /** - * Determines the "least upper bound" (LUB) type of an {@link IfExpressionNode}'s branches - the - * common type that both the {@code then} and {@code else} expressions' types can be implicitly - * converted to. If neither type can be converted to the other, the types are incompatible and the - * resulting type is unknown. + * Resolves the type of an {@code if} expression as the "least upper bound" of its {@code then} + * and {@code else} branch types, mirroring the type combinations accepted by the Delphi compiler. + * This is not the same computation used for operator/parameter overload resolution: the + * compiler's promotion buckets can produce a result type that is neither branch's type (e.g. + * {@code Byte} combined with {@code Word} produces {@code Integer}), so a pairwise "which side + * converts more cleanly to the other" comparison (as used for overload resolution) is not + * sufficient here. + * + * @see + * Conditional Operators (Delphi) */ - private static Type findLeastUpperBoundType(Type thenType, Type elseType) { - if (thenType.isUnknown() || elseType.isUnknown()) { - return unknownType(); + private Type commonType(Type thenType, Type elseType) { + if (thenType.isUnknown()) { + return elseType; + } + if (elseType.isUnknown()) { + return thenType; } - if (thenType.is(elseType)) { return thenType; } + if (thenType.isInteger() && elseType.isInteger()) { + return integerCommonType((IntegerType) thenType, (IntegerType) elseType); + } + if (isNumeric(thenType) && isNumeric(elseType)) { + return realCommonType(thenType, elseType); + } + if (isTextual(thenType) && isTextual(elseType)) { + return textualCommonType(thenType, elseType); + } + if (thenType.isStruct() && elseType.isStruct()) { + return structCommonType(thenType, elseType); + } + return unknownType(); + } - EqualityType thenToElse = TypeComparer.compare(thenType, elseType); - EqualityType elseToThen = TypeComparer.compare(elseType, thenType); + /** + * Two integers widen to at least {@code Integer}; mixing a 32-bit unsigned ({@code Cardinal}) + * with any signed integer, or involving any 64-bit integer, widens to {@code Int64}. + */ + private Type integerCommonType(IntegerType thenType, IntegerType elseType) { + boolean any64Bit = thenType.size() >= 8 || elseType.size() >= 8; + boolean unsigned32Bit = + (!thenType.isSigned() && thenType.size() == 4) + || (!elseType.isSigned() && elseType.size() == 4); + boolean anySigned = thenType.isSigned() || elseType.isSigned(); + + if (any64Bit || (unsigned32Bit && anySigned)) { + return typeFactory.getIntrinsic(IntrinsicType.INT64); + } + return typeFactory.getIntrinsic(IntrinsicType.INTEGER); + } - if (thenToElse == EqualityType.INCOMPATIBLE_TYPES - && elseToThen == EqualityType.INCOMPATIBLE_TYPES) { - return unknownType(); + /** + * A real combined with another real or an integer collapses to {@code Double}, unless both sides + * are integer-valued ({@code Comp}, {@code Currency}, or an integer), in which case it is {@code + * Comp}. + */ + private Type realCommonType(Type thenType, Type elseType) { + if (isIntegerValued(thenType) && isIntegerValued(elseType)) { + return typeFactory.getIntrinsic(IntrinsicType.COMP); } + return typeFactory.getIntrinsic(IntrinsicType.DOUBLE); + } + + private boolean isIntegerValued(Type type) { + return type.isInteger() + || type.is(typeFactory.getIntrinsic(IntrinsicType.COMP)) + || type.is(typeFactory.getIntrinsic(IntrinsicType.CURRENCY)); + } - if (thenToElse.ordinal() > elseToThen.ordinal()) { - // "then" converts more cleanly onto "else" than vice versa, so "else" is the wider type. + /** + * Characters and strings that involve a {@code UnicodeString} promote to {@code UnicodeString}; + * other combinations are left unknown. + */ + private Type textualCommonType(Type thenType, Type elseType) { + Type unicodeString = typeFactory.getIntrinsic(IntrinsicType.UNICODESTRING); + if (thenType.is(unicodeString) || elseType.is(unicodeString)) { + return unicodeString; + } + return unknownType(); + } + + /** + * When one struct is assignable to the other (e.g. a descendant and its ancestor), the less + * derived type wins; otherwise the nearest shared ancestor is used. + */ + private static Type structCommonType(Type thenType, Type elseType) { + boolean thenToElse = + TypeComparer.compare(thenType, elseType) != EqualityType.INCOMPATIBLE_TYPES; + boolean elseToThen = + TypeComparer.compare(elseType, thenType) != EqualityType.INCOMPATIBLE_TYPES; + + if (thenToElse && !elseToThen) { return elseType; - } else if (elseToThen.ordinal() > thenToElse.ordinal()) { + } + if (elseToThen && !thenToElse) { return thenType; - } else { - // Conversion quality is a tie (e.g. equally-sized, unrelated types) - fall back to picking - // the larger type, mirroring the widening behaviour used for array constructor elements. - return thenType.size() >= elseType.size() ? thenType : elseType; } + return findCommonAncestor(thenType, elseType); + } + + private static boolean isNumeric(Type type) { + return type.isInteger() || type.isReal(); + } + + private static boolean isTextual(Type type) { + return type.isChar() || type.isString(); + } + + private static Type findCommonAncestor(Type left, Type right) { + if (!left.isStruct() || !right.isStruct()) { + return unknownType(); + } + + for (Type leftAncestor = left.parent(); + !leftAncestor.isUnknown(); + leftAncestor = leftAncestor.parent()) { + for (Type rightAncestor = right.parent(); + !rightAncestor.isUnknown(); + rightAncestor = rightAncestor.parent()) { + if (leftAncestor.is(rightAncestor)) { + return leftAncestor; + } + } + } + return unknownType(); } public Type resolve(PrimaryExpressionNode expression) { diff --git a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java index 0e3b62e13..19c82d352 100644 --- a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java @@ -35,7 +35,7 @@ * Conditional Operators (Delphi) */ public interface IfExpressionNode extends ExpressionNode { - ExpressionNode getConditionExpression(); + ExpressionNode getGuardExpression(); ExpressionNode getThenExpression(); diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java index c92a931e5..5ebbbc328 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImplTest.java @@ -34,7 +34,7 @@ void testBranchesShouldBeFound() { " Result := if True then 1 else 2;", "end;"); - assertThat(node.getConditionExpression().getImage()).isEqualTo("True"); + assertThat(node.getGuardExpression().getImage()).isEqualTo("True"); assertThat(node.getThenExpression().getImage()).isEqualTo("1"); assertThat(node.getElseExpression().getImage()).isEqualTo("2"); } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index 2092ba313..5d5aa2fdf 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -65,6 +65,7 @@ import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForToStatementNode; import org.sonar.plugins.communitydelphi.api.ast.GotoStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.IntegerLiteralNode; import org.sonar.plugins.communitydelphi.api.ast.NameDeclarationNode; @@ -345,6 +346,20 @@ void testEmptyIfElse() { block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } + @Test + void testIfExpression() { + test( + List.of("X: Integer"), + "X := if A then Foo else Bar;", + checker( + block(element(NameReferenceNode.class, "A")) + .branchesTo(3, 2) + .withTerminator(IfExpressionNode.class), + block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), + block(element(NameReferenceNode.class, "X")).succeedsTo(0))); + } + @Test void testLocalVarDeclaration() { test( diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java index b286771ff..98fea9ae2 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java @@ -1,6 +1,6 @@ /* * Sonar Delphi Plugin - * Copyright (C) 2019 Integrated Application Development + * Copyright (C) 2026 Integrated Application Development * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -21,10 +21,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.plugins.communitydelphi.api.type.StructKind.CLASS; +import static org.sonar.plugins.communitydelphi.api.type.TypeFactory.unknownType; -import au.com.integradev.delphi.DelphiProperties; -import au.com.integradev.delphi.compiler.Toolchain; -import au.com.integradev.delphi.type.factory.TypeFactoryImpl; +import au.com.integradev.delphi.utils.types.TypeFactoryUtils; +import au.com.integradev.delphi.utils.types.TypeMocker; import org.junit.jupiter.api.Test; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; @@ -32,60 +33,189 @@ import org.sonar.plugins.communitydelphi.api.type.Type; import org.sonar.plugins.communitydelphi.api.type.TypeFactory; +/** + * The expected result type of every {@code if ... then ... else ...} expression in this test was + * obtained from the Delphi 13 (compiler version 37.0) {@code dcc64} compiler, by assigning the + * expression to an incompatible variable and reading the type named in the resulting {@code E2010 + * Incompatible types} error. + */ class ExpressionTypeResolverTest { - private static final TypeFactory TYPE_FACTORY = - new TypeFactoryImpl(Toolchain.DCC64, DelphiProperties.COMPILER_VERSION_DEFAULT); - private static final ExpressionTypeResolver RESOLVER = new ExpressionTypeResolver(TYPE_FACTORY); + private static final TypeFactory FACTORY = TypeFactoryUtils.defaultFactory(); + private static final ExpressionTypeResolver RESOLVER = new ExpressionTypeResolver(FACTORY); + + private static Type intrinsic(IntrinsicType type) { + return FACTORY.getIntrinsic(type); + } + + // ---- same type / unknown ------------------------------------------------- + + @Test + void testSameTypeOnBothBranchesReturnsThatType() { + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.INTEGER, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.CARDINAL, IntrinsicType.CARDINAL, IntrinsicType.CARDINAL); + assertResolvesBoth(IntrinsicType.UINT64, IntrinsicType.UINT64, IntrinsicType.UINT64); + assertResolvesBoth(IntrinsicType.CURRENCY, IntrinsicType.CURRENCY, IntrinsicType.CURRENCY); + assertResolvesBoth(IntrinsicType.COMP, IntrinsicType.COMP, IntrinsicType.COMP); + } + + @Test + void testUnknownThenBranchReturnsElseType() { + Type integer = intrinsic(IntrinsicType.INTEGER); + assertThat(resolve(unknownType(), integer).is(integer)).isTrue(); + } + + @Test + void testUnknownElseBranchReturnsThenType() { + Type integer = intrinsic(IntrinsicType.INTEGER); + assertThat(resolve(integer, unknownType()).is(integer)).isTrue(); + } + + // ---- integer widening ---------------------------------------------------- + + @Test + void testTwoSignedIntegersWidenToSignedAtLeastInteger() { + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.SMALLINT, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.INTEGER, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.SMALLINT, IntrinsicType.INTEGER, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.INT64, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.INT64, IntrinsicType.INT64); + } + + @Test + void testTwoUnsignedIntegersWidenToSignedInteger() { + assertResolvesBoth(IntrinsicType.BYTE, IntrinsicType.WORD, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.BYTE, IntrinsicType.CARDINAL, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.WORD, IntrinsicType.CARDINAL, IntrinsicType.INTEGER); + } + + @Test + void testSmallSignedAndUnsignedWidenToInteger() { + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.BYTE, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.WORD, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.SMALLINT, IntrinsicType.WORD, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.BYTE, IntrinsicType.INTEGER, IntrinsicType.INTEGER); + assertResolvesBoth(IntrinsicType.WORD, IntrinsicType.INTEGER, IntrinsicType.INTEGER); + } @Test - void testIdenticalTypesResolveToThatType() { - Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); - assertThat(resolveIfExpression(integer, integer)).isEqualTo(integer); + void testSignedMixedWithCardinalWidenToInt64() { + assertResolvesBoth(IntrinsicType.SHORTINT, IntrinsicType.CARDINAL, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.SMALLINT, IntrinsicType.CARDINAL, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.CARDINAL, IntrinsicType.INT64); } @Test - void testIntegerAndRealResolveToReal() { - Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); - Type extended = TYPE_FACTORY.getIntrinsic(IntrinsicType.EXTENDED); - assertThat(resolveIfExpression(integer, extended)).isEqualTo(extended); - assertThat(resolveIfExpression(extended, integer)).isEqualTo(extended); + void testAnythingMixedWith64BitWidensToInt64() { + assertResolvesBoth(IntrinsicType.CARDINAL, IntrinsicType.INT64, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.CARDINAL, IntrinsicType.UINT64, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.UINT64, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.INT64, IntrinsicType.UINT64, IntrinsicType.INT64); + assertResolvesBoth(IntrinsicType.WORD, IntrinsicType.INT64, IntrinsicType.INT64); + } + + // ---- real promotion ------------------------------------------------------ + + @Test + void testTwoRealsCollapseToDouble() { + assertResolvesBoth(IntrinsicType.SINGLE, IntrinsicType.DOUBLE, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.SINGLE, IntrinsicType.EXTENDED, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.DOUBLE, IntrinsicType.EXTENDED, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.SINGLE, IntrinsicType.CURRENCY, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.DOUBLE, IntrinsicType.CURRENCY, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.EXTENDED, IntrinsicType.CURRENCY, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.SINGLE, IntrinsicType.COMP, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.DOUBLE, IntrinsicType.COMP, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.EXTENDED, IntrinsicType.COMP, IntrinsicType.DOUBLE); } @Test - void testNarrowerIntegerWidensToWiderIntegerRegardlessOfBranchOrder() { - Type shortInt = TYPE_FACTORY.getIntrinsic(IntrinsicType.SHORTINT); - Type int64 = TYPE_FACTORY.getIntrinsic(IntrinsicType.INT64); - assertThat(resolveIfExpression(shortInt, int64)).isEqualTo(int64); - assertThat(resolveIfExpression(int64, shortInt)).isEqualTo(int64); + void testCurrencyAndCompCollapseToComp() { + assertResolvesBoth(IntrinsicType.CURRENCY, IntrinsicType.COMP, IntrinsicType.COMP); } + // ---- integer mixed with real --------------------------------------------- + @Test - void testIncompatibleTypesResolveToUnknown() { - Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); - Type string = TYPE_FACTORY.getIntrinsic(IntrinsicType.UNICODESTRING); - assertThat(resolveIfExpression(integer, string).isUnknown()).isTrue(); + void testIntegerMixedWithFloatingRealPromotesToDouble() { + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.SINGLE, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.DOUBLE, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.EXTENDED, IntrinsicType.DOUBLE); + assertResolvesBoth(IntrinsicType.INT64, IntrinsicType.DOUBLE, IntrinsicType.DOUBLE); } @Test - void testUnknownBranchTypeResolvesToUnknown() { - Type integer = TYPE_FACTORY.getIntrinsic(IntrinsicType.INTEGER); - assertThat(resolveIfExpression(integer, TypeFactory.unknownType()).isUnknown()).isTrue(); - assertThat(resolveIfExpression(TypeFactory.unknownType(), integer).isUnknown()).isTrue(); + void testIntegerMixedWithCurrencyOrCompPromotesToComp() { + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.CURRENCY, IntrinsicType.COMP); + assertResolvesBoth(IntrinsicType.INTEGER, IntrinsicType.COMP, IntrinsicType.COMP); + assertResolvesBoth(IntrinsicType.BYTE, IntrinsicType.CURRENCY, IntrinsicType.COMP); + assertResolvesBoth(IntrinsicType.INT64, IntrinsicType.COMP, IntrinsicType.COMP); } - private static Type resolveIfExpression(Type thenType, Type elseType) { - ExpressionNode thenExpression = mockExpressionNode(thenType); - ExpressionNode elseExpression = mockExpressionNode(elseType); + // ---- characters and strings ---------------------------------------------- + + @Test + void testCharMixedWithUnicodeStringPromotesToString() { + assertResolvesBoth( + IntrinsicType.CHAR, IntrinsicType.UNICODESTRING, IntrinsicType.UNICODESTRING); + } - IfExpressionNode ifExpression = mock(IfExpressionNode.class); - when(ifExpression.getThenExpression()).thenReturn(thenExpression); - when(ifExpression.getElseExpression()).thenReturn(elseExpression); - return RESOLVER.resolve(ifExpression); + @Test + void testStringMixedWithOtherStringPromotesToUnicodeString() { + assertResolvesBoth( + IntrinsicType.UNICODESTRING, IntrinsicType.ANSISTRING, IntrinsicType.UNICODESTRING); + assertResolvesBoth( + IntrinsicType.UNICODESTRING, IntrinsicType.SHORTSTRING, IntrinsicType.UNICODESTRING); } - private static ExpressionNode mockExpressionNode(Type type) { - ExpressionNode result = mock(); - when(result.getType()).thenReturn(type); - return result; + // ---- class types --------------------------------------------------------- + + @Test + void testDescendantAndAncestorClassReturnsAncestor() { + Type base = TypeMocker.struct("TBase", CLASS); + Type derived = TypeMocker.struct("TDerived", CLASS, base); + assertThat(resolve(derived, base).is(base)).isTrue(); + assertThat(resolve(base, derived).is(base)).isTrue(); + } + + @Test + void testSiblingClassesReturnsCommonAncestor() { + Type base = TypeMocker.struct("TBase", CLASS); + Type left = TypeMocker.struct("TLeft", CLASS, base); + Type right = TypeMocker.struct("TRight", CLASS, base); + assertThat(resolve(left, right).is(base)).isTrue(); + assertThat(resolve(right, left).is(base)).isTrue(); + } + + // ---- incompatible branches (a compile error in Delphi) ------------------- + + @Test + void testUnrelatedTypesReturnUnknown() { + Type fooClass = TypeMocker.struct("TFoo", CLASS); + Type integer = intrinsic(IntrinsicType.INTEGER); + assertThat(resolve(fooClass, integer).isUnknown()).isTrue(); + assertThat(resolve(integer, fooClass).isUnknown()).isTrue(); + assertThat(resolve(intrinsic(IntrinsicType.UNICODESTRING), integer).isUnknown()).isTrue(); + } + + private void assertResolvesBoth( + IntrinsicType then, IntrinsicType elseType, IntrinsicType expect) { + assertThat(resolve(intrinsic(then), intrinsic(elseType)).is(intrinsic(expect))) + .as("%s | %s -> %s", then, elseType, expect) + .isTrue(); + assertThat(resolve(intrinsic(elseType), intrinsic(then)).is(intrinsic(expect))) + .as("%s | %s -> %s", elseType, then, expect) + .isTrue(); + } + + private static Type resolve(Type thenType, Type elseType) { + ExpressionNode thenExpression = mock(ExpressionNode.class); + ExpressionNode elseExpression = mock(ExpressionNode.class); + when(thenExpression.getType()).thenReturn(thenType); + when(elseExpression.getType()).thenReturn(elseType); + + IfExpressionNode node = mock(IfExpressionNode.class); + when(node.getThenExpression()).thenReturn(thenExpression); + when(node.getElseExpression()).thenReturn(elseExpression); + return RESOLVER.resolve(node); } } diff --git a/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas index 0bd1b9be1..fa7389f86 100644 --- a/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas +++ b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/IfExpression.pas @@ -28,4 +28,15 @@ procedure Test; X := if (X > 0) then (X + 1) else (X - 1); end; +function Nested(A, B: Boolean): Integer; +begin + // An unparenthesized nested `if` expression is unambiguous in both the then and else + // branches, since the mandatory ELSE always closes the inner expression first. + Result := + if A then + if B then 1 else 2 + else + if B then 3 else 4; +end; + end.