Skip to content

Commit f038e2f

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #19589 from michaelnebel/csharp/dereference
C#: Improve `cs/dereference-*` queries and add to the Code Quality suite.
2 parents 75caa18 + d2b8bd5 commit f038e2f

File tree

26 files changed

+834
-783
lines changed

26 files changed

+834
-783
lines changed

‎csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ ql/csharp/ql/src/API Abuse/CallToGCCollect.ql
22
ql/csharp/ql/src/API Abuse/FormatInvalid.ql
33
ql/csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql
44
ql/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql
5+
ql/csharp/ql/src/CSI/NullAlways.ql
6+
ql/csharp/ql/src/CSI/NullMaybe.ql
57
ql/csharp/ql/src/Dead Code/DeadStoreOfLocal.ql
68
ql/csharp/ql/src/Language Abuse/MissedReadonlyOpportunity.ql
79
ql/csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql

‎csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,13 @@ class Dereference extends G::DereferenceableExpr {
544544
p.hasExtensionMethodModifier() and
545545
not emc.isConditional()
546546
|
547-
p.fromSource() // assume all non-source extension methods perform a dereference
548-
implies
547+
// Assume all non-source extension methods on
548+
// (1) nullable types are null-safe
549+
// (2) non-nullable types are doing a dereference.
550+
p.fromLibrary() and
551+
not p.getAnnotatedType().isNullableRefType()
552+
or
553+
p.fromSource() and
549554
exists(
550555
Ssa::ImplicitParameterDefinition def,
551556
AssignableDefinitions::ImplicitParameterDefinition pdef

‎csharp/ql/lib/semmle/code/csharp/frameworks/system/Diagnostics.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ class SystemDiagnosticsDebugClass extends SystemDiagnosticsClass {
4141
/** Gets an `Assert(bool, ...)` method. */
4242
Method getAssertMethod() {
4343
result.getDeclaringType() = this and
44-
result.hasName("Assert") and
45-
result.getParameter(0).getType() instanceof BoolType and
46-
result.getReturnType() instanceof VoidType
44+
result.hasName("Assert")
4745
}
4846
}
4947

‎csharp/ql/src/CSI/NullAlways.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* correctness
1010
* exceptions
1111
* external/cwe/cwe-476
12+
* quality
1213
*/
1314

1415
import csharp

‎csharp/ql/src/CSI/NullMaybe.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* correctness
1111
* exceptions
1212
* external/cwe/cwe-476
13+
* quality
1314
*/
1415

1516
import csharp

‎csharp/ql/src/change-notes/2025-06-03-dereferece-extension-method.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The queries `cs/dereferenced-value-is-always-null` and `cs/dereferenced-value-may-be-null` have been improved to reduce false positives. The queries no longer assume that expressions are dereferenced when passed as the receiver (`this` parameter) to extension methods where that parameter is a nullable type.

‎csharp/ql/test/query-tests/Nullness/A.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class A
55
public void Lock()
66
{
77
object synchronizedAlways = null;
8-
lock (synchronizedAlways) // BAD (always)
8+
lock (synchronizedAlways) // $ Alert[cs/dereferenced-value-is-always-null]
99
{
1010
synchronizedAlways.GetHashCode(); // GOOD
1111
}
@@ -14,7 +14,7 @@ public void Lock()
1414
public void ArrayAssignTest()
1515
{
1616
int[] arrayNull = null;
17-
arrayNull[0] = 10; // BAD (always)
17+
arrayNull[0] = 10; // $ Alert[cs/dereferenced-value-is-always-null]
1818

1919
int[] arrayOk;
2020
arrayOk = new int[10];
@@ -28,10 +28,10 @@ public void Access()
2828
object methodAccess = null;
2929
object methodCall = null;
3030

31-
Console.WriteLine(arrayAccess[1]); // BAD (always)
32-
Console.WriteLine(fieldAccess.Length); // BAD (always)
33-
Func<String> tmp = methodAccess.ToString; // BAD (always)
34-
Console.WriteLine(methodCall.ToString()); // BAD (always)
31+
Console.WriteLine(arrayAccess[1]); // $ Alert[cs/dereferenced-value-is-always-null]
32+
Console.WriteLine(fieldAccess.Length); // $ Alert[cs/dereferenced-value-is-always-null]
33+
Func<String> tmp = methodAccess.ToString; // $ Alert[cs/dereferenced-value-is-always-null]
34+
Console.WriteLine(methodCall.ToString()); // $ Alert[cs/dereferenced-value-is-always-null]
3535

3636
Console.WriteLine(arrayAccess[1]); // GOOD
3737
Console.WriteLine(fieldAccess.Length); // GOOD
@@ -47,7 +47,7 @@ public void OutOrRef()
4747

4848
object varRef = null;
4949
TestMethod2(ref varRef);
50-
varRef.ToString(); // BAD (always)
50+
varRef.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
5151

5252
varRef = null;
5353
TestMethod3(ref varRef);

‎csharp/ql/test/query-tests/Nullness/Assert.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,23 @@ void Fn(bool b)
1212

1313
s = b ? null : "";
1414
Assert.IsNull(s);
15-
Console.WriteLine(s.Length); // BAD (always)
15+
Console.WriteLine(s.Length); // $ Alert[cs/dereferenced-value-is-always-null]
1616

1717
s = b ? null : "";
1818
Assert.IsNotNull(s);
1919
Console.WriteLine(s.Length); // GOOD
2020

2121
s = b ? null : "";
2222
Assert.IsTrue(s == null);
23-
Console.WriteLine(s.Length); // BAD (always)
23+
Console.WriteLine(s.Length); // $ Alert[cs/dereferenced-value-is-always-null]
2424

2525
s = b ? null : "";
2626
Assert.IsTrue(s != null);
2727
Console.WriteLine(s.Length); // GOOD
2828

2929
s = b ? null : "";
3030
Assert.IsFalse(s != null);
31-
Console.WriteLine(s.Length); // BAD (always)
31+
Console.WriteLine(s.Length); // $ Alert[cs/dereferenced-value-is-always-null]
3232

3333
s = b ? null : "";
3434
Assert.IsFalse(s == null);
@@ -44,10 +44,10 @@ void Fn(bool b)
4444

4545
s = b ? null : "";
4646
Assert.IsTrue(s == null && b);
47-
Console.WriteLine(s.Length); // BAD (always)
47+
Console.WriteLine(s.Length); // $ Alert[cs/dereferenced-value-is-always-null]
4848

4949
s = b ? null : "";
5050
Assert.IsFalse(s != null || !b);
51-
Console.WriteLine(s.Length); // BAD (always)
51+
Console.WriteLine(s.Length); // $ Alert[cs/dereferenced-value-is-always-null]
5252
}
5353
}

‎csharp/ql/test/query-tests/Nullness/B.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ public void OperatorCall()
1010
B neqCallAlways = null;
1111

1212
if (eqCallAlways == null)
13-
eqCallAlways.ToString(); // BAD (always)
13+
eqCallAlways.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
1414

1515
if (b2 != null)
1616
b2.ToString(); // GOOD
@@ -21,7 +21,7 @@ public void OperatorCall()
2121

2222
if (neqCallAlways != null) { }
2323
else
24-
neqCallAlways.ToString(); // BAD (always)
24+
neqCallAlways.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
2525
}
2626

2727
public static bool operator ==(B b1, B b2)

‎csharp/ql/test/query-tests/Nullness/C.cs

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void NotTest()
1515

1616
if (!(o != null))
1717
{
18-
o.GetHashCode(); // BAD (always)
18+
o.GetHashCode(); // $ Alert[cs/dereferenced-value-is-always-null]
1919
}
2020
}
2121

@@ -39,7 +39,7 @@ public void AssertTest()
3939
{
4040
var s = Maybe() ? null : "";
4141
Debug.Assert(s == null);
42-
s.ToString(); // BAD (always)
42+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
4343

4444
s = Maybe() ? null : "";
4545
Debug.Assert(s != null);
@@ -50,22 +50,22 @@ public void AssertNullTest()
5050
{
5151
var o1 = new object();
5252
AssertNull(o1);
53-
o1.ToString(); // BAD (always) (false negative)
53+
o1.ToString(); // $ MISSING: Alert[cs/dereferenced-value-is-always-null]
5454

5555
var o2 = Maybe() ? null : "";
5656
Assert.IsNull(o2);
57-
o2.ToString(); // BAD (always)
57+
o2.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
5858
}
5959

6060
public void AssertNotNullTest()
6161
{
62-
var o1 = Maybe() ? null : new object();
62+
var o1 = Maybe() ? null : new object(); // $ Source[cs/dereferenced-value-may-be-null]
6363
AssertNonNull(o1);
64-
o1.ToString(); // GOOD (false positive)
64+
o1.ToString(); // $ SPURIOUS (false positive): Alert[cs/dereferenced-value-may-be-null]
6565

66-
var o2 = Maybe() ? null : new object();
66+
var o2 = Maybe() ? null : new object(); // $ Source[cs/dereferenced-value-may-be-null]
6767
AssertNonNull(o1);
68-
o2.ToString(); // BAD (maybe)
68+
o2.ToString(); // $ Alert[cs/dereferenced-value-may-be-null]
6969

7070
var o3 = Maybe() ? null : new object();
7171
Assert.IsNotNull(o3);
@@ -91,16 +91,16 @@ public void InstanceOf()
9191

9292
public void Lock()
9393
{
94-
var o = Maybe() ? null : new object();
95-
lock (o) // BAD (maybe)
94+
var o = Maybe() ? null : new object(); // $ Source[cs/dereferenced-value-may-be-null]
95+
lock (o) // $ Alert[cs/dereferenced-value-may-be-null]
9696
o.ToString(); // GOOD
9797
}
9898

9999
public void Foreach(IEnumerable<int> list)
100100
{
101101
if (Maybe())
102-
list = null;
103-
foreach (var x in list) // BAD (maybe)
102+
list = null; // $ Source[cs/dereferenced-value-may-be-null]
103+
foreach (var x in list) // $ Alert[cs/dereferenced-value-may-be-null]
104104
{
105105
x.ToString(); // GOOD
106106
list.ToString(); // GOOD
@@ -159,23 +159,23 @@ public void DoWhile()
159159
s = null;
160160
do
161161
{
162-
s.ToString(); // BAD (always)
162+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
163163
s = null;
164164
}
165165
while (s != null);
166166

167167
s = null;
168168
do
169169
{
170-
s.ToString(); // BAD (always)
170+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
171171
}
172172
while (s != null);
173173

174174
s = "";
175175
do
176176
{
177-
s.ToString(); // BAD (maybe)
178-
s = null;
177+
s.ToString(); // $ Alert[cs/dereferenced-value-may-be-null]
178+
s = null; // $ Source[cs/dereferenced-value-may-be-null]
179179
}
180180
while (true);
181181
}
@@ -193,15 +193,15 @@ public void While()
193193
s = null;
194194
while (b)
195195
{
196-
s.ToString(); // BAD (always)
196+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
197197
s = null;
198198
}
199199

200200
s = "";
201201
while (true)
202202
{
203-
s.ToString(); // BAD (maybe)
204-
s = null;
203+
s.ToString(); // $ Alert[cs/dereferenced-value-may-be-null]
204+
s = null; // $ Source[cs/dereferenced-value-may-be-null]
205205
}
206206
}
207207

@@ -215,12 +215,12 @@ public void If()
215215
}
216216

217217
if (s == null)
218-
s.ToString(); // BAD (always)
218+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
219219

220220
s = "";
221221
if (s != null && s.Length % 2 == 0)
222-
s = null;
223-
s.ToString(); // BAD (maybe)
222+
s = null; // $ Source[cs/dereferenced-value-may-be-null]
223+
s.ToString(); // $ Alert[cs/dereferenced-value-may-be-null]
224224
}
225225

226226
public void For()
@@ -230,23 +230,23 @@ public void For()
230230
{
231231
s.ToString(); // GOOD
232232
}
233-
s.ToString(); // BAD (always)
233+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
234234

235235
for (s = null; s == null; s = null)
236236
{
237-
s.ToString(); // BAD (always)
237+
s.ToString(); // $ Alert[cs/dereferenced-value-is-always-null]
238238
}
239239

240-
for (s = ""; ; s = null)
240+
for (s = ""; ; s = null) // $ Source[cs/dereferenced-value-may-be-null]
241241
{
242-
s.ToString(); // BAD (maybe)
242+
s.ToString(); // $ Alert[cs/dereferenced-value-may-be-null]
243243
}
244244
}
245245

246246
public void ArrayAssignTest()
247247
{
248248
int[] a = null;
249-
a[0] = 10; // BAD (always)
249+
a[0] = 10; // $ Alert[cs/dereferenced-value-is-always-null]
250250

251251
a = new int[10];
252252
a[0] = 42; // GOOD
@@ -257,8 +257,8 @@ public void Access()
257257
int[] ia = null;
258258
string[] sa = null;
259259

260-
ia[1] = 0; // BAD (always)
261-
var temp = sa.Length; // BAD (always)
260+
ia[1] = 0; // $ Alert[cs/dereferenced-value-is-always-null]
261+
var temp = sa.Length; // $ Alert[cs/dereferenced-value-is-always-null]
262262

263263
ia[1] = 0; // BAD (always), but not first
264264
temp = sa.Length; // BAD (always), but not first

0 commit comments

Comments
 (0)