Compilation fails with partial classes

[Danielku15]

If you try to compile a project which contains partial classes the compiler crashes.

This is due to the ToDictionary call in the Compiler.SweepSort method. The List passed to this method contains duplicate entries for partial classes. In this case the ToDictionary fails because there will be duplicated keys.

The real problem seems to be that JavaScript transformation is done on SyntaxTree base. A partial class will be in two different SyntaxTrees. Partial classes need to be merged into a single SyntaxTree before transforming it to JavaScript.

[kswoll]

Thanks for the bug report. As you can see, I haven't tried using partial classes, but I agree it's an important feature. I'll take a look into fixing it this weekend.

[kswoll]

I've implemented this missing feature. You can see the unit tests in PartialClassTests. The solution was as you advised, and the syntax trees are merged before transformation (You can see the logic in PartialClassReassembler). Let me know if you run into any problems. Thanks again for submitting this.

[Danielku15]

Thanks for the fast implementation. But it seems there are some minor bugs left. Just merging the Syntax Trees seems to break the semantic model in some cases. Here a small example:

File01.cs

partial class Environment {
    static Environment() { 
        PlatformInit();
    }
}

File02.cs

using System;
partial class Environment {
    static void PlatformInit() { 
        Func<object> factory = () => null;
    }
}

The PlatformInit method will be merged into syntax tree of File01.cs. But the using System; is not merged, therefore the Func is not resolved to System.Func. This will cause the compiler to crash at several places because the Symbol returned for the syntax nodes parts are null.

I'm not yet sure how to solve this problem easily. Rewriting the full method to fully qualified class names might be quite time consuming. Merging the using statements might cause side effects. Assume you are using a System.IO.Stream in the target file. Additionally you have a MyNamespace.Stream and the merging causes a using MyNamespace to be added to the target file. Then you have two Stream classes imported and if the wrong one is used the compilation might fail.

I hope Roslyn provides some proper Refactoring API to move the partial declarations into a single class.

[kswoll]

This is terrific, Daniel. Thanks for stressing the edge cases, it's invaluable for making this API robust. I agree with the difficulties you highlight, I'm not at the moment sure what the best solution will be. As far as I know, there is no straightforward way via Roslyn to merge the classes by enlisting its help directly. (If you know otherwise, please let me know! :) )

I'll mull this over, with particular attention to the ways you've described in which it can be broken, and see what we can come up with. Most likely, you're initial suggestion of using fully-qualified type names will be the most straightforward approach -- it's pretty easy to make that transformation as that info is known when merging the syntax trees.

[kswoll]

Went with your suggestion of using fully-qualified names. Performance cost is negligible and it the logic is nice and simple.

[Danielku15]

Your tests did not really cover the actual problem. The PartialClassPart2.cs needs to use a type that's not imported in PartialClassPart1.cs. I created a small test case for you as the compiler still crashes.

https://github.com/Danielku15/WootzJs/commit/8e2e07a130c1a3af9b13442fe46c00c294856098

[kswoll]

Sorry about that, Daniel, good catch! I screwed up with my logic and was using the wrong type to constrain for partial types. Upon review, since the partial types were already filtered, I removed the check and it has fixed the bug.

[Danielku15]

I don't know exactly how I did it, but I found a case where the compilation still fails. I uploaded the VS Solution here: http://www.alphatab.net/downloads/WootzJsPartial.zip The compilation fails if the csproj file contains (JS file first).

    <Compile Include="Environment_JS.cs" />
    <Compile Include="Environment_Core.cs" />

But the compilation works if you flip them to:

    <Compile Include="Environment_Core.cs" />
    <Compile Include="Environment_JS.cs" />

I think it has something to do with the direction you merge.

I ran into this problem because in my real setup I use shared projects. I reference the projitems (Core) in my csproj (JavaScript) and Visual Studio injects the tags into the MsBuild context. This way I get exactly this order that causes the exception.

[kswoll]

Thanks for the repro sample! Will take a look this evening.

[kswoll]

I'm not entirely sure why the order mattered, but the ultimate problem was that I was not handling generic types (VisitGenericName) but had only been handling simple identifiers (VisitIdentifierName). I put the equivalent logic for fully qualified names in VisitGenericName as well, and that has fixed the problem.