Fix S3247 FP: Rule should not trigger when variable is used for data binding on Razor/MVC views #8973
Conversation
gregory-paidis-sonarsource
force-pushed
the
greg/fix-S3247-FP
branch
from
996c0d3
to
8394783
Compare
| : parentStatement | ||
| .DescendantNodes() | ||
| .OfType<CastExpressionSyntax>() | ||
| .Where(x => x.Type.WithoutTrivia().IsEquivalentTo(castType.WithoutTrivia()) | ||
| && IsCastOnSameSymbol(x)) | ||
| && IsCastOnSameSymbol(x) | ||
| && !CSharpFacade.Instance.Syntax.IsInExpressionTree(context.SemanticModel, x)) // see https://github.com/SonarSource/sonar-dotnet/issues/8735#issuecomment-1943419398 |
There was a problem hiding this comment.
This is the relevant change, rest is cayc
| @@ -0,0 +1,10 @@ | |||
| @addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers | |||
There was a problem hiding this comment.
This is necessary for asp-for to be properly interpreted by the generator.
If it's not included, we get nothing of value from the razor generator, since it parses asp-for as a random tag that has no specific meaning.
If it is included, we get the thing on the right of asp-for to be bound inside a lambda on the .g.cs file, which is what we are trying to exclude from the rule.
gregory-paidis-sonarsource
force-pushed
the
greg/fix-S3247-FP
branch
from
8394783
to
c40ebb6
Compare
| .Single(); | ||
|
|
||
| compilation.AdditionalSourceFiles.Should().ContainSingle(x => x.EndsWith("Dummy.cshtml")); | ||
| compilation.Compilation.SyntaxTrees.Select(x => x.FilePath).Should().Contain(x => x.EndsWith("snippet.0.cs")); |
There was a problem hiding this comment.
These four tests are testing that:
- If a snippet name ends with
.cshtml, they are added as additional file to the compilation, to be processed by the razor generators. - If they either do not have a name or their name ends with
.cs, they are not added as additional file, but they are still part of the compilation as a SyntaxTree.
|
I do not understand the low coverage. |
…binding on Razor/MVC views
gregory-paidis-sonarsource
force-pushed
the
greg/fix-S3247-FP
branch
from
c40ebb6
to
943a216
Compare
|
|
I agree that it's strange, I debugged the code and some of the branches reported as uncovered are actually covered. |
|
Peach validation: S3247 no changes |
@martin-strecker-sonarsource that's fine, right? |
|
Yes. That's fine. |
Fixes #8735