ksc: prohibit sub-type and field name collision

[koczkatamas]

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 ...

• CPP-STL: typename = mz_header_t, private field = m_mz_header, getter = mz_header()
• Java: typename = MzHeader, private field = mzHeader, getter = mzHeader()
• JavaScript: typename = MzHeader, field = mzHeader (also instance vs static?)
• Perl: typename = MzHeader, field = mz_header (also instance vs static?)
• PHP: typename = MzHeader, private field = _m_mzHeader, getter = mzHeader()
• Python: typename = MzHeader, field = mzHeader
• Ruby: typename = MzHeader, field = mzHeader

Summary: only C# uses UpperCase properties.

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

[koczkatamas]

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]

I started implementing a workaround but it gets messy fast: kaitai-io/kaitai_struct_compiler@918d4cc

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

[koczkatamas]

Here is a bit complex .ksy:

nameconflict.ksy (click to open)

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)

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)

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:

• 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)

The pros of this solution:

• we don't need the ugly XYTypes namespaces
• we don't put enums into classes (we put them into the class' namespace), so there won't be name conflicts with enums either
• using a namespace includes the class and every used subtypes and enums for that type

NameConflict.cs (click to open)

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]

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]

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

@koczkatamas (#86 (comment)):

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:

• Part One: Collisions amongst referenced assemblies
• Part Two: Machine-generated code
• Part Three: Bad hierarchical design
• Part Four: Making the problem worse