From 65743cb6221cf2fa17218c1df64f731c06ede82e Mon Sep 17 00:00:00 2001 From: Yassin Kammoun <52890329+yassin-kammoun-sonarsource@users.noreply.github.com> Date: Tue, 17 Oct 2023 19:44:58 +0200 Subject: [PATCH] Modify S3353: Migrate To LayC (#3302) ## Review A dedicated reviewer checked the rule description successfully for: - [ ] logical errors and incorrect information - [ ] information gaps and missing content - [ ] text style and tone - [ ] PR summary and labels follow [the guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule) --------- Co-authored-by: Zsolt Kolbay <121798625+zsolt-kolbay-sonarsource@users.noreply.github.com> --- rules/S3353/csharp/metadata.json | 1 - rules/S3353/csharp/rule.adoc | 50 +++++++++++++++----------- rules/S3353/how.adoc | 3 ++ rules/S3353/javascript/metadata.json | 1 - rules/S3353/javascript/rule.adoc | 52 +++++++++++++++++++--------- rules/S3353/metadata.json | 2 +- rules/S3353/why.adoc | 7 ++++ 7 files changed, 76 insertions(+), 40 deletions(-) create mode 100644 rules/S3353/how.adoc create mode 100644 rules/S3353/why.adoc diff --git a/rules/S3353/csharp/metadata.json b/rules/S3353/csharp/metadata.json index f3c2071d901..3f9951567b4 100644 --- a/rules/S3353/csharp/metadata.json +++ b/rules/S3353/csharp/metadata.json @@ -1,5 +1,4 @@ { - "title": "Unchanged local variables should be \"const\"", "quickfix": "partial", "tags": [ "performance" diff --git a/rules/S3353/csharp/rule.adoc b/rules/S3353/csharp/rule.adoc index ac9713e312d..d3218bdaa2a 100644 --- a/rules/S3353/csharp/rule.adoc +++ b/rules/S3353/csharp/rule.adoc @@ -1,10 +1,12 @@ -== Why is this an issue? +include::../why.adoc[] -Marking a variable that is unchanged after initialization ``++const++`` is an indication to future maintainers that "no this isn't updated, and it's not supposed to be". ``++const++`` should be used in these situations in the interests of code clarity. +include::../how.adoc[] -=== Noncompliant code example +=== Code examples -[source,csharp] +==== Noncompliant code example + +[source,csharp,diff-id=1,diff-type=noncompliant] ---- public bool Seek(int[] input) { @@ -19,22 +21,10 @@ public bool Seek(int[] input) return false; } ---- -or -[source,csharp] ----- -public class Sample -{ - public void Method() - { - var context = $"{nameof(Sample)}.{nameof(Method)}"; // Noncompliant (C# 10 and above only) - } -} ----- +==== Compliant solution -=== Compliant solution - -[source,csharp] +[source,csharp,diff-id=1,diff-type=compliant] ---- public bool Seek(int[] input) { @@ -49,9 +39,23 @@ public bool Seek(int[] input) return false; } ---- -or -[source,csharp] +==== Noncompliant code example + +[source,csharp,diff-id=2,diff-type=noncompliant] +---- +public class Sample +{ + public void Method() + { + var context = $"{nameof(Sample)}.{nameof(Method)}"; // Noncompliant (C# 10 and above only) + } +} +---- + +==== Compliant solution + +[source,csharp,diff-id=2,diff-type=compliant] ---- public class Sample { @@ -62,6 +66,12 @@ public class Sample } ---- +== Resources + +=== Documentation + +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/const[const] + ifdef::env-github,rspecator-view[] ''' diff --git a/rules/S3353/how.adoc b/rules/S3353/how.adoc new file mode 100644 index 00000000000..cb33089df8e --- /dev/null +++ b/rules/S3353/how.adoc @@ -0,0 +1,3 @@ +== How to fix it + +Mark the given variable with the `const` modifier. diff --git a/rules/S3353/javascript/metadata.json b/rules/S3353/javascript/metadata.json index e89f1373a1a..d3d36101ebf 100644 --- a/rules/S3353/javascript/metadata.json +++ b/rules/S3353/javascript/metadata.json @@ -1,5 +1,4 @@ { - "title": "Unchanged variables should be marked \"const\"", "tags": [ "es2015" ], diff --git a/rules/S3353/javascript/rule.adoc b/rules/S3353/javascript/rule.adoc index 8db6d7385c1..4ec76c5c287 100644 --- a/rules/S3353/javascript/rule.adoc +++ b/rules/S3353/javascript/rule.adoc @@ -1,47 +1,65 @@ -== Why is this an issue? +include::../why.adoc[] -Marking a variable that is unchanged after initialization ``++const++`` is an indication to future maintainers that "no this isn't updated, and it's not supposed to be". ``++const++`` should be used in these situations in the interests of code clarity. +include::../how.adoc[] -=== Noncompliant code example +=== Code examples -[source,javascript] +==== Noncompliant code example + +[source,javascript,diff-id=1,diff-type=noncompliant] ---- function seek(input) { let target = 32; // Noncompliant - for (let i of input) { // Noncompliant - if (i == target) { + for (const i of input) { + if (i === target) { return true; } } return false; } - -function getUrl(query) {     - let url; // Noncompliant - url = "http://example.com"; - return url; -} ---- -=== Compliant solution +==== Compliant solution -[source,javascript] +[source,javascript,diff-id=1,diff-type=compliant] ---- function seek(input) { const target = 32; for (const i of input) { - if (i == target) { + if (i === target) { return true; } } return false; } +---- -function getUrl(query) {   - const url = "http://example.com"; +[source,javascript,diff-id=2,diff-type=noncompliant] +---- + +function getUrl(protocol, domain, path) {     + let url; // Noncompliant + url = `${protocol}/${domain}/${path}`; return url; } ---- + +==== Compliant solution + +[source,javascript,diff-id=2,diff-type=compliant] +---- +function getUrl(protocol, domain, path) {   + const url = `${protocol}/${domain}/${path}`; + return url; +} +---- + +== Resources + +=== Documentation + +* MDN - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const[const] + ifdef::env-github,rspecator-view[] ''' diff --git a/rules/S3353/metadata.json b/rules/S3353/metadata.json index d892c29154f..86fd0b2c935 100644 --- a/rules/S3353/metadata.json +++ b/rules/S3353/metadata.json @@ -1,5 +1,5 @@ { - "title": "Unchanged local variables should be \"final\"", + "title": "Unchanged variables should be marked as \"const\"", "type": "CODE_SMELL", "code": { "impacts": { diff --git a/rules/S3353/why.adoc b/rules/S3353/why.adoc new file mode 100644 index 00000000000..2dad22834a6 --- /dev/null +++ b/rules/S3353/why.adoc @@ -0,0 +1,7 @@ +== Why is this an issue? + +If a variable that is not supposed to change is not marked as `const`, it could be accidentally reassigned elsewhere in the code, leading to unexpected behavior and bugs that can be hard to track down. + +By declaring a variable as `const`, you ensure that its value remains constant throughout the code. It also signals to other developers that this value is intended to remain constant. This can make the code easier to understand and maintain. + +In some cases, using `const` can lead to performance improvements. The compiler might be able to make optimizations knowing that the value of a `const` variable will not change.