-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
DotNet format linter does not work properly for projects using .NET versions other than .NET 6 #2310
Comments
It's clear that it would be impossible to have .Net framework in MegaLinter's image, since it's only available on Windows. Dotnet format as a linter is not working nicely and behaves very differently as all the other linters. It is quite difficult to integrate it correctly since they included dotnet format as part of the SDK instead of the standalone tool that could be installed. In my opinion, the solution you expect is unmaintainable as it would have a great impact for more users than the benefit that it has for only one or a couple users. The required size of the sdks asked is huge. |
See #1680 (comment) for why we couldn't already use .net 7. MegaLinter was stuck on .net 5 and with hard work, we could only recently upgrade to .net 6. And that caused some change in behaviour, since the dotnet-format tool that was available for up to .NET 5 has a really different behaviour with its version 6. See #2248 for a longer discussion. |
Would you be willing to try this custom Dockerfile to see if the dotnet format with the .NET 7 project works fine? FROM oxsecurity/megalinter-dotnet
RUN wget --tries=5 -q -O dotnet-install.sh https://dot.net/v1/dotnet-install.sh \
&& chmod +x dotnet-install.sh \
&& ./dotnet-install.sh --install-dir /usr/share/dotnet -channel 7.0 -version latest
(I made a scratch repo just for that https://github.com/echoix/dotnet-megalinter-extended-test, not to maintain at all) |
@nvuillam, I see that there is a dotnet-format install in the Linter dockerfile. megalinter/flavors/dotnet/Dockerfile Lines 293 to 294 in f8d535e
and https://github.com/echoix/megalinter/blob/f8d535e8f1b5be62df8ea5c9c8548035fc298788/Dockerfile#L407-L408C64 It is not supposed to, since we use the .NET 6 SDK, the dotnet-format tool isn't needed and was supposed to be removed with #1680, but wasn't. So we use the command |
@echoix I thought the same thing when I checked that descriptor recently. In the PR to upgrade .NET 5 to .NET 6 we didn't realize that we don't need to install it explicitly anymore. I'm going to create a PR to remove it. |
I also saw that the Dockerfile s in the linter directory had different Python Alpine versions and thought: that's weird. But I remembered you talked about it on a PR, I don't remember which one, where you saw that they weren't updated. And that's one of the effects of it. |
@Isalgeon that means that for now, dotnet-format of version 5 is still installed, are you able to use it or do something with it. |
@echoix ah, I completely forgot about that fact in this context, which is crucial! That means that, for the scope of MegaLinter, my expectation cannot be met for .NET Framework specifically. This detail is a good one to mention explicitly in the documentation for the DotNet format linter, in case someone else encounters this particular issue.
Armed with the knowledge that the DotNet linter has become part of the SDK... yes, I agree with your assessment @echoix. That would neither be feasible nor fair to expect from MegaLinter.
I'm currently trying this out, @echoix. Thanks!
No need, thankfully! I thought I had 1 such project (besides my little test project), but the TargetFramework setting was overridden in a |
@Isalgeon for your missing restore error, two things that might help you:
|
@echoix, I've just tried it out but eventually ran into the below issue during the lining process. I'm not sure what's happening, as this also occurs in a newly generated class library project. 🤔
|
What did you try out that lead to the MSBuild error? The reference assemblies or a custom image with another install? It seems to mention .net 7, so it's not the released image for sure. If it was a test of both, could you at least try with what is released first, to rule out a possible install error? (I saw that there was a megalinter/flavors/dotnet/Dockerfile Lines 235 to 240 in b0bb50f
|
I've specifically tried out the following:
|
I tested it again using your ENV suggestion to no avail unfortunately. I'll look into your other comment regarding the references tomorrow, although it doesn't seem applicable to my specific case on first glance. Thanks for the help so far, @echoix! |
Good luck for your use case! |
@Isalgeon, no rush, but curious what you learned and/or where you got stuck if you have had a chance to investigate further. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
Hi @Kurt-von-Laven, to give an update on this issue: My team and I decided it's better to avoid using the DotNet format linter via MegaLinter and instead invoke it directly in our CI pipeline on a Windows-based build agent via the CLI. Our reasoning is as follows:
I did verify whether the DevSkim linter has upgraded its solution to .NET 7, and it appears to be in the works (look up the individual TargetFramework elements in the .csproj files), just not yet released. From our perspective, this issue is closed. As said before, thanks for your input and help, especially @echoix! 🙂 |
Sorry we couldn't be of more help. That seems like a wise decision. FYI, CSharpier was added to MegaLinter not so long ago, so for ourselves we decided to disable dotnet format as we consider it superceded by its faster, more opinionated cousin. |
Describe the bug
The DotNet format linter does not apply most of the configured rules in the EditorConfig when the project uses any framework other than .NET 6.
To Reproduce
Steps to reproduce the behavior:
Create the most basic C# project (e.g., a class library) that targets any other .NET version other than .NET 6.
Open the generated
Class1.cs
file and note the placement of thenamespace
. It currently wraps the class which should be replaced (given the example EditorConfig) with the self-closing, file-scoped namespace syntax.Run MegaLinter using the example configuration.
Inspect the earlier generated
Class1.cs
file; it is unchanged regarding the namespace syntax.If you would repeat the above reproduction steps using .NET 6 it will work as expected.
Expected behavior
I expect the DotNet format linter to work properly, regardless of the used .NET version.
This would likely mean including each .NET version in the All and DotNet Docker images.
One nuance to that expectation is that Microsoft should still support the .NET version.
In other words, I do not think end-of-life .NET versions should be incorporated into MegaLinter.
See the Microsoft documentation for the current support policy.
Logs
Example logs for .NET Framework 4.8.1, which are the clearest what the issue is exactly.
Example logs for .NET 7.
Additional context
I have quite a few enterprise application extension projects.
The enterprise applications extended still use .NET Framework, which is out of my control to upgrade.
On the other hand, I also have quite a few applications that use the latest .NET 7.
Ideally, I would still like to format all of these projects.
Note that I use the
--no-restore
argument for the DotNet linter.This is because I have several NuGet packages on a private NuGet feed that the Docker image cannot access without further configuration. I still have to look into that how to resolve that particular issue.
Just so you know, it does not matter for the main issue, namely the formatting. That only affects whether DotNet format can act on rules related to namespace imports that are not enabled by default.
Example configurations
MegaLinter configuration:
EditorConfig configuration:
The text was updated successfully, but these errors were encountered: