Fix S6802 FP: Reduce scope to raise only within loop blocks (#8387)

analyzers/rspec/cs/S6802.html

@@ -5,8 +5,13 @@ <h2>Why is this an issue?</h2>
 elements.</p>
 <p>The reason behind this is that Blazor rebuilds all lambda expressions within the loop every time the UI elements are rendered.</p>
 <h2>How to fix it</h2>
-<p>Ensure to not use a delegate in elements rendered in loops by using a collection of objects containing the delegate as an <a
-href="https://learn.microsoft.com/en-us/dotnet/api/system.action">Action</a>.</p>
+<p>Ensure to not use a delegate in elements rendered in loops, you can try:</p>
+<ul>
+  <li> using a collection of objects containing the delegate as an <a href="https://learn.microsoft.com/en-us/dotnet/api/system.action">Action</a>,
+  </li>
+  <li> or extracting the elements into a dedicated component and using an <a
+  href="https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling#eventcallback">EventCallback</a> to call the delegate </li>
+</ul>
 <h3>Code examples</h3>
 <h4>Noncompliant code example</h4>
 <pre data-diff-id="1" data-diff-type="noncompliant">
@@ -15,7 +20,7 @@ <h4>Noncompliant code example</h4>
     var buttonNumber = i;
 
     &lt;button @onclick="@(e =&gt; DoAction(e, buttonNumber))"&gt; @* Noncompliant *@
-        Button #buttonNumber
+        Button #@buttonNumber
     &lt;/button&gt;
 }
 
@@ -62,14 +67,77 @@ <h4>Compliant solution</h4>
     }
 }
 </pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+@* Component.razor *@
+
+@for (var i = 1; i &lt; 100; i++)
+{
+    var buttonNumber = i;
+
+    &lt;button @onclick="@(e =&gt; DoAction(e, buttonNumber))"&gt; @* Noncompliant *@
+        Button #@buttonNumber
+    &lt;/button&gt;
+}
+
+@code {
+    private void DoAction(MouseEventArgs e, int button)
+    {
+        // Do something here
+    }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+@* MyButton.razor *@
+
+&lt;button @onclick="OnClickCallback"&gt;
+    @ChildContent
+&lt;/button&gt;
+
+@code {
+    [Parameter]
+    public int Id { get; set; }
+
+    [Parameter]
+    public EventCallback&lt;int&gt; OnClick { get; set; }
+
+    [Parameter]
+    public RenderFragment ChildContent { get; set; }
+
+    private void OnClickCallback()
+    {
+        OnClick.InvokeAsync(Id);
+    }
+}
+
+@* Component.razor *@
+
+@for (var i = 1; i &lt; 100; i++)
+{
+    var buttonNumber = i;
+    &lt;MyButton Id="buttonNumber" OnClick="DoAction"&gt;
+        Button #@buttonNumber
+    &lt;/MyButton&gt;
+}
+
+@code {
+    private void DoAction(int button)
+    {
+        // Do something here
+    }
+}
+</pre>
 <h2>Resources</h2>
 <h3>Documentation</h3>
 <ul>
   <li> Microsoft Learn - <a
   href="https://learn.microsoft.com/en-us/aspnet/core/blazor/performance#avoid-recreating-delegates-for-many-repeated-elements-or-components">ASP.NET
   Core Blazor performance best practices</a> </li>
   <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling#lambda-expressions">ASP.NET Core
-  Blazor event handling</a> </li>
+  Blazor event handling - Lambda expressions</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling#eventcallback">Event handling -
+  EventCallback Struct</a> </li>
 </ul>
 <h3>Benchmarks</h3>
 <p>The results were generated with the help of <a href="https://github.com/dotnet/BenchmarkDotNet">BenchmarkDotNet</a> and <a

analyzers/rspec/cs/Sonar_way_profile.json

@@ -294,7 +294,6 @@
     "S6797",
     "S6798",
     "S6800",
-    "S6802",
     "S6803"
   ]
 }

analyzers/src/SonarAnalyzer.CSharp/Rules/AvoidLambdaExpressionInLoopsInBlazor.cs

@@ -45,7 +45,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
                 {
                     var node = (LambdaExpressionSyntax)c.Node;
 
-                    if (IsWithinLoop(node)
+                    if (IsWithinLoopBody(node)
                         && IsWithinRenderTreeBuilderInvocation(node, c.SemanticModel))
                     {
                         c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation()));
@@ -61,6 +61,6 @@ private static bool IsWithinRenderTreeBuilderInvocation(SyntaxNode node, Semanti
                                          && semanticModel.GetSymbolInfo(invocation.Expression).Symbol is IMethodSymbol symbol
                                          && symbol.ContainingType.GetSymbolType().Is(KnownType.Microsoft_AspNetCore_Components_Rendering_RenderTreeBuilder));
 
-    private static bool IsWithinLoop(SyntaxNode node) =>
-        node.AncestorsAndSelf().Any(x => x is ForStatementSyntax or ForEachStatementSyntax or WhileStatementSyntax or DoStatementSyntax);
+    private static bool IsWithinLoopBody(SyntaxNode node) =>
+        node.AncestorsAndSelf().Any(x => x is BlockSyntax && x.Parent is ForStatementSyntax or ForEachStatementSyntax or WhileStatementSyntax or DoStatementSyntax);
 }

analyzers/tests/SonarAnalyzer.UnitTest/Rules/AvoidLambdaExpressionInLoopsInBlazorTest.cs

@@ -37,6 +37,12 @@ public void AvoidLambdaExpressionInLoopsInBlazor_Blazor() =>
             .WithAdditionalFilePath(AnalysisScaffolding.CreateSonarProjectConfig(TestContext, ProjectType.Product))
             .Verify();
 
+    [TestMethod]
+    public void AvoidLambdaExpressionInLoopsInBlazor_BlazorLoopsWithNoBody() =>
+        builder.AddPaths("AvoidLambdaExpressionInLoopsInBlazor.LoopsWithNoBody.razor")
+            .WithAdditionalFilePath(AnalysisScaffolding.CreateSonarProjectConfig(TestContext, ProjectType.Product))
+            .Verify();
+
     [TestMethod]
     public void AvoidLambdaExpressionInLoopsInBlazor_UsingRenderFragment() =>
         builder.AddPaths("AvoidLambdaExpressionInLoopsInBlazor.RenderFragment.razor", "AvoidLambdaExpressionInLoopsInBlazor.RenderFragmentConsumer.razor")

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AvoidLambdaExpressionInLoopsInBlazor.LoopsWithNoBody.razor

@@ -0,0 +1,102 @@
+@* https://github.com/SonarSource/sonar-dotnet/issues/8394 *@
+
+@foreach (var item in Buttons.Where(x => x.Id == "idToFind"))
+    if (item.Id == "idToFind")
+    {
+        <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+    }
+
+@for (int i = 0; i < Buttons.Count; i++)
+    @if (i % 2 == 0)
+    {
+        var buttonNumber = i;
+        <button @onclick="@(e => DoAction(e, Buttons[buttonNumber]))"> @* FN *@
+            Button #@buttonNumber
+        </button>
+    }
+
+@{
+    var j = 0;
+    while (j < 5)
+        if (j % 2 == 0)
+        {
+            j += 2;
+            <button @onclick="(e) => Reset(e)"> @* FN *@
+                Reset @j
+            </button>
+            j += 2;
+        }
+
+    do
+        if (j % 2 == 0)
+        {
+            <button @onclick="(e) => Reset(e)"> @* FN *@
+                Reset @j
+            </button>
+            j += 2;
+        }
+    while (j < 10);
+}
+
+@foreach (var item in Buttons.Where(x => x.Id == "idToFind"))
+    @if (item.Id == "idToFind")
+    {
+        <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+    }
+    else if (item.Id == "idToFind")
+    {
+        <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+    }
+    else
+    {
+        <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+    }
+
+@foreach (var item in Buttons.Where(x => x.Id == "idToFind"))
+    @if (true)
+        @if (item.Id == "idToFind")
+        {
+            <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+        }
+
+@foreach (var item in Buttons.Where(x => x.Id == "idToFind"))
+    @switch(item.Id)
+    {
+        case "idToFind":
+            <button @onclick="(e) => Reset(e)">Reset #3</button> @* FN *@
+            break;
+        default:
+        {
+            <button @onclick="(e) => Reset(e)">Reset #4</button> @* FN *@
+            break;
+        }
+    }
+
+@foreach (var button in Buttons)
+{
+    {
+        <button @key="button.Id" @onclick="(e) => button.Action(e)"> @* Noncompliant, there is a direct block for the loop *@
+            Button #@button.Id
+        </button>
+    }
+}
+
+@code {
+    private List<Button> Buttons { get; } = new();
+
+    private void DoAction(MouseEventArgs e, Button button) { }
+
+    private class Button
+    {
+        public string? Id { get; } = Guid.NewGuid().ToString();
+        public Action<MouseEventArgs> Action { get; set; } = e => { };
+    }
+
+    private void Reset(MouseEventArgs mouseEventArgs) 
+    { 
+        foreach (var button in Buttons)
+        {
+            button.Action = e => { }; // Compliant
+        }
+    }
+}

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AvoidLambdaExpressionInLoopsInBlazor.RenderFragmentConsumer.razor

@@ -5,6 +5,13 @@
             Button #@button.Id
         </button>
     }
+
+    @foreach (var button in Buttons.Where(x => x.Id == "SomeId")) @* Compliant *@
+    {
+        <button @key="button.Id" @onclick="(e) => button.Action(e)"> @* Noncompliant *@
+            Button #@button.Id
+        </button>
+    }
 </AvoidLambdaExpressionInLoopsInBlazor_RenderFragment>
 
 @code {

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AvoidLambdaExpressionInLoopsInBlazor.cs

@@ -2,6 +2,7 @@
 using Microsoft.AspNetCore.Components.Rendering;
 using Microsoft.AspNetCore.Components.Web;
 using System;
+using System.Linq;
 using System.Collections.Generic;
 
 class LambdaInLoopInMethod
@@ -77,5 +78,7 @@ protected override void BuildRenderTree(RenderTreeBuilder builder)
             builder.AddMarkupContent(14, "\r\n        Button");
             builder.CloseElement();
         }
+
+        foreach (var button in Buttons.OrderByDescending(x => x.Id)) { } // Compliant, the lambda is executed outside of the loop
     }
 }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AvoidLambdaExpressionInLoopsInBlazor.razor

@@ -46,6 +46,11 @@
     <button @onclick="(e) => Reset(e)">Reset #2</button> @* Compliant *@
 }
 
+@foreach (var button in Buttons.OrderByDescending(x => x.Id)) @* Compliant, the lambda is executed outside of the loop *@
+{
+    <p>@button.Id</p>
+}
+
 @code {
     private List<Button> Buttons { get; } = new();
 

its/profiles/blazor_rules.xml

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?><!-- Generated by Sonar -->
+<profile>
+  <name>blazor_rules</name>
+  <language>cs</language>
+  <rules>
+      <rule>
+          <repositoryKey>csharpsquid</repositoryKey>
+          <key>S6797</key>
+          <priority>MAJOR</priority>
+      </rule>
+      <rule>
+          <repositoryKey>csharpsquid</repositoryKey>
+          <key>S6798</key>
+          <priority>MAJOR</priority>
+      </rule>
+      <rule>
+          <repositoryKey>csharpsquid</repositoryKey>
+          <key>S6800</key>
+          <priority>MAJOR</priority>
+      </rule>
+      <rule>
+          <repositoryKey>csharpsquid</repositoryKey>
+          <key>S6802</key>
+          <priority>MAJOR</priority>
+      </rule>
+      <rule>
+          <repositoryKey>csharpsquid</repositoryKey>
+          <key>S6803</key>
+          <priority>MAJOR</priority>
+      </rule>
+  </rules>
+</profile>

its/src/test/java/com/sonar/it/csharp/RazorClassLibProjectTest.java

@@ -19,8 +19,6 @@
  */
 package com.sonar.it.csharp;
 
-import com.sonar.it.shared.TestUtils;
-import com.sonar.orchestrator.build.ScannerForMSBuild;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -29,6 +27,7 @@
 import java.nio.file.Path;
 import java.util.stream.Collectors;
 
+import static com.sonar.it.csharp.Tests.ORCHESTRATOR;
 import static com.sonar.it.csharp.Tests.getComponent;
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -44,12 +43,13 @@ public class RazorClassLibProjectTest {
   private static final String S6798_FOLDER = "RazorClassLib:S6798";
   private static final String S6800_FOLDER = "RazorClassLib:S6800";
   private static final String S6802_FOLDER = "RazorClassLib:S6802";
-  private static final String SONAR_RULE_S6802 = "csharpsquid:S6802";
-  private static final String S6802_COMPONENT_RAZOR_FILE = "RazorClassLib:S6802/S6802.razor";
-  private static final String S6802_COMPONENT_CS_FILE = "RazorClassLib:S6802/S6802.cs";
 
   @BeforeAll
   public static void beforeAll() throws Exception {
+    // Create the project in SQ before setting the associated QualityPorfile
+    ORCHESTRATOR.getServer().provisionProject(PROJECT, PROJECT);
+    // Enable only Blazor rules, include those in non-SonarWay, through the blazor_rules profile
+    ORCHESTRATOR.getServer().associateProjectToQualityProfile(PROJECT, "cs", "blazor_rules");
     Tests.analyzeProject(PROJECT, temp, PROJECT);
   }
 
@@ -99,7 +99,7 @@ void issuesS6802AreRaised() {
 
     assertThat(issues).hasSize(4);
     assertThat(issues.stream().filter(issue -> issue.getComponent().equals(S6802_FOLDER + "/S6802.razor"))).hasSize(4);
-    assertThat(issues.stream().filter(issue -> issue.getComponent().equals(S6802_FOLDER + "/S6802.cs"))).hasSize(0);
+    assertThat(issues.stream().filter(issue -> issue.getComponent().equals(S6802_FOLDER + "/S6802.cs"))).isEmpty();
   }
 
   @Test

its/src/test/java/com/sonar/it/csharp/Tests.java

@@ -49,6 +49,7 @@ public class Tests implements BeforeAllCallback, AfterAllCallback {
     // otherwise .cshtml file are not pushed to SQ
     .addPlugin(MavenLocation.of("org.sonarsource.html", "sonar-html-plugin", "LATEST_RELEASE"))
     .restoreProfileAtStartup(FileLocation.of("profiles/no_rule.xml"))
+    .restoreProfileAtStartup(FileLocation.of("profiles/blazor_rules.xml"))
     .restoreProfileAtStartup(FileLocation.of("profiles/class_name.xml"))
     .restoreProfileAtStartup(FileLocation.of("profiles/template_rule.xml"))
     .restoreProfileAtStartup(FileLocation.of("profiles/custom_parameters.xml"))