Move formatting analyzer to `Analyzers` layer by Youssef1313 · Pull Request #60549 · dotnet/roslyn

Youssef1313 · 2022-04-03T22:27:27Z

There were two versions of formatting analyzer. One in Features and the other is in CodeStyle. This PR makes a single analyzer in Analyzers layer, which is shared between both CodeStyle and Features.

Youssef1313 · 2022-04-03T22:48:49Z

src/Analyzers/Core/Analyzers/Formatting/FormattingAnalyzerHelper.cs

-#if CODE_STYLE
 using Formatter = Microsoft.CodeAnalysis.Formatting.FormatterHelper;
 using FormattingProvider = Microsoft.CodeAnalysis.Formatting.ISyntaxFormatting;
-#else
-using FormattingProvider = Microsoft.CodeAnalysis.Host.HostWorkspaceServices;
-#endif


Previously, Features was using Mef and CodeStyle was using ISyntaxFormatting. Now both are using ISyntaxFormatting. @mavasani Can you confirm this is the right action?

I think it should be fine, @sharwell to confirm

Youssef1313 · 2022-04-04T07:59:42Z

src/Analyzers/Core/CodeFixes/Formatting/FormattingCodeFixProvider.cs

            foreach (var diagnostic in context.Diagnostics)
            {
+#pragma warning disable RS0005 // Do not use generic 'CodeAction.Create' to create 'CodeAction'
                context.RegisterCodeFix(


@mavasani The one that used to live in Features was inheriting SyntaxEditorBasedCodeFixProvider. While the one that lived in CodeStyle was inheriting CodeFixProvider directly. Does it matter what do I inherit when merging both in Analyzers layer?

Moving to SyntaxEditorBasedCodeFixProvider is preferable if possible, and you don't see any breaking changes.

@mavasani Done in last commit (5e4b835)

Youssef1313 · 2022-04-04T19:25:13Z

@mavasani Is this ready to merge?

mavasani · 2022-04-06T13:22:20Z

@Youssef1313 There are merge conflicts

Youssef1313 · 2022-04-06T14:04:43Z

@mavasani I resolved the conflicts.

src/Analyzers/Core/CodeFixes/Formatting/FormattingCodeFixProvider.cs

...eStyle/CSharp/Tests/AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_OptionHelpers.cs

src/CodeStyle/Core/Analyzers/Microsoft.CodeAnalysis.CodeStyle.csproj

...nosticsTestUtilities/Microsoft.CodeAnalysis.EditorFeatures.DiagnosticsTests.Utilities.csproj

CyrusNajmabadi · 2022-04-06T16:14:40Z

This is AWESOME. Thanks so much @Youssef1313 !

mavasani

Thank you @Youssef1313

Youssef1313 · 2022-04-06T18:03:02Z

Can someone restart the failing tests please?

CyrusNajmabadi · 2022-04-06T18:05:30Z

Done!

Youssef1313 · 2022-04-06T19:38:11Z

@CyrusNajmabadi I fixed merge conflicts so auto merge got disabled. Can you enable it again? Thanks!

CyrusNajmabadi · 2022-04-06T20:05:45Z

Done!

mavasani · 2022-04-09T11:56:56Z

src/CodeStyle/CSharp/Tests/Microsoft.CodeAnalysis.CSharp.CodeStyle.UnitTests.csproj

-    <ProjectReference Include="..\..\..\Compilers\Test\Core\Microsoft.CodeAnalysis.Test.Utilities.csproj" />
-    <ProjectReference Include="..\..\..\Workspaces\CoreTestUtilities\Microsoft.CodeAnalysis.Workspaces.Test.Utilities.csproj" />
-  </ItemGroup>
-  <Import Project="..\..\..\Analyzers\CSharp\Tests\CSharpAnalyzers.UnitTests.projitems" Label="Shared" />


@Youssef1313 My bad that I did not notice that your change actually deletes a bunch of test projects. C# and VB CodeStyle unit test projects test the analyzers that ship as part of respective CodeStyle NuGet package. I am going to file an issue to restore these projects back.

Oops. Yeah sorry for that.

Youssef1313 added 3 commits April 4, 2022 00:10

Move formatting analyzer to shared Analyzers layer

79b4313

Move tests to shared Analyzers layer and more other fixes

3a931a8

Few more fixes

07e44fd

Youssef1313 requested a review from a team as a code owner April 3, 2022 22:27

ghost added Community The pull request was submitted by a contributor who is not a Microsoft employee. Area-IDE labels Apr 3, 2022

Youssef1313 added 4 commits April 4, 2022 00:31

Fix bad changes

404b9d4

Fix bad changes

7d0775e

Fix bad changes

5c381c8

Remove IVTs of deleted test projects

e439bff

Youssef1313 requested a review from a team as a code owner April 3, 2022 22:40

Youssef1313 mentioned this pull request Apr 3, 2022

Delete unused files in CodeStyle layer #60548

Closed

Youssef1313 commented Apr 3, 2022

View reviewed changes

mavasani assigned sharwell Apr 4, 2022

Youssef1313 added 2 commits April 4, 2022 09:53

Merge branch 'main' into formatting

700f3e3

Fix failing test

3c0d140

Youssef1313 commented Apr 4, 2022

View reviewed changes

Move to SyntaxEditorBasedCodeFixProvider

5e4b835

Merge branch 'main' into formatting

0345549

mavasani reviewed Apr 6, 2022

View reviewed changes

src/Analyzers/Core/CodeFixes/Formatting/FormattingCodeFixProvider.cs Outdated Show resolved Hide resolved

Address review comment

b1a97ee

mavasani reviewed Apr 6, 2022

View reviewed changes

...eStyle/CSharp/Tests/AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_OptionHelpers.cs Show resolved Hide resolved

mavasani reviewed Apr 6, 2022

View reviewed changes

src/CodeStyle/Core/Analyzers/Microsoft.CodeAnalysis.CodeStyle.csproj Outdated Show resolved Hide resolved

mavasani reviewed Apr 6, 2022

View reviewed changes

...nosticsTestUtilities/Microsoft.CodeAnalysis.EditorFeatures.DiagnosticsTests.Utilities.csproj Show resolved Hide resolved

Remove unneeded IVTs

5c8461d

mavasani approved these changes Apr 6, 2022

View reviewed changes

mavasani enabled auto-merge April 6, 2022 16:45

Merge branch 'main' into formatting

83f7f93

auto-merge was automatically disabled April 6, 2022 19:37
Head branch was pushed to by a user without write access

CyrusNajmabadi enabled auto-merge April 6, 2022 20:05

CyrusNajmabadi merged commit f8a1185 into dotnet:main Apr 7, 2022

ghost added this to the Next milestone Apr 7, 2022

Youssef1313 deleted the formatting branch April 7, 2022 06:31

mavasani reviewed Apr 9, 2022

View reviewed changes

mavasani mentioned this pull request Apr 9, 2022

Restore back C# and VB CodeStyle layer unit test projects #60666

Closed

dibarbet modified the milestones: Next, 17.3.P1 Apr 25, 2022

Conversation

Youssef1313 commented Apr 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 Apr 3, 2022

Choose a reason for hiding this comment

Uh oh!

mavasani Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

mavasani Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Apr 4, 2022

Uh oh!

mavasani commented Apr 6, 2022

Uh oh!

Youssef1313 commented Apr 6, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CyrusNajmabadi commented Apr 6, 2022

Uh oh!

mavasani left a comment

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Apr 6, 2022

Uh oh!

CyrusNajmabadi commented Apr 6, 2022

Uh oh!

Youssef1313 commented Apr 6, 2022

Uh oh!

CyrusNajmabadi commented Apr 6, 2022

Uh oh!

mavasani Apr 9, 2022

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 9, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Youssef1313 commented Apr 3, 2022 •

edited

Loading