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

Fix type and enum leaking from one imported KSY to another #303

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Apr 17, 2024

This PR implements solution from my comment. Each top-level ClassSpec now have a list of other top-level ClassSpecs which is explicitly import it. When resolve types and enums now checked if spec with the type/enum was explicitly imported or not and if not, it does not resolves.

Related links:

Mingun added 16 commits October 4, 2024 21:35
The same names makes it difficult to recognize where which function is used even with help from IDE
Also make some functions private to not pollute public namespace. The other methods will be tested
so cannot be made private because it creates difficulties to test them
…ssage is a type name

Especially when the type name is something very generic, like 'test'
Error text does not checked yet, because it will be changed and even those tests
which passed now, would fail due to that

Failures (12):

[info] ClassTypeProvider$Test:
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:287)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:324)
[info]   - doesn't resolve 'two::one' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:336)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:361)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:398)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:435)
[info]   - doesn't resolve 'two::one' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:447)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:472)
[info]   - doesn't resolve 'two::one' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:484)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:509)
[info]   - doesn't resolve 'one::two' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:546)
[info]   - doesn't resolve 'two::one' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.TypeNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:558)
Error text does not checked yet, because it will be changed and even those tests
which passed now, would fail due to that

Failures (6):

[info] ClassTypeProvider$Test:
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:690)
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:740)
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:765)
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:790)
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:815)
[info]   - doesn't resolve 'one::e' *** FAILED ***
[info]     Expected exception io.kaitai.struct.precompile.EnumNotFoundError to be thrown, but no exception was thrown (ClassTypeProvider$Test.scala:840)
This commit also unify enum resolution in expression language and in `enum` keys

Fixes kaitai-io/kaitai_struct#1028
This commit also unify type resolution in expression language and in `type` keys

Fixes kaitai-io/kaitai_struct#786
It not so big and this unifies handling of types and enums
Because of new types type system enforses fixing kaitai-io/kaitai_struct#857

This test need to be updated:

[info] - expr_compare_enum2 *** FAILED ***
[info]   [expr_compare_enum2.ksy: /seq/1/if:
[info]          error: can't compare EnumType(EnumRef(false,List(),animal),Int1Type(false)) and Int1Type(true)
[info]   ]
[info]     did not equal
[info]   [expr_compare_enum2.ksy: /seq/1/if:
[info]          error: can't compare EnumType(List(animal),Int1Type(false)) and Int1Type(true)
[info]   ] (SimpleMatchers.scala:34)
…lative

In all cases this methods called with Some(...)
@Mingun
Copy link
Contributor Author

Mingun commented Oct 5, 2024

Rebased on top of #310 (which includes #309) and test set updated to have enums in value instances. With this PR errors reported correctly, but strangely only the one (for seq, but not for value instance, when file contains both error as in the tests PR), even when I apply #313

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.

Imports "leak" across to other imported files
1 participant