Skip to content

C#: Fewer alerts in cs/useless-if-statement. #18935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 6, 2025
Merged

C#: Fewer alerts in cs/useless-if-statement. #18935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9d45a2
C#: Add some tests for cs/useless-if-statement.
michaelnebel Mar 5, 2025
361bdfa
C#: Add a testcase with an empty if statement containing a comment.
michaelnebel Mar 5, 2025
35fbaf4
C#: Do flag empty if statements if there is a comment in cs/useless-i…
michaelnebel Mar 5, 2025
dd7d5d0
C#: Update test expected output.
michaelnebel Mar 5, 2025
c73eeec
C#: Add change note.
michaelnebel Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion csharp/ql/src/Useless code/FutileConditional.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ predicate emptyStmt(Stmt s) {
or
s =
any(BlockStmt bs |
bs.getNumberOfStmts() = 0
bs.getNumberOfStmts() = 0 and
not any(CommentBlock cb).getParent() = bs
or
bs.getNumberOfStmts() = 1 and
emptyStmt(bs.getStmt(0))
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Don't consider an if-statement to be *useless* in `cs/useless-if-statement` if there is at least a comment.
34 changes: 34 additions & 0 deletions csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using System;

class FutileConditionalTest
{

public void M(string s)
{
if (s.Length > 0) ; // $ Alert

if (s.Length > 1)
{
} // $ Alert

if (s.Length > 2) // GOOD: because of else-branch
{
}
Comment on lines +14 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to your change, I find it odd to consider this okay.

Copy link
Contributor Author

@michaelnebel michaelnebel Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. Maybe an idea for a quality query:
Create a new quality query which states that the condition should be inverted (we can also provide good QHelp for that - and I suspect an LLM will be good at figuring out how the fix should look like).

Copy link
Contributor Author

@michaelnebel michaelnebel Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm there is already a button for that in VSCode, so it might not be relevant (but you will have to actively click that).

else
{
Console.WriteLine("hello");
}

if (s.Length > 3)
{
}
else
{
} // $ Alert

if (s.Length > 4)
{
// GOOD: Because of the comment.
}
}
}
3 changes: 3 additions & 0 deletions csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
| FutileConditional.cs:8:9:8:27 | if (...) ... | If-statement with an empty then-branch and no else-branch. |
| FutileConditional.cs:10:9:12:9 | if (...) ... | If-statement with an empty then-branch and no else-branch. |
| FutileConditional.cs:22:9:27:9 | if (...) ... | If-statement with an empty then-branch and no else-branch. |
2 changes: 2 additions & 0 deletions csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.qlref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Useless code/FutileConditional.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql
2 changes: 2 additions & 0 deletions csharp/ql/test/query-tests/Useless Code/FutileConditional/options
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
semmle-extractor-options: /nostdlib /noconfig
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj