mirror of
https://github.com/github/codeql.git
synced 2026-04-25 08:45:14 +02:00
Merge pull request #4170 from aibaars/drop-inefficient-toarray
Java: remove InefficientToArray.ql
This commit is contained in:
@@ -1,28 +0,0 @@
|
||||
class Company {
|
||||
private List<Customer> customers = ...;
|
||||
|
||||
public Customer[] getCustomerArray() {
|
||||
// AVOID: Inefficient call to 'toArray' with zero-length argument
|
||||
return customers.toArray(new Customer[0]);
|
||||
}
|
||||
}
|
||||
|
||||
class Company {
|
||||
private List<Customer> customers = ...;
|
||||
|
||||
public Customer[] getCustomerArray() {
|
||||
// GOOD: More efficient call to 'toArray' with argument that is big enough to store the list
|
||||
return customers.toArray(new Customer[customers.size()]);
|
||||
}
|
||||
}
|
||||
|
||||
class Company {
|
||||
private static final Customer[] NO_CUSTOMERS = new Customer[0];
|
||||
|
||||
private List<Customer> customers = ...;
|
||||
|
||||
public Customer[] getCustomerArray() {
|
||||
// GOOD: Reuse same zero-length array on every invocation
|
||||
return customers.toArray(NO_CUSTOMERS);
|
||||
}
|
||||
}
|
||||
@@ -1,61 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>The <code>java.util.Collection</code> interface provides a <code>toArray</code>
|
||||
method that can be used to convert a collection of objects into an array of a particular
|
||||
type. This method takes an array as an argument, which is used for two purposes.
|
||||
Firstly, it determines the type of the returned array. Secondly, if it is big enough to
|
||||
hold all values in the collection, it is filled with those values and returned;
|
||||
otherwise, a new array of sufficient size and the appropriate type is allocated and
|
||||
filled.</p>
|
||||
|
||||
<p>It is common to pass a fresh zero-length array to <code>toArray</code>,
|
||||
simply because it is easy to construct one. Unfortunately, this allocation is wasteful,
|
||||
because the array clearly is not big enough to hold the elements of the collection. This can cause
|
||||
considerable garbage collector churn, impacting performance.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>There are at least two ways to address this issue.</p>
|
||||
|
||||
<p>The first is to always call <code>toArray</code> with a new array allocated with a
|
||||
sufficient size to hold the contents of the collection. Usually, this involves calling
|
||||
the collection's <code>size</code> method and allocating an array with that many
|
||||
elements. While it may seem odd that adding a call to <code>size</code> improves performance, if you
|
||||
do not pass a large enough array, the <code>toArray</code> method makes this call automatically.
|
||||
Calling <code>size</code> explicitly and then calling <code>toArray</code> with a large enough array
|
||||
avoids the possible creation of two arrays (one too small and consequently unused).</p>
|
||||
|
||||
<p>The second approach is to add a static field holding a constant zero-length array to the
|
||||
enclosing class, and pass that field to <code>toArray</code>. In this case, <code>toArray</code>
|
||||
will end up allocating a new array in (almost) every case, but because the same zero-length array
|
||||
is reused every time, there is almost no overhead. (Note that if <code>toArray</code>
|
||||
is invoked on an empty collection, it will return the passed-in array. If your code expects
|
||||
a new array from every invocation of <code>toArray</code>, you should use the first method.)</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>In the following example, the first version of class <code>Company</code> uses an inefficient
|
||||
call to <code>toArray</code> by passing a zero-length array. The second and third version
|
||||
illustrate the two ways of addressing this issue outlined above.</p>
|
||||
|
||||
<sample src="InefficientToArray.java" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
|
||||
<li>
|
||||
Java Platform, Standard Edition 6, API Specification:
|
||||
<a href="http://docs.oracle.com/javase/6/docs/api/java/util/Collection.html#toArray%28T[]%29">toArray</a>.
|
||||
</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -1,53 +0,0 @@
|
||||
/**
|
||||
* @name Inefficient toArray on zero-length argument
|
||||
* @description Calling 'Collection.toArray' with a zero-length array argument is inefficient.
|
||||
* @kind problem
|
||||
* @problem.severity recommendation
|
||||
* @precision low
|
||||
* @id java/inefficient-to-array
|
||||
* @deprecated
|
||||
*/
|
||||
|
||||
import java
|
||||
|
||||
/*
|
||||
* This method uses the toArray() method of a collection derived class, and passes in a
|
||||
* zero-length prototype array argument.
|
||||
*
|
||||
* It is more efficient to use myCollection.toArray(new Foo[myCollection.size()]).
|
||||
* If the array passed in is big enough to store all of the elements of the collection,
|
||||
* then it is populated and returned directly. This avoids the need to create a
|
||||
* second array (by reflection) to return as the result.
|
||||
*
|
||||
* The new array has to be created as the argument value. Values in both branches of
|
||||
* a conditional has to be an empty array.
|
||||
*/
|
||||
|
||||
predicate emptyArrayLiteral(Expr e) {
|
||||
// ArrayCreationExpr with zero-length init expr
|
||||
exists(ArrayCreationExpr arr | arr = e |
|
||||
exists(arr.getInit()) and
|
||||
not exists(arr.getInit().getAnInit())
|
||||
)
|
||||
or
|
||||
// ArrayCreationExpr where 0th dimension is zero literal
|
||||
e.(ArrayCreationExpr).getDimension(0).(IntegerLiteral).getIntValue() = 0
|
||||
or
|
||||
exists(ConditionalExpr cond | cond = e |
|
||||
emptyArrayLiteral(cond.getTrueExpr()) and
|
||||
emptyArrayLiteral(cond.getFalseExpr())
|
||||
)
|
||||
}
|
||||
|
||||
class EmptyArrayLiteral extends Expr {
|
||||
EmptyArrayLiteral() { emptyArrayLiteral(this) }
|
||||
}
|
||||
|
||||
from EmptyArrayLiteral l, MethodAccess ma, Method m, GenericInterface coll
|
||||
where
|
||||
coll.hasQualifiedName("java.util", "Collection") and
|
||||
ma.getMethod() = m and
|
||||
m.hasName("toArray") and
|
||||
m.getDeclaringType().getASupertype*().getSourceDeclaration() = coll and
|
||||
ma.getArgument(0) = l
|
||||
select ma, "Collection.toArray(...) called with zero-length array argument."
|
||||
Reference in New Issue
Block a user