Fix that replaced visibility was not correctly applied to the entire …

…override hierarchy.
touchlab · May 17, 2024 · 1327638 · 1327638
1 parent 5a12834
commit 1327638
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 6 deletions.
diff --git a/SKIE/acceptance-tests b/SKIE/acceptance-tests
diff --git a/...ations/src/commonMain/kotlin/co/touchlab/skie/configuration/annotations/SkieVisibility.kt b/...ations/src/commonMain/kotlin/co/touchlab/skie/configuration/annotations/SkieVisibility.kt
@@ -26,6 +26,8 @@ annotation class SkieVisibility {
      * The declaration will be visible from external modules, but it will be:
      *  - marked as `swift-private` (Xcode will not include it in autocomplete suggestions.),
      *  - renamed in Swift by adding the `__` prefix (Obj-C name remains the same)
+     *
+     * The `__` prefix will be added to all overrides of the declaration even if they have different visibility.
      */
     @Retention(AnnotationRetention.BINARY)
     @Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY)
@@ -41,6 +43,8 @@ annotation class SkieVisibility {
     /**
      * The declaration will be visible only for declarations within the Kotlin module (including custom Swift code bundled by SKIE).
      * Additionally, the declaration will be renamed in Swift by adding the `__` prefix (Obj-C name remains the same)
+     *
+     * The `__` prefix will be added to all overrides of the declaration even if they have different visibility.
      */
     @Retention(AnnotationRetention.BINARY)
     @Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY)

diff --git a/...ration/declaration/src/commonMain/kotlin/co/touchlab/skie/configuration/SkieVisibility.kt b/...ration/declaration/src/commonMain/kotlin/co/touchlab/skie/configuration/SkieVisibility.kt
@@ -25,6 +25,8 @@ object SkieVisibility : ConfigurationKey.Enum<co.touchlab.skie.configuration.Ski
          * The declaration will be visible from external modules, but it will be:
          *  - marked as `swift-private` (Xcode will not include it in autocomplete suggestions.),
          *  - renamed in Swift by adding the `__` prefix (Obj-C name remains the same); constructors are not affected.
+         *
+         * The `__` prefix will be added to all overrides of the declaration even if they have different visibility.
          */
         PublicButReplaced(SkieVisibility.PublicButReplaced::class),
 
@@ -36,6 +38,8 @@ object SkieVisibility : ConfigurationKey.Enum<co.touchlab.skie.configuration.Ski
         /**
          * The declaration will be visible only for declarations within the Kotlin module (including custom Swift code bundled by SKIE).
          * Additionally, the declaration will be renamed in Swift by adding the `__` prefix (Obj-C name remains the same); constructors are not affected.
+         *
+         * The `__` prefix will be added to all overrides of the declaration even if they have different visibility.
          */
         InternalAndReplaced(SkieVisibility.InternalAndReplaced::class),
 

diff --git a/...ore/src/commonMain/kotlin/co/touchlab/skie/phases/sir/CommitSirIsReplacedPropertyPhase.kt b/...ore/src/commonMain/kotlin/co/touchlab/skie/phases/sir/CommitSirIsReplacedPropertyPhase.kt
@@ -8,9 +8,11 @@ import co.touchlab.skie.sir.element.SirExtension
 import co.touchlab.skie.sir.element.SirProperty
 import co.touchlab.skie.sir.element.SirSimpleFunction
 import co.touchlab.skie.sir.element.SirTypeDeclaration
+import co.touchlab.skie.sir.element.applyToEntireOverrideHierarchy
 
 /**
  * Prefixes all isReplaced declarations with the appropriate prefix and sets all isReplaced to false.
+ * Unifies the configuration across all overrides.
  *
  * This phase ensures that SKIE can use isReplaced internally without having to worry about the declarations already being replaced.
  * (For example, isReplaced is used by CreateSirMembersPhase to implement isRefinedInSwift and user-configurable SkieVisibility.)
@@ -38,15 +40,33 @@ object CommitSirIsReplacedPropertyPhase : SirPhase {
     }
 
     private fun SirSimpleFunction.commitIsReplaced() {
-        this.identifier = this.identifierAfterVisibilityChange
+        // Already processed by some member of the override hierarchy.
+        if (!this.isReplaced) {
+            return
+        }
 
-        this.isReplaced = false
+        this.applyToEntireOverrideHierarchy {
+            isReplaced = true
+
+            identifier = identifierAfterVisibilityChange
+
+            isReplaced = false
+        }
     }
 
     private fun SirProperty.commitIsReplaced() {
-        this.identifier = this.identifierAfterVisibilityChange
+        // Already processed by some member of the override hierarchy.
+        if (!this.isReplaced) {
+            return
+        }
 
-        this.isReplaced = false
+        this.applyToEntireOverrideHierarchy {
+            isReplaced = true
+
+            identifier = identifierAfterVisibilityChange
+
+            isReplaced = false
+        }
     }
 
     private fun SirTypeDeclaration.commitIsReplaced() {

diff --git a/...r/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirDeclarationWithVisibility.kt b/...r/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirDeclarationWithVisibility.kt
@@ -6,6 +6,8 @@ sealed interface SirDeclarationWithVisibility : SirDeclaration {
 
     /**
      * If true, the "__" will be added to the name.
+     *
+     * Must be consistent across all overrides.
      */
     val isReplaced: Boolean
 

diff --git a/...iler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirOverridableDeclaration.kt b/...iler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirOverridableDeclaration.kt
@@ -1,6 +1,5 @@
 package co.touchlab.skie.sir.element
 
-import co.touchlab.skie.kir.util.BaseOverridableDeclaration
 import co.touchlab.skie.util.pop
 
 sealed interface SirOverridableDeclaration<T : SirOverridableDeclaration<T>> : SirCallableDeclaration {