kaitai_struct_compiler icon indicating copy to clipboard operation
kaitai_struct_compiler copied to clipboard

Remove `id` from paths to instances in style warnings

Open Mingun opened this issue 1 year ago • 2 comments

It seems relatively new error, but the code was not changed since 2021...

Fixes errors like:

[info] - style_bad_num_inst_value *** FAILED ***
[info]   [style_bad_num_inst_value.ksy: /instances/size_of_foo/id:
[info]   	warning: use `num_foos` instead of `size_of_foo`, given that it's only used as repeat count of `foos` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[info]   ]
[info]     did not equal
[info]   [style_bad_num_inst_value.ksy: /instances/size_of_foo:
[info]   	warning: use `num_foos` instead of `size_of_foo`, given that it's only used as repeat count of `foos` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[info]   ] (SimpleMatchers.scala:34)

/instances/size_of_foo/id is not-existing path. Instances does not have id, the name of instance is its id.

Fixes https://github.com/kaitai-io/kaitai_struct/issues/920.

Also fixes a couple of other errors

Mingun avatar Sep 08 '24 10:09 Mingun

@Mingun:

It seems relatively new error

It is. I recently extended the formats_err test suite: https://github.com/kaitai-io/kaitai_struct_tests/compare/c5f366f4ce108650d585f9218deb9de345c44e7f...0427d48a0da213241f75f7029cd08217d0e1c7ef

The tests you're fixing here were added in https://github.com/kaitai-io/kaitai_struct_tests/commit/c911e0a302939208d796d344294d0caa026b6941 to demonstrate https://github.com/kaitai-io/kaitai_struct/issues/920. So ideally you should add Fixes https://github.com/kaitai-io/kaitai_struct/issues/920 to the PR description and commit message.

generalmimon avatar Sep 08 '24 10:09 generalmimon

@generalmimon, could you look again? This PR fixes 13 errors

Mingun avatar Sep 10 '24 16:09 Mingun

@GreyCat, @generalmimon , this PR reduces error count from currently 54 to 42. It is small win, do you mind to merge it?

Mingun avatar Aug 22 '25 08:08 Mingun

@generalmimon, your suggestion applied

Mingun avatar Aug 23 '25 14:08 Mingun