Skip to content

Command line way to apply code fixes #48561

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

Closed
mikadumont opened this issue Oct 13, 2020 · 21 comments
Closed

Command line way to apply code fixes #48561

mikadumont opened this issue Oct 13, 2020 · 21 comments
Labels
Area-IDE Dev17 IDE Priority Feature Request Need Design Review The end user experience design needs to be reviewed and approved.
Milestone

Comments

@mikadumont
Copy link
Contributor

No description provided.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 13, 2020
@jinujoseph jinujoseph added Feature Request Need Design Review The end user experience design needs to be reviewed and approved. and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 15, 2020
@jinujoseph jinujoseph added this to the Backlog milestone Oct 15, 2020
@halter73
Copy link
Member

The ASP.NET team would really like this feature. Using the roslyn analyzer to track public API changes is made a lot more difficult for us and external contributors since it requires VS. What would it take to get his out of the backlog?

@davidfowl @javiercn @TanayParikh

@davidfowl
Copy link
Member

Is this something that should be part of dotnet-format?
cc @jaredpar ?

@dougbu
Copy link

dougbu commented Nov 4, 2020

@mavasani @sharwell @jaredpar @Pilchie this issue is one that comes up too frequently in dotnet/aspnetcore PRs, especially those from external contributors. The extremely limited ways available to update the PublicApi.*.txt files (basically, VS or bust) is raising new barriers to entry for our repo. Even a fix focused just on adding or removing APIs in those files would be awesome.

Side note: Part of the pain here relates to dotnet/roslyn-analyzers#3901 because the current PublicApi.*.txt syntax is obscure and we can't recommend making manual edits. (But, would admit the current syntax not capturing public attributes on public API is good in the short term. Limits how many times our contributors need to open VS 😺.)

@JamesNK
Copy link
Member

JamesNK commented Nov 18, 2020

+1 here.

I'm adding nullable annotations to a library and almost every public API is complaining. Applying Roslyn fixes from VS an API at a time is tedious.

@dougbu
Copy link

dougbu commented Nov 18, 2020

@masachs @sharwell @jaredpar if it's possible for the solution to be part of dotnet format, should this issue be moved to https://github.com/dotnet/format/issues❔ If not, what might work better for the PublicAPI analyzer's fixers in particular❔

@Pilchie
Copy link
Member

Pilchie commented Nov 18, 2020

@JamesNK I would suggest running the fixer 1 project at a type using the "Fix all in project" link on the preview of an individual fix. That's what I did when I was getting the files set up in the first place.

@mikadumont
Copy link
Contributor Author

@jmarolf and @JoeRobich doesn't dotnet-format already support this scenario and has the ability to apply third party analyzer fixers?

@JoeRobich
Copy link
Member

@mikadumont dotnet-format does support running 3rd party analyzers in the nightly builds. Not sure whether we persist changes to AdditionalFiles properly, but this would be a good test for it.

@dougbu
Copy link

dougbu commented Nov 19, 2020

Does dotnet format place restrictions on "3rd party analyzers" preventing use of analyzers and fixers that require a semantic understanding of the classes e.g. Microsoft.CodeAnalysis.PublicApi.Analyzers❔

@JamesNK
Copy link
Member

JamesNK commented Nov 19, 2020

@JamesNK I would suggest running the fixer 1 project at a type using the "Fix all in project" link on the preview of an individual fix. That's what I did when I was getting the files set up in the first place.

For some reason I thought "Fix all in project" was a context menu option. I forgot it was in the preview window 😮

Doing that does clean up most related errors quickly.

@JoeRobich
Copy link
Member

Does dotnet format place restrictions on "3rd party analyzers" preventing use of analyzers and fixers that require a semantic understanding of the classes e.g. Microsoft.CodeAnalysis.PublicApi.Analyzers❔

@dougbu When running analyzers dotnet-format will open your solution/project in a MSBuildWorkspace, so it will have semantic information available to it.

@Pilchie
Copy link
Member

Pilchie commented Feb 4, 2021

@JoeRobich Okay, I spent some time playing with this today, and it doesn't seem to work, at least for PublicAPIAnalyzers (maybe because the files they change are not sources)? I put a little repro together at https://github.com/Pilchie/DotNetFormatTest, and was testing by running dotnet format -a warn, but no files get changed.

(aspnetcore) pilchie@SAIDIN:~/code/PublicApiTest$ dotnet-format --version
5.0.210401+7cfa7b18f790226f4db76ce6360f39646f8b5589
(aspnetcore) pilchie@SAIDIN:~/code/PublicApiTest$ dotnet format -a warn
  Formatting code files in workspace '/home/pilchie/code/PublicApiTest/PublicApiTest.csproj'.
  Class1.cs(5,18): Symbol 'Class1' is not part of the declared API. (RS0016)
  Class1.cs(5,18): Symbol 'implicit constructor for 'Class1'' is not part of the declared API. (RS0016)
  Formatted code file '/home/pilchie/code/PublicApiTest/Class1.cs'.
  Format complete in 4934ms.
(aspnetcore) pilchie@SAIDIN:~/code/PublicApiTest$ git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
(aspnetcore) pilchie@SAIDIN:~/code/PublicApiTest$

@Pilchie
Copy link
Member

Pilchie commented Feb 4, 2021

Side note - why not diagnostics in the output the way that csc does so that existing regexes will find them?

@JoeRobich
Copy link
Member

it doesn't seem to work, at least for PublicAPIAnalyzers (maybe because the files they change are not sources)?

Yeah, it as I expected. We will need to take extra steps to persist changes to these additional files. Opened dotnet/format#952 to track.

Side note - why not diagnostics in the output the way that csc does so that existing regexes will find them?

We can look into formatting our output to match csc. Opened dotnet/format#953 to track.

@josefpihrt
Copy link
Contributor

Roslynator CLI has functionality to analyze project/solution and to apply code fixes automatically.

@joaopgrassi
Copy link
Member

@josefpihrt, maybe I missed something, but I tried using Roslynator to fix the aforementioned public api warnings (RS0016), and got this:

Compile 'OpenTelemetry.SemanticConventions(netstandard2.0)'
  Analyze 'OpenTelemetry.SemanticConventions(netstandard2.0)'
  Found 356 diagnostics in 'OpenTelemetry.SemanticConventions(netstandard2.0)'
  Fix 356 RS0016 'Add public types and members to the declared API'
One or more errors occurred. (Changing additional documents is not supported.)
  Inner exception: Changing additional documents is not supported.

I guess this is happening since to apply the fix, Roslynator would have to change the correct PublicApi.*.txt files?

Currently stuck in this, since Rider also has not implemented this. Ppl running on Linux have no automatic way to fix this, it seems :( https://youtrack.jetbrains.com/issue/RIDER-38610

@josefpihrt
Copy link
Contributor

@joaopgrassi This seems to be the limitation of MSBuildWorkspace 3.9.0:

Good news is that I found the commit which should allow to change additional documents so I hope that it will work as soon as I publish a new version of Roslynator CLI that references Roslyn 3.10.0.

@JoeRobich
Copy link
Member

Just wanted to respond back that dotnet-format global tool currently supports applying code fixes from the commandline including those for the PublicApi analyzer.

@mikadumont
Copy link
Contributor Author

@jmarolf should we close this now that dotnet format is apart of the .NET 6.0 CLI?

@jmarolf
Copy link
Contributor

jmarolf commented Aug 13, 2021

yeah, I think this work is done now

@jmarolf
Copy link
Contributor

jmarolf commented Aug 18, 2021

see docs here for how to run analyzers from the commandline

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 Need Design Review The end user experience design needs to be reviewed and approved.
Projects
None yet
Development

No branches or pull requests