rspec/rules/S2175/java/rule.adoc

147 lines
6.1 KiB
Plaintext

== Why is this an issue?
The `java.util.Collection` type and its subtypes provide methods to access and modify collections such as `Collection.remove(Object o)` and `Collection.contains(Object o)`.
Some of these methods accept arguments of type `java.lang.Object` and will compare said argument with objects already in the collection.
If the actual type of the argument is unrelated to the type of object contained in the collection, these methods will always return `false`, `null`, or `-1`.
This behavior is most likely unintended and can be indicative of a design issue.
This rule raises an issue when the type of the argument provided to one of the following methods is unrelated to the type used for the collection declaration:
* `Collection.remove(Object o)`
* `Collection.removeAll(Collection<?>)`
* `Collection.contains(Object o)`
* `List.indexOf(Object o)`
* `List.lastIndexOf(Object o)`
* `Map.containsKey(Object key)`
* `Map.containsValue(Object value)`
* `Map.get(Object key)`
* `Map.getOrDefault(Object key, V defaultValue)`
* `Map.remove(Object key)`
* `Map.remove(Object key, Object value)`
== How to fix it
Ask yourself what the purpose of this method call is.
Check whether the provided argument and collection are correct in this context and for the desired purpose.
Remove unnecessary calls and otherwise provide an argument of which the type is compatible with the list content's type.
=== Code examples
==== Noncompliant code example
[source,java,diff-id=1,diff-type=noncompliant]
----
void removeFromMap(Map<Integer, Object> map, String strKey) {
map.remove(strKey); // Noncompliant, this call will remove nothing and always return 'null' because 'map' is handling only Integer keys and String cannot be cast to Integer.
}
void listContains(List<String> list, Integer integer) {
if (list.contains(integer)) { // Noncompliant; always false as the list only contains Strings, not integers.
// ...
}
}
----
==== Compliant solution
[source,java,diff-id=1,diff-type=compliant]
----
void removeFromMap(Map<Integer, Object> map, String strKey) {
map.remove(Integer.parseInt(strKey)); // Compliant, strKey is parsed into an Integer before trying to remove it from the map.
}
void listContains(List<String> list, Integer integer) {
if (list.contains(integer.toString())) { // Compliant, 'integer' is converted to a String before checking if the list contains it.
// ...
}
}
----
== Resources
* https://wiki.sei.cmu.edu/confluence/x/uDdGBQ[CERT, EXP04-J.] - Do not pass arguments to certain Java Collections Framework methods that are a different type than the collection parameter type
* https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Collection.html[Java SE 17 & JDK 17] - Collection interface
ifdef::env-github,rspecator-view[]
'''
== Implementation Specification
(visible only on this page)
=== Message
A "[class]<[type]>" cannot contain a "[ytype]".
'''
== Comments And Links
(visible only on this page)
=== on 21 Nov 2024, 16:48:00 Erwan Serandour wrote:
[test-code-support-investigation-for-java] Good candidate to move to 'All'. Custom implementation of collections is not the common case, I was not able to reproduce the other FPs.
=== on 21 Nov 2024, 16:48:00 Alban Auzeill wrote:
[test-code-support-investigation-for-java] Decision for scope: Keep 'Main'. FP on custom implementation of collections
=== on 5 Feb 2015, 17:44:14 Michael Gumowski wrote:
As I am currently encountering difficulties implementing the rule, I think that expressly mentioning the names of the variables in the issue message does not worth the effort.
Indeed, gathering the names of the variables to build a the proposed message imply several problems:
* It is costly (and complex) in terms of operations to gather them, and does not provides a lot of information (the issue is already detected on the line, it should not be hard to locate the issue);
* Variables are not always present in expressions manipulating these methods, implying that multiple messages are possible (increasing complexity of the rule as well). As shown in the following code:
----
List<String> getList() {
return new ArrayList<String>();
}
Integer getIntegerValue() {
return Integer.valueOf(1);
}
void myMethod() {
if (getList().contains(getIntegerValue())) { // Noncompliant. Always false. <<-- What would be the message
getList().remove(getIntegerValue()); // Noncompliant. list.add(iger) doesn't compile, so this will always return false <<-- Same problem
}
}
----
I would like to change the message to the following proposition, which I think is much simpler without loosing its pertinence:
____{code}"[class]<[type]>" will not contain any "[ytype]"{code}____
For the previous examples we would then have the syme following message :
____"List<String>" will not contain any "Integer"____
=== on 5 Feb 2015, 18:40:55 Ann Campbell wrote:
\[~michael.gumowski] how about
* A "[class]<[type]>" cannot contain a "[ytype]".
* You cannot add a "[ytype]" to a "[class]<[type]>".
=== on 6 Feb 2015, 07:43:26 Michael Gumowski wrote:
I'll take your first proposition if it's ok for you!
=== on 16 Nov 2018, 20:02:03 Jens Bannmann wrote:
The first paragraph of the rule description ends in an incomplete sentence, and it is redundant with the second and third paragraphs. Is it a leftover from revising it, or is there some kind of rendering error here in Jira?
____A couple Collection methods can be called with arguments of an incorrect type, but doing so is pointless and likely the result of using the wrong argument. This rule will raise an issue
The java.util.Collection API is having methods accepting Object as parameter such as Collection.remove(Object o) or Collection.contains(Object o). When the effective type of the object provided to these methods is not consistent with the type declared on the Collection instantiation, those methods will always return false or null. This is most likely unintended and hide a design problem.
This rule raises an issue when the type of the argument of the following APIs is unrelated to the type used for the Collection declaration:
(...)____
=== on 16 Nov 2018, 22:09:59 Alexandre Gigleux wrote:
\[~bannmann] Fixed
endif::env-github,rspecator-view[]