From 2cc4281f217bb2318fbb37dc169ad6e166c76b0f Mon Sep 17 00:00:00 2001 From: Sammy Valtonen Date: Wed, 22 Apr 2026 09:34:29 +0200 Subject: [PATCH 1/2] Add support for `if` expressions Resolves #407. Introduces parsing, type resolution, and control-flow modelling of Delphi 13's conditional `if` operator (e.g. `X := if Foo then Bar else Baz`). The LUB of the two branch types is used as the expression's type, and the branches are modelled as mutually-exclusive paths in the CFG. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + .../au/com/integradev/delphi/antlr/Delphi.g | 5 ++ .../node/ConditionalExpressionNodeImpl.java | 80 +++++++++++++++++++ .../visitors/CognitiveComplexityVisitor.java | 14 ++++ .../visitors/CyclomaticComplexityVisitor.java | 7 ++ .../ast/visitors/DelphiParserVisitor.java | 5 ++ .../delphi/cfg/ControlFlowGraphVisitor.java | 22 +++++ .../resolve/ExpressionTypeResolver.java | 25 ++++++ .../api/ast/ConditionalExpressionNode.java | 27 +++++++ .../integradev/delphi/antlr/GrammarTest.java | 5 ++ .../delphi/cfg/ControlFlowGraphTest.java | 15 ++++ .../delphi/grammar/ConditionalExpressions.pas | 36 +++++++++ 12 files changed, 242 insertions(+) create mode 100644 delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java create mode 100644 delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/ConditionalExpressionNode.java create mode 100644 delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/ConditionalExpressions.pas diff --git a/CHANGELOG.md b/CHANGELOG.md index 66c6d84e8..fee33984b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support for `noreturn` routines, introduced in Delphi 13. - `NoreturnContract` analysis rule, which flags `noreturn` routines that return normally. +- Support for `if` expressions (e.g. `X := if Foo then Bar else Baz;`), introduced in Delphi 13. ## [1.18.3] - 2025-11-11 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 54ff76185..710c4f23e 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 @@ -72,6 +72,7 @@ tokens { TkArgument; TkAnonymousMethod; TkAnonymousMethodHeading; + TkConditionalExpression; TkLessThanEqual; TkGreaterThanEqual; } @@ -920,6 +921,10 @@ attribute : (ASSEMBLY ':')? expression (':' expression)* //---------------------------------------------------------------------------- expression : relationalExpression | anonymousMethod + | conditionalExpression + ; +conditionalExpression : IF expression THEN expression ELSE expression + -> ^(TkConditionalExpression IF expression THEN expression ELSE expression) ; // ANTLR sets the begin and end tokens for nested binary expression nodes // in relationalOperator, not relationalExpression, meaning that their diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java new file mode 100644 index 000000000..797d4bbd1 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java @@ -0,0 +1,80 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2024 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.ConditionalExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; +import org.sonar.plugins.communitydelphi.api.type.Type; + +public final class ConditionalExpressionNodeImpl extends ExpressionNodeImpl + implements ConditionalExpressionNode { + private String image; + + public ConditionalExpressionNodeImpl(Token token) { + super(token); + } + + public ConditionalExpressionNodeImpl(int tokenType) { + super(tokenType); + } + + @Override + public T accept(DelphiParserVisitor visitor, T data) { + return visitor.visit(this, data); + } + + @Override + public ExpressionNode getGuardExpression() { + return (ExpressionNode) getChild(1); + } + + @Override + public ExpressionNode getThenExpression() { + return (ExpressionNode) getChild(3); + } + + @Override + public ExpressionNode getElseExpression() { + return (ExpressionNode) getChild(5); + } + + @Override + public String getImage() { + if (image == null) { + image = + "if " + + getGuardExpression().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/CognitiveComplexityVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/CognitiveComplexityVisitor.java index 32ccaade4..8c8f41360 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 @@ -27,6 +27,7 @@ import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.ExceptBlockNode; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; @@ -90,6 +91,19 @@ public Data visit(IfStatementNode statement, Data data) { return data; } + @Override + public Data visit(ConditionalExpressionNode 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..ef636464a 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 @@ -22,6 +22,7 @@ import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ForStatementNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode; @@ -80,6 +81,12 @@ public Data visit(IfStatementNode statement, Data data) { return DelphiParserVisitor.super.visit(statement, data); } + @Override + public Data visit(ConditionalExpressionNode 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/antlr/ast/visitors/DelphiParserVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DelphiParserVisitor.java index 81ebb7af7..a72f2b0e4 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 @@ -46,6 +46,7 @@ import org.sonar.plugins.communitydelphi.api.ast.ClassTypeNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ConstArrayElementTypeNode; import org.sonar.plugins.communitydelphi.api.ast.ConstDeclarationNode; import org.sonar.plugins.communitydelphi.api.ast.ConstSectionNode; @@ -622,6 +623,10 @@ default T visit(AnonymousMethodNode node, T data) { return visit((ExpressionNode) node, data); } + default T visit(ConditionalExpressionNode node, T data) { + return visit((ExpressionNode) node, data); + } + default T visit(ArrayConstructorNode 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 99b3ed841..c07e61071 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 @@ -37,6 +37,7 @@ import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ConstStatementNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.ExceptBlockNode; @@ -277,6 +278,27 @@ public ControlFlowGraphBuilder visit(IfStatementNode node, ControlFlowGraphBuild return buildCondition(builder, node.getGuardExpression(), thenBlock, elseBlock); } + /* + * Modeled the same way as an `if` statement: the guard branches into two expressions that both + * merge back to the block following the conditional expression. + */ + @Override + public ControlFlowGraphBuilder visit( + ConditionalExpressionNode 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); + ProtoBlock thenBlock = builder.getCurrentBlock(); + + builder.addBlock(ProtoBlockFactory.branch(node, thenBlock, elseBlock)); + return buildCondition(builder, node.getGuardExpression(), thenBlock, elseBlock); + } + private ControlFlowGraphBuilder buildCondition( ControlFlowGraphBuilder builder, ExpressionNode node, 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..5bbec6cc1 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 @@ -34,6 +34,7 @@ import org.sonar.plugins.communitydelphi.api.ast.ArrayAccessorNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; @@ -93,6 +94,30 @@ public Type resolve(UnaryExpressionNode expression) { } } + public Type resolve(ConditionalExpressionNode expression) { + Type thenType = expression.getThenExpression().getType(); + Type elseType = expression.getElseExpression().getType(); + + if (thenType.isUnknown()) { + return elseType; + } + if (elseType.isUnknown()) { + return thenType; + } + + EqualityType thenToElse = TypeComparer.compare(thenType, elseType); + EqualityType elseToThen = TypeComparer.compare(elseType, thenType); + + if (thenToElse.ordinal() >= elseToThen.ordinal() + && thenToElse != EqualityType.INCOMPATIBLE_TYPES) { + return elseType; + } + if (elseToThen != EqualityType.INCOMPATIBLE_TYPES) { + return thenType; + } + return thenType; + } + 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/ConditionalExpressionNode.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/ConditionalExpressionNode.java new file mode 100644 index 000000000..9a439b7b7 --- /dev/null +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/ConditionalExpressionNode.java @@ -0,0 +1,27 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2024 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; + +public interface ConditionalExpressionNode extends ExpressionNode { + ExpressionNode getGuardExpression(); + + 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 5d7977810..b1b4c50cd 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 @@ -141,6 +141,11 @@ void testParseAnonymousMethods() { assertParsed("AnonymousMethods.pas"); } + @Test + void testParseConditionalExpressions() { + assertParsed("ConditionalExpressions.pas"); + } + @Test void testParseGenerics() { assertParsed("Generics.pas"); 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 7c6571d5b..405f24972 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 @@ -58,6 +58,7 @@ import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.FinallyBlockNode; import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode; @@ -342,6 +343,20 @@ void testEmptyIfElse() { block(element(NameReferenceNode.class, "A")).succeedsTo(0))); } + @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(ConditionalExpressionNode.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/resources/au/com/integradev/delphi/grammar/ConditionalExpressions.pas b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/ConditionalExpressions.pas new file mode 100644 index 000000000..8d86f63ee --- /dev/null +++ b/delphi-frontend/src/test/resources/au/com/integradev/delphi/grammar/ConditionalExpressions.pas @@ -0,0 +1,36 @@ +unit ConditionalExpressions; + +interface + +implementation + +function Describe(aValue : Integer) : string; +begin + Result := if aValue > 0 then 'positive' else 'non-positive'; +end; + +function PickInt(aFlag : Boolean; aLhs, aRhs : Integer) : Integer; +begin + Result := if aFlag then aLhs else aRhs; +end; + +procedure UseInArgument; +begin + Describe(if True then 1 else 2); +end; + +function Nested(aA, aB : Boolean) : Integer; +begin + Result := + if aA then + if aB then 1 else 2 + else + if aB then 3 else 4; +end; + +function Mixed(aFlag : Boolean) : Integer; +begin + Result := (if aFlag then 10 else 20) + 1; +end; + +end. From aa3d5b36d6ca2ef428a83bb33c4e211613e3f090 Mon Sep 17 00:00:00 2001 From: Bazooper-blip Date: Fri, 1 May 2026 13:01:40 +0200 Subject: [PATCH 2/2] Address review feedback on if expressions - Rename ConditionalExpressionNode to IfExpressionNode (and the Impl class + grammar token) to mirror Delphi's keyword and IfStatementNode. - Bump copyright year on the new public-API and impl files. - Drop the redundant ControlFlowGraphVisitor.visit comment. - Replace the naive type resolution with a real LUB: - Handle exact-equal types directly. - When only one branch's type is assignable to the other, the receiver is the result. - When both directions are assignable, prefer the wider type (the one that accepts the other with a higher equality score). - When neither is assignable, walk the parent chain of both struct types to find a common ancestor. - Otherwise, fall back to unknownType instead of silently returning the then-branch type. --- .../au/com/integradev/delphi/antlr/Delphi.g | 8 +- ...odeImpl.java => IfExpressionNodeImpl.java} | 11 +- .../visitors/CognitiveComplexityVisitor.java | 4 +- .../visitors/CyclomaticComplexityVisitor.java | 4 +- .../ast/visitors/DelphiParserVisitor.java | 4 +- .../delphi/cfg/ControlFlowGraphVisitor.java | 9 +- .../resolve/ExpressionTypeResolver.java | 44 +++++-- ...ressionNode.java => IfExpressionNode.java} | 4 +- .../delphi/cfg/ControlFlowGraphTest.java | 4 +- .../resolve/ExpressionTypeResolverTest.java | 107 ++++++++++++++++++ 10 files changed, 164 insertions(+), 35 deletions(-) rename delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/{ConditionalExpressionNodeImpl.java => IfExpressionNodeImpl.java} (85%) rename delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/{ConditionalExpressionNode.java => IfExpressionNode.java} (88%) create mode 100644 delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java 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 710c4f23e..c39ba3133 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 @@ -72,7 +72,7 @@ tokens { TkArgument; TkAnonymousMethod; TkAnonymousMethodHeading; - TkConditionalExpression; + TkIfExpression; TkLessThanEqual; TkGreaterThanEqual; } @@ -921,10 +921,10 @@ attribute : (ASSEMBLY ':')? expression (':' expression)* //---------------------------------------------------------------------------- expression : relationalExpression | anonymousMethod - | conditionalExpression + | ifExpression ; -conditionalExpression : IF expression THEN expression ELSE expression - -> ^(TkConditionalExpression IF expression THEN expression ELSE expression) +ifExpression : IF expression THEN expression ELSE expression + -> ^(TkIfExpression IF expression THEN expression ELSE expression) ; // ANTLR sets the begin and end tokens for nested binary expression nodes // in relationalOperator, not relationalExpression, meaning that their diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java similarity index 85% rename from delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java rename to delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java index 797d4bbd1..601eb93ec 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ConditionalExpressionNodeImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/IfExpressionNodeImpl.java @@ -1,6 +1,6 @@ /* * Sonar Delphi Plugin - * Copyright (C) 2024 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 @@ -22,19 +22,18 @@ import au.com.integradev.delphi.symbol.resolve.ExpressionTypeResolver; import javax.annotation.Nonnull; import org.antlr.runtime.Token; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; 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 ConditionalExpressionNodeImpl extends ExpressionNodeImpl - implements ConditionalExpressionNode { +public final class IfExpressionNodeImpl extends ExpressionNodeImpl implements IfExpressionNode { private String image; - public ConditionalExpressionNodeImpl(Token token) { + public IfExpressionNodeImpl(Token token) { super(token); } - public ConditionalExpressionNodeImpl(int tokenType) { + public IfExpressionNodeImpl(int tokenType) { super(tokenType); } 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 8c8f41360..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 @@ -27,11 +27,11 @@ import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; 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; @@ -92,7 +92,7 @@ public Data visit(IfStatementNode statement, Data data) { } @Override - public Data visit(ConditionalExpressionNode expression, Data data) { + public Data visit(IfExpressionNode expression, Data data) { data.increaseComplexityByNesting(); expression.getGuardExpression().accept(this, data); 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 ef636464a..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 @@ -22,8 +22,8 @@ import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; 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; @@ -82,7 +82,7 @@ public Data visit(IfStatementNode statement, Data data) { } @Override - public Data visit(ConditionalExpressionNode expression, Data data) { + public Data visit(IfExpressionNode expression, Data data) { ++data.complexity; return DelphiParserVisitor.super.visit(expression, data); } 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 a72f2b0e4..b88a5f078 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 @@ -46,7 +46,6 @@ import org.sonar.plugins.communitydelphi.api.ast.ClassTypeNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ConstArrayElementTypeNode; import org.sonar.plugins.communitydelphi.api.ast.ConstDeclarationNode; import org.sonar.plugins.communitydelphi.api.ast.ConstSectionNode; @@ -82,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; @@ -623,7 +623,7 @@ default T visit(AnonymousMethodNode node, T data) { return visit((ExpressionNode) node, data); } - default T visit(ConditionalExpressionNode node, T data) { + default T visit(IfExpressionNode 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 c07e61071..8cb8366a4 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 @@ -37,7 +37,6 @@ import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.ConstStatementNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.ExceptBlockNode; @@ -51,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; @@ -278,13 +278,8 @@ public ControlFlowGraphBuilder visit(IfStatementNode node, ControlFlowGraphBuild return buildCondition(builder, node.getGuardExpression(), thenBlock, elseBlock); } - /* - * Modeled the same way as an `if` statement: the guard branches into two expressions that both - * merge back to the block following the conditional expression. - */ @Override - public ControlFlowGraphBuilder visit( - ConditionalExpressionNode node, ControlFlowGraphBuilder builder) { + public ControlFlowGraphBuilder visit(IfExpressionNode node, ControlFlowGraphBuilder builder) { ProtoBlock after = builder.getCurrentBlock(); builder.addBlockBefore(after); 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 5bbec6cc1..c67a7f249 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 @@ -34,9 +34,9 @@ import org.sonar.plugins.communitydelphi.api.ast.ArrayAccessorNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; 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; @@ -94,7 +94,7 @@ public Type resolve(UnaryExpressionNode expression) { } } - public Type resolve(ConditionalExpressionNode expression) { + public Type resolve(IfExpressionNode expression) { Type thenType = expression.getThenExpression().getType(); Type elseType = expression.getElseExpression().getType(); @@ -105,17 +105,45 @@ public Type resolve(ConditionalExpressionNode expression) { return thenType; } - EqualityType thenToElse = TypeComparer.compare(thenType, elseType); - EqualityType elseToThen = TypeComparer.compare(elseType, thenType); + if (thenType.is(elseType)) { + return thenType; + } - if (thenToElse.ordinal() >= elseToThen.ordinal() - && thenToElse != EqualityType.INCOMPATIBLE_TYPES) { + EqualityType thenAsCommon = TypeComparer.compare(elseType, thenType); + EqualityType elseAsCommon = TypeComparer.compare(thenType, elseType); + + if (thenAsCommon == EqualityType.INCOMPATIBLE_TYPES + && elseAsCommon == EqualityType.INCOMPATIBLE_TYPES) { + return findCommonAncestor(thenType, elseType); + } + + if (thenAsCommon == EqualityType.INCOMPATIBLE_TYPES) { return elseType; } - if (elseToThen != EqualityType.INCOMPATIBLE_TYPES) { + if (elseAsCommon == EqualityType.INCOMPATIBLE_TYPES) { return thenType; } - return thenType; + + return elseAsCommon.ordinal() >= thenAsCommon.ordinal() ? elseType : thenType; + } + + 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/ConditionalExpressionNode.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java similarity index 88% rename from delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/ConditionalExpressionNode.java rename to delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java index 9a439b7b7..c1ba4ff80 100644 --- a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/ConditionalExpressionNode.java +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/IfExpressionNode.java @@ -1,6 +1,6 @@ /* * Sonar Delphi Plugin - * Copyright (C) 2024 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 @@ -18,7 +18,7 @@ */ package org.sonar.plugins.communitydelphi.api.ast; -public interface ConditionalExpressionNode extends ExpressionNode { +public interface IfExpressionNode extends ExpressionNode { ExpressionNode getGuardExpression(); ExpressionNode getThenExpression(); 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 405f24972..b4194da48 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 @@ -58,7 +58,7 @@ import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; -import org.sonar.plugins.communitydelphi.api.ast.ConditionalExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.IfExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.FinallyBlockNode; import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode; @@ -351,7 +351,7 @@ void testIfExpression() { checker( block(element(NameReferenceNode.class, "A")) .branchesTo(3, 2) - .withTerminator(ConditionalExpressionNode.class), + .withTerminator(IfExpressionNode.class), block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "X")).succeedsTo(0))); 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..5686a51c9 --- /dev/null +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/ExpressionTypeResolverTest.java @@ -0,0 +1,107 @@ +/* + * Sonar Delphi Plugin + * 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 + * 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 static org.sonar.plugins.communitydelphi.api.type.StructKind.CLASS; +import static org.sonar.plugins.communitydelphi.api.type.TypeFactory.unknownType; + +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; +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 FACTORY = TypeFactoryUtils.defaultFactory(); + private static final ExpressionTypeResolver RESOLVER = new ExpressionTypeResolver(FACTORY); + + @Test + void testIfExpressionWithSameTypeOnBothBranchesReturnsThatType() { + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolve(integer, integer).is(integer)).isTrue(); + } + + @Test + void testIfExpressionWithUnknownThenBranchReturnsElseType() { + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolve(unknownType(), integer).is(integer)).isTrue(); + } + + @Test + void testIfExpressionWithUnknownElseBranchReturnsThenType() { + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolve(integer, unknownType()).is(integer)).isTrue(); + } + + @Test + void testIfExpressionPromotesIntegerToReal() { + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + Type real = FACTORY.getIntrinsic(IntrinsicType.DOUBLE); + assertThat(resolve(integer, real).is(real)).isTrue(); + assertThat(resolve(real, integer).is(real)).isTrue(); + } + + @Test + void testIfExpressionWidensSmallerIntegerToLarger() { + Type byteType = FACTORY.getIntrinsic(IntrinsicType.BYTE); + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolve(byteType, integer).is(integer)).isTrue(); + assertThat(resolve(integer, byteType).is(integer)).isTrue(); + } + + @Test + void testIfExpressionWithDescendantAndAncestorClassReturnsAncestor() { + 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 testIfExpressionWithSiblingClassesReturnsCommonAncestor() { + 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(); + } + + @Test + void testIfExpressionWithUnrelatedTypesReturnsUnknown() { + Type record = TypeMocker.struct("TFoo", CLASS); + Type integer = FACTORY.getIntrinsic(IntrinsicType.INTEGER); + assertThat(resolve(record, integer).isUnknown()).isTrue(); + } + + private static Type resolve(Type thenType, Type elseType) { + IfExpressionNode node = mock(IfExpressionNode.class); + ExpressionNode thenExpression = mock(ExpressionNode.class); + ExpressionNode elseExpression = mock(ExpressionNode.class); + when(thenExpression.getType()).thenReturn(thenType); + when(elseExpression.getType()).thenReturn(elseType); + when(node.getThenExpression()).thenReturn(thenExpression); + when(node.getElseExpression()).thenReturn(elseExpression); + return RESOLVER.resolve(node); + } +}