Skip to content

Add Code Cleanup support for Visual Basic#48092

Merged
sharwell merged 48 commits intodotnet:mainfrom
paul1956:VBCodeCleanup
Mar 19, 2021
Merged

Add Code Cleanup support for Visual Basic#48092
sharwell merged 48 commits intodotnet:mainfrom
paul1956:VBCodeCleanup

Conversation

@paul1956
Copy link
Contributor

@paul1956 paul1956 commented Sep 27, 2020

Tests would fail because of "IFixAllGetFixesService is missing from workspace" and are Skipped or the CodeFix is just not run. Format Document, System Imports first and separate Import Groups are all working. There is also something missing that prevents Visual Studio from running Visual Basic Code Cleanup or providing a separate list of what features are exposed.

Fixes #47876

… are Skipped. Format Document, System Imports first and separate Import Groups are working.
@paul1956 paul1956 requested a review from a team as a code owner September 27, 2020 04:20
…preferences and header preferences thought (not covered in C# either) most fail and are skipped for 1 or 2 reasons "IFixAllGetFixesService is missing" or the fixes for Compiler issues are not running.
@paul1956 paul1956 changed the title WIP Add Code Cleanup support for Visual Basic fixes #47876 DRAFT Add Code Cleanup support for Visual Basic fixes #47876 Sep 28, 2020
@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Sep 28, 2020
@jasonmalinowski jasonmalinowski marked this pull request as draft September 28, 2020 17:57
@jasonmalinowski jasonmalinowski removed the request for review from a team September 28, 2020 17:58
@paul1956
Copy link
Contributor Author

Ping @sharwell

@paul1956 paul1956 marked this pull request as ready for review November 20, 2020 02:29
@paul1956
Copy link
Contributor Author

paul1956 commented Dec 5, 2020

Ping

End Using
End If

Return currentDocument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic looks like it would be identical to C#. i would have expected there to be a shared base class extracted from the C# version, and the Vb and C# versions would subclass it and only od things specific to those languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi It was and worked perfectly in 2 previews (though no tests) and then someone disabled it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. so to move forward. we should move all the duplicated logic you have here to a single place (ideally an abstract base class). VB and C# will then subclass that.

' Add a progress item for each enabled option we're going to fix-up.
progressTracker.AddItems(enabledDiagnosticSets.Length)

For Each diagnosticSet1 As DiagnosticSet In enabledDiagnosticSets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For Each diagnosticSet1 As DiagnosticSet In enabledDiagnosticSets
For Each diagnosticSet1 In enabledDiagnosticSets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@paul1956
Copy link
Contributor Author

paul1956 commented Jan 9, 2021

Ping

@paul1956
Copy link
Contributor Author

@sharwell @CyrusNajmabadi Anything else I need to do to move this forward?

@CyrusNajmabadi
Copy link
Contributor

@sharwell this LGTM. Do you have any concerns here?

Base automatically changed from master to main March 3, 2021 23:52
@paul1956
Copy link
Contributor Author

paul1956 commented Mar 5, 2021

Ping

@paul1956 paul1956 changed the title DRAFT Add Code Cleanup support for Visual Basic fixes #47876 Add Code Cleanup support for Visual Basic fixes #47876 Mar 7, 2021
@paul1956
Copy link
Contributor Author

Ping @dotnet/roslyn-compiler

@AlekseyTs
Copy link
Contributor

It doesn't look like anything ander Compilers is touched.


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

@sharwell
Copy link
Contributor

@CyrusNajmabadi it looks like we reached a working state. As a follow-up, it would be good to consolidate the fixer exports to avoid duplicating items in the configuration dialog.

@sharwell sharwell changed the title Add Code Cleanup support for Visual Basic fixes #47876 Add Code Cleanup support for Visual Basic Mar 18, 2021
@sharwell sharwell merged commit 37cb7f8 into dotnet:main Mar 19, 2021
@ghost ghost added this to the Next milestone Mar 19, 2021
@sharwell
Copy link
Contributor

Thank you @paul1956 ! This should be in 16.10 Preview 2. 👍

@paul1956 paul1956 deleted the VBCodeCleanup branch March 19, 2021 01:53
@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code Cleanup does not support Visual Basic

6 participants