Some expression language operators seems to produce invalid ruby output

[wader]

Reproduction:

$ docker pull kaitai/ksv
Using default tag: latest
latest: Pulling from kaitai/ksv
Digest: sha256:444ac8b2ad80a01efa62f09eb3fbce7b2faddb18362017494c5cf3a2eb6d105a
Status: Image is up to date for kaitai/ksv:latest
docker.io/kaitai/ksv:latest
$ docker run -v "$PWD:/share" -it --entrypoint=ksc kaitai/ksv --version
kaitai-struct-compiler 0.10

$ cat ruby_op_bug.ksy
meta:
  id: ruby_op_bug
instances:
  a:
    value: (true == true) == true

$ docker run -v "$PWD:/share" -it --entrypoint=ksc kaitai/ksv -t ruby ruby_op_bug.ksy
$ cat ruby_op_bug.rb
# This is a generated file! Please edit source .ksy file and use kaitai-struct-compiler to rebuild

require 'kaitai/struct/struct'

unless Gem::Version.new(Kaitai::Struct::VERSION) >= Gem::Version.new('0.9')
  raise "Incompatible Kaitai Struct Ruby API: 0.9 or later is required, but you have #{Kaitai::Struct::VERSION}"
end

class RubyOpBug < Kaitai::Struct::Struct
  def initialize(_io, _parent = nil, _root = self)
    super(_io, _parent, _root)
    _read
  end

  def _read
    self
  end
  def a
    return @a unless @a.nil?
    @a = true == true == true
    @a
  end
end

$ ruby ruby_op_bug.rb
ruby_op_bug.rb:20: syntax error, unexpected ==
    @a = true == true == true

Seems parentheses got stripped but == is no associative in ruby. For some other operators liks + the parentheses seems to be kept.

[generalmimon]

@wader Thanks for reporting. I'm personally not too surprised about this (although I probably should be, because this bug still exists on the main branch and I don't think there has been a specific issue about this 😛). This is quite a long-running bug and although some instances (places/situations) were fixed in some languages, I'd say most of the possible occurrences still don't work properly.

Some time ago, I created a test ExprOpsParens (https://github.com/kaitai-io/kaitai_struct_tests/pull/74) and you can observe at https://ci.kaitai.io/ that it still fails in Ruby. But although this test attempted to test the cases I knew of back then, now I know that it by far doesn't cover all cases.

I also encountered instances of this issue when working on serialization - i.e. in Java when testing enum values for equality (https://github.com/kaitai-io/kaitai_struct_compiler/commit/793f58b99945f80bf51bdc85db64123477bc382e#diff-72a5f940fe59dd50fd56699451d4c08908452eb0f55f826366d39994d0109359). And this case wasn't even revealed by any of the formats in https://github.com/kaitai-io/kaitai_struct_tests - I only found about it by trying to compile the specs from the format library, and seeing that the generated output isn't valid Java code.

[generalmimon]

Actually, I've come to realize that the current testing approach used in https://github.com/kaitai-io/kaitai_struct_tests is highly inefficient and insufficient. Yes, it can definitely reveal some accidental bug from time to time (when we have 11+ targets with various degrees of maturity, you can always catch something with any simple test). But the problem is that all the .ksy specs are written directly by a human, and usually when it comes to adding a test for a new feature, only the simplest case is tested and all combinations with other existing features are merely assumed to work too. This has been proven wrong many times, and it's always embarrassing to say "oh, this new feature doesn't work on a field with if? Shocking, we didn't think of testing that!", and this repeats over and over again for all features.

You can see that most bugs in the issue tracker is not like "this feature alone is completely broken", it's almost always "this feature doesn't work in combination with this other feature".

---

To address this, I'm currently working on a .ksy test format generator in Rust. It's in the very early stages so far (I haven't even pushed it anywhere yet), but my idea is to say "these are possible data types in the KS language", "these are the various ways each data type can be produced", "what works as a seq field must also work in instances the same way", "what works as a singular field must work when combined with repeat" etc., and the generator would produce all sorts of combinations. I hope that one day this approach will be able to supersede the current manually written .ksy specs in https://github.com/kaitai-io/kaitai_struct_tests/tree/master/formats.

[Mingun]

@generalmimon , while you ideas can be the right in theory, in practice, bugs will be only increasing if no one of existing maintainers will not review and merge PRs that can improve situation step-by-step. Most of my attempts to improve situation just stalled because I cannot move forward without fixing issues that I've already offered in PRs. Current code is so smell, that it's just impractical to dig into it without small victories