Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove id from paths to instances in style warnings #306

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,19 @@ object ClassSpec {
private def checkDupId(prevAttrOpt: Option[MemberSpec], id: String, nowAttr: YAMLPath): Unit = {
prevAttrOpt match {
case Some(prevAttr) =>
// Report error at position where referenced param / attribute / instance is defined.
// Add `id` for attributes in `seq` and `params`, do not add for instances
val path = nowAttr match {
case _: InstanceSpec => nowAttr.path
case _ => nowAttr.path :+ "id"
}
val prevPath = prevAttr match {
case _: InstanceSpec => prevAttr.path
case _ => prevAttr.path :+ "id"
}
throw KSYParseError.withText(
s"duplicate attribute ID '$id', previously defined at /${prevAttr.pathStr}",
nowAttr.path
s"duplicate attribute ID '$id', previously defined at /${prevPath.mkString("/")}",
path
)
case None =>
// no dups, ok
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ object MetaSpec {
ParseUtils.getOptValueStr(srcMap, "ks-version", path).foreach { (verStr) =>
val ver = KSVersion.fromStr(verStr)
if (ver > KSVersion.current)
throw KSYParseError.incompatibleVersion(ver, KSVersion.current, path)
throw KSYParseError.incompatibleVersion(ver, KSVersion.current, path :+ "ks-version")
}

val endian: Option[Endianness] = Endianness.fromYaml(srcMap.get("endian"), path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,47 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
}

def resolveUserTypeForMember(curClass: ClassSpec, attr: MemberSpec): Iterable[CompilationProblem] =
resolveUserType(curClass, attr.dataType, attr.path)
resolveUserType(curClass, attr.dataType, attr.path, None)

def resolveUserType(curClass: ClassSpec, dataType: DataType, path: List[String]): Iterable[CompilationProblem] = {
/**
* Resolves the type of the `dataType` of an attribute defined in `curClass`.
*
* @param curClass The owner of attribute which type is being resolved
* @param dataType The type of the attribute being resolved
* @param path YAML path to the attribute where diagnostics should bу reported
* @param caseExpr If attribute type is switchable type then contains the specific case name
*/
def resolveUserType(
curClass: ClassSpec,
dataType: DataType,
path: List[String],
caseExpr: Option[String],
): Iterable[CompilationProblem] = {
dataType match {
case ut: UserType =>
val (resClassSpec, problems) = resolveUserType(curClass, ut.name, path ++ List("type"))
val (resClassSpec, problems) = resolveUserType(
curClass,
ut.name,
caseExpr match {
case Some(case_) => path ++ List("type", "cases", case_)
case None => path :+ "type"
}
)
ut.classSpec = resClassSpec
problems
case et: EnumType =>
et.enumSpec = resolveEnumSpec(curClass, et.name)
if (et.enumSpec.isEmpty) {
Some(EnumNotFoundErr(et.name, curClass, path ++ List("enum")))
Some(EnumNotFoundErr(et.name, curClass, path :+ "enum"))
} else {
None
}
case st: SwitchType =>
st.cases.flatMap { case (caseName, ut) =>
resolveUserType(curClass, ut, path ++ List("type", "cases", caseName.toString))
resolveUserType(curClass, ut, path, Some(caseName.toString))
}
case at: ArrayType =>
resolveUserType(curClass, at.elType, path)
resolveUserType(curClass, at.elType, path, caseExpr)
case _ =>
// not a user type, nothing to resolve
None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,33 +39,53 @@ class StyleCheckIds(specs: ClassSpecs) extends PrecompileStep {
}
}

/**
* @param spec Owner of `attr`
* @param attr Attribute that references attribute with probably not-conformant name
*/
def getSizeRefProblem(spec: ClassSpec, attr: MemberSpec): Option[CompilationProblem] = {
getSizeReference(spec, attr.dataType).flatMap(sizeRefAttr => {
val existingName = sizeRefAttr.id.humanReadable
val goodName = s"len_${attr.id.humanReadable}"
if (existingName != goodName) {
// Report error at position where referenced attribute is defined.
// Add `id` for attributes in `seq`, do not add for instances
val path = sizeRefAttr match {
case _: InstanceSpec => sizeRefAttr.path
case _ => sizeRefAttr.path :+ "id"
}
Some(StyleWarningSizeLen(
goodName,
existingName,
attr.id.humanReadable,
ProblemCoords(path = Some(sizeRefAttr.path ++ List("id")))
ProblemCoords(path = Some(path))
))
} else {
None
}
})
}

/**
* @param spec Owner of `attr`
* @param attr Attribute that references attribute with probably not-conformant name
*/
def getRepeatExprRefProblem(spec: ClassSpec, attr: AttrLikeSpec): Option[CompilationProblem] = {
getRepeatExprReference(spec, attr).flatMap(repeatExprRefAttr => {
val existingName = repeatExprRefAttr.id.humanReadable
val goodName = s"num_${attr.id.humanReadable}"
if (existingName != goodName) {
// Report error at position where referenced attribute is defined.
// Add `id` for attributes in `seq`, do not add for instances
val path = repeatExprRefAttr match {
case _: InstanceSpec => repeatExprRefAttr.path
case _ => repeatExprRefAttr.path :+ "id"
}
Some(StyleWarningRepeatExprNum(
goodName,
existingName,
attr.id.humanReadable,
ProblemCoords(path = Some(repeatExprRefAttr.path ++ List("id")))
ProblemCoords(path = Some(path))
))
} else {
None
Expand Down
Loading