Skip to content

Commit 91a1074

Browse files
committed
Ruby: generate require_relative calls for imported types
... instead of expecting the calling code to load everything that the generated modules need. Fixes these tests for Ruby: * ImportsCastToImported * ImportsCastToImported2 * ImportsParamsDefEnumImported * ImportsParamsDefUsertypeImported We intentionally use `require_relative` (instead of `require` as usual) and only for imported types (not opaque types) - see the added code comment in `RubyCompiler`'s implementation of `externalTypeDeclaration` for more details. Until now, we've been kind of living under the impression that the generated Ruby files don't need explicit imports, because until recently, all of our import-related tests in https://github.com/kaitai-io/kaitai_struct_tests were passing despite the fact that KSC has never generated any `require`s in Ruby. However, it turned out that this was just a coincidence described in kaitai-io/kaitai_struct#1099, caused by the fact that the Ruby's `require` pollutes the global namespace and that the imported specs for which we were missing `require`s were already required by some other test. This was not the case in the 4 tests that this commit fixes as mentioned above, which were added recently and all import a format spec that no other test uses. This "missing `require`s" problem would have been an issue in [KSV](https://github.com/kaitai-io/kaitai_struct_visualizer) too, but KSV works around it by requiring every single `*.rb` file that KSC has generated (which will include any imported .ksy specs), and opaque types must be explicitly imported using the `-r` command-line option. This should still work after this commit, although calling `require` with each `*.rb` file in the directory of generated format modules becomes unnecessary.
1 parent bb53205 commit 91a1074

11 files changed

+69
-33
lines changed

shared/src/main/scala/io/kaitai/struct/ClassCompiler.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ class ClassCompiler(
3030
}
3131

3232
def compileExternalTypes(topClass: ClassSpec) = {
33-
TypeProcessor.getExternalTypes(topClass).foreach((name) =>
34-
lang.externalTypeDeclaration(name)
33+
TypeProcessor.getExternalTypes(topClass).foreach((extType) =>
34+
lang.externalTypeDeclaration(extType)
3535
)
3636
}
3737

shared/src/main/scala/io/kaitai/struct/TypeProcessor.scala

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,22 @@ import io.kaitai.struct.format._
66

77
import scala.collection.mutable
88

9+
sealed abstract class ExternalType {
10+
def name: List[String]
11+
def isOpaque: Boolean
12+
}
13+
case class ExternalUserType(classSpec: ClassSpec) extends ExternalType {
14+
def name: List[String] = classSpec.name
15+
def isOpaque: Boolean = classSpec.meta.isOpaque
16+
}
17+
case class ExternalEnum(enumSpec: EnumSpec) extends ExternalType {
18+
def name: List[String] = enumSpec.name
19+
def isOpaque: Boolean = false
20+
}
21+
922
object TypeProcessor {
10-
def getExternalTypes(curClass: ClassSpec): Iterable[List[String]] = {
11-
val res = mutable.Set[List[String]]()
23+
def getExternalTypes(curClass: ClassSpec): Iterable[ExternalType] = {
24+
val res = mutable.Set[ExternalType]()
1225
curClass.seq.foreach((attr) =>
1326
res ++= getExternalTypesFromDataType(attr.dataType, curClass)
1427
)
@@ -31,17 +44,17 @@ object TypeProcessor {
3144
res
3245
}
3346

34-
def getExternalTypesFromDataType(dataType: DataType, curClass: ClassSpec): Iterable[List[String]] = {
47+
def getExternalTypesFromDataType(dataType: DataType, curClass: ClassSpec): Iterable[ExternalType] = {
3548
dataType match {
3649
case ut: UserType =>
3750
if (ut.isExternal(curClass)) {
38-
List(ut.classSpec.get.name)
51+
List(ExternalUserType(ut.classSpec.get))
3952
} else {
4053
List()
4154
}
4255
case et: EnumType =>
4356
if (et.isExternal(curClass)) {
44-
List(et.enumSpec.get.name)
57+
List(ExternalEnum(et.enumSpec.get))
4558
} else {
4659
List()
4760
}

shared/src/main/scala/io/kaitai/struct/languages/CppCompiler.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ class CppCompiler(
118118
}
119119
}
120120

121-
override def externalTypeDeclaration(name: List[String]): Unit =
122-
importListHdr.addLocal(outFileNameHeader(name.head))
121+
override def externalTypeDeclaration(extType: ExternalType): Unit =
122+
importListHdr.addLocal(outFileNameHeader(extType.name.head))
123123

124124
override def classHeader(name: List[String]): Unit = {
125125
val className = types2class(List(name.last))

shared/src/main/scala/io/kaitai/struct/languages/JavaScriptCompiler.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import io.kaitai.struct.exprlang.Ast.expr
77
import io.kaitai.struct.format._
88
import io.kaitai.struct.languages.components._
99
import io.kaitai.struct.translators.JavaScriptTranslator
10-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils}
10+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils, ExternalType}
1111

1212
class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1313
extends LanguageCompiler(typeProvider, config)
@@ -58,8 +58,8 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
5858
out.puts("});")
5959
}
6060

61-
override def externalTypeDeclaration(name: List[String]): Unit = {
62-
val className = type2class(name.head)
61+
override def externalTypeDeclaration(extType: ExternalType): Unit = {
62+
val className = type2class(extType.name.head)
6363
importList.add(s"./$className")
6464
}
6565

shared/src/main/scala/io/kaitai/struct/languages/LuaCompiler.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.kaitai.struct.languages
22

3-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils}
3+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils, ExternalType}
44
import io.kaitai.struct.datatype.{DataType, FixedEndian, InheritedEndian, KSError, ValidationNotEqualError}
55
import io.kaitai.struct.datatype.DataType._
66
import io.kaitai.struct.exprlang.Ast
@@ -32,8 +32,8 @@ class LuaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
3232
override def outImports(topClass: ClassSpec) =
3333
importList.toList.mkString("", "\n", "\n")
3434

35-
override def externalTypeDeclaration(name: List[String]): Unit =
36-
importList.add("require(\"" + name.head + "\")")
35+
override def externalTypeDeclaration(extType: ExternalType): Unit =
36+
importList.add("require(\"" + extType.name.head + "\")")
3737

3838
override def fileHeader(topClassName: String): Unit = {
3939
outHeader.puts(s"-- $headerComment")

shared/src/main/scala/io/kaitai/struct/languages/NimCompiler.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import io.kaitai.struct.exprlang.Ast
66
import io.kaitai.struct.format._
77
import io.kaitai.struct.languages.components._
88
import io.kaitai.struct.translators.NimTranslator
9-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils}
9+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils, ExternalType}
1010

1111
class NimCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1212
extends LanguageCompiler(typeProvider, config)
@@ -54,8 +54,8 @@ class NimCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
5454
out.puts
5555
}
5656

57-
override def externalTypeDeclaration(name: List[String]): Unit =
58-
importList.add(name.head)
57+
override def externalTypeDeclaration(extType: ExternalType): Unit =
58+
importList.add(extType.name.head)
5959
override def innerEnums = false
6060
override val translator: NimTranslator = new NimTranslator(typeProvider, importList)
6161
override def universalFooter: Unit = {

shared/src/main/scala/io/kaitai/struct/languages/PerlCompiler.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import io.kaitai.struct.format._
88
import io.kaitai.struct.languages.components.{ExceptionNames, _}
99
import io.kaitai.struct.translators.{PerlTranslator, TypeProvider}
1010
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig}
11+
import io.kaitai.struct.ExternalType
1112

1213
class PerlCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1314
extends LanguageCompiler(typeProvider, config)
@@ -50,8 +51,8 @@ class PerlCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
5051
out.puts("1;")
5152
}
5253

53-
override def externalTypeDeclaration(name: List[String]): Unit =
54-
importList.add(type2class(name.head))
54+
override def externalTypeDeclaration(extType: ExternalType): Unit =
55+
importList.add(type2class(extType.name.head))
5556

5657
override def classHeader(name: List[String]): Unit = {
5758
out.puts

shared/src/main/scala/io/kaitai/struct/languages/PythonCompiler.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import io.kaitai.struct.exprlang.Ast.expr
77
import io.kaitai.struct.format._
88
import io.kaitai.struct.languages.components._
99
import io.kaitai.struct.translators.PythonTranslator
10-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, StringLanguageOutputWriter, Utils}
10+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, StringLanguageOutputWriter, Utils, ExternalType}
1111

1212
class PythonCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1313
extends LanguageCompiler(typeProvider, config)
@@ -77,8 +77,8 @@ class PythonCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
7777
out.puts
7878
}
7979

80-
override def externalTypeDeclaration(name: List[String]): Unit = {
81-
val moduleName = name.head
80+
override def externalTypeDeclaration(extType: ExternalType): Unit = {
81+
val moduleName = extType.name.head
8282
importList.add(
8383
if (config.pythonPackage.nonEmpty) {
8484
s"from ${config.pythonPackage} import $moduleName"

shared/src/main/scala/io/kaitai/struct/languages/RubyCompiler.scala

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import io.kaitai.struct.exprlang.Ast.expr
77
import io.kaitai.struct.format._
88
import io.kaitai.struct.languages.components._
99
import io.kaitai.struct.translators.RubyTranslator
10-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils}
10+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils, ExternalType}
1111

1212
class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1313
extends LanguageCompiler(typeProvider, config)
@@ -34,13 +34,13 @@ class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
3434
override def indent: String = " "
3535

3636
override def outImports(topClass: ClassSpec) =
37-
importList.toList.map((x) => s"require '$x'").mkString("\n") + "\n"
37+
importList.toList.mkString("", "\n", "\n")
3838

3939
override def fileHeader(topClassName: String): Unit = {
4040
outHeader.puts(s"# $headerComment")
4141
outHeader.puts
4242

43-
importList.add("kaitai/struct/struct")
43+
importList.add("require 'kaitai/struct/struct'")
4444

4545
out.puts
4646

@@ -61,6 +61,28 @@ class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
6161
out.puts
6262
}
6363

64+
override def externalTypeDeclaration(extType: ExternalType): Unit = {
65+
// Do not generate imports for opaque types
66+
if (extType.isOpaque)
67+
return
68+
// We intentionally use `require_relative` instead of `require`. This is
69+
// because in our case, it's more reasonable to assume that the imported
70+
// format module is in the same directory as the importing one (because KSC
71+
// generates them to the same directory) than to assume that the imported
72+
// format module is in `$LOAD_PATH`. In addition, since `$LOAD_PATH` is an
73+
// array of paths and the first matching path wins with `require`, by
74+
// avoiding `require` we eliminate the possibility that we load a different
75+
// module than intended if the directory with compiled format modules is not
76+
// the first path in `$LOAD_PATH`.
77+
//
78+
// However, we are careful not to generate any imports for opaque types.
79+
// Opaque types are not generated by KSC, so there's no reason to assume
80+
// that they are defined in .rb files in the same directory as the generated
81+
// format module that uses them. Instead, we expect the user to make them
82+
// globally available, and it's up to them how they do that.
83+
importList.add(s"require_relative '${extType.name.head}'")
84+
}
85+
6486
override def classHeader(name: String): Unit = {
6587
out.puts(s"class ${type2class(name)} < $kstructName")
6688
out.inc
@@ -188,7 +210,7 @@ class RubyCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
188210
}
189211
s"$kstreamName::$procName($srcExpr, ${expression(xorValue)})"
190212
case ProcessZlib =>
191-
importList.add("zlib")
213+
importList.add("require 'zlib'")
192214
s"Zlib::Inflate.inflate($srcExpr)"
193215
case ProcessRotate(isLeft, rotValue) =>
194216
val expr = if (isLeft) {

shared/src/main/scala/io/kaitai/struct/languages/RustCompiler.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import io.kaitai.struct.exprlang.Ast
66
import io.kaitai.struct.format.{NoRepeat, RepeatEos, RepeatExpr, RepeatSpec, _}
77
import io.kaitai.struct.languages.components._
88
import io.kaitai.struct.translators.RustTranslator
9-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils}
9+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, Utils, ExternalType}
1010

1111
class RustCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
1212
extends LanguageCompiler(typeProvider, config)
@@ -54,9 +54,9 @@ class RustCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
5454
out.puts
5555
}
5656

57-
override def externalTypeDeclaration(name: List[String]): Unit = {
58-
val className = type2class(name.last)
59-
val pkg = type2classAbs(name)
57+
override def externalTypeDeclaration(extType: ExternalType): Unit = {
58+
val className = type2class(extType.name.last)
59+
val pkg = type2classAbs(extType.name)
6060

6161
importList.add(s"$pkg::$className")
6262
}

shared/src/main/scala/io/kaitai/struct/languages/components/LanguageCompiler.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import io.kaitai.struct.datatype.{DataType, Endianness, FixedEndian, InheritedEn
44
import io.kaitai.struct.exprlang.Ast
55
import io.kaitai.struct.format._
66
import io.kaitai.struct.translators.AbstractTranslator
7-
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig}
7+
import io.kaitai.struct.{ClassTypeProvider, RuntimeConfig, ExternalType}
88

99
import scala.collection.mutable.ListBuffer
1010

@@ -81,7 +81,7 @@ abstract class LanguageCompiler(
8181
* type.
8282
* @param name absolute path to the type in KS notation (lower underscore)
8383
*/
84-
def externalTypeDeclaration(name: List[String]): Unit = {}
84+
def externalTypeDeclaration(extType: ExternalType): Unit = {}
8585

8686
def classDoc(name: List[String], doc: DocSpec): Unit = {}
8787
def classHeader(name: List[String]): Unit

0 commit comments

Comments
 (0)