kaitai-io / kaitai_struct

Kaitai Struct: declarative language to generate binary data parsers in C++ / C# / Go / Java / JavaScript / Lua / Nim / Perl / PHP / Python / Ruby
https://kaitai.io
4.01k stars 196 forks source link

PHP: top-level type cannot be named `kaitai_struct` #1126

Open generalmimon opened 2 months ago

generalmimon commented 2 months ago

I ran into some suspicious code again - PHPTranslator.scala:156-161:

  def types2classAbs(names: List[String]) =
    names match {
      case List("kaitai_struct") => PHPCompiler.kstructName
      case _ =>
        namespaceRef + "\\" + PHPCompiler.types2classRel(names)
    }

Is this some kind of easter egg? In terms of language design, why couldn't a user name the top-level type kaitai_struct?

A quick test confirms that kaitai_struct as /meta/id really breaks things for PHP:

kaitai_struc.ksy

meta:
  id: kaitai_struc
seq:
  - id: recursive
    type: kaitai_struc
    if: false

kaitai_struct.ksy

meta:
  id: kaitai_struct
seq:
  - id: recursive
    type: kaitai_struct
    if: false
$ kaitai-struct-compiler -- --verbose file -t php kaitai_struc.ksy kaitai_struct.ksy
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF-8
parsing kaitai_struc.ksy...
reading kaitai_struc.ksy...
... compiling it for php...
.... writing KaitaiStruc.php
parsing kaitai_struct.ksy...
reading kaitai_struct.ksy...
... compiling it for php...
.... writing KaitaiStruct.php

Note: using KSC in revision 3c855b38

diff --git 1/KaitaiStruc.php 2/KaitaiStruct.php
index 76e786e..678c30d 100644
--- 1/KaitaiStruc.php
+++ 2/KaitaiStruct.php
@@ -2,15 +2,15 @@
 // This is a generated file! Please edit source .ksy file and use kaitai-struct-compiler to rebuild

 namespace {
-    class KaitaiStruc extends \Kaitai\Struct\Struct {
-        public function __construct(\Kaitai\Struct\Stream $_io, \Kaitai\Struct\Struct $_parent = null, \KaitaiStruc $_root = null) {
+    class KaitaiStruct extends \Kaitai\Struct\Struct {
+        public function __construct(\Kaitai\Struct\Stream $_io, \Kaitai\Struct\Struct $_parent = null, \Kaitai\Struct\Struct $_root = null) {
             parent::__construct($_io, $_parent, $_root === null ? $this : $_root);
             $this->_read();
         }

         private function _read() {
             if (false) {
-                $this->_m_recursive = new \KaitaiStruc($this->_io, $this, $this->_root);
+                $this->_m_recursive = new \Kaitai\Struct\Struct($this->_io, $this, $this->_root);
             }
         }
         protected $_m_recursive;
generalmimon commented 2 months ago

I think the case List("kaitai_struct") => branch is actually never used for the stated purpose (the only scenario in which it is used is when someone names the top-level type kaitai_struct as I did), because this is one of the only 2 occurrences of "kaitai_struct" in the compiler codebase (the other one in RustTranslator.scala looks basically the same). So it's probably a legacy of older code and is no longer used.

GreyCat commented 2 months ago

Totally agree. Moreover, I would contest removing just about anything in the codebase which does not result in any regression in the tests.