Skip to content

Make InvalidIdentifier exception subclass of ExpressionError to report invalid attributes correctly#312

Open
Mingun wants to merge 1 commit intokaitai-io:masterfrom
Mingun:fix-bad-id-test
Open

Make InvalidIdentifier exception subclass of ExpressionError to report invalid attributes correctly#312
Mingun wants to merge 1 commit intokaitai-io:masterfrom
Mingun:fix-bad-id-test

Conversation

@Mingun
Copy link
Copy Markdown
Contributor

@Mingun Mingun commented Oct 5, 2024

The test

# expr_bad_id_inst_value.ksy: /instances/foo/value:
# 	error: invalid ID: 'BAD', expected /^[a-z][a-z0-9_]*$/
#
meta:
  id: expr_bad_id_inst_value
instances:
  foo:
    value: BAD

Fixes test which instead of reporting error shows this exception:

[info] - expr_bad_id_inst_value *** FAILED ***
[info]   io.kaitai.struct.format.InvalidIdentifier: invalid ID: 'BAD', expected /^[a-z][a-z0-9_]*$/
[info]   at io.kaitai.struct.format.Identifier$.checkIdentifier(Identifier.scala:52)
[info]   at io.kaitai.struct.format.InstanceIdentifier.<init>(Identifier.scala:113)
[info]   at io.kaitai.struct.ClassTypeProvider.resolveMember(ClassTypeProvider.scala:83)
[info]   at io.kaitai.struct.ClassTypeProvider.determineType(ClassTypeProvider.scala:46)
[info]   at io.kaitai.struct.ClassTypeProvider.determineType(ClassTypeProvider.scala:20)
[info]   at io.kaitai.struct.translators.TypeDetector.detectTypeRaw(TypeDetector.scala:61)
[info]   at io.kaitai.struct.translators.TypeDetector.detectType(TypeDetector.scala:25)
[info]   at io.kaitai.struct.precompile.ValueTypesDeriver.$anonfun$deriveValueType$2(ValueTypesDeriver.scala:28)
[info]   at io.kaitai.struct.precompile.ValueTypesDeriver.$anonfun$deriveValueType$2$adapted(ValueTypesDeriver.scala:21)
[info]   at scala.collection.immutable.RedBlackTree$.foreach(RedBlackTree.scala:291)
[info]   ...

Actually, I think, that this is incorrect test. You should check attribute IDs when you defines attribute, not when you use it. Obviously, you use that name with which attribute is defined. I expect, that this test will report unknown attribute BAD even when it is obviously violates ID requirements

Fixes test which instead of reporting error shows this exception:

[info] - expr_bad_id_inst_value *** FAILED ***
[info]   io.kaitai.struct.format.InvalidIdentifier: invalid ID: 'BAD', expected /^[a-z][a-z0-9_]*$/
[info]   at io.kaitai.struct.format.Identifier$.checkIdentifier(Identifier.scala:52)
[info]   at io.kaitai.struct.format.InstanceIdentifier.<init>(Identifier.scala:113)
[info]   at io.kaitai.struct.ClassTypeProvider.resolveMember(ClassTypeProvider.scala:83)
[info]   at io.kaitai.struct.ClassTypeProvider.determineType(ClassTypeProvider.scala:46)
[info]   at io.kaitai.struct.ClassTypeProvider.determineType(ClassTypeProvider.scala:20)
[info]   at io.kaitai.struct.translators.TypeDetector.detectTypeRaw(TypeDetector.scala:61)
[info]   at io.kaitai.struct.translators.TypeDetector.detectType(TypeDetector.scala:25)
[info]   at io.kaitai.struct.precompile.ValueTypesDeriver.$anonfun$deriveValueType$2(ValueTypesDeriver.scala:28)
[info]   at io.kaitai.struct.precompile.ValueTypesDeriver.$anonfun$deriveValueType$2$adapted(ValueTypesDeriver.scala:21)
[info]   at scala.collection.immutable.RedBlackTree$.foreach(RedBlackTree.scala:291)
[info]   ...
@Mingun
Copy link
Copy Markdown
Contributor Author

Mingun commented Dec 24, 2025

For the same strange reason as in #313, this PR adds new error (in comparison to 83dde4b)

[info] - encoding_str_missing *** FAILED ***
[info]   [encoding_str_missing.ksy: /seq/0:
[info]   	error: string type, but no encoding found
[info]   ]
[info]     did not equal
[info]   [encoding_str_missing.ksy: /seq/0:
[info]   	error: string type, but no encoding found
[info]
[info]   encoding_str_missing.ksy: /seq/1:
[info]   	error: string type, but no encoding found
[info]
[info]   encoding_str_missing.ksy: /instances/baz:
[info]   	error: string type, but no encoding found
[info]   ] (SimpleMatchers.scala:34)

It seems to me, that the test was changed after running tests on master (83dde4b).

@generalmimon, could you review this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant