Skip to content

Commit

Permalink
Modify S1128: Migrate To LayC (#3227)
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource authored Oct 10, 2023
1 parent daff220 commit 82ea93b
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 146 deletions.
2 changes: 1 addition & 1 deletion rules/S1128/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Unused \"using\" should be removed",
"title": "Unnecessary \"using\" should be removed",
"defaultQualityProfiles": [

],
Expand Down
80 changes: 55 additions & 25 deletions rules/S1128/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,49 +1,79 @@
== Why is this an issue?

Although unnecessary ``++using++`` won't change anything to the produced application, removing them:
Unnecessary `using` directives refer to importing namespaces, types or creating aliases that are not used or referenced anywhere in the code.

* Will help readability and maintenance.
* Will help reduce the number of items in the IDE auto-completion list when coding.
* May avoid some name collisions.
* May improve compilation time because the compiler has fewer namespaces to look-up when it resolves types.
* The build will fail if this namespace is removed from the project.
include::../description.adoc[]

=== Noncompliant code example
Starting with C# 10, it's possible to define https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-directive#global-modifier[global usings] for an entire project. They reduce the need for repetitive namespace inclusions, but can also mask which namespaces are truly necessary for the code at hand. Over-relying on them can lead to less transparent code dependencies, especially for newcomers to the project.

=== Exceptions

The rule will not raise a warning for `global using` directives, even if none of the types of that namespace are used in the project:

[source,csharp]
----
using System.Collections.Generic; // Noncompliant - unnecessary using
global using System.Net.Sockets; // Compliant by exception
----

Unnecessary `using` directives are ignored in ASP.NET Core projects in the following files:

* `_Imports.razor`
* `_ViewImports.cshtml`

== How to fix it

namespace Foo
While it's not difficult to remove these unneeded lines manually, modern code editors support the removal of every unnecessary `using` directive with a single click from every file of the project.

=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
using System.IO;
using System.Linq;
using System.Collections.Generic; // Noncompliant - no types are used from this namespace
using MyApp.Helpers; // Noncompliant - FileHelper is in the same namespace
using MyCustomNamespace; // Noncompliant - no types are used from this namespace
namespace MyApp.Helpers
{
public class Bar
public class FileHelper
{
public Bar(string path)
{
File.ReadAllLines(path);
}
public static string ReadFirstLine(string filePath) =>
File.ReadAllLines(filePath).First();
}
}
----

=== Compliant solution
==== Compliant solution

[source,csharp]
[source,csharp,diff-id=1,diff-type=compliant]
----
using System.IO;
using System.Linq;
namespace Foo
namespace MyApp.Helpers
{
public class Bar
public class FileHelper
{
public Bar(string path)
{
File.ReadAllLines(path);
}
public static string ReadFirstLine(string filePath) =>
File.ReadAllLines(filePath).First();
}
}
----

== Resources
=== Documentation

* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-directive[MSDN - using directives]
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/namespace[MSDN - namespaces]

=== Related rules

* S1144 - Unused private types or members should be removed
* S1481 - Unused local variables should be removed

ifdef::env-github,rspecator-view[]

'''
Expand All @@ -52,13 +82,15 @@ ifdef::env-github,rspecator-view[]

=== Message

* Remove this unused "using".
* Remove this unnecessary 'using'.


'''
== Comments And Links
(visible only on this page)

include::../comments-and-links.adoc[]

=== on 23 Sep 2019, 16:37:14 Nicolas Harraudeau wrote:
*OUT OF SCOPE*

Expand All @@ -68,6 +100,4 @@ Duplicate imports are out of scopes as Roslyn already raises an issue in this ca
=== on 18 Dec 2020, 10:06:15 Andrei Epure wrote:
We are removing this rule from SonarWay due to its performance issues. After the rule gets re-designed to avoid perf issues, (see https://github.com/SonarSource/sonar-dotnet/issues/3761[#3761]), we should bring it back to SonarWay.

include::../comments-and-links.adoc[]

endif::env-github,rspecator-view[]
9 changes: 5 additions & 4 deletions rules/S1128/description.adoc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
The imports part of a file should be handled by the Integrated Development Environment (IDE), not manually by the developer.
Although they don't affect the runtime behavior of the application after compilation, removing them will:

Unused and useless imports should not occur if that is the case.

Leaving them in reduces the code's readability, since their presence can be confusing.
* Improve the readability and maintainability of the code.
* Help avoid potential naming conflicts.
* Improve the build time, as the compiler has fewer lines to read and fewer types to resolve.
* Reduce the number of items the code editor will show for auto-completion, thereby showing fewer irrelevant suggestions.
59 changes: 56 additions & 3 deletions rules/S1128/java/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,20 +1,73 @@
== Why is this an issue?

include::../description.adoc[]
Unnecessary imports refer to importing types that are not used or referenced anywhere in the code.

include::../noncompliant.adoc[]
include::../description.adoc[]

=== Exceptions

Imports for types mentioned in Javadocs are ignored.

== How to fix it

While it's not difficult to remove these unneeded lines manually, modern code editors support the removal of every unnecessary import with a single click from every file of the project.

=== Code examples

==== Noncompliant code example

[source,java,diff-id=1,diff-type=noncompliant]
----
package myapp.helpers;
import java.io.IOException;
import java.nio.file.*;
import java.nio.file.*; // Noncompliant - package is imported twice
import java.lang.Runnable; // Noncompliant - java.lang is imported by default
public class FileHelper {
public static String readFirstLine(String filePath) throws IOException {
return Files.readAllLines(Paths.get(filePath)).get(0);
}
}
----

==== Compliant solution

[source,java,diff-id=1,diff-type=compliant]
----
package myapp.helpers;
import java.io.IOException;
import java.nio.file.*;
public class FileHelper {
public static String readFirstLine(String filePath) throws IOException {
return Files.readAllLines(Paths.get(filePath)).get(0);
}
}
----

== Resources
=== Documentation

* https://docs.oracle.com/javase/tutorial/java/package/usepkgs.html[Java packages]

=== Related rules

* S1144 - Unused "private" methods should be removed
* S1481 - Unused local variables should be removed

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::../message.adoc[]
=== Message

* Unnecessary imports should be removed.


'''
== Comments And Links
Expand Down
63 changes: 38 additions & 25 deletions rules/S1128/kotlin/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,58 +1,71 @@
== Why is this an issue?

Unnecessary imports refer to importing types that are not used or referenced anywhere in the code.

include::../description.adoc[]

=== Noncompliant code example
=== Exceptions

[source,kotlin]
----
package my.company
Imports for types mentioned in KDoc are ignored.

import kotlin.String // Noncompliant; "kotlin" classes are always implicitly imported
import my.company.SomeClass // Noncompliant; same-package files are always implicitly imported
import java.io.File // Noncompliant; "File" is not used
== How to fix it

import my.company2.SomeType
While it's not difficult to remove these unneeded lines manually, modern code editors support the removal of every unnecessary import with a single click from every file of the project.

class ExampleClass {
=== Code examples

val someString = ""
val something = SomeType()
==== Noncompliant code example

}
[source,kotlin,diff-id=1,diff-type=noncompliant]
----
package myapp.helpers
import java.io.IOException
import java.nio.file.*
import java.nio.file.* // Noncompliant - package is imported twice
import java.nio.* // Noncompliant - nothing is used from that package
=== Compliant solution

[source,kotlin]
object FileHelper {
fun readFirstLine(filePath: String)
= Files.readAllLines(Paths.get(filePath)).first()
}
----
package my.company

import java.io.File
import my.company2.SomeType
==== Compliant solution

class ExampleClass {
[source,kotlin,diff-id=1,diff-type=compliant]
----
package myapp.helpers
val someString = ""
val something = SomeType()
lateinit var fileUsage: File
import java.io.IOException
import java.nio.file.*
object FileHelper {
fun readFirstLine(filePath: String)
= Files.readAllLines(Paths.get(filePath)).first()
}
----

== Resources
=== Documentation

=== Exceptions
* https://kotlinlang.org/docs/packages.html[Kotlin packages and imports]

=== Related rules

Imports for types mentioned in KDoc comments are ignored.
* S1144 - Unused "private" methods should be removed
* S1481 - Unused local variables should be removed

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::../message.adoc[]
=== Message

* Unnecessary imports should be removed.


'''
== Comments And Links
Expand Down
2 changes: 1 addition & 1 deletion rules/S1128/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "2min"
"constantCost": "1min"
},
"tags": [
"unused"
Expand Down
20 changes: 0 additions & 20 deletions rules/S1128/noncompliant.adoc

This file was deleted.

5 changes: 0 additions & 5 deletions rules/S1128/rule.adoc

This file was deleted.

6 changes: 0 additions & 6 deletions rules/S1128/vbnet/metadata.json

This file was deleted.

Loading

0 comments on commit 82ea93b

Please sign in to comment.