From 98de60b23c8f60332ab4b5ffd9e7063a03b056fb Mon Sep 17 00:00:00 2001 From: Mingun Date: Wed, 17 Apr 2024 22:29:50 +0500 Subject: [PATCH] Fix leaking of types and enums from other imported specs to specs which does not import them explicitly _(review in whitespace ignored mode)_ --- .../io/kaitai/struct/format/ClassSpec.scala | 3 ++ .../struct/precompile/LoadImports.scala | 14 +++--- .../struct/precompile/ResolveTypes.scala | 47 ++++++++++++++----- 3 files changed, 46 insertions(+), 18 deletions(-) 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 6a3cb0135..5d58b10c1 100644 --- a/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala +++ b/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala @@ -81,6 +81,9 @@ case class ClassSpec( var seqSize: Sized = NotCalculatedSized + /** The list of top-level type specifications which contains import of this type. */ + var importedInto = mutable.ListBuffer[ClassSpec]() + def toDataType: DataType = { val cut = CalcUserType(name, None) cut.classSpec = Some(this) diff --git a/shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala b/shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala index d25bfeb9b..55d74795a 100644 --- a/shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala +++ b/shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala @@ -32,7 +32,7 @@ class LoadImports(specs: ClassSpecs) { loadImport( name, curClass.meta.path ++ List("imports", idx.toString), - Some(curClass.fileNameAsStr), + curClass, workDir ) }).map((x) => x.flatten) @@ -44,9 +44,10 @@ class LoadImports(specs: ClassSpecs) { Future.sequence(List(thisMetaFuture, nestedFuture)).map((x) => x.flatten) } - private def loadImport(name: String, path: List[String], inFile: Option[String], workDir: ImportPath): Future[List[ClassSpec]] = { + private def loadImport(name: String, path: List[String], curClass: ClassSpec, workDir: ImportPath): Future[List[ClassSpec]] = { Log.importOps.info(() => s".. LoadImports: loadImport($name, workDir = $workDir)") + val inFile = Some(curClass.fileNameAsStr) val impPath = ImportPath.fromString(name) val fullPath = ImportPath.add(workDir, impPath) @@ -63,8 +64,9 @@ class LoadImports(specs: ClassSpecs) { s".. LoadImports: loadImport($name, workDir = $workDir), got spec=$specNameAsStr" }) optSpec match { - case Some(spec) => - val specName = spec.name.head + case Some(importedSpec) => + importedSpec.importedInto += curClass + val specName = importedSpec.name.head // Check if spec name does not match file name. If it doesn't match, // it is probably already a serious error. if (name != specName) @@ -88,12 +90,12 @@ class LoadImports(specs: ClassSpecs) { val isNewSpec = specs.synchronized { val isNew = !specs.contains(specName) if (isNew) { - specs(specName) = spec + specs(specName) = importedSpec } isNew } if (isNewSpec) { - processClass(spec, ImportPath.updateWorkDir(workDir, impPath)) + processClass(importedSpec, ImportPath.updateWorkDir(workDir, impPath)) } else { Log.importOps.warn(() => s"... we have that already, ignoring") Future { List() } diff --git a/shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala b/shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala index f6f0c888b..f4b706ff6 100644 --- a/shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala +++ b/shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala @@ -9,8 +9,11 @@ import io.kaitai.struct.problems._ /** * A collection of methods that resolves user types and enum types, i.e. * converts names into ClassSpec / EnumSpec references. + * + * This step runs for each top-level [[format.ClassSpec]]. */ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) extends PrecompileStep { + /** Resolves references to types and enums in `topClass` and all its nested types. */ override def run(): Iterable[CompilationProblem] = topClass.mapRec(resolveUserTypes).map(problem => problem.localizedInType(topClass)) @@ -68,6 +71,14 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) } } + /** + * Resolves `typeName` reference used in `curClass` to a type definition, or + * returns [[TypeNotFoundErr]] error. + * + * @param curClass Class that contains member + * @param typeName A reference to a type that need to be resolved + * @param path A path to the attribute in KSY where the error should be reported if reference is unknown + */ def resolveUserType(curClass: ClassSpec, typeName: List[String], path: List[String]): (Option[ClassSpec], Option[CompilationProblem]) = { val res = realResolveUserType(curClass, typeName, path) @@ -103,7 +114,7 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) // No further names to resolve, here's our answer Some(nestedClass) } else { - // Try to resolve recursively + // Try to resolve recursively in all nested classes realResolveUserType(nestedClass, restNames, path) } ) @@ -124,12 +135,18 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) // If there's None => no luck at all val resolvedTop = specs.get(firstName) resolvedTop match { - case None => None - case Some(classSpec) => if (restNames.isEmpty) { - resolvedTop - } else { - realResolveUserType(classSpec, restNames, path) + // We should use that spec if it is imported in our file (which is represented + // by our top-level class). It `topClass` imports `classSpec`, we could try to + // resolve type in it + // TODO: if type is defined in spec, we could add a suggestion to error to add missing import + case Some(classSpec) if (classSpec.importedInto.contains(topClass)) => { + if (restNames.isEmpty) { + resolvedTop + } else { + realResolveUserType(classSpec, restNames, path) + } } + case _ => None } } } @@ -183,13 +200,19 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean) // If there's None => no luck at all val resolvedTop = specs.get(firstName) resolvedTop match { - case None => None - case Some(classSpec) => if (restNames.isEmpty) { - // resolved everything, but this points to a type name, not enum name - None - } else { - resolveEnumSpec(classSpec, restNames) + // We should use that spec if it is imported in our file (which is represented + // by our top-level class). It `topClass` imports `classSpec`, we could try to + // resolve type in it + // TODO: if type is defined in spec, we could add a suggestion to error to add missing import + case Some(classSpec) if (classSpec.importedInto.contains(topClass)) => { + if (restNames.isEmpty) { + // resolved everything, but this points to a type name, not enum name + None + } else { + resolveEnumSpec(classSpec, restNames) + } } + case _ => None } } }