From dd9def2840ce710e69c0775b8f1a526c644ecb16 Mon Sep 17 00:00:00 2001 From: Gregory Paidis <115458417+gregory-paidis-sonarsource@users.noreply.github.com> Date: Thu, 29 Jun 2023 14:07:20 +0200 Subject: [PATCH] Modify rule S2114: LaYC format (#2316) --- rules/S2114/csharp/rule.adoc | 34 ++++++++++++++++++++++------------ rules/S2114/description.adoc | 4 ---- rules/S2114/java/rule.adoc | 24 ++++++++++++++++++++++-- rules/S2114/kotlin/rule.adoc | 16 +++++++++++----- rules/S2114/message.adoc | 4 ---- rules/S2114/rule.adoc | 18 ------------------ 6 files changed, 55 insertions(+), 45 deletions(-) delete mode 100644 rules/S2114/description.adoc delete mode 100644 rules/S2114/message.adoc delete mode 100644 rules/S2114/rule.adoc diff --git a/rules/S2114/csharp/rule.adoc b/rules/S2114/csharp/rule.adoc index a8ec6e9f23..8cdd285a17 100644 --- a/rules/S2114/csharp/rule.adoc +++ b/rules/S2114/csharp/rule.adoc @@ -1,6 +1,9 @@ == Why is this an issue? -Passing a collection as an argument to the collection's own method is either an error - some other argument was intended - or simply nonsensical code. +Passing a collection as an argument to the collection's own method is a code defect. Doing so might either have unexpected side effects or always have the same result. + +Another case is using set-like operations. For example, using https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.union[Union] between a list and itself will always return the same list. +Conversely, using https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.except[Except] between a list and itself will always return an empty list. [source,csharp] ---- @@ -10,23 +13,30 @@ list.AddRange(list); // Noncompliant list.Concat(list); // Noncompliant list.Union(list); // Noncompliant: always returns list -list.Except(list); // Noncompliant: always empty -list.Intersect(list); // Noncompliant: always list -list.SequenceEqual(list); // Noncompliant: always true +list.Intersect(list); // Noncompliant: always returns list +list.Except(list); // Noncompliant: always returns empty +list.SequenceEqual(list); // Noncompliant: always returns true var set = new HashSet(); set.UnionWith(set); // Noncompliant: no changes -set.ExceptWith(set); // Noncompliant: always empty set.IntersectWith(set); // Noncompliant: no changes -set.IsProperSubsetOf(set); // Noncompliant: always false -set.IsProperSupersetOf(set); // Noncompliant: always false -set.IsSubsetOf(set); // Noncompliant: always true -set.IsSupersetOf(set); // Noncompliant: always true -set.Overlaps(set); // Noncompliant: always true -set.SetEquals(set); // Noncompliant: always true -set.SymmetricExceptWith(set); // Noncompliant: always empty +set.ExceptWith(set); // Noncompliant: always returns empty +set.SymmetricExceptWith(set); // Noncompliant: always returns empty +set.IsProperSubsetOf(set); // Noncompliant: always returns false +set.IsProperSupersetOf(set); // Noncompliant: always returns false +set.IsSubsetOf(set); // Noncompliant: always returns true +set.IsSupersetOf(set); // Noncompliant: always returns true +set.Overlaps(set); // Noncompliant: always returns true +set.SetEquals(set); // Noncompliant: always returns true ---- + +== Resources + +=== Documentation + +* https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/collections[Collections] + ifdef::env-github,rspecator-view[] ''' diff --git a/rules/S2114/description.adoc b/rules/S2114/description.adoc deleted file mode 100644 index 11104fc29b..0000000000 --- a/rules/S2114/description.adoc +++ /dev/null @@ -1,4 +0,0 @@ -Passing a collection as an argument to the collection's own method is either an error - some other argument was intended - or simply nonsensical code. - - -Further, because some methods require that the argument remain unmodified during the execution, passing a collection to itself can result in undefined behavior. diff --git a/rules/S2114/java/rule.adoc b/rules/S2114/java/rule.adoc index fe598e7f71..34d09c5122 100644 --- a/rules/S2114/java/rule.adoc +++ b/rules/S2114/java/rule.adoc @@ -1,4 +1,22 @@ -include::../rule.adoc[] +== Why is this an issue? + +Passing a collection as an argument to the collection's own method is either an error - some other argument was intended - or simply nonsensical code. + +Further, because some methods require that the argument remain unmodified during the execution, passing a collection to itself can result in undefined behavior. + +=== Noncompliant code example + +[source,java] +---- +List objs = new ArrayList(); +objs.add("Hello"); + +objs.add(objs); // Noncompliant; StackOverflowException if objs.hashCode() called +objs.addAll(objs); // Noncompliant; behavior undefined +objs.containsAll(objs); // Noncompliant; always true +objs.removeAll(objs); // Noncompliant; confusing. Use clear() instead +objs.retainAll(objs); // Noncompliant; NOOP +---- ifdef::env-github,rspecator-view[] @@ -6,7 +24,9 @@ ifdef::env-github,rspecator-view[] == Implementation Specification (visible only on this page) -include::../message.adoc[] +=== Message + +Remove or correct this "xxx" call. ''' == Comments And Links diff --git a/rules/S2114/kotlin/rule.adoc b/rules/S2114/kotlin/rule.adoc index aec505788a..dc2f9a8f90 100644 --- a/rules/S2114/kotlin/rule.adoc +++ b/rules/S2114/kotlin/rule.adoc @@ -1,10 +1,16 @@ == Why is this an issue? -include::../description.adoc[] +Passing a collection as an argument to the collection's own method is either an error - some other argument was intended - or simply nonsensical code. -=== Noncompliant code example +Further, because some methods require that the argument remain unmodified during the execution, passing a collection to itself can result in undefined behavior. -[source,kotlin] +== How to fix it + +=== Code examples + +==== Noncompliant code example + +[source,kotlin,diff-id=1,diff-type=noncompliant] ---- val objs = mutableListOf() objs.add("Hello") @@ -16,9 +22,9 @@ objs.removeAll(objs) // Noncompliant; confusing. Use clear() instead objs.retainAll(objs) // Noncompliant; NOOP ---- -=== Compliant solution +==== Compliant solution -[source,kotlin] +[source,kotlin,diff-id=1,diff-type=compliant] ---- val newList = mutableListOf() val objs = mutableListOf() diff --git a/rules/S2114/message.adoc b/rules/S2114/message.adoc deleted file mode 100644 index d418bf8c97..0000000000 --- a/rules/S2114/message.adoc +++ /dev/null @@ -1,4 +0,0 @@ -=== Message - -Remove or correct this "xxx" call. - diff --git a/rules/S2114/rule.adoc b/rules/S2114/rule.adoc deleted file mode 100644 index 3a76971e24..0000000000 --- a/rules/S2114/rule.adoc +++ /dev/null @@ -1,18 +0,0 @@ -== Why is this an issue? - -include::description.adoc[] - -=== Noncompliant code example - -[source,text] ----- -List objs = new ArrayList(); -objs.add("Hello"); - -objs.add(objs); // Noncompliant; StackOverflowException if objs.hashCode() called -objs.addAll(objs); // Noncompliant; behavior undefined -objs.containsAll(objs); // Noncompliant; always true -objs.removeAll(objs); // Noncompliant; confusing. Use clear() instead -objs.retainAll(objs); // Noncompliant; NOOP ----- -