Source Generators: Please consider allowing duplicate hintNames

[canton7]

In the current design of source generators, the hintName given to GeneratorExecutionContext.AddSource / SourceProductionContext.AddSource must be unique across all outputs of the generator.

However, in most cases, the hintName is derived from user input (since source generators mainly generate extensions to user-defined code), and it's hard to come up with a source of hintName which is meaningful to the user, and is also guaranteed to be unique.

The most obvious source of hintName is the name of the type being generated, but it's easy for these to be non-unique:

namespace Test
{
    public partial class Foo { }
}

namespace Test2
{
    public partial class Foo { }
}

Or:

public partial class Test
{
    public partial class Foo { }
}

public partial class Test2
{
    public partial class Foo { }
}

It turns out that even Microsoft-authored source generators fall into this trap: dotnet/runtime#66851.

For non-incremental source generators, one possible workaround is something like:

var fileNames = new HashSet<string>();

void AddSource(string hintName, string sourceText)
{
    string fileName = hintName;
    if (!fileNames.Add(fileName))
    {
        for (int i = 2; ; i++)
        {
            fileName = hintName + i;
            if (fileNames.Add(fileName))
            {
                break;
            }
        }
    }

    context.AddSource(fileName, SourceText.From(sourceText, Encoding.UTF8));
}

Given two hintNames of "Foo", this will output "Foo" and then "Foo2". However, which file gets called "Foo" and which gets called "Foo2" is (probably) non-deterministic, which breaks the rule that hintNames are relatively stable. Other options are using GUIDs (again breaking this rule), or appending the hash of the file contents to the hint (which then starts potentially running into path length limits). It's also boilerplate which (almost) every source generator will need to add, just to work around a seemingly arbitrary limitation in the source generator API.

With incremental SGs, this gets significantly harder, as naively when we call SourceProductionContext.AddSource for one source, we don't know what other sources have been generated and what their hintNames are, so it's much harder to make sure that we're not using a hintName which has already been used elsewhere.

The best approach I can come up with is to .Collect all of the outputs into a single IncrementalValueProvider<ImmutableArray<(SourceText, string desiredHintName)>> or so, then have a single call to IncrementalGeneratorInitializationContext.RegisterSourceOutput which takes this and does all of the .AddSource calls for every file your SG needs to output in one go, making sure that none of the hintNames collide. This seems to somewhat defeat the point of the incremental SG design however, as we need to re-output every generated file on every change (even if we're able to cache the actual content being output).

I'm sure there's good reason to require that hintNames are unique and to put the burden of ensuring this on the SG author, but it results in a source of bugs which every SG author is going to discover by themselves, and then have to work around. These workarounds are particularly painful for incremental SGs.

My request is to relax this restriction. Remove all restrictions on hintNames (including allowing duplicate values and invalid characters). If the source generator machinery needs to turn this collection of hints into file names, that can be handled in a suitable way by the source generator machinery itself, using some sensible scheme.

[jasonmalinowski]

So if it wasn't for file path limits (which is really unfortunate here), those generators should just be using the fully qualified name of the types with the namespace, to avoid the collisions. @chsienki do you know if that was the concern for the runtime generators or this was just an oversight? I don't want to spend a lot of time designing around an "oops" if this was just that. I know famously our API has had no easy way to get a full qualified type name...this might finally be a forcing function.

The hint name is used so that way users can see the stable output of a file. If you're running real builds and telling the compiler to output files on disk and you are trying to debug something, it might be handy to be able to open that file in an editor and just let it reload between builds. In the IDE, you can open the output of a generator into a virtual file, and we'll refresh that as we run it; that uses the hint path to know what is the "same" file from one run to the next. We absolutely could do something to automatically differentiate the files like you were with automatically applying suffixes, as you observed there are still determinism problems if the order of calls varies from one run to the next. And even if the order is deterministic, there's still a problem that one disappearing "in the middle" adjusts the suffixes the rest all get, which could be confusing as well.

[canton7]

If the solution ends end being "you must use the full type name", can you please make this more obvious in the api / otherwise enforce this somehow? As I noted, even Microsoft fell into this trap, so I expect lots of independent developers will too!

[jasonmalinowski]

It's hard to "enforce" but we definitely should doc it as best practice somewhere.

[canton7]

Even using fully-qualified type names doesn't entirely safe you. If you give JsonSourceGenerator a type called MyContext, it will generate MyContext.g.cs, but also e.g. MyContext.GetJsonTypeInfo.g.cs and MyContext.PropertyNames.g.cs. Even if you use the fully-qualified name of MyContext there, all the user has to do is define an inner class called PropertyNames or GetJsonTypeInfo, and you've got an exception.

Mulling it over, if we do get duplicate hintNames, there are basically two options:

1. If the user has manually opened one of the affected files, they may be inconvenienced by the fact that the file's name changes at some point.
2. The source generator crashes and doesn't generate anything.

Of those, I'm not sure that 2. is the right one to be aiming for! We're comparing a small number of users being inconvenienced in a small number of cases (those users who happen to have opened one of the affected files directly from disk, which requires them to define a property in their csproj, and that affected file happens to have its name changed by the SG machinery for some reason), with all of the users being majorly affected all of the time.

Would an API such as this help?

void AddSource(SourceText sourceText, INamedTypeSymbol augmentedType, string? suffix = null);
void AddSource(SourceText sourceText, string fullyQualifiedAugmentedTypeName, string? suffix = null);

Most SG-generated files augment a type defined by the user, which the SG author probably has an INamedTypeSymbol for. By prompting the user to pass this to AddSource, the SG machinery can ensure that it uses its fully-qualified type name. A string fallback exists for those cases where there's no corresponding type being augmented in the compilation, and the parameter's name strongly indicates that it should be fully-qualified. A separate optional suffix parameter lets e.g. JsonSourceGenerator pass its PropertyNames suffix, and the SG machinery can ensure that it uses a suitable separator which isn't a valid identifier to denote the suffix, to generate e.g. Namespace.MyContext-PropertyNames.g.cs.

[SteveDunn]

Hi - any updates on this? I'm getting reports of this on my source generator: SteveDunn/Vogen#418

[jasonmalinowski]

I don't think we're tracking this as a known issue anywhere. The requirement that all outputted documents have unique hint names is currently a design requirement and changing that would cause some interesting downstream problems, especially for something like the ability to open a generated file in the editor, since suddenly we wouldn't know which one is which anymore. I'm not saying we couldn't change it, but that changing it is not trivial and so we'd need some pretty good evidence the right fix here isn't just generators getting cleverer.

That all said, if you're seeing reports of that, that would mean that either:

1. Your generator has a bug and isn't meeting this requirement. If that's the case, we can help you figure out how to get your generator to meet this requirement. Opening a new discussion here might be wise before this gets too confusing with the original conversation.
2. We've got a bug in our generator driver that means previously outputted files are hanging around in ways you don't expect. This wouldn't be unheard of, but we'd need more info to investigate. But before we go down this path, let's really convince ourselves your generator is bug free first!

[SteveDunn]

Hi - it's entirely likely that my source generator has a bug! This particular bug report though seems to suggest the bug is intermittent. I think it might be caused by the file being locked and therefore not deleted (and therefore reported as duplicated).

My generator is at https://github.com/SteveDunn/Vogen

The bit where the generator uses the hintPath is at https://github.com/SteveDunn/Vogen/blob/main/src/Vogen/WriteWorkItems.cs#L87

To alleviate the problem that was reported, I attempt 10 different filenames and give up after that.

[sharwell]

Incremental source generator implementations that derive hint names from user-provided identifiers are expected to internally process them in a deterministic manner such that the final hint names are not duplicates. An example of this process can be seen here:
dotnet/roslyn-analyzers#6615

This pull request can be reviewed commit-by-commit to see how it addresses the potential duplication problem (dotnet/roslyn-analyzers@a1e72641), and then ensures that the result does not impact the overall efficiency of the source generator (dotnet/roslyn-analyzers@9ad38533).

[JasonBock]

FWIW I'm tracking this issue as well: JasonBock/Rocks#219. The input I get is the type a user wants to mock. But just using the name of the type isn't sufficient. Even using a FQN (e.g. global::ANamespace.AType) isn't enough, as it's possible that the user has references to two assemblies that (amazingly) have the same FQN. But....the point is, if I try to use the FQN (which should work in most cases), if it's actually persisted as a file name, then it may have characters that aren't valid, and then I'm not quite sure how you sanitize for all OSes (https://stackoverflow.com/questions/309485/c-sharp-sanitize-file-name).

(Side note: With a Type, I'm guessing AssemblyQualifiedName would truly be unique, but that's Reflection, not Roslyn/ITypeSymbol).

I guess my main question is: how do I generate a hint name that will be unique for a given ITypeSymbol and won't cause havoc with the file system and is (hopefully) human-readable? I could generate a SHA hashcode for a unique name so it's repeatable, but that would look really ugly :)

[sharwell]

I guess my main question is: how do I generate a hint name that will be unique for a given ITypeSymbol and won't cause havoc with the file system and is (hopefully) human-readable?

See #60272 (comment) for one solution that would certainly work here.

[sgf]

[BitX]
public struct TestStruct1 {

    public Bit3_0 BF1;
}

[BitX]
[StructLayout(LayoutKind.Explicit, Pack = 1)]
public struct TestStruct
{
    [ByteOffset(0)]
    public Bit3_0 BF1;
}

I have the same problem, now I need to generate Bit3_0, but the above code doesn't work,
Because Bit3_0 is used in two structures.
I don't know how to get the source code generator to avoid calling AddSource() twice with the same hintName entered.
If possible, is there any API that can detect whether the current hintName has been generated to avoid generating it again?

[CyrusNajmabadi]

@sgf As stated in discord, you should only be generating Bit3_0 once. And you can figure this out by aggregating all the models you want to generate. I'd also recommend just generatin gthem to a single file.