From 13276389d94e82ecab474d0f8f9349e410a5b87d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Filip=20Doln=C3=ADk?= <github@filipdolnik.com>
Date: Fri, 17 May 2024 13:41:50 +0200
Subject: [PATCH] Fix that replaced visibility was not correctly applied to the
 entire override hierarchy.

---
 SKIE/acceptance-tests                         |  2 +-
 .../annotations/SkieVisibility.kt             |  4 +++
 .../skie/configuration/SkieVisibility.kt      |  4 +++
 .../sir/CommitSirIsReplacedPropertyPhase.kt   | 28 ++++++++++++++++---
 .../element/SirDeclarationWithVisibility.kt   |  2 ++
 .../sir/element/SirOverridableDeclaration.kt  |  1 -
 6 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/SKIE/acceptance-tests b/SKIE/acceptance-tests
index 84a0999f..7c09e892 160000
--- a/SKIE/acceptance-tests
+++ b/SKIE/acceptance-tests
@@ -1 +1 @@
-Subproject commit 84a0999fe4c921fa28782945319e985c1bc4718d
+Subproject commit 7c09e89295634ac0ecb4098bd0fb4ad0328439f0
diff --git a/SKIE/common/configuration/annotations/src/commonMain/kotlin/co/touchlab/skie/configuration/annotations/SkieVisibility.kt b/SKIE/common/configuration/annotations/src/commonMain/kotlin/co/touchlab/skie/configuration/annotations/SkieVisibility.kt
index f1000128..8c187fc7 100644
--- a/SKIE/common/configuration/annotations/src/commonMain/kotlin/co/touchlab/skie/configuration/annotations/SkieVisibility.kt
+++ b/SKIE/common/configuration/annotations/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/SKIE/common/configuration/declaration/src/commonMain/kotlin/co/touchlab/skie/configuration/SkieVisibility.kt b/SKIE/common/configuration/declaration/src/commonMain/kotlin/co/touchlab/skie/configuration/SkieVisibility.kt
index 921df7df..15e80592 100644
--- a/SKIE/common/configuration/declaration/src/commonMain/kotlin/co/touchlab/skie/configuration/SkieVisibility.kt
+++ b/SKIE/common/configuration/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/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/phases/sir/CommitSirIsReplacedPropertyPhase.kt b/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/phases/sir/CommitSirIsReplacedPropertyPhase.kt
index 62b80e75..f584060b 100644
--- a/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/phases/sir/CommitSirIsReplacedPropertyPhase.kt
+++ b/SKIE/kotlin-compiler/core/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/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirDeclarationWithVisibility.kt b/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirDeclarationWithVisibility.kt
index d56d017f..8dfc9dc1 100644
--- a/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirDeclarationWithVisibility.kt
+++ b/SKIE/kotlin-compiler/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/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirOverridableDeclaration.kt b/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirOverridableDeclaration.kt
index 2b590c4b..247e06c3 100644
--- a/SKIE/kotlin-compiler/core/src/commonMain/kotlin/co/touchlab/skie/sir/element/SirOverridableDeclaration.kt
+++ b/SKIE/kotlin-compiler/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 {