Handle function pointer and xml normalization by Youssef1313 · Pull Request #51981 · dotnet/roslyn

Youssef1313 · 2021-03-19T08:02:57Z

Fixes #50664 (commit 1)
Fixes #49732 (commit 2)

333fred · 2021-03-19T17:50:47Z

src/Compilers/CSharp/Portable/Syntax/SyntaxNormalizer.cs

            }

+            // No space after asterisk in function pointer.
+            if (token.IsKind(SyntaxKind.AsteriskToken) && token.Parent.IsKind(SyntaxKind.FunctionPointerType))


It's a bit more complicated than this. The IDE rules I implemented are:

If there is no calling convention, no spaces.

If there is a calling convention, put a space between the asterisk and the calling convention. So, for example, delegate* unmanaged<void>. See the IDE spacing rules here: https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/Rules/SpacingFormattingRule.cs#L321-L413. #Closed

Thanks @333fred. This makes it much easier. #Closed

src/Compilers/CSharp/Portable/Syntax/SyntaxNormalizer.cs

333fred · 2021-03-19T17:54:40Z

Done review pass (commit 2)

Youssef1313 · 2021-03-20T12:21:40Z

src/Compilers/CSharp/Portable/Syntax/SyntaxNormalizer.cs

+            // No space after the < in function pointer parameter lists
+            // delegate*<void
+            if (token.IsKind(SyntaxKind.LessThanToken) && token.Parent.IsKind(SyntaxKind.FunctionPointerParameterList))
+            {
+                return false;
+            }
+
+            // No space before the > in function pointer parameter lists
+            // delegate*<void>
+            if (next.IsKind(SyntaxKind.GreaterThanToken) && next.Parent.IsKind(SyntaxKind.FunctionPointerParameterList))
+            {
+                return false;


The IDE spacing is using an IDE option, which I don't think is available to use here. I defaulted to the more common convention, which is no space before > and no space after <. #Resolved

Yeah, the normalizer isn't customized. There's one normalization.

In reply to: 598105046 [](ancestors = 598105046)

Youssef1313 · 2021-03-20T13:01:47Z

src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNormalizerTests.cs

            TestNormalizeDeclaration("public (string prefix,string uri)Foo()", "public (string prefix, string uri) Foo()");
        }

+        [Fact]


Tests are copy from IDE tests, except 2 tests that produce an invalid tree and have different behavior (which I think is acceptable):

roslyn/src/Workspaces/CSharpTest/Formatting/FormattingTests_FunctionPointers.cs

Lines 88 to 122 in 6da1274

[Fact]

public async Task FormatFunctionPointerWithUnrecognizedCallingConvention()

{

var content = @"

unsafe class C

{

delegate *invalid < int , int > functionPointer;

}";

var expected = @"

unsafe class C

{

delegate*invalid <int, int> functionPointer;

}";

await AssertFormatAsync(expected, content);

}

[Fact]

public async Task FormatFunctionPointerWithInvalidCallingConventionAndSpecifiers()

{

var content = @"

unsafe class C

{

delegate *invalid [ Cdecl , Thiscall ] < int , int > functionPointer;

}";

var expected = @"

unsafe class C

{

delegate*invalid [Cdecl, Thiscall]<int, int> functionPointer;

}";

await AssertFormatAsync(expected, content);

}

333fred

LGTM (commit 7). Thanks for the ping, this had slipped off my radar. @dotnet/roslyn-compiler for a second review.

jcouv

LGTM Thanks (iteration 7)

jcouv · 2021-04-02T14:39:31Z

Merged/squashed. Thanks @Youssef1313

Handle function pointer normalization

4da0b03

Youssef1313 requested a review from a team as a code owner March 19, 2021 08:02

ghost added the Area-Compilers label Mar 19, 2021

Handle xml normalization

021549a

Youssef1313 changed the title ~~Handle function pointer normalization~~ Handle function pointer and xml normalization Mar 19, 2021

Youssef1313 mentioned this pull request Mar 19, 2021

NormalizeWhitespace breaks XML attributes in comments #47363

Open

333fred reviewed Mar 19, 2021

View reviewed changes

src/Compilers/CSharp/Portable/Syntax/SyntaxNormalizer.cs Outdated Show resolved Hide resolved

Youssef1313 added 4 commits March 20, 2021 12:15

Address feedback

f2b4cad

Fix

1764c19

Fix

07b5e95

Fix

019b007

Youssef1313 commented Mar 20, 2021

View reviewed changes

Fix typo and add tests

88b117f

Youssef1313 commented Mar 20, 2021

View reviewed changes

Youssef1313 requested a review from 333fred April 1, 2021 13:11

333fred approved these changes Apr 1, 2021

View reviewed changes

jcouv added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Apr 2, 2021

jcouv approved these changes Apr 2, 2021

View reviewed changes

jcouv enabled auto-merge (squash) April 2, 2021 04:28

jcouv merged commit f48d930 into dotnet:main Apr 2, 2021

ghost added this to the Next milestone Apr 2, 2021

Youssef1313 deleted the normalizewhitespace branch April 2, 2021 06:01

dibarbet modified the milestones: Next, 16.10.P3 Apr 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle function pointer and xml normalization#51981

Handle function pointer and xml normalization#51981
jcouv merged 7 commits intodotnet:mainfrom
Youssef1313:normalizewhitespace

Youssef1313 commented Mar 19, 2021 •

edited

Loading

Uh oh!

333fred Mar 19, 2021 •

edited

Loading

Uh oh!

Youssef1313 Mar 20, 2021 •

edited by 333fred

Loading

Uh oh!

Uh oh!

333fred commented Mar 19, 2021

Uh oh!

Youssef1313 Mar 20, 2021 •

edited by 333fred

Loading

Uh oh!

333fred Apr 1, 2021

Uh oh!

Youssef1313 Mar 20, 2021

Uh oh!

333fred left a comment

Uh oh!

jcouv left a comment

Uh oh!

jcouv commented Apr 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	[Fact]
	public async Task FormatFunctionPointerWithUnrecognizedCallingConvention()
	{
	var content = @"
	unsafe class C
	{
	delegate *invalid < int , int > functionPointer;
	}";

	var expected = @"
	unsafe class C
	{
	delegate*invalid <int, int> functionPointer;
	}";

	await AssertFormatAsync(expected, content);
	}

	[Fact]
	public async Task FormatFunctionPointerWithInvalidCallingConventionAndSpecifiers()
	{
	var content = @"
	unsafe class C
	{
	delegate *invalid [ Cdecl , Thiscall ] < int , int > functionPointer;
	}";

	var expected = @"
	unsafe class C
	{
	delegate*invalid [Cdecl, Thiscall]<int, int> functionPointer;
	}";

	await AssertFormatAsync(expected, content);
	}

Conversation

Youssef1313 commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

333fred Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Mar 20, 2021 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

333fred commented Mar 19, 2021

Uh oh!

Youssef1313 Mar 20, 2021 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Mar 20, 2021

Choose a reason for hiding this comment

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv commented Apr 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Youssef1313 commented Mar 19, 2021 •

edited

Loading

333fred Mar 19, 2021 •

edited

Loading

Youssef1313 Mar 20, 2021 •

edited by 333fred

Loading

Youssef1313 Mar 20, 2021 •

edited by 333fred

Loading