Provide an alternative strategy to handling collisions

[motivity-chris]

Thanks for releasing this project -- it's very useful for our needs!

We did encounter one edge case that led to confusion, and we hoped that you might consider introducing a change to address it:

When two class names produce a collision using a naming strategy that allows for it (e.g. Naming.Underscores), a _2 suffix is added as per the documentation, but unfortunately when transitioning from a scenario where a class name using hyphens exists without any collision to a scenario where a new class name is introduced differing only by the use of a underscore instead of a hyphen in at least one of the characters, the provided property lacking the suffix will then refer to the newly introduced class name using the underscores, and the existing class name using the hyphens will then be referred to by the provided property having the suffix.

Example:

Baseline (.test--foo): Provided property mappings: test__foo -> .test--foo

After adding class name .test__foo: Provided property mappings: test__foo -> .test__foo, test__foo_2 -> .test--foo

The reason this causes a problem for us is that we have widespread usage of a particular BEM variant that separates blocks and elements with hyphens and elements and modifiers with dashes, leading to the occasional collision. If a developer introduces a collision-causing class name using underscores when its analog using hyphens already exists unbeknownst to them, either an alternative class name should be used to avoid the collision, or all of the references to the provided property for the existing property must be updated to use the _2 suffix.

We hope you might consider having the type provider use an alternative strategy to handling collisions (when configured). Presumably the simplest such alternative strategy would be to simply throw an exception. An alternative to that might be omitting all variations of the provided properties involved in a collision (although this would likely only be appropriate in limited scenarios where the absence of the provided properties would be expected and the reason for it understood).

[zanaptak]

Note that Naming.Verbatim might be a better fit for this scenario where hyphens are significant in the naming scheme and not just generic word separators.

I do think it would be nice to have a safer collision alternative. I'm not sure about exceptions, I've found the IDE sometimes doesn't report exceptions and just fails silently so I'm not sure I want to rely on that. The omit option could be useful, or maybe an alternative where all involved properties are suffixed and include the count, e.g.:

Before collision:
  test__foo

After introducing a collision:
  test__foo_1_2
  test__foo_2_2

After introducing another collision:
  test__foo_1_3
  test__foo_2_3
  test__foo_3_3

[motivity-chris]

Thanks for the quick response!

Note that Naming.Verbatim might be a better fit for this scenario where hyphens are significant in the naming scheme and not just generic word separators

That is certainly true. I should have added that our team is opposed to this due to the excessive "noisiness" of four backticks per property, so we're in the position of preferring to risk missing the correction of a collision (which we hope won't be frequent, but can't really predict) over the noise.

... or maybe an alternative where all involved properties are suffixed and include the count

That's an interesting idea. The primary advantage over the omission strategy that I see here is not making devs who aren't familiar with the details of how the type provider is expected to work wonder whether the type provider is somehow "broken" or exhibiting a bug when they don't find any property for the collided class name. It would certainly help to make it clearer that a collision has occurred (or another collision has occurred). I imagine you may have already considered it, but a more explicit ${M}_of_${N} format might make this more intuitive of course at the cost of verbosity.

Obviously the downside here would be that all existing references would need to be updated, but for us in particular, this would be a welcomed trade off.

[zanaptak]

I think I'll add a new parameter with something like the following:

• NameCollisions.BasicSuffix: default, current behavior
• NameCollisions.ExtendedSuffix: test__foo__1_of_2 etc. -- use two leading underscores and "of"
• NameCollisions.Omit: omit

Let me know if you have any additional ideas or concerns with this.

[motivity-chris]

That approach makes sense to me.

Thank you for taking this on, and again for the work that has already gone into this project!