DotNetAnalyzers / StyleCopAnalyzers

An implementation of StyleCop rules using the .NET Compiler Platform
MIT License
2.67k stars 508 forks source link

SA1501 applied differently when used in .editorconfig vs .ruleset #3290

Open kornelijepetak opened 3 years ago

kornelijepetak commented 3 years ago

There's this piece of code

if(variable == value) return;

Before. I used to use the .ruleset file in the past to set up the analyzers.

// project.csproj

<PropertyGroup>
  <CodeAnalysisRuleSet>$(SolutionDir)styles.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>

// styles.ruleset
<Rule Id="SA1501" Action="Error" />

That setup would result in this: image

After. I have since migrated to the .editorconfig, removed the .ruleset entry from the csproj file and I now have this:

// .editorconfig
dotnet_diagnostic.SA1501.severity = error

Now, this situation is no longer detected. image

While I understand that SA1501 may not even deal with this particular case, as the cause for the SA1501 as per docs requires braces to be present, I don't understand why it works differently with the .ruleset file and not with the .editorconfig.

I am mostly interested in a rule that would (regardless of the braces) disallow putting return/continue/break on the same line as any other flow control statement (if, while, etc.). Is there any such rule that can be used with the .editorconfig?

💻 Technical context

sharwell commented 3 years ago

Well that's weird 😄

Note that .globalconfig is a better replacement for rule sets than .editorconfig. Have you tried moving the configuration there?

ThomasMader commented 3 years ago

I am experiencing the same problem. Moving the settings from a ruleset to an .editorconfig doesn't seem to work at all. Moving SA1201 for example did not work.

manfred-brands commented 3 years ago

Rule SA1501 depends on the configuration for SA1503 source

[*.cs]
# Statement should not be on a single line
dotnet_diagnostic.SA1501.severity = error
# Braces should not be omitted
dotnet_diagnostic.SA1503.severity = none

But even with this file, I can only get SA1501 to fire on

   lock (args) { return args.Length; }

but not on

   lock (args) return args.Length;

Indicating that somehow it doesn't detect SA1503 is suppressed.

sharwell commented 3 years ago

Due to the way configuration is checked, these rules may need to be configured via .globalconfig (from my earlier comment) and not via .editorconfig in order for the relation between SA1501 and SA1503 to be detected during analysis. While such a requirement would not be intentional, do note that .globalconfig (and not .editorconfig) is the true replacement for .ruleset files.

manfred-brands commented 3 years ago

I can confirm that renaming the file to .globalconfig and removing the section header, the SA1501 fires in both cases. @sharwell Is this specific to StyleCop as I haven't noticed this in the past for other analyzers. Although I don't know if any other rules are linked in this way either.

sharwell commented 3 years ago

Yes, this implementation detail is specific to the way StyleCop Analyzers tries to link/coordinate the behavior of SA1501 and SA1503. In hindsight this wasn't a great idea but removing the link between the two is difficult now. Now that the repository has proper support for .editorconfig (#3285), the implementation of these two rules could be updated to work with both .editorconfig and .globalconfig.