codecentric / net_automatic_interface

.Net Source Generator for Automatic Interfaces
MIT License
62 stars 14 forks source link

Fails if class has any overloaded members #13

Closed allsorts46 closed 1 year ago

allsorts46 commented 2 years ago

I was getting an error in my build output when trying to use this generator in my project:

CSC : warning CS8785: Generator 'AutomaticInterfaceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'InvalidOperationException' with message 'Sequence contains more than one matching element'

However I could not reproduce it in your own demo project.

I guessed that this exception method usually comes from Single() or SingleOrDefault(), so searched your code for these and found only three, in GetDocumentationFor(), GetDocumentationForProperty() and GetDocumentationForEvent(). I don't know anything about how source generators are written but it looks like you're trying to compare members to source code tokens based purely upon their names.

I had an idea it might be caused by multiple members of the same name, and modified your example code (DemoClass) to have an overload of AMethod():

        public string AMethod(string x, string y) // included
        {
            return BMethod(x,y);
        }

        public string AMethod(string x, string y, string crash) // included
        {
            return BMethod(x,y);
        }

Indeed the generator now fails with the same error I was getting.

I can see where a fix needs to be made, and had a go, but I've got no experience with source generators and got stuck. I replaced:

.FirstOrDefault(x => x.Identifier.ValueText == method.Name);

with

.SingleOrDefault(x => IsMatch(x, method));

Implemented as:

        private static bool IsMatch(MethodDeclarationSyntax x, IMethodSymbol method)
        {
            if (x.Identifier.ValueText != method.Name)
                return false;

            var LeftParams = x.ParameterList.Parameters.ToArray();
            var RightParams = method.Parameters.ToArray();

            if (LeftParams.Length != RightParams.Length)
                return false;

            for(var i = 0; i < LeftParams.Length; i++)
            {
                var Left = LeftParams[i];
                var Right = RightParams[i];

                log.LogMessage($"IsMatch: {Left.Type.ToString()}=={Right.Type.Name}?");

                if (Left.Type.ToString() != Right.Type.Name)
                    return false;
            }

            return true;
        }

which sort of almost works, except in the log I see "IsMatch: string==String?" which is clearly false. Seems like the right side is getting the type name as the compiler has eventually interpreted it (String?) but the left side is just whatever is literally written in the source code (string).

I have no idea how to reconcile these. For now I just changed SingleOrDefault() into FirstOrDefault() so at least I can use this without errors, but obviously that's not really a fix. I could strip both sides of any non-alphanumeric characters and do a case-insensitive compare which would pass for this particular case, but that seems very hacky. And anyway, would still fail if the overloads had the same number of parameters and the types differered only by namespace not by local name - really need to compare the fully-qualified type name.

I hope this information is of use to you in creating a proper fix.

ChristianSauer commented 2 years ago

Thank you for your finding.

That crash is unfortunate. I'll have to try to find the correct member. It should be possible, but I am not an export of the roslyn API either. I am trying to find time for that, unfortunately we have child birthdays coming up. But I'll do my best.

allsorts46 commented 2 years ago

Thanks for looking at it, completely understand it might be hard to find the time.

I managed to work around it just using FirstOrDefault() instead of SingleOrDefault() for now, since I don't care too much about the documentation being carried over anyway. And overall the fact that this project even exists saves me from having to implement it myself, so thanks, It's not much of an inconvenience to just reference my local modified copy of the project directly instead of the nuget package, Visual Studio seems fine with that.

I did find a couple of other small issues too but I think I might be able to fix those myself. I'll create issues here on GitHub for them just so they're documented, but I'll try to resolve them if I can, and send a PR if I'm able.

ChristianSauer commented 1 year ago

Ok, that took way longer than I hoped for, but life took a toll. I just uploaded 1.4.x - which should fix this problem. I found a way to leverage Roslyn to do the comparison, but it was really difficult to find that. But hopefully it works now, thanks!