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

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

Open
Tracked by #40163
CyrusNajmabadi opened this issue Nov 4, 2016 · 32 comments
Open
Tracked by #40163

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

CyrusNajmabadi opened this issue Nov 4, 2016 · 32 comments
Assignees
Labels
Area-IDE Dev17 IDE Priority Feature Request User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@CyrusNajmabadi
Copy link
Member

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

@CyrusNajmabadi CyrusNajmabadi added this to the 2.0 (RC) milestone Nov 4, 2016
@Pilchie
Copy link
Member

Pilchie commented Nov 4, 2016

Clearing milestone - this is not an RC issue.

@Pilchie Pilchie removed this from the 2.0 (RC) milestone Nov 4, 2016
@Pilchie
Copy link
Member

Pilchie commented Nov 14, 2016

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
Copy link

kuhlenh commented Nov 14, 2016

@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
Copy link
Member Author

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

@jnm2
Copy link
Contributor

jnm2 commented Aug 28, 2017

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

@dpoeschl dpoeschl self-assigned this Aug 28, 2017
@jnm2
Copy link
Contributor

jnm2 commented Oct 3, 2017

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

@kendrahavens
Copy link
Contributor

@sharwell
Copy link
Member

sharwell commented Sep 7, 2019

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
Copy link

I'd be happy to see this implemented too.

@matthew-j-clark
Copy link

I also would like to see this implemented.

@mikadumont mikadumont added the User Story A single user-facing feature. Can be grouped under an epic. label Apr 5, 2022
@LeftofZen
Copy link

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

@mwagnerEE
Copy link

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
Copy link
Member Author

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
Copy link

that enhancement would really speed up fixing such places

@jnm2
Copy link
Contributor

jnm2 commented Jun 14, 2022

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.

@mikadumont mikadumont modified the milestones: 17.3, 17.4 Jul 21, 2022
@wesleyscaldwell
Copy link

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
Copy link
Member

Cosifne commented Aug 23, 2022

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
Copy link

Prologh commented Oct 6, 2022

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
Copy link

``\uconverts first letter of$2` group to uppercase.

This works in VS Code, but not Visual Studio.

@arkalyanms arkalyanms modified the milestones: 17.4, 17.6 P3 Jan 17, 2023
@fdellasoppa
Copy link

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
Copy link

mavaddat commented Jul 22, 2023

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
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link

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

@Varorbc
Copy link

Varorbc commented Mar 6, 2024

Any update on this?

@CyrusNajmabadi
Copy link
Member Author

@Varorbc Nope.

@ChairmanMawh
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Dev17 IDE Priority Feature Request User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests