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

Add Roslynator #2269

Closed
bdovaz opened this issue Jan 16, 2023 · 13 comments · Fixed by #3155
Closed

Add Roslynator #2269

bdovaz opened this issue Jan 16, 2023 · 13 comments · Fixed by #3155
Labels
enhancement New feature or request nostale This issue or pull request is not stale, keep it open

Comments

@bdovaz bdovaz added the enhancement New feature or request label Jan 16, 2023
@nvuillam
Copy link
Member

Great idea to go even further in C# coverage in MegaLinter :)

@Kurt-von-Laven
Copy link
Collaborator

Neat find; I knew of Roslyn but not Roslynator.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Feb 17, 2023
@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Feb 17, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Mar 20, 2023
@bdovaz bdovaz added nostale This issue or pull request is not stale, keep it open and removed O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity labels Mar 20, 2023
@bdovaz
Copy link
Collaborator Author

bdovaz commented Mar 24, 2023

I have not advanced in this linter because there is a blocking problem and today I have taken some time to create the issue: dotnet/roslynator#1060

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 9, 2023

cc: @nvuillam @echoix @Kurt-von-Laven

So far I have not implemented it because I have encountered several problems in this issue over the last months: dotnet/roslynator#1060

I have reached the point where from a Dockerfile (outside of megalinter) it works for me but:

  1. This cli only works with *.sln or *.csproj files which are .NET solution / project files. The rest of the .NET linters work on *.cs files although I still think we should use them with *.csproj / *.sln because .NET linters by default the input is always going to be a solution or project because a *.cs file by definition "is nothing" without its project or solution.
  2. It requires that a dotnet restore has been done in the context of docker before to download the NuGet packages because the cli requires them to get the information from the analyzers (the most relevant ones: Meziantou.Analyzer, SonarAnalyzer.CSharp, Roslynator.Analyzers, ...).

The second point is the most problematic because it implies that before executing the linter command itself we have to previously execute another dotnet restore command but as I said that it has to be done in the context of Docker, it is not enough for the user to do it on his own, we have to do it ourselves on linter side.

@Kurt-von-Laven
Copy link
Collaborator

  1. I get what you mean about running on *.csproj|*.sln rather than *.cs. It's possible there are some exceptions out there, but in general that seems to be the preferred approach. I wonder if CSharpier is perfectly happy running on *.cs files?
  2. I support running dotnet restore on our side, although I also don't have a great proposal for how exactly we ought to do this. I suspect this would also address CSharpier Requires dotnet tool restore #2868. I take it it wouldn't be sufficient to run dotnet restore in the Dockerfile (or that we already do this)?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 13, 2023

@nvuillam can you answer what @Kurt-von-Laven and I asked? That is, how to execute the dotnet restore command prior to the linter command itself? Have you ever encountered that use case or any similar one?

I don't know how to move forward otherwise, I need your advice @nvuillam.

@nvuillam
Copy link
Member

nvuillam commented Nov 13, 2023

@bdovaz sorry for the delay, you're right to beep me it's kind of crazy these days/weeks/months :)

If you need to run a command before running a linter, you can define a before_lint_files in the linter python class to add a pre_command, like for tflint :)

https://github.com/oxsecurity/megalinter/blob/main/megalinter/linters/TfLintLinter.py

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 13, 2023

But this function does not receive the input of the path to the file, the dotnet restore command needs this input.

@nvuillam
Copy link
Member

Do you have an example of the full dotnet restore command that is required ?

Most of properties are available from python linter classes, there is probably a way !

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 13, 2023

@echoix
Copy link
Collaborator

echoix commented Nov 13, 2023

We have to note that its quite possible that not all project types, like WPF or winforms, would be able to run in a Linux environment. For the rest, it could work ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request nostale This issue or pull request is not stale, keep it open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants