Skip to content

JS-1493 Fix open SonarCloud maintainability issues#6642

Open
zglicz wants to merge 3 commits intomasterfrom
codex/fix-sonar-maintainability
Open

JS-1493 Fix open SonarCloud maintainability issues#6642
zglicz wants to merge 3 commits intomasterfrom
codex/fix-sonar-maintainability

Conversation

@zglicz
Copy link
Contributor

@zglicz zglicz commented Mar 19, 2026

Summary

This PR fixes the currently open SonarCloud maintainability issues for SonarSource_SonarJS that are in this codebase slice.

Main changes:

  • TypeScript cleanup for maintainability rules:
    • Replace escaped regex strings with String.raw in config/test helper patterns
    • Use optional chaining where recommended
    • Replace whitelist array lookups with Set.has
    • Remove unnecessary type assertion
  • Java cleanup for maintainability rules:
    • Remove unused import
    • Avoid eager logging argument computation by guarding calls where needed
    • Use parameterized logging instead of String.format
    • Use isEmpty() for empty-string check
    • Improve exception handling by rethrowing with contextual message

Validation

  • npx prettier --check on touched files
  • npx tsx --tsconfig packages/tsconfig.test.json --test ...
    • packages/jsts/src/rules/S1192/unit.test.ts
    • packages/jsts/src/rules/S134/unit.test.ts
    • packages/jsts/src/rules/S3776/unit.test.ts
    • packages/jsts/src/rules/S3782/unit.test.ts
    • packages/jsts/src/rules/S4158/unit.test.ts
    • packages/jsts/src/rules/S6486/cb.test.ts
    • packages/jsts/tests/tools/testers/comment-based/framework.test.ts
  • Java compile checks:
    • mvn -pl sonar-plugin/bridge -DskipTests -Dexec.skip=true compile
    • mvn -pl sonar-plugin/sonar-javascript-plugin -DskipTests -Dexec.skip=true compile

@sonar-review-alpha
Copy link

sonar-review-alpha bot commented Mar 19, 2026

Summary

This PR fixes ~15 SonarCloud maintainability violations through targeted refactorings:

TypeScript: Uses String.raw for cleaner regex literals, converts array lookups to Set.has() for O(1) performance, replaces verbose null checks with optional chaining (?.), and removes an unnecessary type assertion.

Java: Guards debug/warn log calls with isDebugEnabled()/isWarnEnabled() to avoid computing arguments when logging is disabled, switches from String.format() to parameterized logging, uses isEmpty() instead of length() > 0, removes unused imports, and improves an exception handler to include contextual details instead of logging separately.

All changes maintain backward compatibility and are validated by the existing test suite and compilation checks.

What reviewers should know

Where to start: The TypeScript and Java sections are independent—reviewers can evaluate them separately. Most changes are mechanical replacements with no behavioral impact.

Key points to watch:

  • The Java logging guards (if (LOG.isDebugEnabled())) are intentional performance improvements, not error handling—they prevent string concatenation/method calls when logs are disabled.
  • The String.raw usage in TypeScript test helpers unescapes regex patterns for readability (e.g., \s instead of \\s).
  • In BridgeServerImpl.java, the exception handler now throws with a contextual message instead of logging separately—this consolidates error reporting into one place.
  • The Set conversion in S1192/rule.ts improves lookup performance from O(n) to O(1) for the ignore-strings check.

Trivial changes: The escaped quote removal in S134 config and the toString()this swap in NodeCommand are pure style fixes.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Fix open SonarCloud maintainability issues JS-1493 Fix open SonarCloud maintainability issues Mar 19, 2026
@hashicorp-vault-sonar-prod
Copy link

hashicorp-vault-sonar-prod bot commented Mar 19, 2026

JS-1493

@zglicz
Copy link
Contributor Author

zglicz commented Mar 19, 2026

@zglicz zglicz requested a review from a team March 19, 2026 13:52
@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

Ruling Report

No changes to ruling expected issues in this PR

@sonar-review-alpha
Copy link

sonar-review-alpha bot commented Mar 19, 2026

Summary

This PR fixes 8 SonarCloud maintainability issues across the TypeScript and Java codebase. Changes fall into three main categories:

  1. Optional chaining refactoring: Simplified null checks (e.g., !x || x.propx?.prop) in 5 rules to improve code readability
  2. Logging optimization: Added performance guards (isDebugEnabled(), isWarnEnabled()) before expensive logging calls in 4 Java files to avoid unnecessary string composition
  3. Pattern modernization: Converted escaped regex strings to String.raw templates for clarity in test helpers and config files; improved regex patterns for quoted message content in test fixtures; replaced array lookups with Set; removed unused imports and type assertions

What reviewers should know

Key areas to review:

  • Optional chaining logic: Spot-check the conversions in S3776, S3782, S4158, S6486 to confirm they're semantically equivalent (they eliminate falsy checks, which is safe since ?. short-circuits on null/undefined)
  • Regex refactors in quickfixes.ts & locations.ts: The new MESSAGE_CONTENT and BRACED_TEXT_CONTENT patterns use negative lookahead to match content safely inside braces—verify these don't break existing test fixtures
  • Exception handling change in BridgeServerImpl.java: Now throws the exception directly with a contextual message instead of logging separately; confirm this preserves the intended error reporting behavior
  • Type assertion removal in module.ts: The cast was redundant since isRequire() already narrows the type—verify TypeScript is satisfied

All TypeScript test files mentioned in the validation section should pass, and the Java changes are straightforward guard additions + string improvements.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-next
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants