[ObjCExport] Implement initial sorting and ordering for ObjC translation

^KT-65237
2024-02-01 14:29:58 +01:00
parent 27fa3ebfb3
commit 32da1b70f5
14 changed files with 307 additions and 13 deletions
@@ -27,7 +27,11 @@ fun KtClassOrObjectSymbol.translateToObjCClass(): ObjCClass? {
    val comment: ObjCComment? = annotationsList.translateToObjCComment()
    val origin: ObjCExportStubOrigin = getObjCExportStubOrigin()
    val superProtocols: List<String> = superProtocols()
-    val members: List<ObjCExportStub> = getAllMembers().flatMap { it.translateToObjCExportStubs() }
+
+    val members: List<ObjCExportStub> = getAllMembers()
+        .sortedWith(StableSymbolOrder)
+        .flatMap { it.translateToObjCExportStubs() }
+
    val categoryName: String? = null
    val generics: List<ObjCGenericTypeDeclaration> = emptyList()
    val superClassGenerics: List<ObjCNonNullReferenceType> = emptyList()
@@ -17,7 +17,11 @@ import org.jetbrains.kotlin.psi.KtFile

 context(KtAnalysisSession, KtObjCExportSession)
 fun translateToObjCHeader(files: List<KtFile>): ObjCHeader {
-    val declarations = files.flatMap { ktFile -> ktFile.translateToObjCExportStubs() }.toMutableList()
+    val declarations = files
+        .sortedWith(StableFileOrder)
+        .flatMap { file -> file.getFileSymbol().translateToObjCExportStubs() }
+        .toMutableList()
+
    val classForwardDeclarations = getClassForwardDeclarations(declarations).toMutableSet()
    val protocolForwardDeclarations = getProtocolForwardDeclarations(declarations)

@@ -66,14 +70,11 @@ private fun getProtocolForwardDeclarations(declarations: List<ObjCExportStub>) =
    .flatMap { it.superProtocols }
    .toSet()

-context(KtAnalysisSession, KtObjCExportSession)
-fun KtFile.translateToObjCExportStubs(): List<ObjCExportStub> {
-    return this.getFileSymbol().translateToObjCExportStubs()
-}

 context(KtAnalysisSession, KtObjCExportSession)
 fun KtFileSymbol.translateToObjCExportStubs(): List<ObjCExportStub> {
    return listOfNotNull(translateToObjCTopLevelInterfaceFileFacade()) + getFileScope().getClassifierSymbols()
+        .sortedWith(StableClassifierOrder)
        .flatMap { classifierSymbol -> classifierSymbol.translateToObjCExportStubs() }
 }

@@ -26,7 +26,9 @@ fun KtClassOrObjectSymbol.translateToObjCProtocol(): ObjCProtocol? {
    // TODO: Check error type!
    val name = getObjCClassOrProtocolName()

-    val members = getDeclaredMembers().flatMap { it.translateToObjCExportStubs() }
+    val members = getDeclaredMembers()
+        .sortedWith(StableSymbolOrder)
+        .flatMap { it.translateToObjCExportStubs() }

    val comment: ObjCComment? = annotationsList.translateToObjCComment()

@@ -44,6 +44,7 @@ import org.jetbrains.kotlin.objcexport.analysisApiUtils.getDefaultSuperClassOrPr
 context(KtAnalysisSession, KtObjCExportSession)
 fun KtFileSymbol.translateToObjCTopLevelInterfaceFileFacade(): ObjCInterface? {
    val topLevelCallableStubs = getFileScope().getCallableSymbols()
+        .sortedWith(StableCallableOrder)
        .flatMap { callableSymbol -> callableSymbol.translateToObjCExportStubs() }
        .toList()
        /* If there are no top level functions or properties, we do not need to export a file facade */
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.objcexport
+
+import org.jetbrains.kotlin.analysis.api.symbols.*
+import org.jetbrains.kotlin.psi.KtFile
+
+internal val StableFileOrder: Comparator<KtFile>
+    get() = compareBy<KtFile> { file -> file.packageFqName.asString() }
+        .thenComparing { file -> file.name }
+
+internal val StablePropertyOrder: Comparator<KtPropertySymbol> = compareBy { it.name }
+
+internal val StableFunctionOrder: Comparator<KtFunctionSymbol>
+    get() = compareBy(
+        { it.isConstructor },
+        { it.name },
+        { it.valueParameters.size },
+        // TODO NOW! { KonanManglerDesc.run { it.signatureString(false) } }
+    )
+
+internal val StableClassifierOrder: Comparator<KtClassifierSymbol> =
+    compareBy<KtClassifierSymbol> { classifier ->
+        if (classifier !is KtClassOrObjectSymbol) return@compareBy 0
+        else 2
+    }.thenComparing { classifier ->
+        if (classifier is KtClassLikeSymbol) classifier.classIdIfNonLocal?.toString().orEmpty()
+        else ""
+    }
+
+internal val StableCallableOrder: Comparator<KtCallableSymbol> = compareBy<KtCallableSymbol> {
+    when (it) {
+        is KtConstructorSymbol -> 0
+        is KtFunctionSymbol -> 1
+        is KtPropertySymbol -> 2
+        else -> 3
+    }
+}
+    .thenComparing(StablePropertyOrder)
+    .thenComparing(StableFunctionOrder)
+
+internal val StableSymbolOrder: Comparator<KtSymbol> = compareBy<KtSymbol> { symbol ->
+    when (symbol) {
+        is KtFileSymbol -> 0
+        is KtClassifierSymbol -> 1
+        is KtCallableSymbol -> 2
+        else -> Int.MAX_VALUE
+    }
+}
+    .thenComparing(StableClassifierOrder)
+    .thenComparing(StableCallableOrder)
+
+private inline fun <T, reified R> Comparator<T>.thenComparing(comparator: Comparator<R>): Comparator<T> where R : T {
+    return thenComparing { a, b ->
+        if (a is R && b is R) comparator.compare(a, b) else 0
+    }
+}
@@ -36,7 +36,16 @@ import java.nio.file.Files
 * ```
 */
 interface InlineSourceCodeAnalysis {
+
    fun createKtFile(@Language("kotlin") sourceCode: String): KtFile
+
+    fun createKtFiles(
+        builder: KtModuleBuilder.() -> Unit,
+    ): Map</* File Name */ String, KtFile>
+
+    interface KtModuleBuilder {
+        fun sourceFile(fileName: String, @Language("kotlin") sourceCode: String)
+    }
 }

 /**
@@ -68,8 +77,25 @@ class InlineSourceCodeAnalysisExtension : ParameterResolver, AfterEachCallback {
 */
 private class InlineSourceCodeAnalysisImpl(private val tempDir: File) : InlineSourceCodeAnalysis {
    override fun createKtFile(@Language("kotlin") sourceCode: String): KtFile {
-        return createStandaloneAnalysisApiSession(tempDir, listOf(sourceCode))
+        return createStandaloneAnalysisApiSession(tempDir, mapOf("TestSources.kt" to sourceCode))
            .modulesWithFiles.entries.single()
            .value.single() as KtFile
    }
+
+    override fun createKtFiles(builder: InlineSourceCodeAnalysis.KtModuleBuilder.() -> Unit): Map<String, KtFile> {
+        val sources = KtModuleBuilderImpl().also(builder).sources.toMap()
+        return createStandaloneAnalysisApiSession(tempDir, sources)
+            .modulesWithFiles.entries.single()
+            .value.map { it as KtFile }
+            .associateBy { it.name }
+    }
+
+    private class KtModuleBuilderImpl : InlineSourceCodeAnalysis.KtModuleBuilder {
+        val sources = mutableMapOf<String, String>()
+
+        override fun sourceFile(fileName: String, sourceCode: String) {
+            check(fileName !in sourceCode)
+            sources[fileName] = sourceCode
+        }
+    }
 }
@@ -23,14 +23,14 @@ import kotlin.io.path.Path
 */
 fun createStandaloneAnalysisApiSession(
    tempDir: File,
-    kotlinSources: List<String>,
+    kotlinSources: Map</* File Name */ String, /* Source Code */ String>,
 ): StandaloneAnalysisAPISession {
    val testModuleRoot = tempDir.resolve("testModule")
    testModuleRoot.mkdirs()

-    kotlinSources.forEachIndexed { index, kotlinSource ->
-        testModuleRoot.resolve("TestSources$index.kt").apply {
-            writeText(kotlinSource)
+    kotlinSources.forEach { (fileName, sourceCode) ->
+        testModuleRoot.resolve(fileName).apply {
+            writeText(sourceCode)
        }
    }
    return createStandaloneAnalysisApiSession(listOf(testModuleRoot))
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.objcexport.tests
+
+import org.jetbrains.kotlin.objcexport.StableFileOrder
+import org.jetbrains.kotlin.objcexport.testUtils.InlineSourceCodeAnalysis
+import org.junit.jupiter.api.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+
+class TranslationOrderTest(
+    private val inlineSourceCodeAnalysis: InlineSourceCodeAnalysis,
+) {
+
+    @Test
+    fun `test - StableFileOrder - sort by packageName`() {
+        val files = inlineSourceCodeAnalysis.createKtFiles {
+            sourceFile("A.kt", "package com.a")
+            sourceFile("B.kt", "package com.b")
+            sourceFile("C.kt", "package com.c")
+        }
+
+        val unsorted = listOf(files.getValue("C.kt"), files.getValue("B.kt"), files.getValue("A.kt"))
+        val sorted = listOf(files.getValue("A.kt"), files.getValue("B.kt"), files.getValue("C.kt"))
+
+        assertNotEquals(sorted, unsorted)
+        assertEquals(sorted, unsorted.sortedWith(StableFileOrder))
+    }
+
+    @Test
+    fun `test - StableFileOrder - sort by packageName then FileName`() {
+        val files = inlineSourceCodeAnalysis.createKtFiles {
+            sourceFile("A.kt", "package com.a")
+            sourceFile("B1.kt", "package com.b")
+            sourceFile("B2.kt", "package com.b")
+        }
+
+        val unsorted = listOf(files.getValue("B2.kt"), files.getValue("A.kt"), files.getValue("B1.kt"))
+        val sorted = listOf(files.getValue("A.kt"), files.getValue("B1.kt"), files.getValue("B2.kt"))
+
+        assertNotEquals(sorted, unsorted)
+        assertEquals(sorted, unsorted.sortedWith(StableFileOrder))
+    }
+}
@@ -83,11 +83,13 @@ class ObjCExportHeaderGeneratorTest(private val generator: HeaderGenerator) {
    }

    @Test
+    @TodoAnalysisApi // Early translate of forward declarations
    fun `test - classImplementingInterface`() {
        doTest(headersTestDataDir.resolve("classImplementingInterface"))
    }

    @Test
+    @TodoAnalysisApi
    fun `test - interfaceImplementingInterface`() {
        doTest(headersTestDataDir.resolve("interfaceImplementingInterface"))
    }
@@ -176,16 +178,26 @@ class ObjCExportHeaderGeneratorTest(private val generator: HeaderGenerator) {
    }

    @Test
+    @TodoAnalysisApi // Early translate of forward declarations
    fun `test - classWithUnresolvedSuperTypeGenerics`() {
        doTest(headersTestDataDir.resolve("classWithUnresolvedSuperTypeGenerics"))
    }

    @Test
-    @TodoAnalysisApi
    fun `test - topLevelFunctionWithNumberReturn`() {
        doTest(headersTestDataDir.resolve("topLevelFunctionWithNumberReturn"))
    }

+    @Test
+    fun `test - classWithManyMembers`() {
+        doTest(headersTestDataDir.resolve("classWithManyMembers"))
+    }
+
+    @Test
+    fun `test - manyClassesAndInterfaces`() {
+        doTest(headersTestDataDir.resolve("manyClassesAndInterfaces"))
+    }
+
    private fun doTest(root: File, configuration: Configuration = Configuration()) {
        if (!root.isDirectory) fail("Expected ${root.absolutePath} to be directory")
        val generatedHeaders = generator.generateHeaders(root, configuration).toString()
@@ -0,0 +1,40 @@
+#import <Foundation/NSArray.h>
+#import <Foundation/NSDictionary.h>
+#import <Foundation/NSError.h>
+#import <Foundation/NSObject.h>
+#import <Foundation/NSSet.h>
+#import <Foundation/NSString.h>
+#import <Foundation/NSValue.h>
+
+NS_ASSUME_NONNULL_BEGIN
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunknown-warning-option"
+#pragma clang diagnostic ignored "-Wincompatible-property-type"
+#pragma clang diagnostic ignored "-Wnullability"
+
+#pragma push_macro("_Nullable_result")
+#if !__has_feature(nullability_nullable_result)
+#undef _Nullable_result
+#define _Nullable_result _Nullable
+#endif
+
+__attribute__((objc_subclassing_restricted))
+@interface Foo : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+- (void)a __attribute__((swift_name("a()")));
+- (void)aP0:(int32_t)p0 __attribute__((swift_name("a(p0:)")));
+- (void)aP0:(int32_t)p0 p1:(int32_t)p1 __attribute__((swift_name("a(p0:p1:)")));
+- (void)b __attribute__((swift_name("b()")));
+- (void)bP0:(int32_t)p0 __attribute__((swift_name("b(p0:)")));
+- (void)bP0:(int32_t)p0 p1:(int32_t)p1 __attribute__((swift_name("b(p0:p1:)")));
+- (void)c __attribute__((swift_name("c()")));
+- (int32_t)d __attribute__((swift_name("d()")));
+@property (readonly) int32_t pA __attribute__((swift_name("pA")));
+@property (readonly) int32_t pB __attribute__((swift_name("pB")));
+@property (readonly) int32_t pC __attribute__((swift_name("pC")));
+@end
+
+#pragma pop_macro("_Nullable_result")
+#pragma clang diagnostic pop
+NS_ASSUME_NONNULL_END
@@ -0,0 +1,22 @@
+class Foo {
+    /* Number of parameters increases, relevant for ordering */
+    fun a() = Unit
+    fun a(p0: Int) = Unit
+    fun a(p0: Int, p1: Int) = Unit
+
+    /* Should be ordered a, b, c, but is explicitly placed as a, c, b in source code */
+    fun c() = Unit
+
+    /* Number of parameters decreases, should be orderd in increasing number */
+    fun b(p0: Int, p1: Int) = Unit
+    fun b(p0: Int) = Unit
+    fun b() = Unit
+
+    /* Short get sorted in alphabetical order */
+    val pA = 42
+    val pC = 42
+    val pB = 42
+
+    /* Functions should be prioritised over properties */
+    fun d() = 42
+}
@@ -0,0 +1,67 @@
+#import <Foundation/NSArray.h>
+#import <Foundation/NSDictionary.h>
+#import <Foundation/NSError.h>
+#import <Foundation/NSObject.h>
+#import <Foundation/NSSet.h>
+#import <Foundation/NSString.h>
+#import <Foundation/NSValue.h>
+
+NS_ASSUME_NONNULL_BEGIN
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunknown-warning-option"
+#pragma clang diagnostic ignored "-Wincompatible-property-type"
+#pragma clang diagnostic ignored "-Wnullability"
+
+#pragma push_macro("_Nullable_result")
+#if !__has_feature(nullability_nullable_result)
+#undef _Nullable_result
+#define _Nullable_result _Nullable
+#endif
+
+@protocol A0
+@required
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface A1 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface A2 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface A3 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface B1 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface B2 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+__attribute__((objc_subclassing_restricted))
+@interface B3 : Base
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
+ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end
+
+@protocol B4
+@required
+@end
+
+#pragma pop_macro("_Nullable_result")
+#pragma clang diagnostic pop
+NS_ASSUME_NONNULL_END
@@ -0,0 +1,6 @@
+package com.a
+
+class A2
+class A1
+interface A0
+class A3
@@ -0,0 +1,6 @@
+package com.b
+
+class B2
+class B1
+class B3
+interface B4