SONARHTML-472 Handle plain C# conditionals in Razor code blocks - #807
SONARHTML-472 Handle plain C# conditionals in Razor code blocks#807victor-peixoto-sonarsource wants to merge 19 commits into
Conversation
Ruling Report✅ No changes to ruling expected issues in this PR |
This comment has been minimized.
This comment has been minimized.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| escapedCSharpStringCharacter = false; | ||
| } else if (!verbatimCSharpString && current == '\\') { | ||
| escapedCSharpStringCharacter = true; | ||
| } else if (current == csharpStringDelimiter) { |
There was a problem hiding this comment.
This closes the C# string on any ", but an interpolated string can contain nested string literals inside its {...} holes. For valid Razor like var marker = $"{(true ? "}" : "{")}";, the " before } is read as ending the string, so the following } is counted as a real brace and closes the @{ } block early. The if/else after it then looks unconditional and S7930 reports a false duplicate id. A test with $"..." containing inner quotes would cover this.
| if (pendingRenderedClosingBraceDepth < 0) { | ||
| return; | ||
| } | ||
| if (currentElementDepth() >= pendingRenderedClosingBraceDepth && !conditionalBraces.isEmpty()) { |
There was a problem hiding this comment.
False positive: a literal } in markup text closes the conditional scope
When a lone } appears in rendered markup text and is followed by a child element (no end tag in between), this guard pops the conditional brace and closes the surrounding @if/else scope early. Both branches then look unconditional and S7930 fires a false-positive duplicate id.
Example (differs only by the } in the text):
@{
@if (Model.ShowPrimary)
{
<div>Total is 100} percent
<span id="choice">First</span>
</div>
}
else
{
<span id="choice">Second</span>
}
}Result: S7930 line 10: Duplicate id "choice" found. First occurrence was on line 5. The same file without the } is clean. Reproduced against the compiled scanner.
Why: the literal } sets pendingRenderedClosingBraceDepth at the <div> depth. The wrapper's end tag would normally discard it, but the child <span> arrives first, so currentElementDepth() >= pending holds and the conditional is popped even though the } was just text.
Suggestion: only resolve a pending brace when it actually balances an opening markup brace, and add a test with a literal } in branch markup text followed by a child element.
🤖 Generated with GitHub Actions
Code Review ✅ Approved 13 resolved / 13 findingsAdds C# conditional branch tracking to the Razor lexer to recognize plain ✅ 13 resolved✅ Bug: elementDepth desync silently disables C# conditional tracking
✅ Bug: Unbalanced brace in markup leaves Razor code block open forever
✅ Edge Case: Verbatim C# string detection misses @$"..." and fragment starts
✅ Bug: Balanced braces in rendered Razor markup now close the branch early
✅ Quality: Cross-fragment verbatim-prefix carry is unreachable via the lexer
...and 8 more resolved from earlier reviews Implementation Status ✅ 5 of 5 objectives covered✅ SONARHTML-472 - 5 of 5 objectives coveredThis PR implements handling of plain C# conditionals and comments inside Razor blocks, adds appropriate regression fixtures, and ensures genuine duplicates remain reported. ✅ 5 covered here
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
2 participants
Part of SONARHTML-472
Summary
Testing