Java: remove InefficientToArray.ql

This query was deprecated 4 years ago.

Fixes: #4167
This commit is contained in:
Arthur Baars
2020-08-31 10:39:00 +02:00
parent 398a95c65f
commit 66d39bb5f6
3 changed files with 0 additions and 142 deletions

View File

@@ -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);
}
}

View File

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

View File

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