Describe the solution you'd like
If a user adds duplicate [Rock] or [RockPartial] attributes, where the target type is duplicated like this:
[assembly: Rock(typeof(IFirst), BuildType.Create | BuildType.Make)]
[assembly: Rock(typeof(ISecond), BuildType.Create | BuildType.Make)]
[assembly: Rock(typeof(IFirst), BuildType.Create | BuildType.Make)]
public interface IFirst
{
void DoFirst();
}
public interface ISecond
{
void DoSecond();
}
Rocks will only generate code for IFirst and ISecond once. However, within the callback provided to ForAttributeWithMetadataName(), four MockModel instances will be created for IFirst, two for the "create", and two for the "make" (how many attributes exist within context.Attributes is a guess - probably depends on internal machinery within the compiler and when it calls the callback and what it provides).
A slight tweak could be made to catch duplicates before they get to CreateCombinedOutput() (where duplicates are definitely filtered out). A HashSet<(ITypeSymbol, BuildFlag)> is created, using a custom equality comparer that uses SymbolEqualityComparer. If that combination has already been seen, it is skipped. Some care must be taken here with the BuildType flag to ensure we don't consider this situation a "duplicate":
[assembly: Rock(typeof(IFirst), BuildType.Create)]
[assembly: Rock(typeof(IFirst), BuildType.Make)]
While odd (and probably very unlikely to happen...which means it will :) ), this should create two MockModel instances.
Once this work is done, I can probably remove the Distinct() calls on the arrays within CreateCombinedOutput(), though the projections will still need a hashset to prevent duplicate generation of those types.
Note that I may want to ignore [RockPartial] here. That one needs to be tied to a partial class, so a bit more work is necessary for that to exist in a code base. A user should know that they've already done this once.
While I'm making this change, might as well remove IncrementalGeneratorInitializationContextExtensions as it's no longer necessary.
Describe alternatives you've considered
Leave things as-is. Even if duplicates exist, the perf hit is probably not too bad, though doing this work should overall be beneficial.
Describe the solution you'd like
If a user adds duplicate
[Rock]or[RockPartial]attributes, where the target type is duplicated like this:Rocks will only generate code for
IFirstandISecondonce. However, within the callback provided toForAttributeWithMetadataName(), fourMockModelinstances will be created forIFirst, two for the "create", and two for the "make" (how many attributes exist withincontext.Attributesis a guess - probably depends on internal machinery within the compiler and when it calls the callback and what it provides).A slight tweak could be made to catch duplicates before they get to
CreateCombinedOutput()(where duplicates are definitely filtered out). AHashSet<(ITypeSymbol, BuildFlag)>is created, using a custom equality comparer that usesSymbolEqualityComparer. If that combination has already been seen, it is skipped. Some care must be taken here with theBuildTypeflag to ensure we don't consider this situation a "duplicate":While odd (and probably very unlikely to happen...which means it will :) ), this should create two
MockModelinstances.Once this work is done, I can probably remove the
Distinct()calls on the arrays withinCreateCombinedOutput(), though the projections will still need a hashset to prevent duplicate generation of those types.Note that I may want to ignore
[RockPartial]here. That one needs to be tied to a partial class, so a bit more work is necessary for that to exist in a code base. A user should know that they've already done this once.While I'm making this change, might as well remove
IncrementalGeneratorInitializationContextExtensionsas it's no longer necessary.Describe alternatives you've considered
Leave things as-is. Even if duplicates exist, the perf hit is probably not too bad, though doing this work should overall be beneficial.