codecadwallader / codemaid

CodeMaid is an open source Visual Studio extension to cleanup and simplify our C#, C++, F#, VB, PHP, PowerShell, JSON, XAML, XML, ASP, HTML, CSS, LESS, SCSS, JavaScript and TypeScript coding.
http://www.codemaid.net
GNU Lesser General Public License v3.0
1.89k stars 356 forks source link

`ref struct`s get code cleaned to `internal struct`, dropping the ref #708

Open lcsondes opened 4 years ago

lcsondes commented 4 years ago

Environment

Description

using System;

ref struct Bug // This will get code cleaned to internal struct, which won't compile
{
    private Span<int> x;
}

internal class Program
{
    public static void Main()
    {
    }
}

Steps to recreate

Code cleanup the above code snippet.

Current behavior

ref struct is changed to internal struct. This is a breaking change even if it didn't contain a Span<T>.

In the case of ref readonly struct it gets cleaned to ref readonly internal struct which does not compile either.

Expected behavior

Not breaking the code.

codecadwallader commented 4 years ago

Thanks for reporting the issue. I am unable to reproduce it placing that code inside an existing C# console application. Can you please provide a minimal solution that reproduces the issue for you, as well as your CodeMaid settings in case there is a combination that may be causing the issue?

lcsondes commented 4 years ago

It's a fresh project. This is the csproj, have it next to the sample file above:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>

</Project>
codecadwallader commented 4 years ago

I do have a .csproj that matches that configuration, but I'm still unable to reproduce it. To make sure there isn't some unexpected difference between our environments, can you please attach your minimal solution?

lcsondes commented 4 years ago

Here: CsSandbox.zip

Clean up Program.cs and ref struct Test will become internal struct Test.

codecadwallader commented 4 years ago

Thanks for providing that example, I am able to reproduce it now. Interestingly enough, the difference of having the trailing comment // This will get code cleaned.. is what makes the bug occur or not occur.

The insert explicit access modifier logic uses the VS API to add the "current" access modifier if it's not found within the declaration. We previously found this would cause a similar issue with explicit interface implementations and had to make an exception case for it. I suspect we may need to do the same thing here and extend the unit tests to cover this scenario.

GeorgeAlexandria commented 2 years ago

Any updates? Issue still persists.

codecadwallader commented 2 years ago

I do not have much of an update other than I would recommend disabling the insert explicit access modifiers option at this time if you're encountering this issue. There are some new issues that have been registered that are also related to the explicit access modifier logic so perhaps this could be addressed as well if that feature gets some attention.

TheKayneGame commented 1 year ago

This is still a problem

maranmaran commented 1 year ago

Still a problem

Sinus32 commented 6 months ago

The problem still persists.