search

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.02k stars 197 forks source link

ksc: prohibit sub-type and field name collision #86

Open koczkatamas opened 7 years ago

koczkatamas commented 7 years ago

The generated C# code from this .ksy won't compile:

meta:
  id: dos_mz
seq:
  - id: mz_header # <-- this cannot be the same as the type below
    type: mz_header
types:
  mz_header: # <-- this cannot be the same as the field above
  ...

The C# compiler throws the following error message:

DosMz.cs(130,25): error CS0102: The type `Kaitai.DosMz' already contains a definition for `MzHeader'
DosMz.cs(32,30): (Location of the symbol related to previous error)

This is not an issue in ... because ...

Summary: only C# uses UpperCase properties.

Other solution could be renaming subclasses if we detect a conflict.

koczkatamas commented 7 years ago

Actually renaming inner classes looks like as a better solution. It's very convenient to use the class name as property name in multiple cases...

And probably it's not worth to deny this opportunity from other languages because of one language.

But I have no idea for a good renaming rule. Maybe appending "Struct" or "KS"?

koczkatamas commented 7 years ago

I started implementing a workaround but it gets messy fast: https://github.com/kaitai-io/kaitai_struct_compiler/commit/918d4ccfd9df11b0a30b6cec08e1e65fe1991cea

The same problem happens with enums too, but I cannot easily overwrite enum names...

koczkatamas commented 7 years ago

Here is a bit complex .ksy:

nameconflict.ksy (click to open) ```yaml meta: id: name_conflict endian: le seq: - id: hdr1 type: hdr1 - id: hdr2 type: hdr2 types: hdr1: seq: - id: type type: u4 enum: type enums: type: 1: y hdr2: seq: - id: type type: u4 enum: type - id: inner type: hdr2_inner types: hdr2_inner: seq: - id: type type: u4 enum: type enums: type: 1: x ```


Currently this C# file is generated:

NameConflict.cs (click to open) ```csharp namespace Kaitai { public partial class NameConflict : KaitaiStruct { public static NameConflict FromFile(string fileName) { return new NameConflict(new KaitaiStream(fileName)); } public NameConflict(KaitaiStream io, KaitaiStruct parent = null, NameConflict root = null) : base(io) { m_parent = parent; m_root = root ?? this; _parse(); } private void _parse() { _hdr1 = new Hdr1(m_io, this, m_root); _hdr2 = new Hdr2(m_io, this, m_root); } public partial class Hdr1 : KaitaiStruct { public static Hdr1 FromFile(string fileName) { return new Hdr1(new KaitaiStream(fileName)); } public enum Type { Y = 1, } public Hdr1(KaitaiStream io, NameConflict parent = null, NameConflict root = null) : base(io) { m_parent = parent; m_root = root; _parse(); } private void _parse() { _type = ((Type) m_io.ReadU4le()); } private Type _type; private NameConflict m_root; private NameConflict m_parent; public Type Type { get { return _type; } } public NameConflict M_Root { get { return m_root; } } public NameConflict M_Parent { get { return m_parent; } } } public partial class Hdr2 : KaitaiStruct { public static Hdr2 FromFile(string fileName) { return new Hdr2(new KaitaiStream(fileName)); } public enum Type { X = 1, } public Hdr2(KaitaiStream io, NameConflict parent = null, NameConflict root = null) : base(io) { m_parent = parent; m_root = root; _parse(); } private void _parse() { _type = ((Type) m_io.ReadU4le()); _inner = new Hdr2Inner(m_io, this, m_root); } public partial class Hdr2Inner : KaitaiStruct { public static Hdr2Inner FromFile(string fileName) { return new Hdr2Inner(new KaitaiStream(fileName)); } public Hdr2Inner(KaitaiStream io, Hdr2 parent = null, NameConflict root = null) : base(io) { m_parent = parent; m_root = root; _parse(); } private void _parse() { _type = ((NameConflict.Hdr2.Type) m_io.ReadU4le()); } private Type _type; private NameConflict m_root; private NameConflict.Hdr2 m_parent; public Type Type { get { return _type; } } public NameConflict M_Root { get { return m_root; } } public NameConflict.Hdr2 M_Parent { get { return m_parent; } } } private Type _type; private Hdr2Inner _inner; private NameConflict m_root; private NameConflict m_parent; public Type Type { get { return _type; } } public Hdr2Inner Inner { get { return _inner; } } public NameConflict M_Root { get { return m_root; } } public NameConflict M_Parent { get { return m_parent; } } } private Hdr1 _hdr1; private Hdr2 _hdr2; private NameConflict m_root; private KaitaiStruct m_parent; public Hdr1 Hdr1 { get { return _hdr1; } } public Hdr2 Hdr2 { get { return _hdr2; } } public NameConflict M_Root { get { return m_root; } } public KaitaiStruct M_Parent { get { return m_parent; } } } } ```


The Hdr1, Hdr2, Hdr1.Type and Hdr2.Type names conflict as there is a nested class / enum and a property with the same name (both are members of the same class).

As far as I know it's a generic advice to use namespaces instead of nested classes (using works on them), this gives us a structure something like this:

NameConflict.cs (click to open) ```csharp namespace Kaitai { public partial class NameConflict : KaitaiStruct { public static NameConflict FromFile(string fileName) { return new NameConflict(new KaitaiStream(fileName)); } public NameConflict(KaitaiStream mIo, KaitaiStruct parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root ?? this; _parse(); } private void _parse() { Hdr1 = new NameConflictTypes.Hdr1(m_io, this, M_Root); Hdr2 = new NameConflictTypes.Hdr2(m_io, this, M_Root); } public NameConflictTypes.Hdr1 Hdr1 { get; protected set; } public NameConflictTypes.Hdr2 Hdr2 { get; protected set; } public NameConflict M_Root { get; protected set; } public KaitaiStruct M_Parent { get; protected set; } } } namespace Kaitai.NameConflictTypes { public partial class Hdr1 : KaitaiStruct { public Hdr1Types.Type Type { get; protected set; } public NameConflict M_Root { get; protected set; } public NameConflict M_Parent { get; protected set; } public Hdr1(KaitaiStream mIo, NameConflict parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Hdr1Types.Type) m_io.ReadU4le(); } public static Hdr1 FromFile(string fileName) { return new Hdr1(new KaitaiStream(fileName)); } } public partial class Hdr2 : KaitaiStruct { public Hdr2Types.Type Type { get; protected set; } public Hdr2Types.Hdr2Inner Inner { get; protected set; } public NameConflict M_Root { get; protected set; } public NameConflict M_Parent { get; protected set; } public Hdr2(KaitaiStream mIo, NameConflict parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Hdr2Types.Type) m_io.ReadU4le(); Inner = new Hdr2Types.Hdr2Inner(m_io, this, M_Root); } public static Hdr2 FromFile(string fileName) { return new Hdr2(new KaitaiStream(fileName)); } } } namespace Kaitai.NameConflictTypes.Hdr1Types { public enum Type { Y = 1, } } namespace Kaitai.NameConflictTypes.Hdr2Types { public enum Type { X = 1, } public partial class Hdr2Inner : KaitaiStruct { public static Hdr2Inner FromFile(string fileName) { return new Hdr2Inner(new KaitaiStream(fileName)); } public Hdr2Inner(KaitaiStream mIo, Hdr2 parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Type) m_io.ReadU4le(); } public Type Type { get; protected set; } public NameConflict M_Root { get; protected set; } public Hdr2 M_Parent { get; protected set; } } } ```


With a bit more complex logic we can generate a bit more nature code.

I created (manually) the code below using this logic:

The pros of this solution:

NameConflict.cs (click to open) ```csharp namespace Kaitai.NameConflict { public partial class NameConflict : KaitaiStruct { public static NameConflict FromFile(string fileName) { return new NameConflict(new KaitaiStream(fileName)); } public NameConflict(KaitaiStream mIo, KaitaiStruct parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root ?? this; _parse(); } private void _parse() { Hdr1 = new Hdr1.Hdr1(m_io, this, M_Root); Hdr2 = new Hdr2.Hdr2(m_io, this, M_Root); } public Hdr1.Hdr1 Hdr1 { get; protected set; } public Hdr2.Hdr2 Hdr2 { get; protected set; } public NameConflict M_Root { get; protected set; } public KaitaiStruct M_Parent { get; protected set; } } } namespace Kaitai.NameConflict.Hdr1 { public enum Type { Y = 1, } public partial class Hdr1 : KaitaiStruct { public Type Type { get; protected set; } public NameConflict M_Root { get; protected set; } public NameConflict M_Parent { get; protected set; } public Hdr1(KaitaiStream mIo, NameConflict parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Type)m_io.ReadU4le(); } public static Hdr1 FromFile(string fileName) { return new Hdr1(new KaitaiStream(fileName)); } } } namespace Kaitai.NameConflict.Hdr2 { public enum Type { X = 1, } public partial class Hdr2 : KaitaiStruct { public Type Type { get; protected set; } public Hdr2Inner Inner { get; protected set; } public NameConflict M_Root { get; protected set; } public NameConflict M_Parent { get; protected set; } public Hdr2(KaitaiStream mIo, NameConflict parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Type) m_io.ReadU4le(); Inner = new Hdr2Inner(m_io, this, M_Root); } public static Hdr2 FromFile(string fileName) { return new Hdr2(new KaitaiStream(fileName)); } } public partial class Hdr2Inner : KaitaiStruct { public Type Type { get; protected set; } public NameConflict M_Root { get; protected set; } public Hdr2 M_Parent { get; protected set; } public Hdr2Inner(KaitaiStream mIo, Hdr2 parent = null, NameConflict root = null) : base(mIo) { M_Parent = parent; M_Root = root; _parse(); } private void _parse() { Type = (Type) m_io.ReadU4le(); } public static Hdr2Inner FromFile(string fileName) { return new Hdr2Inner(new KaitaiStream(fileName)); } } } ```


I also took the opportunity to modify the order of properties, methods, constructors and static parse methods. Also removed the unnecessary backing fields (still using C# 2.0).

What do you think @LogicAndTrick & @GreyCat how does it look?

LogicAndTrick commented 7 years ago

Looks like a good idea. I can't see any obvious downsides, aside from the compiler being a bit more complex. There's still going to be possible conflicts with class names and property names but they are probably less common:

types:
  test:
    seq:
      - id: test
        type: s4
public class Test
{
    // CS0542 'Test': member names cannot be the same as their enclosing type
    public int Test { get; set; } 
}
generalmimon commented 1 year ago

A note for the future when this is being worked on.

@koczkatamas (https://github.com/kaitai-io/kaitai_struct/issues/86#issuecomment-292667485):

With a bit more complex logic we can generate a bit more nature code.

I created (manually) the code below using this logic:

  • if the type has enums or types definition then we use a namespace around it (with the same name, eg. NameConflict.NameConflict or Hdr1.Hdr1).
  • otherwise we can put into the parent's namespace (optional)

(...)

NameConflict.cs (click to open)

We should not use this solution, because naming a class the same as its namespace is actually explicitly discouraged by the C# community - see https://stackoverflow.com/a/2850148/12940655 and a 4-part article series "Do not name a class the same as its namespace" by Eric Lippert: