From a9d45a2aa23130f28f5ea82ee29e19b6428ed898 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 5 Mar 2025 14:32:41 +0100 Subject: [PATCH 1/5] C#: Add some tests for cs/useless-if-statement. --- .../FutileConditional/FutileConditional.cs | 29 +++++++++++++++++++ .../FutileConditional.expected | 3 ++ .../FutileConditional/FutileConditional.qlref | 2 ++ .../Useless Code/FutileConditional/options | 2 ++ 4 files changed, 36 insertions(+) create mode 100644 csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs create mode 100644 csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected create mode 100644 csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.qlref create mode 100644 csharp/ql/test/query-tests/Useless Code/FutileConditional/options diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs new file mode 100644 index 000000000000..6ac6f707b553 --- /dev/null +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs @@ -0,0 +1,29 @@ +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 + { + } + else + { + Console.WriteLine("hello"); + } + + if (s.Length > 3) + { + } + else + { + } // $ Alert + } +} diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected new file mode 100644 index 000000000000..db9b75043cba --- /dev/null +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected @@ -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. | diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.qlref b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.qlref new file mode 100644 index 000000000000..6f3548889789 --- /dev/null +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.qlref @@ -0,0 +1,2 @@ +query: Useless code/FutileConditional.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/options b/csharp/ql/test/query-tests/Useless Code/FutileConditional/options new file mode 100644 index 000000000000..75c39b4541ba --- /dev/null +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/options @@ -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 From 361bdfac1289334c1668a997c8e43a9ae3ddf4cd Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 5 Mar 2025 15:22:22 +0100 Subject: [PATCH 2/5] C#: Add a testcase with an empty if statement containing a comment. --- .../Useless Code/FutileConditional/FutileConditional.cs | 5 +++++ .../FutileConditional/FutileConditional.expected | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs index 6ac6f707b553..fffdaf9de743 100644 --- a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs @@ -25,5 +25,10 @@ public void M(string s) else { } // $ Alert + + if (s.Length > 4) + { + // GOOD: Because of the comment. + } } } diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected index db9b75043cba..475b1c345d1a 100644 --- a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected @@ -1,3 +1,7 @@ +#select | 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. | +| FutileConditional.cs:29:9:32:9 | if (...) ... | If-statement with an empty then-branch and no else-branch. | +testFailures +| FutileConditional.cs:29:9:32:9 | If-statement with an empty then-branch and no else-branch. | Unexpected result: Alert | From 35fbaf4ac33274558d78d6b432bd7e78dbdf73ec Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 5 Mar 2025 15:26:39 +0100 Subject: [PATCH 3/5] C#: Do flag empty if statements if there is a comment in cs/useless-if-statement. --- csharp/ql/src/Useless code/FutileConditional.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/csharp/ql/src/Useless code/FutileConditional.ql b/csharp/ql/src/Useless code/FutileConditional.ql index 1000113185d4..d77fd55a434a 100644 --- a/csharp/ql/src/Useless code/FutileConditional.ql +++ b/csharp/ql/src/Useless code/FutileConditional.ql @@ -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)) From dd7d5d031c9fad3d7dfacaa7b443196a9a827890 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 5 Mar 2025 15:27:01 +0100 Subject: [PATCH 4/5] C#: Update test expected output. --- .../Useless Code/FutileConditional/FutileConditional.expected | 4 ---- 1 file changed, 4 deletions(-) diff --git a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected index 475b1c345d1a..db9b75043cba 100644 --- a/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected +++ b/csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.expected @@ -1,7 +1,3 @@ -#select | 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. | -| FutileConditional.cs:29:9:32:9 | if (...) ... | If-statement with an empty then-branch and no else-branch. | -testFailures -| FutileConditional.cs:29:9:32:9 | If-statement with an empty then-branch and no else-branch. | Unexpected result: Alert | From c73eeec8145888ce29b92a85e3a34fb449e3f124 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 5 Mar 2025 15:33:02 +0100 Subject: [PATCH 5/5] C#: Add change note. --- csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md diff --git a/csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md b/csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md new file mode 100644 index 000000000000..3d62fe373e1e --- /dev/null +++ b/csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md @@ -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.