andrewlock / StronglyTypedId

A Rosyln-powered generator for strongly-typed IDs
MIT License
1.52k stars 80 forks source link

Fixes #74 #77

Closed jo-goro closed 2 years ago

jo-goro commented 2 years ago

Fixes #74.

Two strongly typed ids yielded the same generated filename, e.g.:

namespace A { [StronglyTypedId] partial struct Id {} }
namespace B { [StronglyTypedId] partial struct Id {} }

Both yielded the same file Id.g.cs.

This commit uses the full name as the filename, yielding A.Id.g.cs and B.Id.g.cs for the previous example. Parentclasses are also included. Namespace, parent-names and id-name are seperated a dot. Should a subclass contain generic type parameters like

class A<TKey, TValue> { [StronglyTypedId] partial struct Id {} }

then the generated file name will be A__TKeyTValue.Id.g.cs. Two __ are used to sepereate the generic type parameters from the class name, since SourceProductionContext.AddSource(...) seems the throw an exception for any character other than [0-9a-zA-Z_\.]. Therefore, both

class A<TKey, TValue> { [StronglyTypedId] partial struct Id {} }
class A__TKeyTValue { [StronglyTypedId] partial struct Id {} }

would yield the same name. I don't think this limitation will cause many porblems, since most people won't have a classname like A__TKeyTValue. If anyone has a better idea I would be happy to implement it.

andrewlock commented 2 years ago

Thanks for this @jo-goro, I actually have a similar link sitting locally (though I prefer your implementation as it's far more thorough than mine is).

The trouble is, I'd like to add a unit test for this. I added a snapshot test like the following:

        [Fact]
        public Task CanGenerateMultipleIdsWithSameName()
        {
            // https://github.com/andrewlock/StronglyTypedId/issues/74
            const string input = @"using StronglyTypedIds;

namespace MyContracts.V1
{
    [StronglyTypedId]
    public partial struct MyId {}
}

namespace MyContracts.V2
{
    [StronglyTypedId]
    public partial struct MyId {}
}";
            // This only includes the last ID but that's good enough for this
            var (diagnostics, output) = TestHelpers.GetGeneratedOutput<StronglyTypedIdGenerator>(input);

            Assert.Empty(diagnostics);

            return Verifier.Verify(output)
                .UseDirectory("Snapshots");
        }

But the trouble is the test setup currently assumes that we only generate a single file, and I haven't found time to improve that yet. If you could add a regression test for the old/behaviour like the one above and fix the testing limitations that would be great, thanks!

jo-goro commented 2 years ago

Thanks for preferring my implementation, @andrewlock. I'll add the regression test and fix the current testing limitations.

jo-goro commented 2 years ago

@andrewlock I have added the regression test and changed the snapshot-tester to accomodate multiple generated Ids.

andrewlock commented 2 years ago

LGTM, thanks @jo-goro!