KirillOsenkov / RoslynQuoter

Roslyn tool that for a given C# program shows syntax tree API calls to construct its syntax tree
http://roslynquoter.azurewebsites.net
Apache License 2.0
918 stars 118 forks source link

Numeric literals are not handled properly #35

Open svick opened 6 years ago

svick commented 6 years ago

RoslynQuoter doesn't generate the correct code for numeric literals. For example, consider this expression:

new object[] { 42, 42.0, 42f, 42m, 42.0m, 42.00m }

Those six values all have different syntax and are also different values. But the code generated by RQ is:

ArrayCreationExpression(
        ArrayType(PredefinedType(Token(SyntaxKind.ObjectKeyword))).WithRankSpecifiers(
            SingletonList<ArrayRankSpecifierSyntax>(
                ArrayRankSpecifier(
                    SingletonSeparatedList<ExpressionSyntax>(OmittedArraySizeExpression())))))
    .WithInitializer(
        InitializerExpression(
            SyntaxKind.ArrayInitializerExpression,
            SeparatedList<ExpressionSyntax>(
                new SyntaxNodeOrToken[]
                {
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42.0)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42.00))
                }))).NormalizeWhitespace()

Evaluating this results in:

new object[]{42, 42, 42, 42, 42, 42}

I think that RQ should generate code that at least preserves semantics. (Ideally, it should preserve the exact syntax used, but I think that's less important.)

svick commented 6 years ago

Somewhat related issue: https://github.com/dotnet/roslyn/issues/23403.

KirillOsenkov commented 6 years ago

To solve this, we need to look at the actual syntax trees for these cases and see how the trees are different.

Then make sure that we emit those differences as API calls. There may be another hole where it's impossible to construct the syntax trees using the public API and only the parser is able to construct such trees internally, because it bypasses the public API and uses the internal APIs directly.

We've had such cases in the past a few times.

svick commented 6 years ago

I suspect that using the SyntaxFactory.Literal(string text, SomeType value) overloads is going to work.

KirillOsenkov commented 6 years ago

Aha, so we probably just need to hint it to choose the right overload...

KirillOsenkov commented 3 years ago

Could you please check if this is still a problem?

svick commented 3 years ago

@KirillOsenkov It's much better, but still not fully fixed. The generated code now is:

ArrayCreationExpression(
    ArrayType(
        PredefinedType(
            Token(SyntaxKind.ObjectKeyword)))
    .WithRankSpecifiers(
        SingletonList<ArrayRankSpecifierSyntax>(
            ArrayRankSpecifier(
                SingletonSeparatedList<ExpressionSyntax>(
                    OmittedArraySizeExpression())))))
.WithInitializer(
    InitializerExpression(
        SyntaxKind.ArrayInitializerExpression,
        SeparatedList<ExpressionSyntax>(
            new SyntaxNodeOrToken[]{
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.0)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42f)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42m)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.0m)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.00m))})))
.NormalizeWhitespace()

This produces:

new object[]{42, 42, 42F, 42M, 42.0M, 42.00M}

So the only remaining problematic case is double, which should be fixed if https://github.com/dotnet/roslyn/issues/23403 is fixed. I guess this means you can now either close this issue as a duplicate of the Roslyn issue, or keep it open if you think workaround in RQ would be worth it.