Generated class names can clash

[shilangyu]

Currently, to pick a class name for a contract statement the following is done:

1. Take the statement name
2. If it collides with any other statement name (regardless of their namespace), then go one namespace up and prefix your current name with the namespace name
3. Repeat step 2 until there are no more conflicts

For instance consider two statements One.Two.Three and Four.Two.Three where . denotes a namespace accessor.

Then to find the class name for One.Two.Three:

1. Start with Three
2. It collides with Four.Two.Three, go one namespace up: set current name to TwoThree
3. Still collides with Four.Two.Three, go one namespace up: set current name to OneTwoThree
4. No collisions. Final class name is OneTwoThree

This algorithm was chosen because Dart does not have namespaces and this algorithm produces possibly shortest names without collisions... Or so I thought.

Now consider the following statements: One.One, OneOne, Two.One. If you follow the algorithm, the resulting class names will be respectively OneOne, OneOne, and TwoOne. That's a name clash.

This brings us to the question: how do we want to generate names for such cases? Or, what would be a different (better) way of picking class names?

[bartekpacia]

Off the top of my head: replace . with _? Then for your second example, we'd get:

• One.One -> One_One
• OneOne -> OneOne
• Two.One -> Two_One

[shilangyu]

_ is a valid identifier character, so it could be used in a statement name and lead to clashes

[bartekpacia]

Is $ a valid identifier in C#?

[shilangyu]

Yes, I am pretty sure dart and c# shares the same character set for identifiers there goes my uneducated assumption

[jakubfijalkowski]

Nope, $ is invalid char in an identifier in C#. We might also limit valid identifier character on contracts level if it will be necessary.

Also, I think the "fix" (I still don't like the general algo - it does not generate stable names) for this would be slightly simpler. It relates to point (2):

If it collides with any other statement name (regardless of their namespace),

You should not check if it collides with any other statement, but with already used identifiers. So basically keep a hashset of already used identifiers and check with it. There will also be a case where this fails:
a) NS2.Class
b) NS2.NamedClass
c) NS2.MyNamedClass
d) NS2.NSMyNamedClass
e) NS.My.Named.Class

For (e), it will try:

1. Class collides with (a),
2. NamedClass collides with (b)
3. MyNamedClass collides with (d)
4. NSMyNamedClass collides with (e).

So it also needs some other suffix mechanism (e.g. appending numbers).

[bartekpacia]

What if we duplicated the C# namespace hierarchy in generated Dart contracts? Then it'd be the contract user's responsibility to import the correct namespace. Kinda ugly though, and too complex, and too big of a change.

So for your second example, we'd get:

$ tree
.
├── One
│   └── One.dart
├── One.dart
└── Two
    └── One.dart

[jakubfijalkowski]

That, IMO, is the ideal option, but I think I am the only one who does think that.

[shilangyu]

You should not check if it collides with any other statement, but with already used identifiers

Is this stable though? A new statement comes along and affects others (as long as it is ordered before them). Also, I kinda don't like its stateful nature and dependence on order, it feels weird that some statement will receive a different naming just because it is ordered earlier/later. Plus generic nested classes such as Values will never "stabilize" to a more concrete name.

I feel like the only "stable" solution is when the generated name is fully independent of other statements. Current stable solutions:

1. filesystem simulating namespaces (readable, solves collision problem, DX usability is not very good)
2. names are always the full namespace (ok-ish readability, does not solve collision problem, DX usability ok)
3. simulating namespaces with static classes (bad readability, solves collision problem, DX usability good)

[jakubfijalkowski]

Is this stable though?

Stable in the same way that the current solution is, meaning - not stable at all.

2. names are always the full namespace (ok-ish readability, does not solve collision problem, DX usability ok)

Why not?

---

Also, I'm all for option (1) - with Cmd+. shortcut you basically get good enough DX so that the multitude of directories is not a problem (but I would still truncate some of the "static" directories).

[shilangyu]

2. names are always the full namespace (ok-ish readability, does not solve collision problem, DX usability ok)

Why not?

Same case as in before, example: One.One and OneOne

[jakubfijalkowski]

Ah, got it - I assumed that the "dot" (or any other separator) stays.

[mishioo]

I don't have much to add to the discussion, just wanted to let you guys know that we encountered this issue in one of the projects.