From 5eb305da9310bcd2ff1b6f752ed7dc5142351d12 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 24 Feb 2025 13:42:30 +0100 Subject: [PATCH 1/4] C#: Add some value tuple examples for cs/call-to-object-tostring and update test expected output. --- .../Useless Code/DefaultToString/DefaultToString.cs | 10 ++++++++++ .../DefaultToString/DefaultToString.expected | 6 +++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs index 6fe7850d781b..cd3460a09cc2 100644 --- a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs +++ b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs @@ -35,6 +35,16 @@ void M() IPublic g = null; Console.WriteLine(g); // GOOD + + Console.WriteLine(new ValueTuple(1, 2)); // GOOD [FALSE POSITIVE] + + Console.WriteLine((1, 2)); // GOOD [FALSE POSITIVE] + + var t1 = new ValueTuple(1, new DefaultToString()); + Console.WriteLine(t1); // BAD + + var t2 = new ValueTuple(new A(), new D()); + Console.WriteLine(t2); // GOOD [FALSE POSITIVE] } class A diff --git a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected index b3f3dcdd72de..b03646dc566c 100644 --- a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected +++ b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected @@ -2,7 +2,11 @@ | DefaultToString.cs:10:28:10:28 | access to local variable d | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToString.cs:4:14:4:28 | DefaultToString | DefaultToString | | DefaultToString.cs:16:27:16:30 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | | DefaultToString.cs:19:24:19:27 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | -| DefaultToString.cs:34:27:34:27 | access to local variable f | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToString.cs:62:23:62:30 | IPrivate | IPrivate | +| DefaultToString.cs:34:27:34:27 | access to local variable f | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToString.cs:72:23:72:30 | IPrivate | IPrivate | +| DefaultToString.cs:39:27:39:56 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,Int32) | (Int32,Int32) | +| DefaultToString.cs:41:27:41:32 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,Int32) | (Int32,Int32) | +| DefaultToString.cs:44:27:44:28 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,DefaultToString) | (Int32,DefaultToString) | +| DefaultToString.cs:47:27:47:28 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (A,D) | (A,D) | | DefaultToStringBad.cs:8:35:8:35 | access to local variable p | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToStringBad.cs:14:11:14:16 | Person | Person | | DefaultToStringBad.cs:11:38:11:41 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | | WriteLineArray.cs:7:23:7:26 | access to parameter args | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | String[] | String[] | From a85131bf0fcc811880f7122a409b9d5b5b203538 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 24 Feb 2025 13:53:16 +0100 Subject: [PATCH 2/4] C#: Better handling of (value) tuple types in cs/call-to-object-tostring. --- csharp/ql/src/Useless code/DefaultToStringQuery.qll | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/csharp/ql/src/Useless code/DefaultToStringQuery.qll b/csharp/ql/src/Useless code/DefaultToStringQuery.qll index 9185756b0a95..f11b5a9cd324 100644 --- a/csharp/ql/src/Useless code/DefaultToStringQuery.qll +++ b/csharp/ql/src/Useless code/DefaultToStringQuery.qll @@ -46,6 +46,7 @@ private predicate alwaysInvokesToString(ParameterRead pr) { * method from `System.Object` or `System.ValueType`. */ predicate alwaysDefaultToString(ValueOrRefType t) { + not t instanceof TupleType and exists(ToStringMethod m | t.hasMethod(m) | m.getDeclaringType() instanceof SystemObjectClass or m.getDeclaringType() instanceof SystemValueTypeClass @@ -55,6 +56,11 @@ predicate alwaysDefaultToString(ValueOrRefType t) { overriding.getABaseType+() = t ) and ((t.isAbstract() or t instanceof Interface) implies not t.isEffectivelyPublic()) + or + exists(ValueOrRefType elem | + elem = t.(TupleType).getElementType(_) and + alwaysDefaultToString(elem) + ) } class DefaultToStringType extends ValueOrRefType { From 97f9f0ccc5ed54bdf1cba4303d9060e8d55613fd Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 24 Feb 2025 13:53:50 +0100 Subject: [PATCH 3/4] C#: Update test expected output. --- .../Useless Code/DefaultToString/DefaultToString.cs | 6 +++--- .../Useless Code/DefaultToString/DefaultToString.expected | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs index cd3460a09cc2..2d6ea91f71e0 100644 --- a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs +++ b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.cs @@ -36,15 +36,15 @@ void M() IPublic g = null; Console.WriteLine(g); // GOOD - Console.WriteLine(new ValueTuple(1, 2)); // GOOD [FALSE POSITIVE] + Console.WriteLine(new ValueTuple(1, 2)); // GOOD - Console.WriteLine((1, 2)); // GOOD [FALSE POSITIVE] + Console.WriteLine((1, 2)); // GOOD var t1 = new ValueTuple(1, new DefaultToString()); Console.WriteLine(t1); // BAD var t2 = new ValueTuple(new A(), new D()); - Console.WriteLine(t2); // GOOD [FALSE POSITIVE] + Console.WriteLine(t2); // GOOD } class A diff --git a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected index b03646dc566c..c856d4ff4d00 100644 --- a/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected +++ b/csharp/ql/test/query-tests/Useless Code/DefaultToString/DefaultToString.expected @@ -3,10 +3,7 @@ | DefaultToString.cs:16:27:16:30 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | | DefaultToString.cs:19:24:19:27 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | | DefaultToString.cs:34:27:34:27 | access to local variable f | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToString.cs:72:23:72:30 | IPrivate | IPrivate | -| DefaultToString.cs:39:27:39:56 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,Int32) | (Int32,Int32) | -| DefaultToString.cs:41:27:41:32 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,Int32) | (Int32,Int32) | | DefaultToString.cs:44:27:44:28 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (Int32,DefaultToString) | (Int32,DefaultToString) | -| DefaultToString.cs:47:27:47:28 | (...) ... | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | (A,D) | (A,D) | | DefaultToStringBad.cs:8:35:8:35 | access to local variable p | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | DefaultToStringBad.cs:14:11:14:16 | Person | Person | | DefaultToStringBad.cs:11:38:11:41 | access to local variable ints | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | Int32[] | Int32[] | | WriteLineArray.cs:7:23:7:26 | access to parameter args | Default 'ToString()': $@ inherits 'ToString()' from 'Object', and so is not suitable for printing. | file://:0:0:0:0 | String[] | String[] | From e8f86e41f4fecdd1d065f021644ce1b97bbd9087 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 24 Feb 2025 13:58:55 +0100 Subject: [PATCH 4/4] C#: Add change note. --- csharp/ql/src/change-notes/2025-02-24-object-tostring.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2025-02-24-object-tostring.md diff --git a/csharp/ql/src/change-notes/2025-02-24-object-tostring.md b/csharp/ql/src/change-notes/2025-02-24-object-tostring.md new file mode 100644 index 000000000000..9dff09fb07a3 --- /dev/null +++ b/csharp/ql/src/change-notes/2025-02-24-object-tostring.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* C#: Improve precision of the query `cs/call-to-object-tostring` for value tuples.