From 250ccd2763a8c1bd14fad4ff3d4734f493ed7076 Mon Sep 17 00:00:00 2001 From: Petr Pucil Date: Fri, 8 Mar 2024 12:47:42 +0100 Subject: [PATCH] Ensure that using imported nested types doesn't affect their parent type Related: * https://github.com/kaitai-io/kaitai_struct/issues/703 * https://github.com/kaitai-io/kaitai_struct/issues/963 Until now, using a nested type of an imported spec could affect the derived parent type of that nested type. For example: `main.ksy` ```ksy meta: id: main imports: - imported seq: - id: foo type: imported::nested_type ``` `imported.ksy` ```ksy meta: id: imported types: nested_type: {} ``` If you compile `main.ksy` for Java (`kaitai-struct-compiler -t java main.ksy`), `Imported.java` looks like this: ```java public class Imported extends KaitaiStruct { // ... public static class NestedType extends KaitaiStruct { public NestedType(KaitaiStream _io, Main _parent, Imported _root) { /* ... */ } // ^^^^^^^^^^^^ } // ... } ``` Notice the `Main _parent` parameter - the compiler saw that `imported::nested_type` is only used from the `main` type, so it decided that the `_parent` type of `nested_type` will be `main`. However, this means that the `_parent` crosses KSY spec boundaries and the generated `Imported.java` code will be different depending on whether you compile it as imported from `main.ksy` or standalone. Furthermore, you could even access fields of `main` in `imported::nested_type` via `_parent`, but that would mean that `imported.ksy` would only work when imported and compiled via `main.ksy`. From , I suppose none of this should be possible: > It would be a huge mess if `_root` relied on this particular ksy being > imported from some other ksy, and only work in that case. I agree that `_root` and `_parent` arguably shouldn't cross spec boundaries at all, they should only be passed locally within one .ksy spec, and therefore also parent types should only be derived from local type usages. This commit only adjusts the parent type derivation, not invocations of imported nested types with `_parent` and `_root` still being passed (see also https://github.com/kaitai-io/kaitai_struct/issues/963) - they will be fixed later. --- .../main/scala/io/kaitai/struct/format/ClassSpec.scala | 8 ++++++++ .../scala/io/kaitai/struct/precompile/ParentTypes.scala | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala b/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala index 580c37dd7..f76559e9f 100644 --- a/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala +++ b/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala @@ -78,6 +78,14 @@ case class ClassSpec( def parentType: DataType = parentClass.toDataType + /** + * Determines whether this `ClassSpec` represents a type that is external + * (i.e. not defined in the same .ksy file) from the perspective of the given + * `ClassSpec`. + */ + def isExternal(curClass: ClassSpec): Boolean = + name.head != curClass.name.head + /** * Recursively traverses tree of types starting from this type, calling * certain function for every type, starting from this one. diff --git a/shared/src/main/scala/io/kaitai/struct/precompile/ParentTypes.scala b/shared/src/main/scala/io/kaitai/struct/precompile/ParentTypes.scala index 288267786..c6c002352 100644 --- a/shared/src/main/scala/io/kaitai/struct/precompile/ParentTypes.scala +++ b/shared/src/main/scala/io/kaitai/struct/precompile/ParentTypes.scala @@ -68,7 +68,6 @@ class ParentTypes(classSpecs: ClassSpecs) { } def markupParentAs(curClass: ClassSpec, ut: UserType): Unit = { - Log.typeProcParent.info(() => s"..... class=$ut has parent=${curClass.nameAsStr}") ut.classSpec match { case Some(usedClass) => markupParentAs(curClass, usedClass) @@ -79,6 +78,10 @@ class ParentTypes(classSpecs: ClassSpecs) { } def markupParentAs(parent: ClassSpec, child: ClassSpec): Unit = { + // Don't allow type usages across spec boundaries to affect parent resolution + if (child.isExternal(parent)) + return + Log.typeProcParent.info(() => s"..... class=${child.nameAsStr} has parent=${parent.nameAsStr}") child.parentClass match { case UnknownClassSpec => child.parentClass = parent