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

DotNet format linter does not work properly for projects using .NET versions other than .NET 6 #2310

Closed
Isalgeon opened this issue Feb 5, 2023 · 19 comments
Labels
bug Something isn't working O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity

Comments

@Isalgeon
Copy link
Contributor

Isalgeon commented Feb 5, 2023

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:

  1. Create the most basic C# project (e.g., a class library) that targets any other .NET version other than .NET 6.

    I specifically tested this with .NET Framework 4.8, .NET Framework 4.8.1, .NET Core 3.1, .NET 5, and .NET 7.

    The example EditorConfig in example configurations should result in a change regarding the namespace placement.

  2. Open the generated Class1.cs file and note the placement of the namespace. It currently wraps the class which should be replaced (given the example EditorConfig) with the self-closing, file-scoped namespace syntax.

  3. Run MegaLinter using the example configuration.

  4. 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.

[dotnet-format] command: dotnet format /tmp/lint --no-restore --severity error --verbosity diagnostic | tee /dev/tty2 2>&1 && exit "${PIPESTATUS[0]}"
[dotnet-format] CWD: /tmp/lint
[dotnet-format] result: 0   The dotnet runtime version is '6.0.13'.
  The dotnet CLI version is '6.0.405'.
  Using MSBuild.exe located in '/usr/share/dotnet/sdk/6.0.405/'.
  Formatting code files in workspace '/tmp/lint/TestRepository.NetFramework.sln'.
  Loading workspace.
Msbuild failed when processing the file '/tmp/lint/TestRepository.NetFramework/TestRepository.NetFramework.csproj' with message: /usr/share/dotnet/sdk/6.0.405/Microsoft.Common.CurrentVersion.targets: (1220, 5): The reference assemblies for .NETFramework,Version=v4.8.1 were not found. To resolve this, install the Developer Pack (SDK/Targeting Pack) for this framework version or retarget your application. You can download .NET Framework Developer Packs at https://aka.ms/msbuild/developerpacks
  Project TestRepository.NetFramework is using configuration from '/tmp/lint/.editorconfig'.
  Complete in 2551ms.
  Determining formattable files.
  Complete in 323ms.
  Running formatters.
  Running Code Style analysis.
  Determining diagnostics...
Required references did not load for TestRepository.NetFramework or referenced project. Run `dotnet restore` prior to formatting.
  Complete in 232ms.
  Fixing diagnostics...
  Complete in 8ms.
  Analysis complete in 240ms.
  Running Analyzer Reference analysis.
  Determining diagnostics...
  Complete in 0ms.
  Fixing diagnostics...
  Complete in 0ms.
  Analysis complete in 0ms.
  Complete in 962ms.
  Formatted 0 of 1 files.
  Format complete in 3840ms.

Linter version command: ['/usr/share/dotnet/dotnet', '--version']
Linter version result: 0 6.0.405

✅ Linted [CSHARP] files with [dotnet-format] successfully - (4.97s)
- Using [dotnet-format v6.0.405] https://megalinter.io/latest/descriptors/csharp_dotnet_format
- MegaLinter key: [CSHARP_DOTNET_FORMAT]
- Rules config: identified by [dotnet-format]

Example logs for .NET 7.

[dotnet-format] command: dotnet format /tmp/lint --no-restore --severity error --verbosity diagnostic | tee /dev/tty2 2>&1 && exit "${PIPESTATUS[0]}"
[dotnet-format] CWD: /tmp/lint
[dotnet-format] result: 0   The dotnet runtime version is '6.0.13'.
  The dotnet CLI version is '6.0.405'.
  Using MSBuild.exe located in '/usr/share/dotnet/sdk/6.0.405/'.
  Formatting code files in workspace '/tmp/lint/TestRepository.NetUnified.sln'.
  Loading workspace.
  Project TestRepository.NetUnified is using configuration from '/tmp/lint/.editorconfig'.
  Project TestRepository.NetUnified is using configuration from '/tmp/lint/TestRepository.NetUnified/obj/Debug/net7/TestRepository.NetUnified.GeneratedMSBuildEditorConfig.editorconfig'.
  Project TestRepository.NetUnified is using configuration from '/usr/share/dotnet/sdk/6.0.405/Sdks/Microsoft.NET.Sdk/analyzers/build/config/analysislevel_6_default.editorconfig'.
  Complete in 2237ms.
  Determining formattable files.
  Complete in 422ms.
  Running formatters.
  Running Code Style analysis.
  Determining diagnostics...
Required references did not load for TestRepository.NetUnified or referenced project. Run `dotnet restore` prior to formatting.
  Complete in 297ms.
  Fixing diagnostics...
  Complete in 6ms.
  Analysis complete in 303ms.
  Running Analyzer Reference analysis.
  Determining diagnostics...
Required references did not load for TestRepository.NetUnified or referenced project. Run `dotnet restore` prior to formatting.
  Complete in 25ms.
  Fixing diagnostics...
  Complete in 0ms.
  Analysis complete in 25ms.
  Complete in 1191ms.
  Formatted 0 of 3 files.
  Format complete in 3854ms.

Linter version command: ['/usr/share/dotnet/dotnet', '--version']
Linter version result: 0 6.0.405

✅ Linted [CSHARP] files with [dotnet-format] successfully - (4.61s)
- Using [dotnet-format v6.0.405] https://megalinter.io/latest/descriptors/csharp_dotnet_format
- MegaLinter key: [CSHARP_DOTNET_FORMAT]
- Rules config: identified by [dotnet-format]

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:

APPLY_FIXES: "all"
LOG_LEVEL: DEBUG
VALIDATE_ALL_CODEBASE: true

ENABLE_LINTERS:
    - "CSHARP_DOTNET_FORMAT"

CSHARP_DOTNET_FORMAT_ARGUMENTS:
    - "--no-restore"
    - "--severity error"
    - "--verbosity diagnostic"
CSHARP_DOTNET_FORMAT_CLI_LINT_MODE: "project"

EditorConfig configuration:

root = true

[*.cs]
charset = utf-8
end_of_line = lf
indent_size = 4
indent_style = tab
insert_final_newline = true
tab_width = 4
trim_trailing_whitespace = true

dotnet_diagnostic.IDE0160.severity = none
dotnet_diagnostic.IDE0161.severity = error
csharp_style_namespace_declarations = file_scoped
@Isalgeon Isalgeon added the bug Something isn't working label Feb 5, 2023
@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

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.
I'm searching for a workaround to see if extending the image with a custom Dockerfile would work (that you would build to use). But that implies that the premise that only having some additional SDKs installed would solve your use case.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

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.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

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)

If I connect to the container, it shows up as installed.
image
image

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

@nvuillam, I see that there is a dotnet-format install in the Linter dockerfile.

# dotnet-format installation
&& /usr/share/dotnet/dotnet tool install -g dotnet-format \

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 dotnet format (from the .NET 6 SDK), but we also install dotnet-format (as a global tool, that would have a version like 5.1.250801 (from nuget), and we don't use it. Note that the 5.1 is not related to the .net version, but they happen to align the major version numbers from .net releases

@bdovaz
Copy link
Collaborator

bdovaz commented Feb 5, 2023

@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.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

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.

@bdovaz
Copy link
Collaborator

bdovaz commented Feb 5, 2023

@echoix yes, in #2294 in particular. Don't worry because in that PR I'm already updating them. I still have some work to do in that PR because there are many linters but I'm making good progress.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

@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.

@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.

@Isalgeon
Copy link
Contributor Author

Isalgeon commented Feb 5, 2023

It's clear that it would be impossible to have .Net framework in MegaLinter's image, since it's only available on Windows.

@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.

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.

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.

Would you be willing to try this custom Dockerfile to see if the dotnet format with the .NET 7 project works fine?

I'm currently trying this out, @echoix. Thanks!

that means that for now, dotnet-format of version 5 is still installed, are you able to use it or do something with it.

No need, thankfully! I thought I had 1 such project (besides my little test project), but the TargetFramework setting was overridden in a csproj file but was forgotten to be removed from the Directory.Build.props file.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

@Isalgeon for your missing restore error, two things that might help you:

  • When reading this morning, I saw that there seems to be a difference in the types of csproj, where one can create an error when using a type of project and the other one doesn't. Dependency conflict when project.json references csproj dotnet/sdk#6229 (comment)
  • I already tried to open a really old sample repo written in C# in visual studio in the last month. I had the same error when trying to build (at restoring). It was multitargeting many versions, and had one of them installed, but I could run a debug build since it looked like if I needed all the targeting packs available (but didn't want to). I tried adding the reference assemblies for the framework (like adding a nuget package to the project) as I read it should have enabled me to at least build without having them available, but it didn't really work and I stopped working on trying to get it running. Maybe you'd have more luck. https://github.com/Microsoft/dotnet/tree/main/releases/reference-assemblies. By trying to find the link I just understood that it should've work to build using a dotnet SDK (like building from dotnet build), but I remember that Visual Studio could have been using the msbuild instead, which is implemented with a .Net framework (but can produce the same output). I didn't try that.

@Isalgeon
Copy link
Contributor Author

Isalgeon commented Feb 5, 2023

@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. 🤔

Results of dotnet-format linter (version 7.0.102)
See documentation on https://megalinter.io/latest/descriptors/csharp_dotnet_format/
-----------------------------------------------

❌ [ERROR] for workspace /tmp/lint
Linter raw log:

Welcome to .NET 7.0!
---------------------
SDK Version: 7.0.102

Telemetry
---------
The .NET tools collect usage data in order to help us improve your experience. It is collected by Microsoft and shared with the community. You can opt-out of telemetry by setting the DOTNET_CLI_TELEMETRY_OPTOUT environment variable to '1' or 'true' using your favorite shell.

Read more about .NET CLI Tools telemetry: https://aka.ms/dotnet-cli-telemetry

----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate run 'dotnet dev-certs https --trust' (Windows and macOS only).
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Unable to locate MSBuild. Ensure the .NET SDK was installed with the official installer.

@echoix
Copy link
Collaborator

echoix commented Feb 5, 2023

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 --skip-non-versioned-files option in the docs, https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script, and I wasn't quite sure multiple dotnet versions would work since we were specifying a path. Maybe its an Env issue too, like the ENV line after

# CSHARP installation
&& 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 6.0 -version latest
ENV PATH="${PATH}:/root/.dotnet/tools:/usr/share/dotnet"
)

@Isalgeon
Copy link
Contributor Author

Isalgeon commented Feb 5, 2023

I've specifically tried out the following:

  1. Build a Docker image using the Dockerfile provided by you earlier.
  2. Run MegaLinter by running the built Docker container. The configuration for MegaLinter and the EditorConfig remained the same as provided at the start of this issue.

@Isalgeon
Copy link
Contributor Author

Isalgeon commented Feb 5, 2023

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!

@echoix
Copy link
Collaborator

echoix commented Feb 6, 2023

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!
The ENV comment was me thinking if we were overriding a path, or hiding another, ie: overwriting another dotnet version in the exact same repo. I didn't know how to quickly test that. Eitherway, it was all workaround ideas.

Good luck for your use case!

@Kurt-von-Laven
Copy link
Collaborator

@Isalgeon, no rush, but curious what you learned and/or where you got stuck if you have had a chance to investigate further.

@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 17, 2023
@Isalgeon
Copy link
Contributor Author

Isalgeon commented Mar 17, 2023

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:

  • We use a single linter configuration and CI pipeline template stack across ~65 repositories.
  • Most of our projects (~40) are .NET Framework based, which is not supported on a Linux system. These projects will unfortunately never be upgraded to the newer .NET SDK as the technology they are built upon is reaching the end of its life, meaning no upgrade there either. It will be a few years until they are completely phased out, and they require maintenance until then.
  • We don't want to add complexity to our current setup to determine whether to invoke the DotNet format linter via MegaLinter or separately, depending on whether it's a .NET Framework project. Invoking the DotNet format linter separately from MegaLinter in its own CI pipeline job is acceptable for us.
  • Our CI pipeline build agents are already authenticated towards our private package feed, removing the need to investigate how to make it work in a Docker container context just for this one linter.

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! 🙂

@Kurt-von-Laven
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity
Projects
None yet
Development

No branches or pull requests

4 participants