hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-24 04:36:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13885 from michaelnebel/csharp/linqforeach

Browse Source 
C#: LINQ recommendation queries.
This commit is contained in:
Michael Nebel
2023-08-10 14:55:11 +02:00
committed by GitHub
parent f9fc79b16f f67d5e1dc6
commit f6aca58dbb
14 changed files with 224 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
csharp/ql/lib/Linq/Helpers.qll
View File

@@ -3,7 +3,9 @@
* Provides helper classes and methods related to LINQ.
*/
import csharp
private import csharp
private import semmle.code.csharp.frameworks.system.collections.Generic as GenericCollections
private import semmle.code.csharp.frameworks.system.Collections as Collections
//#################### PREDICATES ####################
private Stmt firstStmt(ForeachStmt fes) {
@@ -29,13 +31,40 @@ predicate isIEnumerableType(ValueOrRefType t) {
)
}
/**
* A class of foreach statements where the iterable expression
* supports the use of the LINQ extension methods on `IEnumerable<T>`.
*/
class ForeachStmtGenericEnumerable extends ForeachStmt {
ForeachStmtGenericEnumerable() {
exists(ValueOrRefType t | t = this.getIterableExpr().getType() |
t.getABaseType*().getUnboundDeclaration() instanceof
GenericCollections::SystemCollectionsGenericIEnumerableTInterface or
t.(ArrayType).getRank() = 1
)
}
}
/**
* A class of foreach statements where the iterable expression
* supports the use of the LINQ extension methods on `IEnumerable`.
*/
class ForeachStmtEnumerable extends ForeachStmt {
ForeachStmtEnumerable() {
exists(ValueOrRefType t | t = this.getIterableExpr().getType() |
t.getABaseType*() instanceof Collections::SystemCollectionsIEnumerableInterface or
t.(ArrayType).getRank() = 1
)
}
}
/**
* Holds if `foreach` statement `fes` could be converted to a `.All()` call.
* That is, the `ForeachStmt` contains a single `if` with a condition that
* accesses the loop variable and with a body that assigns `false` to a variable
* and `break`s out of the `foreach`.
*/
predicate missedAllOpportunity(ForeachStmt fes) {
predicate missedAllOpportunity(ForeachStmtGenericEnumerable fes) {
exists(IfStmt is |
// The loop contains an if statement with no else case, and nothing else.
is = firstStmt(fes) and
@@ -54,12 +83,12 @@ predicate missedAllOpportunity(ForeachStmt fes) {
}
/**
* Holds if `foreach` statement `fes` could be converted to a `.Cast()` call.
* Holds if the `foreach` statement `fes` can be converted to a `.Cast()` call.
* That is, the loop variable is accessed only in the first statement of the
* block, and the access is a cast. The first statement needs to be a
* `LocalVariableDeclStmt`.
* block, the access is a cast, and the first statement is a
* local variable declaration statement `s`.
*/
predicate missedCastOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
predicate missedCastOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
@@ -71,12 +100,12 @@ predicate missedCastOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
}
/**
* Holds if `foreach` statement `fes` could be converted to an `.OfType()` call.
* Holds if `foreach` statement `fes` can be converted to an `.OfType()` call.
* That is, the loop variable is accessed only in the first statement of the
* block, and the access is a cast with the `as` operator. The first statement
* needs to be a `LocalVariableDeclStmt`.
* block, the access is a cast with the `as` operator, and the first statement
* is a local variable declaration statement `s`.
*/
predicate missedOfTypeOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
predicate missedOfTypeOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
@@ -88,12 +117,12 @@ predicate missedOfTypeOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
}
/**
* Holds if `foreach` statement `fes` could be converted to a `.Select()` call.
* Holds if `foreach` statement `fes` can be converted to a `.Select()` call.
* That is, the loop variable is accessed only in the first statement of the
* block, and the access is not a cast. The first statement needs to be a
* `LocalVariableDeclStmt`.
* block, the access is not a cast, and the first statement is a
* local variable declaration statement `s`.
*/
predicate missedSelectOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
predicate missedSelectOpportunity(ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
@@ -107,7 +136,7 @@ predicate missedSelectOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
* variable, and the body of the `if` is either a `continue` or there's nothing
* else in the loop than the `if`.
*/
predicate missedWhereOpportunity(ForeachStmt fes, IfStmt is) {
predicate missedWhereOpportunity(ForeachStmtGenericEnumerable fes, IfStmt is) {
// The very first thing the foreach loop does is test its iteration variable.
is = firstStmt(fes) and
exists(VariableAccess va |

3
csharp/ql/src/Linq/MissedAllOpportunity.ql
View File

@@ -10,7 +10,6 @@
* language-features
*/
import csharp
import Linq.Helpers
/*
@@ -31,7 +30,7 @@ import Linq.Helpers
* bool allEven = lst.All(i => i % 2 == 0);
*/
from ForeachStmt fes
from ForeachStmtGenericEnumerable fes
where missedAllOpportunity(fes)
select fes,
"This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'."

2
csharp/ql/src/Linq/MissedCastOpportunity.ql
View File

@@ -13,7 +13,7 @@
import csharp
import Linq.Helpers
from ForeachStmt fes, LocalVariableDeclStmt s
from ForeachStmtEnumerable fes, LocalVariableDeclStmt s
where missedCastOpportunity(fes, s)
select fes,
"This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'.",

2
csharp/ql/src/Linq/MissedOfTypeOpportunity.ql
View File

@@ -13,7 +13,7 @@
import csharp
import Linq.Helpers
from ForeachStmt fes, LocalVariableDeclStmt s
from ForeachStmtEnumerable fes, LocalVariableDeclStmt s
where missedOfTypeOpportunity(fes, s)
select fes,
"This foreach loop immediately uses 'as' to $@ - consider using '.OfType(...)' instead.", s,

2
csharp/ql/src/Linq/MissedSelectOpportunity.ql
View File

@@ -20,7 +20,7 @@ predicate oversized(LocalVariableDeclStmt s) {
)
}
from ForeachStmt fes, LocalVariableDeclStmt s
from ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s
where
missedSelectOpportunity(fes, s) and
not oversized(s)

2
csharp/ql/src/Linq/MissedWhereOpportunity.ql
View File

@@ -12,7 +12,7 @@
import csharp
import Linq.Helpers
from ForeachStmt fes, IfStmt is
from ForeachStmtGenericEnumerable fes, IfStmt is
where
missedWhereOpportunity(fes, is) and
not missedAllOpportunity(fes)

78
csharp/ql/test/query-tests/Linq/MissedCastOpportunity/MissedCastOpportunity.cs Normal file
View File

@@ -0,0 +1,78 @@
using System;
using System.Collections;
using System.Collections.Generic;
class MissedCastOpportunity
{
public void M1(List<Animal> animals)
{
// BAD: Can be replaced with animals.Cast<Dog>().
foreach (Animal a in animals)
{
Dog d = (Dog)a;
d.Woof();
}
}
public void M2(NonEnumerableClass nec)
{
// GOOD: Not possible to use Linq here.
foreach (Animal a in nec)
{
Dog d = (Dog)a;
d.Woof();
}
}
public void M3(Animal[] animals)
{
// BAD: Can be replaced with animals.Cast<Dog>().
foreach (Animal animal in animals)
{
Dog d = (Dog)animal;
d.Woof();
}
}
public void M4(Array animals)
{
// BAD: Can be replaced with animals.Cast<Dog>().
foreach (Animal animal in animals)
{
Dog d = (Dog)animal;
d.Woof();
}
}
public void M5(IEnumerable animals)
{
// BAD: Can be replaced with animals.Cast<Dog>().
foreach (object animal in animals)
{
Dog d = (Dog)animal;
d.Woof();
}
}
public class NonEnumerableClass
{
public IEnumerator<Animal> GetEnumerator() => throw null;
}
public class Animal { }
class Dog : Animal
{
private string name;
public Dog(string name)
{
this.name = name;
}
public void Woof()
{
Console.WriteLine("Woof! My name is " + name + ".");
}
}
}

4
csharp/ql/test/query-tests/Linq/MissedCastOpportunity/MissedCastOpportunity.expected Normal file
View File

@@ -0,0 +1,4 @@
| MissedCastOpportunity.cs:10:9:14:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'. | MissedCastOpportunity.cs:12:13:12:27 | ... ...; | casts its iteration variable to another type |
| MissedCastOpportunity.cs:30:9:34:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'. | MissedCastOpportunity.cs:32:13:32:32 | ... ...; | casts its iteration variable to another type |
| MissedCastOpportunity.cs:40:9:44:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'. | MissedCastOpportunity.cs:42:13:42:32 | ... ...; | casts its iteration variable to another type |
| MissedCastOpportunity.cs:50:9:54:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'. | MissedCastOpportunity.cs:52:13:52:32 | ... ...; | casts its iteration variable to another type |

1
csharp/ql/test/query-tests/Linq/MissedCastOpportunity/MissedCastOpportunity.qlref Normal file
View File

@@ -0,0 +1 @@
Linq/MissedCastOpportunity.ql

2
csharp/ql/test/query-tests/Linq/MissedCastOpportunity/options Normal file
View File

@@ -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

83
csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.cs Normal file
View File

@@ -0,0 +1,83 @@
using System;
using System.Linq;
using System.Collections.Generic;
class MissedWhereOpportunity
{
public void M1(List<int> lst)
{
// BAD: Can be replaced with lst.Where(e => e % 2 == 0)
foreach (int i in lst)
{
if (i % 2 != 0)
continue;
Console.WriteLine(i);
Console.WriteLine((i / 2));
}
// BAD: Can be replaced with lst.Where(e => e % 2 == 0)
foreach (int i in lst)
{
if (i % 2 == 0)
{
Console.WriteLine(i);
Console.WriteLine((i / 2));
}
}
}
public void M2(NonEnumerableClass nec)
{
// GOOD: Linq can't be used here.
foreach (int i in nec)
{
if (i % 2 == 0)
{
Console.WriteLine(i);
Console.WriteLine((i / 2));
}
}
}
public void M3(int[] arr)
{
// BAD: Can be replaced with arr.Where(e => e % 2 == 0)
foreach (var n in arr)
{
if (n % 2 == 0)
{
Console.WriteLine(n);
Console.WriteLine((n / 2));
}
}
}
public void M4(Array arr)
{
// GOOD: Linq can't be used here.
foreach (var element in arr)
{
if (element.GetHashCode() % 2 == 0)
{
Console.WriteLine(element);
}
}
}
public void M5(IEnumerable<int> elements)
{
// BAD: Can be replaced with elements.Where(e => e.GetHashCode() % 2 == 0)
foreach (var element in elements)
{
if (element.GetHashCode() % 2 == 0)
{
Console.WriteLine(element);
}
}
}
public class NonEnumerableClass
{
public IEnumerator<int> GetEnumerator() => throw null;
}
}

4
csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected Normal file
View File

@@ -0,0 +1,4 @@
| MissedWhereOpportunity.cs:10:9:16:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:12:17:12:26 | ... != ... | implicitly filters its target sequence |
| MissedWhereOpportunity.cs:19:9:26:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:21:17:21:26 | ... == ... | implicitly filters its target sequence |
| MissedWhereOpportunity.cs:45:9:52:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:47:17:47:26 | ... == ... | implicitly filters its target sequence |
| MissedWhereOpportunity.cs:70:9:76:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:72:17:72:46 | ... == ... | implicitly filters its target sequence |

1
csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.qlref Normal file
View File

@@ -0,0 +1 @@
Linq/MissedWhereOpportunity.ql

2
csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/options Normal file
View File

@@ -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