Added support to normalize auto properties on a single line.

[raffaeler]

The discussion started here: #49468

The work consisted in:

• Added support in the SyntaxNormalizer class for the auto-properties (the ones who have null Body property)
• Added more tests for properties including init accessor and initializers
• Fixed the two existing tests (property and indexer) to reflect the changes

The following comparisons shows the results of the actual tests, before and after this PR.

Before:
class a
{
  b c
  {
    get;
  }
}

After:
class a
{
  b c { get; }
}

Before:
class a
{
  int X
  {
    get;
    set;
  }

  = 2;
}

After:
class a
{
  int X { get; set; } = 2;
}

Before:
class a
{
  int Y
  {
    get;
    set;
  }

  = 99;
}

After:
class a
{
  int Y { get; set; } = 99;
}

Before:
class a
{
  int Z
  {
    get;
  }
}

After:
class a
{
  int Z { get; }
}

Before:
class a
{
  int T
  {
    get;
    init;
  }

  int R
  {
    get => 1;
  }
}

After:
class a
{
  int T { get; init; }

  int R { get => 1; }
}

Before:
class a
{
  int Q
  {
    get
    {
      return 0;
    }

    init
    {
    }
  }

  int R
  {
    get => 1;
  }
}

After:
class a
{
  int Q
  {
    get
    {
      return 0;
    }

    init
    {
    }
  }

  int R { get => 1; }
}

Before:
class a
{
  int R
  {
    get => 1;
  }
}

After:
class a
{
  int R { get => 1; }
}

Before:
class a
{
  int S => 2;
}

After:
class a
{
  int S => 2;
}

Before:
class a
{
  b this[c d]
  {
    get;
  }
}

After:
class a
{
  b this[c d] { get; }
}

[CyrusNajmabadi]

in general, the appraoch is good. just needs more clarity :)

[raffaeler]

@CyrusNajmabadi I see errors in the Linux/OSX builds but have no idea why. Do they depend on my PR?

[CyrusNajmabadi]

Yes, you're failing some test in Microsoft.CodeAnalysis.CSharp.Syntax.UnitTests. This should also trigger in the other tests to state what test actually failed.

[raffaeler]

I see them locally. They are TestSpacingOnNullableDatetimeType and TestSpacingOnNullableIntType. I have to fix the expected result as, of course, the formatting has changed for auto properties.
Is that okay for you?

[CyrusNajmabadi]

Is that okay for you?

Yup. If it's adjusting the behavior for just these new constructs, that would be fine. Thanks! :)

[raffaeler]

The check was easy and made sense, so I proceeded. Thank you

[CyrusNajmabadi]

Thank you

You're welcome! Thank you for the PR :)

[raffaeler]

Apparently there is another set of tests suffering from this changes, but this time I run the test locally and they succeeded.

Microsoft.CodeAnalysis.EditorFeatures2.UnitTests
  Tests in group: 7838
   Total Duration: 8 min

Outcomes
   7788 Passed
   50 Skipped

Fail message:

Determinism failed for the following binaries:
	Microsoft.CodeAnalysis.EditorFeatures2.UnitTests.dll

@CyrusNajmabadi do you have suggestions?

[CyrusNajmabadi]

This is not on you. @jaredpar we have a determinism failure as per:

Determinism failed for the following binaries:
Microsoft.CodeAnalysis.EditorFeatures2.UnitTests.dll

How do you generally handle this?

[raffaeler]

I am trying to re-trigger the tests because I saw the failed test is the following one and I can't see how this PR could affect it.

[WpfFact]
public void BclMathCall()
{
    VisualStudio.InteractiveWindow.SubmitText("Math.Sin(1)");
    VisualStudio.InteractiveWindow.WaitForLastReplOutput("0.8414709848078965");
}

[333fred]

I am trying to re-trigger the tests because I saw the failed test is the following one and I can't see how this PR could affect it.

In the future, just let us know and don't use commits to retrigger. We'll take a look and retrigger if necessary.

[raffaeler]

In the future, just let us know and don't use commits to retrigger. We'll take a look and retrigger if necessary.

Sorry, I did it because it failed multiple times last week for random reasons (but not becuse of the PR) and I committed the requested changes in the middle of US festivities.
Should it happen again, I won't do it.

[333fred]

Sorry, I did it because it failed multiple times last week for random reasons (but not becuse of the PR) and I committed the requested changes in the middle of US festivities.
Should it happen again, I won't do it.

No worries, just letting you know for the future.

[CyrusNajmabadi]

Suggested change

	=> node is AccessorListSyntax accessorList &&
	accessorList.Parent is BasePropertyDeclarationSyntax basePropertyDeclarationSyntax &&
	HasInitializer(basePropertyDeclarationSyntax);
	=> node is AccessorListSyntax &&
	node.Parent is PropertyDeclarationSyntax property &&
	property.Initializer != null;

[raffaeler]

@CyrusNajmabadi Are you sure about this?
This prevents any initializers outside the properties (if they will ever exists)

[raffaeler]

@AlekseyTs what do you think about this?
I was searching to avoid looking for properties on purpose so that any derived class from BasePropertyDeclarationSyntax having an accessor fit in the inline formatting.

[AlekseyTs]

what do you think about this?

I am fine with the code and names as it is.

---

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

[CyrusNajmabadi]

This prevents any initializers outside the properties (if they will ever exists)

your existing code already prevents taht as you do a type check for PropertyDeclarationSyntax yourself in HasInitializer.

[raffaeler]

your existing code already prevents taht as you do a type check for PropertyDeclarationSyntax yourself in HasInitializer.

Sure, but is well identified in that point. Currently I can't do differently because properties arei the only one allowed.
Could in the future be possible, let's say, for indexers to be initialized, the changes would be limited to a single point and this function would work correctly.
I don't see how the current version could cause problems. Let me know if you want me to change it anyway, so that I can send the commit before night (here). Thanks

[CyrusNajmabadi]

the changes would be limited to a single point and this function would work correctly.

that is true with the change i suggested as well. in both cases we will need to update the code. so might as well have hte code be correct and precise for the trees we have today.

Let me know if you want me to change it anyway

Yes please :)

[raffaeler]

Okay, and what about the method names?

[CyrusNajmabadi]

i'll defer to Aleksey's judgement on those :)

[raffaeler]

@AlekseyTs I committed all the required changes but not the method renaming.
It's up to you now :)

[CyrusNajmabadi]

tests LGTM. code is almost there. If you would like someone to pick this up to complete this @raffaeler i would be happy to help.

If you'd prefer to finish it yourself, that's 100% ok. I just dont' want you to burn out from all the nitpicking i give :)

THanks!

[raffaeler]

tests LGTM. code is almost there. If you would like someone to pick this up to complete this @raffaeler i would be happy to help.

no problems, thanks. At this point I want to see the end :)

If you'd prefer to finish it yourself, that's 100% ok. I just dont' want you to burn out from all the nitpicking i give :)

Since I startyed renaming on my side, I am not committing your suggestions to avoid merging. That's all, but thanks :)

[AlekseyTs]

LGTM (commit 17)

[AlekseyTs]

@dotnet/roslyn-compiler For the second review. Note, when merging this PR we need to squash commits and adjust the commit title to reflect the broader impact of this change.

[AlekseyTs]

@dotnet/roslyn-compiler For the second review of a small community PR. Note, when merging this PR we need to squash commits and adjust the commit title to reflect the broader impact of this change.

[AlekseyTs]

@dotnet/roslyn-compiler For the second review of a small community PR. Note, when merging this PR we need to squash commits and adjust the commit title to reflect the broader impact of this change.

[AlekseyTs]

@raffaeler Thanks for the contribution.

[raffaeler]

Thank you @AlekseyTs and @CyrusNajmabadi :)