Fix that const definitions referencing imported consts cannot be load…

…ed (#551) * We should check the expectedPath against the thrift file name, instead of the full path, which might contain directories. * Addressing the comment for the test case. * Use an existing API of the Location class. * Remove unintentional format change.
microsoft · Jun 11, 2024 · ef9b63c · ef9b63c
1 parent 3c6d808
commit ef9b63c
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 3 deletions.
diff --git a/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Linker.kt b/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Linker.kt
@@ -443,10 +443,9 @@ internal class Linker(
             if (ix != -1) {
                 val includeName = symbol.substring(0, ix)
                 val qualifiedName = symbol.substring(ix + 1)
-                val expectedPath = "$includeName.thrift"
                 constant = program.includes
                         .asSequence()
-                        .filter { p -> p.location.path == expectedPath } // TODO: Should this be ==, or endsWith?
+                        .filter { p -> p.location.programName == includeName }
                         .mapNotNull { p -> p.constantMap[qualifiedName] }
                         .firstOrNull()
             }

diff --git a/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt b/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt
@@ -264,6 +264,47 @@ class LoaderTest {
         enum.location.path shouldBe listOf("nested", "a.thrift").joinToString(File.separator)
     }
 
+    @Test
+    fun crazyIncludeReferencedConst() {
+        val nestedDir = File(tempDir, "nested").apply { mkdir() }
+        val fileNestedAThrift = File(nestedDir, "a.thrift")
+        val fileAnotherAThrift = File(tempDir, "another_a.thrift")
+        val fileBThrift = File(tempDir, "b.thrift")
+
+        fileNestedAThrift.writeText(
+            """
+                namespace java com.microsoft.thrifty.test.crazyIncludeReferencedConst
+                
+                const string HELLO = "hello"
+            """
+        )
+        fileAnotherAThrift.writeText(
+            """
+                namespace java com.microsoft.thrifty.test.crazyIncludeReferencedConst
+                
+                const string HELLO = "actually goodbye"
+            """
+        )
+        fileBThrift.writeText(
+            """
+                include 'another_a.thrift'
+                include 'nested/a.thrift'
+                namespace java com.microsoft.thrifty.test.crazyIncludeReferencedConst
+                
+                const string HELLO_AGAIN = a.HELLO
+            """
+        )
+
+        val loader = Loader()
+        loader.addIncludePath(tempDir.toPath())
+
+        val schema = loader.load()
+
+        val helloAgain = schema.constants.single { const -> const.name == "HELLO_AGAIN" }
+        val referencedConstant = helloAgain.referencedConstants.single()
+        referencedConstant.location.path shouldBe listOf("nested", "a.thrift").joinToString(File.separator)
+    }
+
     @Test
     fun relativeIncludesConsiderIncludingFileLocation() {
         val thriftDir = File(tempDir, "thrift").apply { mkdir() }
@@ -1370,4 +1411,4 @@ class LoaderTest {
                 name
             }
         }
-}
+}