Sonar-dotnet: S125 (C#) gives false positives far too easily

[perlun]

Description

S125 (C#) gives false positives far too easily

This line illustrates part of the problem: https://github.com/SonarSource/sonar-dotnet/blob/1073fbc662a66e8d79b8a59c1793603c3469d755/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/CommentedOutCode.cs#L191

Repro steps

```c#
// Support optional typing on this form:
// var s: String;

or even just:

```c#
// Foo;

Expected behavior

None of the examples above is really commented out _C#_ code. The first comment contains a Perlang statement (a new programming language I am writing :see_no_evil:); the second could very well be a fraction of a C# program but... it looks kind of naked there without any assignment taking place, any method being called or anything.

The way I understand it, the analyzer in this case just checks if the line ends with ;, { or }. While this probably works for many, many cases, it feels like it's too brittle and susceptible to false positives.

Actual behavior

Both of the above trigger S125 to be raised.

Known workarounds

N/A

Related information

• C#/VB.NET Plugins Version: 8.14.0.22654
• MSBuild / dotnet version: 3.1.302
• Operating System: Linux

See also #2772.

[pavel-mikula-sonarsource]

Hi @perlun,

As you've identified how the rule works, you can see that the code detection is fairly simple (and fast). Main sign of C-based language fragment is presence of ; character that's only rarely used in normal text. Therefore your var s: String; as well as Foo; are code fragments. Even when it doesn't compile, it's still a code fragment.

There's not much we can improve about this rule. If you're writing a new language and you need a lots of code fragments in your comments, you should deactivate this rule because it doesn't fit your project needs.

[perlun]

Thanks @pavel-mikula-sonarsource. I understand that the rule is done in a "quick and dirty" fashion and it perhaps covers many of the typical cases of commented out code. Still, it feels like it would be good to make it more sophisticated somehow. I mean, semicolons _are_ used in regular English as well, so there's a slight chance for false positives with this rule even in "regular", less obscure cases than my own use case... :slightly_smiling_face:

Having that said, I don't have any obvious ideas on how to improve it (except for doing a full C# parsing of the code fragment in question, which is of course a _much_ more complex task...). Thanks for the reply and I guess we'll have to live with it as it currently stands for now.

[aureole82]

I strongly agree with @perlun: Just the presence of ; should never result in S125 indication (even in projects who doesn't implement a compiler). So for me that means: We must deactivate S125 by default!