Discussion: Behavior on qualified name collisions

[slarse]

After the discussion in #3827, I was intrigued by how we deal with name collisions on manipulating the AST. I've done some cursory investigations into this, and there's some truly odd behavior.

1. Creating a type or type member (e.g. with Factory.createClass() or Factory.createMethod()) that has a qualified name collision with some other element causes a silent overwrite of the original.
2. Creating a new package with Factory.createPackage() such that the qualified name collides with some other package, actually just returns the other package, without creating a new one. It's more or less a wrapper around Package.getOrCreate().
3. Creating a new module with Factory.createModule() works the same way as packages.
4. Changing the simple name of a type, type member, package or module so as to make its qualified name collide with some other element causes two elements with the same qualified names to coexist.

This behavior is inconsistent and confusing.

The behavior of 1-3 should be consistent. An exception makes most sense to me, but returning the existing element is also an option (as is the case for 2 and 3) and less breaking as we only have to change 1.

For 4, I think we should raise an exception when applicable (not applicable if the element in question has no parent).

[monperrus]

Thanks for the analysis. I'm all for consistency and least confusion. An exception makes most sense to me, but returning the existing element is also an option (as is the case for 2.). Would prefer the latter, to return the element, it may be more backward compatible.

[slarse]

Would prefer the latter, to return the element, it may be more backward compatible.

Indeed, I think so too. To me, an exception is more "pure" in terms of design, but returning the existing value is more pragmatic given the current behavior.