Skip to content

Commit

Permalink
NET-668 Modify S7130: Add vbnet rspec (#4499)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavel-mikula-sonarsource authored Nov 14, 2024
1 parent f4690e5 commit a0be31c
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 44 deletions.
21 changes: 0 additions & 21 deletions rules/S7130/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,2 @@
{
"title": "First/Single should be used instead of FirstOrDefault/SingleOrDefault on collections that are known to be non-empty",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "1min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7130",
"sqKey": "S7130",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "targeted",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "CLEAR"
}
}
25 changes: 2 additions & 23 deletions rules/S7130/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
When working with collections that are known to be non-empty, using https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.first[First] or https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.single[Single] is generally preferred over https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.firstordefault[FirstOrDefault] or https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.singleordefault[SingleOrDefault].

== Why is this an issue?

Using `FirstOrDefault` or `SingleOrDefault` on collections that are known to be non-empty is an issue due to:

* Code Clarity and intent: When you use `FirstOrDefault` or `SingleOrDefault`, it implies that the collection might be empty, which can be misleading if you know it is not. It can be confusing for other developers who read your code, making it harder for them to understand the actual constraints and behavior of the collection. This leads to confusion and harder-to-maintain code.

* Error handling: If the developer's intend is for the collection not to be empty, using `FirstOrDefault` and `SingleOrDefault` can lead to subtle bugs. These methods return a default value (`null` for reference types and `default` for value types) when the collection is empty, potentially causing issues like `NullReferenceException` later in the code. In contrast, `First` or `Single` will throw an `InvalidOperationException` immediately if the collection is empty, making it easier to detect and address issues early in the development process.

* Code coverage: Potentially, having to check if the result is `null`, you introduces a condition that cannot be fully tested, impacting the code coverage.
include::../description-dotnet.adoc[]

=== Code examples

Expand All @@ -30,17 +20,6 @@ var items = new List<int> { 1, 2, 3 };
int firstItem = items.First(); // Compliant
----

== Resources

=== Documentation

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.single[`Single`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.first[`First`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.singleordefault[`SingleOrDefault`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.firstordefault[`FirstOrDefault`]

=== Articles & blog posts

* https://medium.com/@anyanwuraphaelc/first-vs-firstordefault-single-vs-singleordefault-a-high-level-look-d24db17a2bc3[First vs FirstOrDefault, Single vs SingleOrDefault: A High-level Look]
include::../resources-dotnet.adoc[]

include::../rspecator.adoc[]
12 changes: 12 additions & 0 deletions rules/S7130/description-dotnet.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
When working with collections that are known to be non-empty, using https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.first[First] or https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.single[Single] is generally preferred over https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.firstordefault[FirstOrDefault] or https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.singleordefault[SingleOrDefault].

== Why is this an issue?

Using `FirstOrDefault` or `SingleOrDefault` on collections that are known to be non-empty is an issue due to:

* Code Clarity and intent: When you use `FirstOrDefault` or `SingleOrDefault`, it implies that the collection might be empty, which can be misleading if you know it is not. It can be confusing for other developers who read your code, making it harder for them to understand the actual constraints and behavior of the collection. This leads to confusion and harder-to-maintain code.

* Error handling: If the developer's intend is for the collection not to be empty, using `FirstOrDefault` and `SingleOrDefault` can lead to subtle bugs. These methods return a default value (`null` for reference types and `default` for value types) when the collection is empty, potentially causing issues like `NullReferenceException` later in the code. In contrast, `First` or `Single` will throw an `InvalidOperationException` immediately if the collection is empty, making it easier to detect and address issues early in the development process.

* Code coverage: Potentially, having to check if the result is `null`, you introduces a condition that cannot be fully tested, impacting the code coverage.

21 changes: 21 additions & 0 deletions rules/S7130/metadata.json
Original file line number Diff line number Diff line change
@@ -1,2 +1,23 @@
{
"title": "First/Single should be used instead of FirstOrDefault/SingleOrDefault on collections that are known to be non-empty",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "1min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7130",
"sqKey": "S7130",
"scope": "All",
"defaultQualityProfiles": [ "Sonar way" ],
"quickfix": "targeted",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "CLEAR"
}
}
13 changes: 13 additions & 0 deletions rules/S7130/resources-dotnet.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
== Resources

=== Documentation

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.single[`Single`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.first[`First`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.singleordefault[`SingleOrDefault`]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.firstordefault[`FirstOrDefault`]

=== Articles & blog posts

* https://medium.com/@anyanwuraphaelc/first-vs-firstordefault-single-vs-singleordefault-a-high-level-look-d24db17a2bc3[First vs FirstOrDefault, Single vs SingleOrDefault: A High-level Look]

2 changes: 2 additions & 0 deletions rules/S7130/vbnet/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
25 changes: 25 additions & 0 deletions rules/S7130/vbnet/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
include::../description-dotnet.adoc[]

=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
Dim Items As New list(Of Integer) From {1, 2, 3}
Dim FirstItem As Integer = Items.FirstOrDefault() ' Noncompliant, this implies the collection might be empty, when we know it is not
----

==== Compliant solution

[source,csharp,diff-id=1,diff-type=compliant]
----
Dim Items As New list(Of Integer) From {1, 2, 3}
Dim FirstItem As Integer = Items.First() ' Compliant
----

include::../resources-dotnet.adoc[]

include::../rspecator.adoc[]

0 comments on commit a0be31c

Please sign in to comment.