dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.07k stars 4.04k forks source link

Need 'fix-all' for naming style violations. #14983

Open CyrusNajmabadi opened 8 years ago

CyrusNajmabadi commented 8 years ago

It's just too tedious to have to manually fix up every violation one at a time.

Pilchie commented 8 years ago

Clearing milestone - this is not an RC issue.

Pilchie commented 8 years ago

Tagging @dpoeschl and @kuhlenh have either of you given any thought to this? Should it just be a blanket "fix all my names everywhere", or should it depend on which rule matched, etc?

kuhlenh commented 8 years ago

@Pilchie -- I believe we were thinking of having a "one-click cleanup" button for this (which would fix all formatting and name/style violations).

CyrusNajmabadi commented 8 years ago

I would like both options. One to fixup everything. And one to fix that specific violation.

jnm2 commented 7 years ago

Same! Almost every event handler I've ever written has been camel case. I'd like a way to update an entire solution. :D

jnm2 commented 7 years ago

I'm still blowing so much time on fixing these and then running into more.

kendrahavens commented 5 years ago

Duplicate from developer community

sharwell commented 5 years ago

I've had some thoughts about at least a partial fix all implementation, but haven't completed it yet. For example:

  1. If two code elements that need to be renamed are not visible in each other's scopes, it should be possible to rename both of them in a single operation without conflicts.
  2. If two variables in a shared scope have distinct names before and after the fix, and neither of the fixes applied individually has any conflicts, then it should be possible to rename both of them in a single operation without conflicts.
  3. Rules 1 and 2 should hold for any number of identifiers
artemious7 commented 4 years ago

I'd be happy to see this implemented too.

matthew-j-clark commented 3 years ago

I also would like to see this implemented.

tblong commented 2 years ago

Cannot wait to see this implemented either. Staring at a plethora of name rule violations (cleaning up legacy code) after adding a much needed .editorconfig to the solution.

LeftofZen commented 2 years ago

+1, this is a pretty important feature to have for me

mwagnerEE commented 2 years ago

This seems like a relatively simple implementation. The error list already captures IDE1006 "Naming rule violation" and we can already hover over a single violation and "Fix Naming Violation"

It seems like the aspect up for discussion is whether it should be a one-click fix all or a filtered approach.

The answer is both.

In the analyzer tab, put a "Naming Violations..." item that brings up a new window that lists all the violations from the Error List with properties like "Rule Violated", "Member Type", "Project", "File" etc. for filtering and a "Fix Selected" button.

You can even add a preview to the side like we currently see when hovering over them one by one.

All the pieces are there... Please

CyrusNajmabadi commented 2 years ago

The impl is unfortunately not simple. We are working on this now. But it requires a lot of investment in making the implementation have an acceptable level of performance.

AndrewEastwood commented 2 years ago

that enhancement would really speed up fixing such places

jnm2 commented 2 years ago

As long as it takes less time and attention than me doing it by hand, it's valuable to me no matter how long it otherwise takes to execute.

wesleyscaldwell commented 2 years ago

at the very least an option to do this in code clean up for the page would alleviate some of the pain.
We have implemented the editorconfig and now every the clean up process is extremely time consuming.

If there is work around in the meantime that anyone could share, please let me (and i'm assuming others) know about it.

Thanks

Cosifne commented 2 years ago

Hi, For those who still looking for an update on this issue. It is currently being implemented in https://github.com/dotnet/roslyn/tree/features/FixAllNamingViolation

Prologh commented 2 years ago

For some of you struggling with renaming properties after generating them from JSON, which for me was the most common case of naming style violations, Visual Studio Code has support for modifying results from regular expression find, so you can use the find and replace tool:

public ([\w\[\]]+) (\w+) \{ get; set; \}

public $1 \u$2 { get; set; }

$1 and $2 are groups and \u converts first letter of $2 group to uppercase. Just make sure you tick the "use regular expression box".

image

This surely does not fix all other naming style violations, but I'm leaving this here in case somebody else is trying to find an alternative to renaming hundreds of camel case properties manually.

kerrpeter commented 1 year ago

`\u converts first letter of $2 group to uppercase.

This works in VS Code, but not Visual Studio.

fdellasoppa commented 1 year ago

It happens every time I paste special JSON as Class. I need to then move all properties to upper. I found that Alt+Enter and then Enter again is somewhat "fast" if you don't have many classes. Mmm, I've just realized there might be a better "paste JSON as class" option online.

mavaddat commented 1 year ago

Here is a simple PowerShell Core script to change the method names to the correct casing:

$tokens = @()  # Will retain all method names to change

# Build the set of method names as token to replace (in case methods are referred to across classes)
foreach ($txt in Get-ChildItem -Path . -Include @('*.cs') -Recurse) {
    [array]$tokens += Select-String -Pattern '(?<=(public|private|protected|internal)+ [\w\[\]<>\s]+ )[a-z]\w+(?=\s*\()' -CaseSensitive -Path $txt | ForEach-Object { $_.Matches.Value } | Select-Object -Unique
}

# Replace all tokens
foreach ($txt in Get-ChildItem -Path . -Include @('*.cs') -Recurse) {
    $tmp = Get-Content -Path $txt | ForEach-Object { if (-not [string]::IsNullOrEmpty($tokens)) { $_ -creplace "($($tokens -join '|'))(?=\()", { $_.Value[0].ToString().ToUpperInvariant() + $_.Value.Substring(1) } } }  # Assign to temp to avoid file locking
    Set-Content -Value $tmp -Path $txt
}

If you want to preview the changes before committing them to disk:

$tokens = @()  # Will retain all method names to change

# Build the set of method names as token to replace (in case methods are referred to across classes)
foreach ($txt in Get-ChildItem -Path . -Include @('*.cs') -Recurse) {
    [array]$tokens += Select-String -Pattern '(?<=(public|private|protected|internal)+ [\w\[\]<>\s]+ )[a-z]\w+(?=\s*\()' -CaseSensitive -Path $txt | ForEach-Object { $_.Matches.Value } | Select-Object -Unique
}

# Replace all tokens with preview
foreach ($txt in Get-ChildItem -Path . -Include @('*.cs') -Recurse) {
    $before = Select-String -Pattern "($($tokens -join '|'))(?=\()" -Path $txt -CaseSensitive -AllMatches
    $tmp = Get-Content -Path $txt | ForEach-Object { if (-not [string]::IsNullOrEmpty($tokens)) { $_ -creplace "($($tokens -join '|'))(?=\()", { $_.Value[0].ToString().ToUpperInvariant() + $_.Value.Substring(1) } } }  # Assign to temp to avoid file locking
    $after = $tmp | Where-Object -FilterScript { $_ -imatch "($($tokens -join '|'))(?=\()" } | Select-String -Pattern "($($tokens -join '|'))(?=\()" -AllMatches
    $(for($i=0;$i -lt $before.Count; $i++) {
        [PSCustomObject]@{
            Before = $before[$i]
            After = $after[$i]
        }
    }) | Format-Table -AutoSize -Wrap
    Set-Content -Value $tmp -Path $txt -Confirm
}

Result:

Diff svejdo1/TreeDistance/compare/master...mavaddat:TreeDistance:master

wesleyscaldwell commented 11 months ago

Hi, For those who still looking for an update on this issue. It is currently being implemented in https://github.com/dotnet/roslyn/tree/features/FixAllNamingViolation

There doesn't seem to be much movement on this? Running a powershell script to update large amounts of code just doesn't seem like a good idea.

Other than going one field at a time. What options exist?

CyrusNajmabadi commented 11 months ago

Other than going one field at a time. What options exist?

The Roslyn API is public. So you could write a console tool that loads your solution, then performs all the renames you want, one at a time.

mwagnerEE commented 11 months ago

The Roslyn API is public. So you could write a console tool that loads your solution, then performs all the renames you want, one at a time.

I believe I tried this before and ran into the issue of the NamingStyle settings being internal so you'd need to create a separate system. How can I read the CodeStyle and NamingStyle settings into something like a json file?

mwagnerEE commented 10 months ago

I figured it out with reflection. if anyone wants to give it a try I made it open source. Merry Christmas!

Varorbc commented 8 months ago

Any update on this?

CyrusNajmabadi commented 8 months ago

@Varorbc Nope.

ChairmanMawh commented 8 months ago

It happens every time I paste special JSON as Class.

If you use a more sophisticated generator like http://app.quicktype.io then you get C# properties to C# naming standards, annotated with attributes (for Newtonsoft or STJ) to JSON standards. They (QuickType) also have a VS extension

sharwell commented 8 months ago

I figured it out with reflection. if anyone wants to give it a try I made it open source. Merry Christmas!

You should be able to avoid reflection by passing the settings in using .editorconfig format. The pattern would be similar to this: https://github.com/dotnet/roslyn-sdk/blob/ab34bc50931ac89fdcf16a99462ea3e846944e4c/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest%601.cs#L1448-L1453

quantfreedom commented 2 months ago

how in the world is this not an option with this many people asking for it ... what is going on here ... surely the people who even run this repo are dealing with this same problem ... how are they not making a change for this ... makes no sense

CyrusNajmabadi commented 2 months ago

how in the world is this not an option with this many people asking for it ... what is going on here ... surely the people who even run this repo are dealing with this same problem ... how are they not making a change for this ... makes no sense

@quantfreedom we're not running into the issue because our code matches the naming styles we've chosen. We also don't have a solution here as we don't know how to accomplish this efficiently.

jerviscui commented 3 weeks ago

Is there a solution?

mavaddat commented 1 week ago

Is there a solution?

Hi @jerviscui, you can check the discussion here for the current status of this feature from Visual Studio.