sonar-dotnet
sonar-dotnet copied to clipboard
Fix S4487 FP: recognize usages of private fields in partial Blazor components on Rider
UPDATE: Issue blocked until Rider and Omnisharp include Razor in the Compilation
.
See this comment for more details.
Description
When declaring and setting a private field in a razor.cs file, but only accessing it in the related razor file, sonar marks it as not used. Sonar should check the related razor file for S4487 as well.
Repro steps
Create a Blazor component with two files:
MyBlazorComponent.razor
<div>
<p>@_myPrivateField</p>
</div>
MyBlazorComponent.razor.cs
public partial class MyBlazorComponent
{
private string _myPrivateField; // Sonar marks this field as not used
void Some()
{
_myPrivateField = "hi";
}
}
Expected behavior
Recognize that the field actually is being used in razor file.
Actual behavior
False positive for S4487.
Known workarounds
Mark every occurrence as false positive, or disable rule csharpsquid:S4487
for **/*.razor.cs
completely.
Probably not using code in separate *razor.cs file but declare the private field in the razor file itself. Did not test that.
Related information
- C# 12
- Rider 2023.3.2
- .Net 8
- SonarLint plugin 10.2.1.77304 connected to SonarCloud
- Windows 11
Hi @naefp, I'm investigating this issue as well. The implementation is shared between S4487 and S1144 so the issue is probably linked to #8537
I confirm this as a FP. To provide additional context, the issue does not reproduce in the following:
- SL4VS
- local analysis run with SonarQube
- analyzer UTs
It seems like S2933 (Fields that are only assigned in the constructor should be "readonly") is affected as well.
MyBlazorComponent.razor
<SomeComponant @bind-Value="@_myPrivateField " />
MyBlazorComponent.razor.cs
public partial class MyBlazorComponent
{
private string _myPrivateField = "Some intial value"; // Sonar want to make this 'readonly'
}
As for #8537, the issue seems to be related to SonarLint only. Since it is not yet able to analyze Razor generated files, it is not able to see the field usage in the component. We have an internal ticket to tackle the support of Razor in SonarLint which should fix this issue.
TL;DR: issue blocked until Omnisharp starts supporting Razor compilation
We have reproduced and debugged live the issue from the IDE with the following setups:
- Rider 2024.1.2 (Roslyn 4.10.0-3.24216.12) and Visual Studio 17.10.1 (Roslyn 4.10.0-3.24270.2)
- notice that the Roslyn version is recent enough to include changes around razor files generation and location mapping
- latest version of SonarLint 10.6.2.78685 for Rider
- latest version of SonarLint 8.0.0.92083 for Visual Studio
- a simple Blazor project with a
private
field declared in thecs
part of a component (inheriting fromComponentBase
) and used in therazor
part of the component - a
Debugger.Launch
in the rule
The difference between Visual Studio and Rider, even after recent changes around Razor file generation from Microsoft, is that Rider uses Omnisharp, which deal with projects containing razor
files differently.
As of the time of our testing, what we observed is that the Compilation
does not contain any razor generated file, which are present when building in Visual Studio, with extension .ide.g.cs
.
As an example, for the simple Blazor project mentioned above, only 5 Syntax Trees are visible in the Compilation
when in Rider, as opposed to the 14 files of the Compilation
in Visual Studio.
Given that the original razor
files are not even additional files in the Compilation
, there is currently no way to distinguish between a false positive case induced by a missing partial
class, produced by lack of razor generation, and a true positive case, where all the partial
classes are available.
This primarily happens:
- in the context of SonarLint, where the analysis is run by the IDE (and not via
dotnet build
) - and in the context of Rider, where Omnisharp is used
Another context where the issue may happen is dotnet format
.
We could disable the rule in the presence of partial
classes that inherit from BaseComponent
. That, however, would introduce false negatives in CI as well as in Visual Studio, where generated files are included in the Compilation
and the rule works as expected.
Therefore, given the limited scope of the issue, we are not going to change the rule, and instead we are going to wait for Rider to expose generated files in the Compilation
.