Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial Roslyn rewrite, supporting new language features #980

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

TimothyMakkison
Copy link

@TimothyMakkison TimothyMakkison commented Feb 25, 2023

Hey, kept running into #708 and wanted to learn some roslyn and got carried away.
I combined CodeMaidPlus and used a lot of code from Roslyn.CSharpAddAccessibilityModifiersDiagnosticAnalyzer to add explicit access modifiers for all the currently codemaid supported types in addition to records and record struct. Because the logic is a modifed default VS analyzer/rewriter, cleanup will account for new features like ref struct, required and file instead of erasing them.

I've tried to avoid using Format(SyntaxNode, SyntaxAnnotation, Workspace) as it would add line padding between declarations if a change occured, potentially conflicting with the chosen 'Insert blank line before' options. To do this I copied CSharpSyntaxGenerator.WithAccessibility and modified it to add trailing trivia to each accesor token.

  • Rewrote insert blank line padding before, insert blank line padding between, insert blank line padding after and insert explicit access modifiers.
  • Added additional tests that now pass CI, regardless of line endings.
  • Added blank line padding insertion for all types including comments, region and record.
  • Fixed the "bug" where single line brackets are moved to a new line if insert explicit access modifiers is enabled. If this is intentional the behaviour can be reimplemented.

After updating Visual Studio I experienced the same bug as #979 so I've been unable to debug this extension and instead relied on my unit test 😄

This PR is incomplete and likely contains some obvious mistakes, hopefully someone with roslyn knowlefge can improve on this.

Assuming this PR is successful and roslyn is used, would you consider adding do while to "between chained statements" or "insert blank line padding" before switch expression arms?

@TimothyMakkison TimothyMakkison marked this pull request as draft February 26, 2023 01:01
@TimothyMakkison TimothyMakkison marked this pull request as ready for review March 8, 2023 19:49
@TimothyMakkison TimothyMakkison changed the title Roslyn insert explicit modifers. POC Roslyn insert explicit modifers. Mar 18, 2023
@TimothyMakkison TimothyMakkison changed the title Roslyn insert explicit modifers. Partial Roslyn rewrite, supporting new language featuresq Apr 6, 2023
@TimothyMakkison TimothyMakkison changed the title Partial Roslyn rewrite, supporting new language featuresq Partial Roslyn rewrite, supporting new language features Apr 6, 2023
@TimothyMakkison
Copy link
Author

Hey, @codecadwallader sorry to bother you, but have you been able to look at this PR or resolve #979?

@codecadwallader
Copy link
Owner

Sorry Timothy, I have not yet the time to review it. I really appreciate you putting it together, we've been meaning to move towards Roslyn for a long time. Have you been able to play with it yourself? I'm curious if there will be any performance implications from mixing the old APIs and new APIs together.

@TimothyMakkison
Copy link
Author

Have you been able to play with it yourself?

Very early on, I was able to debug the extension, but after updating Visual Studio 2022 code maid would throw an error during InitializeAsync (I had the same popup as #979). Rolling back Visual Studio didn't fix this and running the dev branch didn't work either. I thought that Visual Studio was causing this and that it would fixed later on. I'd be curious to see if someone else can run it.

// Error is throw in one of these methods
await RegisterCommandsAsync();
await RegisterEventListenersAsync();

I'm curious if there will be any performance implications from mixing the old APIs and new APIs together.

I don't know enough about Roslyn or Microsoft.VisualStudio.Text to say. My guess is the Roslyn part will be faster as it won't be scanning and writing text for each cleanup logic. On the other hand I don't know how long it takes to create and then edit a Roslyn syntax tree. On my laptop codemaid will often pause for a second whereas using Run Code Cleanup Profile is instant.

I have not yet the time to review it.

I wouldn't bother putting too much effort looking at this. As I haven't been able to properly debug it. It's definitely full of bugs. tbh I'd hoped someone experienced with Roslyn would take pity and take over 😄

we've been meaning to move towards Roslyn for a long time

If you want to progressively transition then InsertExplicitAccessorMiddleware is the important section. Most of the bugs that are reported are related to this.

@TimothyMakkison
Copy link
Author

TimothyMakkison commented May 1, 2023

I tried this again in a fresh instance of Visual Studio 2022 with Visual Studio Extensions Development installed and got this error.

image
Error message at the bottom of the large xml document.

<entry>
  <record>399</record>
  <time>2023/05/01 11:49:51.458</time>
  <type>Error</type>
  <source>VisualStudio</source>
  <description>SetSite failed for package [CodeMaidPackage]Source: &apos;SteveCadwallader.CodeMaid&apos; Description: Method not found: &apos;Void EnvDTE._dispBuildEvents_OnBuildBeginEventHandler..ctor(System.Object, UIntPtr)&apos;.&#x000D;&#x000A;System.MissingMethodException: Method not found: &apos;Void EnvDTE._dispBuildEvents_OnBuildBeginEventHandler..ctor(System.Object, UIntPtr)&apos;.&#x000D;&#x000A;   at SteveCadwallader.CodeMaid.CodeMaidPackage.&lt;RegisterEventListenersAsync&gt;d__37.MoveNext()&#x000D;&#x000A;   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[TStateMachine](TStateMachine&amp; stateMachine)&#x000D;&#x000A;   at SteveCadwallader.CodeMaid.CodeMaidPackage.RegisterEventListenersAsync()&#x000D;&#x000A;   at SteveCadwallader.CodeMaid.CodeMaidPackage.&lt;InitializeAsync&gt;d__33.MoveNext() in C:\Users\timma\Desktop\coding\C#\codemaid\CodeMaidShared\CodeMaidPackage.cs:line 207&#x000D;&#x000A;--- End of stack trace from previous location where exception was thrown ---&#x000D;&#x000A;   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()&#x000D;&#x000A;   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)&#x000D;&#x000A;   at Microsoft.VisualStudio.Shell.AsyncPackage.&lt;&gt;c__DisplayClass21_0.&lt;&lt;Microsoft-VisualStudio-Shell-Interop-IAsyncLoadablePackageInitialize-Initialize&gt;b__1&gt;d.MoveNext()&#x000D;&#x000A;--- End of stack trace from previous location where exception was thrown ---&#x000D;&#x000A;   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()&#x000D;&#x000A;   at Microsoft.VisualStudio.Services.VsTask.RethrowException(AggregateException e)&#x000D;&#x000A;   at Microsoft.VisualStudio.Services.VsTask.InternalGetResult(Boolean ignoreUIThreadCheck)</description>
  <guid>{4C82E17D-927E-42D2-8460-B473AC7DF316}</guid>
  <hr>0x80131513</hr>
  <errorinfo></errorinfo>
</entry>

Bottom of the file created when Visual Studio 2019 fails:

<entry>
  <record>3001</record>
  <time>2023/05/01 12:04:19.638</time>
  <type>Error</type>
  <source>VisualStudio</source>
  <description>CreateInstance failed for package [CodeMaidPackage]Source: &apos;mscorlib&apos; Description: Could not load file or assembly &apos;Microsoft.VisualStudio.Shell.15.0, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a&apos; or one of its dependencies. The system cannot find the file specified.&#x000D;&#x000A;System.IO.FileNotFoundException: Could not load file or assembly &apos;Microsoft.VisualStudio.Shell.15.0, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a&apos; or one of its dependencies. The system cannot find the file specified.&#x000D;&#x000A;File name: &apos;Microsoft.VisualStudio.Shell.15.0, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a&apos;&#x000D;&#x000A;   at System.Reflection.RuntimeAssembly.GetType(RuntimeAssembly assembly, String name, Boolean throwOnError, Boolean ignoreCase, ObjectHandleOnStack type)&#x000D;&#x000A;   at System.Reflection.RuntimeAssembly.GetType(String name, Boolean throwOnError, Boolean ignoreCase)&#x000D;&#x000A;   at System.Activator.CreateInstanceFromInternal(String assemblyFile, String typeName, Boolean ignoreCase, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes, Evidence securityInfo)&#x000D;&#x000A;   at System.AppDomain.CreateInstanceFrom(String assemblyFile, String typeName)&#x000D;&#x000A;&#x000D;&#x000A;=== Pre-bind state information ===&#x000D;&#x000A;LOG: DisplayName = Microsoft.VisualStudio.Shell.15.0, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a&#x000A; (Fully-specified)&#x000D;&#x000A;LOG: Appbase = file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/&#x000D;&#x000A;LOG: Initial PrivatePath = NULL&#x000D;&#x000A;Calling assembly : SteveCadwallader.CodeMaid.VS2022, Version=12.0.0.0, Culture=neutral, PublicKeyToken=5e10ffd6b4d2c330.&#x000D;&#x000A;===&#x000D;&#x000A;LOG: This bind starts in LoadFrom load context.&#x000D;&#x000A;WRN: Native image will not be probed in LoadFrom context. Native image will only be probed in default load context, like with Assembly.Load().&#x000D;&#x000A;LOG: Using application configuration file: C:\Users\timma\AppData\Local\Microsoft\VisualStudio\16.0_98ff926eExp\devenv.exe.config&#x000D;&#x000A;LOG: Using host configuration file: &#x000D;&#x000A;LOG: Using machine configuration file from C:\Windows\Microsoft.NET\Framework\v4.0.30319\config\machine.config.&#x000D;&#x000A;LOG: Post-policy reference: Microsoft.VisualStudio.Shell.15.0, Version=17.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PublicAssemblies/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;WRN: Comparing the assembly name resulted in the mismatch: Major Version&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PublicAssemblies/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/TestWindow/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/TestWindow/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Platform/Debugger/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Platform/Debugger/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/x86/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/x86/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PublicAssemblies/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PublicAssemblies/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/TestWindow/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/TestWindow/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Platform/Debugger/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Platform/Debugger/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/x86/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/PrivateAssemblies/DataCollectors/x86/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///c:/users/timma/appdata/local/microsoft/visualstudio/16.0_98ff926eexp/extensions/steve cadwallader/codemaid vs2022/12.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///c:/users/timma/appdata/local/microsoft/visualstudio/16.0_98ff926eexp/extensions/steve cadwallader/codemaid vs2022/12.0/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.DLL.&#x000D;&#x000A;LOG: Attempting download of new URL file:///c:/users/timma/appdata/local/microsoft/visualstudio/16.0_98ff926eexp/extensions/steve cadwallader/codemaid vs2022/12.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;LOG: Attempting download of new URL file:///c:/users/timma/appdata/local/microsoft/visualstudio/16.0_98ff926eexp/extensions/steve cadwallader/codemaid vs2022/12.0/Microsoft.VisualStudio.Shell.15.0/Microsoft.VisualStudio.Shell.15.0.EXE.&#x000D;&#x000A;</description>
  <guid>{4C82E17D-927E-42D2-8460-B473AC7DF316}</guid>
  <hr>80004005 - E_FAIL</hr>
  <errorinfo></errorinfo>
</entry>

It looks like _dispBuildEvents_OnBuildBeginEventHandler is broken.

@codecadwallader
Copy link
Owner

I have been having issues with getting the VS debugger to work as well. I recently switched to a new computer and it started working again so I'm curious if there isn't some kind of state that is being maintained in the experimental instance and interfering, or if there's something that has changed across versions that requires a clean install.

@TimothyMakkison
Copy link
Author

TimothyMakkison commented May 6, 2023

I have been having issues with getting the VS debugger to work as well. I recently switched to a new computer and it started working again so I'm curious if there isn't some kind of state that is being maintained in the experimental instance and interfering, or if there's something that has changed across versions that requires a clean install.

Have you been able to run/debug the roslyn version?

I tried with an old computer with VS 2019, I installed VS extension tools and cloned the main branch. I got the same error popup and it didn't work. I'll try using a clean install. What bugs me is that I could definitely debug the extension until I did a VS update, after that even backdating didnt work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants