SONARRUBY-164 fix: resolve 5 SonarQube issues across Ruby plugin

[sonarqube-agent]

This PR fixes 5 MAJOR SonarQube issues including adding generic type parameters, renaming a restricted identifier, reducing excessive assertions in tests, removing commented-out code, and eliminating unnecessary String.format calls. These changes improve code quality, maintainability, and runtime safety by following Java best practices and reducing technical debt.

View Project in SonarCloud

---

Fixed Issues

java:S3740 - Provide the parametrized type for this generic. • MAJOR • View issue

Location: sonar-ruby-plugin/src/main/java/org/sonarsource/ruby/converter/AstNode.java:39

Why is this an issue?

A generic type is a generic class or interface that is parameterized over types. For example, java.util.List has one type parameter: the type of its elements.

What changed

This hunk adds the type parameter <Object> to the raw List type on line 39 of AstNode.java. The static analysis warning flagged the use of a raw generic type (List without type arguments), which prevents compile-time type checking and could lead to ClassCastException at runtime. By parameterizing it as List<Object>, the generic type is no longer used raw, resolving the code smell.

--- a/sonar-ruby-plugin/src/main/java/org/sonarsource/ruby/converter/AstNode.java
+++ b/sonar-ruby-plugin/src/main/java/org/sonarsource/ruby/converter/AstNode.java
@@ -39,1 +39,1 @@ public interface AstNode {
-  List availableAttributes();
+  List<Object> availableAttributes();

java:S6213 - Rename this variable to not match a restricted identifier. • MAJOR • View issue

Location: sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/converter/visitor/AssignmentVisitorTest.java:56

Why is this an issue?

Even if it is technically possible, Restricted Identifiers should not be used as identifiers. This is only possible for compatibility reasons, using it in Java code is confusing and should be avoided.

What changed

This hunk renames the variable var to varDecl in the test file. The variable name var is a restricted identifier in Java (used for local variable type inference since Java 10), and using it as a variable name is confusing and flagged as a code smell. By renaming it to varDecl, the code avoids using a restricted identifier as a variable name, resolving the static analysis warning.

--- a/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/converter/visitor/AssignmentVisitorTest.java
+++ b/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/converter/visitor/AssignmentVisitorTest.java
@@ -56,2 +56,2 @@ class AssignmentVisitorTest extends AbstractRubyConverterTest {
-    VariableDeclarationTree var = (VariableDeclarationTree) rubyStatement("a = a");
-    assertTree(var.identifier()).isEquivalentTo(var.initializer());
+    VariableDeclarationTree varDecl = (VariableDeclarationTree) rubyStatement("a = a");
+    assertTree(varDecl.identifier()).isEquivalentTo(varDecl.initializer());

java:S5961 - Refactor this method to reduce the number of assertions from 26 to less than 25. • MAJOR • View issue

Location: sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/externalreport/rubocop/RuboCopSensorTest.java:70

Why is this an issue?

A common good practice is to write test methods targeting only one logical concept, that can only fail for one reason.

What changed

Replaces 6 inline assertions for the first external issue with a single call to the extracted helper method assertExternalIssue. This reduces the assertion count in the issues_with_sonarqube test method, helping bring the total number of assertions from 26 down below the threshold of 25.

--- a/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/externalreport/rubocop/RuboCopSensorTest.java
+++ b/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/externalreport/rubocop/RuboCopSensorTest.java
@@ -74,7 +74,7 @@ class RuboCopSensorTest {
-    ExternalIssue first = externalIssues.get(0);
-    assertThat(first.primaryLocation().inputComponent().key()).isEqualTo("rubocop-project:useless-assignment.rb");
-    assertThat(first.ruleKey()).hasToString("external_rubocop:Lint/UselessAssignment");
-    assertThat(first.type()).isEqualTo(RuleType.CODE_SMELL);
-    assertThat(first.severity()).isEqualTo(Severity.MAJOR);
-    assertThat(first.primaryLocation().message()).isEqualTo("Lint/UselessAssignment: Useless assignment to variable - `param`.");
-    assertThat(location(first)).isEqualTo("from line 3 offset 2 to line 3 offset 7");
+    assertExternalIssue(externalIssues.get(0),
+      "rubocop-project:useless-assignment.rb",
+      "external_rubocop:Lint/UselessAssignment",
+      RuleType.CODE_SMELL,
+      Severity.MAJOR,
+      "Lint/UselessAssignment: Useless assignment to variable - `param`.",
+      "from line 3 offset 2 to line 3 offset 7");

java:S125 - This block of commented-out lines of code should be removed. • MAJOR • View issue

Location: sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/RubySensorTest.java:48

Why is this an issue?

Commented-out code distracts the focus from the actual executed code. It creates a noise that increases maintenance code. And because it is never executed, it quickly becomes out of date and invalid.

What changed

This hunk removes the commented-out line of code assertThat(logTester.logs()).contains("1 source files to be analyzed"); and replaces it with just an empty comment. This addresses the code smell about commented-out code that should be removed, as the static analysis rule (java:S125) flags blocks of commented-out lines of code that distract from actual executed code and create maintenance noise. By removing the commented-out assertion, the line no longer contains commented-out code that could become stale or misleading.

--- a/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/RubySensorTest.java
+++ b/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/RubySensorTest.java
@@ -48,1 +48,1 @@ class RubySensorTest extends AbstractSensorTest {
-    //assertThat(logTester.logs()).contains("1 source files to be analyzed");
+    //

java:S3457 - String contains no format specifiers. • MAJOR • View issue

Location: sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/SimpleCovSensorTest.java:203

Why is this an issue?

A printf--style format string is a string that contains placeholders, usually represented by special characters such as "%s" or "{}", depending on the technology in use. These placeholders are replaced by values when the string is printed or logged.

What changed

This hunk removes the unnecessary String.format() call that wrapped a string literal containing no format specifiers. The original code used String.format("Cannot read coverage report file...") but the string had no placeholders like %s or %d, making the String.format() call pointless. The fix replaces it with a simple string assignment, eliminating the code smell flagged by the static analysis rule about format strings containing no format specifiers.

--- a/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/SimpleCovSensorTest.java
+++ b/sonar-ruby-plugin/src/test/java/org/sonarsource/ruby/plugin/SimpleCovSensorTest.java
@@ -203,2 +203,2 @@ class SimpleCovSensorTest {
-    String expectedMessage = String.format(
-      "Cannot read coverage report file, expecting standard SimpleCov JSON formatter output: 'invalid_resultset.json'");
+    String expectedMessage =
+      "Cannot read coverage report file, expecting standard SimpleCov JSON formatter output: 'invalid_resultset.json'";

Have a suggestion or found an issue? Share your feedback here.

---

SonarQube Remediation Agent uses AI. Check for mistakes.

[hashicorp-vault-sonar-prod]

SONARRUBY-164

[sonar-review-alpha]

Summary

⚠️ The PR description exceeded the analysis limit and was truncated. The review may not reflect all context.

Five SonarQube code quality fixes across the Ruby plugin:

1. Generic type parameter (AstNode.java): Added `` type parameter to List return type in interface to eliminate raw type usage and improve compile-time type safety.

2. Restricted identifier (AssignmentVisitorTest.java): Renamed variable var to varDecl to avoid using Java's restricted identifier var (introduced in Java 10).

3. Test refactoring (RuboCopSensorTest.java): Extracted a new helper method assertExternalIssue() to consolidate repetitive external issue validation logic. This reduces assertion count in the test method from 26 to 12 while improving readability. Note: This refactoring also fixes a latent bug where the second issue's assertions were incorrectly checking the first issue object.

4. Commented code removal (RubySensorTest.java): Removed a commented-out assertion line to eliminate dead code.

5. Unnecessary String.format() removal (SimpleCovSensorTest.java): Replaced String.format() with direct string assignment since no formatting parameters are used.

All changes are in test code and interfaces—no production logic is modified.

What reviewers should know

Key reviewer observations:

• Low risk: No production code logic changes, only quality improvements to tests and interface signatures.
• Bug fix bonus: The RuboCopSensorTest refactoring also fixes a copy-paste bug in the original code where the second external issue validation was checking properties of the first issue.
• Start with RuboCopSensorTest.java: This is the most substantial change. Verify that the new assertExternalIssue() helper correctly validates all 6 properties and that the method signature and calls are correct.
• Simple validations: The other four changes are straightforward—verify that renamed variables are consistently updated throughout their scope and that generic types are appropriate.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[sonar-review-alpha]

LGTM! ✅

Clean PR. The RuboCopSensorTest refactoring is the most notable change: the assertExternalIssue() helper is well-structured and, as a side effect, fixes a latent copy-paste bug in the second-issue block where first.ruleKey(), first.type(), first.severity(), and first.primaryLocation().message() were all asserting against the wrong object. The bug was masked because both issues share the same rule and severity values. All other changes are straightforward and correct.

🗣️ Give feedback

[sonarqube-next]

Quality Gate passed for 'Ruby'

Issues
0 New issues
5 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

[github-actions]

This PR is stale because it has been open 7 days with no activity. If there is no activity in the next 7 days it will be closed automatically