From d9f9fc510f395a85851b583b496c452db886efb3 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 5 Feb 2020 19:48:47 +0100 Subject: [PATCH 1/3] C#: Add more tests for `cs/useless-assignment-to-local` --- .../Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs | 14 ++++++++++++++ .../DeadStoreOfLocal/DeadStoreOfLocal.expected | 2 ++ 2 files changed, 16 insertions(+) diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs index 0c6a4f44bbcd..b787401f9411 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs @@ -389,6 +389,20 @@ string M7(bool b) return s; return null; } + + string M8() + { + string s = default; // "GOOD" + s = ""; + return s; + } + + string M9() + { + var s = (string)null; // "GOOD" + s = ""; + return s; + } } class Anonymous diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected index 9720249c28d3..2c1b2fcabfa0 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected @@ -16,6 +16,8 @@ | DeadStoreOfLocal.cs:320:9:320:32 | ... = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:316:23:316:23 | b | b | | DeadStoreOfLocal.cs:361:13:361:20 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:361:13:361:13 | s | s | | DeadStoreOfLocal.cs:387:13:387:21 | ... = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:385:13:385:13 | s | s | +| DeadStoreOfLocal.cs:395:16:395:26 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:395:16:395:16 | s | s | +| DeadStoreOfLocal.cs:402:13:402:28 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:402:13:402:13 | s | s | | DeadStoreOfLocalBad.cs:7:13:7:48 | Boolean success = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:7:13:7:19 | success | success | | DeadStoreOfLocalBad.cs:23:32:23:32 | FormatException e | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:23:32:23:32 | e | e | | DeadStoreOfLocalBad.cs:32:22:32:22 | String s | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:32:22:32:22 | s | s | From 85e6b24c49627cb178ee070ff8f29bcb1c050e77 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 5 Feb 2020 20:03:50 +0100 Subject: [PATCH 2/3] C#: Remove false positives for `cs/useless-assignment-to-local` --- csharp/ql/src/Dead Code/DeadStoreOfLocal.ql | 2 +- .../Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/csharp/ql/src/Dead Code/DeadStoreOfLocal.ql b/csharp/ql/src/Dead Code/DeadStoreOfLocal.ql index 7181879ef8c2..2e8d1b92e02a 100644 --- a/csharp/ql/src/Dead Code/DeadStoreOfLocal.ql +++ b/csharp/ql/src/Dead Code/DeadStoreOfLocal.ql @@ -114,7 +114,7 @@ class RelevantDefinition extends AssignableDefinition { */ private predicate isDefaultLikeInitializer() { this.isInitializer() and - exists(Expr e | e = this.getSource() | + exists(Expr e | e = this.getSource().stripCasts() | exists(string val | val = e.getValue() | val = "0" or val = "-1" or diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected index 2c1b2fcabfa0..9720249c28d3 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.expected @@ -16,8 +16,6 @@ | DeadStoreOfLocal.cs:320:9:320:32 | ... = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:316:23:316:23 | b | b | | DeadStoreOfLocal.cs:361:13:361:20 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:361:13:361:13 | s | s | | DeadStoreOfLocal.cs:387:13:387:21 | ... = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:385:13:385:13 | s | s | -| DeadStoreOfLocal.cs:395:16:395:26 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:395:16:395:16 | s | s | -| DeadStoreOfLocal.cs:402:13:402:28 | String s = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.cs:402:13:402:13 | s | s | | DeadStoreOfLocalBad.cs:7:13:7:48 | Boolean success = ... | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:7:13:7:19 | success | success | | DeadStoreOfLocalBad.cs:23:32:23:32 | FormatException e | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:23:32:23:32 | e | e | | DeadStoreOfLocalBad.cs:32:22:32:22 | String s | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocalBad.cs:32:22:32:22 | s | s | From 69d9d4122abe9e181f04a602019f23de11c5a2d7 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 5 Feb 2020 20:08:54 +0100 Subject: [PATCH 3/3] C#: Add change note --- change-notes/1.24/analysis-csharp.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.24/analysis-csharp.md b/change-notes/1.24/analysis-csharp.md index bf2ae180eb15..e76890135061 100644 --- a/change-notes/1.24/analysis-csharp.md +++ b/change-notes/1.24/analysis-csharp.md @@ -20,6 +20,7 @@ The following changes in version 1.24 affect C# analysis in all applications. | Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the variable is named `_` in a `foreach` statement. | | Potentially dangerous use of non-short-circuit logic (`cs/non-short-circuit`) | Fewer false positive results | Results have been removed when the expression contains an `out` parameter. | | Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. | +| Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the value assigned is an (implicitly or explicitly) cast default-like value. For example, `var s = (string)null` and `string s = default`. | ## Removal of old queries