diff --git a/CHANGELOG.md b/CHANGELOG.md index fe26971d6..47ae27e43 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] +### Fixed + +- Ambiguous type comparisons from variants to real types. + ## [1.17.1] - 2025-06-10 ### Fixed diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationResolver.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationResolver.java index c8a2f1e85..337f0198c 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationResolver.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationResolver.java @@ -530,7 +530,7 @@ private static int isBetterVariantConversion(Type current, Type best) { .compare(current, best, Comparator.comparing(InvocationResolver::isIInterface)) .compare(current, best, Comparator.comparing(Type::isUntyped)) .compare(current, best, InvocationResolver::compareNumericType) - .compare(current, best, InvocationResolver::compareRealSize) + .compare(current, best, InvocationResolver::compareRealTypes) .compare(current, best, InvocationResolver::compareIntegerRange) .compare(current, best, InvocationResolver::compareStringType) .result(); @@ -550,20 +550,35 @@ private static int compareNumericType(Type a, Type b) { } } - private static int compareRealSize(Type a, Type b) { + private static int compareRealTypes(Type a, Type b) { if (!a.isReal() || !b.isReal()) { return 0; } + if (isCurrencyCompConflict(a, b) || isCurrencyCompConflict(b, a)) { return 0; } - return Objects.compare(a, b, Comparator.comparingInt(Type::size)); + + return Comparator.comparingInt( + type -> { + type = TypeUtils.findBaseType(type); + if (type.is(IntrinsicType.EXTENDED)) { + return 1; + } else if (type.is(IntrinsicType.DOUBLE)) { + return 2; + } else if (type.is(IntrinsicType.REAL48)) { + return 3; + } else { + return 4; + } + }) + .reversed() + .compare(a, b); } private static boolean isCurrencyCompConflict(Type currencyComp, Type real) { currencyComp = TypeUtils.findBaseType(currencyComp); return (currencyComp.is(IntrinsicType.CURRENCY) || currencyComp.is(IntrinsicType.COMP)) - && real.isReal() && currencyComp.size() >= real.size(); } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/InvocationResolverTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/InvocationResolverTest.java index 85e6ed8a9..63410200a 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/InvocationResolverTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/resolve/InvocationResolverTest.java @@ -60,6 +60,8 @@ import static org.sonar.plugins.communitydelphi.api.type.StructKind.RECORD; import static org.sonar.plugins.communitydelphi.api.type.TypeFactory.untypedType; +import au.com.integradev.delphi.DelphiProperties; +import au.com.integradev.delphi.compiler.Toolchain; import au.com.integradev.delphi.type.factory.ArrayOption; import au.com.integradev.delphi.type.factory.TypeFactoryImpl; import au.com.integradev.delphi.type.parameter.FormalParameter; @@ -72,7 +74,10 @@ import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.sonar.plugins.communitydelphi.api.ast.FormalParameterNode.FormalParameterData; import org.sonar.plugins.communitydelphi.api.symbol.Invocable; import org.sonar.plugins.communitydelphi.api.symbol.scope.DelphiScope; @@ -83,23 +88,28 @@ import org.sonar.plugins.communitydelphi.api.type.TypeFactory; class InvocationResolverTest { - private static final TypeFactory FACTORY = TypeFactoryUtils.defaultFactory(); + private TypeFactory factory; private Set resolved; + @BeforeEach + void setup() { + factory = TypeFactoryUtils.defaultFactory(); + } + private void assertResolved(Type argumentType, Type winnerType, Type loserType) { assertResolved(List.of(argumentType), List.of(winnerType), List.of(loserType)); } - private static Type type(IntrinsicType intrinsic) { - return FACTORY.getIntrinsic(intrinsic); + private Type type(IntrinsicType intrinsic) { + return factory.getIntrinsic(intrinsic); } - private static Type subrange(String image, int min, int max) { - return FACTORY.subrange(image, BigInteger.valueOf(min), BigInteger.valueOf(max)); + private Type subrange(String image, int min, int max) { + return factory.subrange(image, BigInteger.valueOf(min), BigInteger.valueOf(max)); } - private static Type subrange(String image, Type type) { - return FACTORY.subrange(image, type); + private Type subrange(String image, Type type) { + return factory.subrange(image, type); } private void assertResolved( @@ -181,7 +191,7 @@ void testIntegerTypes() { assertResolved(type(INTEGER), type(WORD), type(BYTE)); assertResolved(type(SHORTINT), type(NATIVEINT), type(NATIVEUINT)); - Type hwnd = FACTORY.strongAlias("HWND", type(NATIVEUINT)); + Type hwnd = factory.strongAlias("HWND", type(NATIVEUINT)); assertResolved( List.of(hwnd, type(NATIVEUINT), type(SHORTINT), type(SHORTINT)), List.of(hwnd, type(NATIVEUINT), type(NATIVEINT), type(NATIVEINT)), @@ -189,20 +199,28 @@ void testIntegerTypes() { assertResolved(type(BYTE), type(INTEGER), type(DOUBLE)); - assertResolved(FACTORY.strongAlias("MyWord", type(LONGWORD)), type(INT64), type(INTEGER)); - assertResolved(type(LONGWORD), FACTORY.strongAlias("MyInt64", type(INT64)), type(INTEGER)); + assertResolved(factory.strongAlias("MyWord", type(LONGWORD)), type(INT64), type(INTEGER)); + assertResolved(type(LONGWORD), factory.strongAlias("MyInt64", type(INT64)), type(INTEGER)); } - @Test - void testFloatingPointTypes() { + @ParameterizedTest + @EnumSource( + value = Toolchain.class, + names = {"DCC32", "DCC64"}) + void testRealTypes(Toolchain toolchain) { + factory = new TypeFactoryImpl(toolchain, DelphiProperties.COMPILER_VERSION_DEFAULT); assertResolved(type(EXTENDED), type(DOUBLE), type(SINGLE)); assertResolved(type(EXTENDED), type(REAL), type(SINGLE)); assertResolved(type(DOUBLE), type(EXTENDED), type(SINGLE)); assertResolved(type(REAL), type(EXTENDED), type(SINGLE)); } - @Test - void testMixedToFloatingPointTypes() { + @ParameterizedTest + @EnumSource( + value = Toolchain.class, + names = {"DCC32", "DCC64"}) + void testMixedToRealTypes(Toolchain toolchain) { + factory = new TypeFactoryImpl(toolchain, DelphiProperties.COMPILER_VERSION_DEFAULT); assertResolved( List.of(type(EXTENDED), type(INTEGER)), List.of(type(EXTENDED), type(EXTENDED)), @@ -217,8 +235,12 @@ void testMixedToFloatingPointTypes() { List.of(type(EXTENDED), type(EXTENDED))); } - @Test - void testIntegerToFloatingPointTypes() { + @ParameterizedTest + @EnumSource( + value = Toolchain.class, + names = {"DCC32", "DCC64"}) + void testIntegerToRealTypes(Toolchain toolchain) { + factory = new TypeFactoryImpl(toolchain, DelphiProperties.COMPILER_VERSION_DEFAULT); assertResolved( List.of(type(SHORTINT), type(SHORTINT)), List.of(type(INTEGER), type(INTEGER)), @@ -236,12 +258,12 @@ void testIntegerToFloatingPointTypes() { @Test void testTextTypes() { assertResolved( - FACTORY.strongAlias("MyString", type(UNICODESTRING)), + factory.strongAlias("MyString", type(UNICODESTRING)), type(UNICODESTRING), type(SHORTSTRING)); assertResolved( type(UNICODESTRING), - FACTORY.strongAlias("MyString", type(UNICODESTRING)), + factory.strongAlias("MyString", type(UNICODESTRING)), type(SHORTSTRING)); assertResolved( List.of(type(ANSICHAR), type(ANSICHAR)), @@ -291,8 +313,8 @@ void testTextTypes() { assertResolved(type(ANSISTRING), type(VARIANT), type(SHORTSTRING)); assertResolved(type(SHORTSTRING), type(ANSISTRING), type(VARIANT)); - assertResolved(type(PANSICHAR), type(ANSISTRING), FACTORY.ansiString(CodePages.CP_1252)); - assertResolved(type(PANSICHAR), FACTORY.ansiString(CodePages.CP_1252), type(UNICODESTRING)); + assertResolved(type(PANSICHAR), type(ANSISTRING), factory.ansiString(CodePages.CP_1252)); + assertResolved(type(PANSICHAR), factory.ansiString(CodePages.CP_1252), type(UNICODESTRING)); assertResolved(type(PANSICHAR), type(UNICODESTRING), type(WIDESTRING)); assertResolved(type(PANSICHAR), type(WIDESTRING), type(SHORTSTRING)); @@ -302,12 +324,17 @@ void testTextTypes() { assertIncompatible(type(PWIDECHAR), type(SHORTSTRING)); } - @Test - void testVariantTypes() { + @ParameterizedTest + @EnumSource( + value = Toolchain.class, + names = {"DCC32", "DCC64"}) + void testVariantTypes(Toolchain toolchain) { + factory = new TypeFactoryImpl(toolchain, DelphiProperties.COMPILER_VERSION_DEFAULT); + Type incompatibleType = TypeMocker.struct("Incompatible", RECORD); - Type enumeration = ((TypeFactoryImpl) FACTORY).enumeration("E", DelphiScope.unknownScope()); + Type enumeration = ((TypeFactoryImpl) factory).enumeration("E", DelphiScope.unknownScope()); Type dynamicArray = - ((TypeFactoryImpl) FACTORY).array(null, type(INTEGER), Set.of(ArrayOption.DYNAMIC)); + ((TypeFactoryImpl) factory).array(null, type(INTEGER), Set.of(ArrayOption.DYNAMIC)); assertResolved( List.of(type(UNICODESTRING), incompatibleType, type(BOOLEAN)), @@ -318,9 +345,9 @@ void testVariantTypes() { assertResolved(type(SHORTSTRING), type(ANSISTRING), type(VARIANT)); assertResolved(type(ANSISTRING), type(VARIANT), type(SHORTSTRING)); assertResolved(type(ANSISTRING), type(UNICODESTRING), type(VARIANT)); - assertResolved(type(ANSISTRING), FACTORY.ansiString(CodePages.CP_UTF8), type(VARIANT)); - assertResolved(type(ANSISTRING), FACTORY.ansiString(CodePages.CP_1252), type(VARIANT)); - assertResolved(FACTORY.ansiString(1251), FACTORY.ansiString(1252), type(VARIANT)); + assertResolved(type(ANSISTRING), factory.ansiString(CodePages.CP_UTF8), type(VARIANT)); + assertResolved(type(ANSISTRING), factory.ansiString(CodePages.CP_1252), type(VARIANT)); + assertResolved(factory.ansiString(1251), factory.ansiString(1252), type(VARIANT)); assertResolved( List.of(type(VARIANT), type(BYTE)), @@ -338,9 +365,7 @@ void testVariantTypes() { assertResolved(type(VARIANT), TypeFactory.untypedType(), type(UNICODESTRING)); assertResolved(type(VARIANT), TypeFactory.untypedType(), type(BOOLEAN)); - assertResolved(type(VARIANT), type(EXTENDED), type(CURRENCY)); assertResolved(type(VARIANT), type(CURRENCY), type(UINT64)); - assertResolved(type(VARIANT), type(EXTENDED), type(COMP)); assertResolved(type(VARIANT), type(COMP), type(UINT64)); assertAmbiguous(type(VARIANT), type(CURRENCY), type(DOUBLE)); @@ -391,6 +416,19 @@ void testVariantTypes() { assertIncompatible(type(VARIANT), type(ANSICHAR)); } + @Test + void testVariantToExtendedCurrencyShouldResolveOnDcc32() { + assertResolved(type(VARIANT), type(EXTENDED), type(CURRENCY)); + assertResolved(type(VARIANT), type(EXTENDED), type(COMP)); + } + + @Test + void testVariantToExtendedCurrencyShouldBeAmbiguousOnDcc64() { + factory = new TypeFactoryImpl(Toolchain.DCC64, DelphiProperties.COMPILER_VERSION_DEFAULT); + assertAmbiguous(type(VARIANT), type(EXTENDED), type(CURRENCY)); + assertAmbiguous(type(VARIANT), type(EXTENDED), type(COMP)); + } + @Test void testInheritedTypes() { Type grandparent = TypeMocker.struct("Grandparent", CLASS); @@ -404,16 +442,16 @@ void testInheritedTypes() { @Test void testVarParameters() { Type openArray = - ((TypeFactoryImpl) FACTORY).array(null, type(INTEGER), Set.of(ArrayOption.OPEN)); + ((TypeFactoryImpl) factory).array(null, type(INTEGER), Set.of(ArrayOption.OPEN)); Type dynamicArray = - ((TypeFactoryImpl) FACTORY).array(null, type(INTEGER), Set.of(ArrayOption.DYNAMIC)); + ((TypeFactoryImpl) factory).array(null, type(INTEGER), Set.of(ArrayOption.DYNAMIC)); assertResolvedVar(type(INTEGER), untypedType()); assertResolvedVar(dynamicArray, openArray); assertResolvedVar(type(INTEGER), openArray); - assertResolvedVar(FACTORY.untypedPointer(), FACTORY.untypedPointer()); + assertResolvedVar(factory.untypedPointer(), factory.untypedPointer()); assertResolvedVar( - FACTORY.pointerTo(null, type(INTEGER)), FACTORY.pointerTo(null, type(SHORTINT))); - assertResolvedVar(FACTORY.fileOf(type(INTEGER)), FACTORY.untypedFile()); + factory.pointerTo(null, type(INTEGER)), factory.pointerTo(null, type(SHORTINT))); + assertResolvedVar(factory.fileOf(type(INTEGER)), factory.untypedFile()); } }