bUnit icon indicating copy to clipboard operation
bUnit copied to clipboard

Remove #pragma disable BL0007

Open linkdotnet opened this issue 4 years ago • 7 comments

With dotnet/aspnetcore#670 we now target against .NET 7 (preview). .NET 7 preview 2 introduced a new analyzer (BL0007) which checks that component parameters should be auto-properties. Unfortunately on Linux builds the .editorconfig or better the severity of that specific analyzer in the .editorconfig is ignored.

We can remove the #pragma disable BL0007 from ThrowsOnParameterSet once the following issue is closed: dotnet/razor-compiler#102

linkdotnet avatar Apr 06 '22 19:04 linkdotnet

I tried a repro of our BL0007 and it seems if you force it, none of the severity settings in .editorconfig are taken into account. Also for some reason on windows and Linux. See: https://github.com/linkdotnet/BL0007Bug/runs/5863424196?check_suite_focus=true

linkdotnet avatar Apr 07 '22 06:04 linkdotnet

Hmm weird, does that also happen with Windows or is it a Linux thing only? If it is Linux only, it might be that the casing of the .editorconfig file is not what is expected there.

egil avatar Apr 07 '22 10:04 egil

To be completely honest with you I am not sure what is going on. I created a small repro I want to share with the asp.net core team: https://github.com/linkdotnet/BL0007Bug

I have this issue also with other BL000X analyzer rules plus I also have it on windows machines. In bUnit we only see it on the Linux containers.

Locally dotnet build will also lead to the same error(s). Seems like the toolchain is here the problem and not necessarily the asp.net core team. https://github.com/linkdotnet/BL0007Bug/actions/runs/2107054876

EDIT: Do we need this file at all? This seems redundant? Can this cause an issue?

linkdotnet avatar Apr 07 '22 10:04 linkdotnet

Besides that: our first nightly verifcation build was ✔ 🎉 yeah

linkdotnet avatar Apr 07 '22 10:04 linkdotnet

EDIT: Do we need this file at all? This seems redundant? Can this cause an issue?

It's just part of a standard set of coding rules related files we use at work. It should not cause problems, .editorconfig files inherits from those higher up the tree if they don't have root=true in them.

But we can remove it.

egil avatar Apr 07 '22 10:04 egil

No I am totally fine with that don't get me wrong (even though the file is basically empty?). I was just not sure if it works like Directoy.Build.props where the "closest" one is taking into account. You have to explicitly import other Directoy.Build.props to make inheritance work.

PS: Sorry for the off-topic. Let's wait for the response from the ASP.NET Core team.

linkdotnet avatar Apr 07 '22 10:04 linkdotnet

Directoy.Build.props in src and test are actually set up to inherit from the one in the root folder, but that is done explicitly and not the default. This is the line that does this:

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., Directory.Build.props))\Directory.Build.props" />

egil avatar Apr 07 '22 12:04 egil

Maybe its due to [*.{cs,razor}] on linux. Can you try separate entry for .cs and .razor to see if it makes any difference

mikes-gh avatar Oct 21 '22 11:10 mikes-gh

@linkdotnet I am on macos and got the same issue. I moved the problem to a code behind file razor.cs and editorconfig seems to work against that.

mikes-gh avatar Oct 21 '22 12:10 mikes-gh

Hey @mikes-gh thanks for the tip! Refactoring the stuff into a code-behind did the trick :D Funny enough that should trigger the analyzer as well. Maybe they forgot to include .cs. Anyway great tip.

linkdotnet avatar Oct 21 '22 12:10 linkdotnet

Funny enough that should trigger the analyzer as well. Maybe they forgot to include .cs. Anyway great tip.

Because you ignore it via .editorconfig in your repo https://github.com/bUnit-dev/bUnit/blob/0e8d73cc5a71c01e9d7c12fb289b7dae7ca39f34/.editorconfig#L466 The issue is the analyzer doesnt seem to respect the editorconfig for .razor files. Thats still a mystery.

mikes-gh avatar Oct 21 '22 14:10 mikes-gh

Funny enough that should trigger the analyzer as well. Maybe they forgot to include .cs. Anyway great tip.

Because you ignore it via .editorconfig in your repo

https://github.com/bUnit-dev/bUnit/blob/0e8d73cc5a71c01e9d7c12fb289b7dae7ca39f34/.editorconfig#L466

The issue is the analyzer doesnt seem to respect the editorconfig for .razor files. Thats still a mystery.

The funny part is that other BL000X issues are ignored if properly configured in .editorconfig. Unfortunately they don't seem to work on that issue at all.

linkdotnet avatar Oct 21 '22 14:10 linkdotnet

Yeh it is weird. The editorconfig does have an affect on BL0007 though on my machine. If I remove it from the editorconfig the warning reappears. It only works on .cs though. It doesn't seem to work for razor file extensions.

mikes-gh avatar Oct 21 '22 15:10 mikes-gh