Skip to content

Commit

Permalink
Fix leaking of types and enums from other imported specs to specs whi…
Browse files Browse the repository at this point in the history
…ch does not import them explicitly

_(review in whitespace ignored mode)_
  • Loading branch information
Mingun committed Apr 17, 2024
1 parent 2191070 commit 98de60b
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 18 deletions.
3 changes: 3 additions & 0 deletions shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
)
Expand All @@ -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
}
}
}
Expand Down Expand Up @@ -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
}
}
}
Expand Down

0 comments on commit 98de60b

Please sign in to comment.