Skip to content

CPP-7789 Add shared SonarResolve parser to analyzer commons#387

Merged
mostafa-mohammed-sonarsource merged 7 commits intomasterfrom
mm/CPP-7789-add-SonarResolve
Apr 14, 2026
Merged

CPP-7789 Add shared SonarResolve parser to analyzer commons#387
mostafa-mohammed-sonarsource merged 7 commits intomasterfrom
mm/CPP-7789-add-SonarResolve

Conversation

@mostafa-mohammed-sonarsource
Copy link
Copy Markdown
Contributor

@mostafa-mohammed-sonarsource mostafa-mohammed-sonarsource commented Apr 8, 2026

This PR moves the SonarResolve parser to sonar-analyzer-commons so it can be reused by other analyzers.

The initial implementation was released in CFamily 6.79. The main reason for moving it here is that we now need to improve and share the parsing logic, especially around code-formatting cases that can split or otherwise alter comments across multiple lines and break sonar-resolve directives.

This PR adds the shared parser and its tests to analyzer commons so follow-up fixes and enhancements can be implemented once and consumed from a common place. It is consumed in https://github.com/SonarSource/sonar-cpp/pull/6115.

@mostafa-mohammed-sonarsource
Copy link
Copy Markdown
Contributor Author

mostafa-mohammed-sonarsource commented Apr 8, 2026

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod bot commented Apr 8, 2026

CPP-7789

@mostafa-mohammed-sonarsource mostafa-mohammed-sonarsource changed the title mm/CPP-7789-add-SonarResolve CPP-7789 Add shared SonarResolve parser to analyzer commons Apr 8, 2026
@sonar-review-alpha
Copy link
Copy Markdown

sonar-review-alpha bot commented Apr 8, 2026

Summary

This PR moves the SonarResolve directive parser into shared analyzer commons so other SonarSource analyzers can reuse it. The parser handles multi-line sonar-resolve comments in source code, which is needed because code formatters can split directives across multiple comment lines.

The implementation adds:

  • SonarResolve.java (~437 lines): A data class with nested StreamingParser for parsing directives incrementally, supporting case-insensitive keywords, multiple justification delimiters, escape sequences, and explicit error messages
  • SonarResolveTest.java (~358 lines): Comprehensive parametrized tests for single-line, multi-line, and error cases
  • Updated pom.xml files: JAR size thresholds and Sonar plugin API version

What reviewers should know

Start reading: The core implementation is in SonarResolve.java. The architecture has three levels:

  1. SonarResolve data class (lines 47-115): Simple immutable value object with constructor, accessors, equals/hashCode, toString
  2. StreamingParser (lines 117-187): Public API that accumulates lines and tracks parsing state (INCOMPLETE/COMPLETE/INVALID)
  3. Parser inner class (lines 189-456): Does the actual parsing, with a Cursor helper (lines 384-451) for text navigation

Key design decisions:

  • Streaming design: The StreamingParser.consumeLine() method deliberately doesn't close over newlines (lines 170-175 append with \n). This lets code formatters split directives across multiple comment lines naturally.
  • Case-insensitivity: expectLiteralIgnoreCase() at line 401 uses String.regionMatches(true, ...) so "SONAR-RESOLVE", "sonar-resolve", and mixed case all work.
  • Flexible delimiters (lines 365-382): Supports quotes, single quotes, backticks, and bracket pairs for justification to handle various comment styles.
  • Error recovery: Parser tracks detailed error messages (e.g., "unterminated status", "duplicate rule key") to help users fix invalid directives.

Testing coverage: Test file uses parametrized tests with @MethodSource streaming test data, covering:

  • Single-line success cases (16 variants with different delimiters, escapes, etc.)
  • Single-line failures (15 invalid syntax patterns)
  • Multi-line success cases (8 variants with breaks at different positions)
  • Multi-line failures (6 error conditions)
  • Edge cases like empty continuation lines inside justification (line 576-585)

Worth reviewing: The escape sequence handling (lines 334-345) and the multi-line accumulation logic—these are the areas most likely to interact with real formatter edge cases.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@lijun-chen-sonarsource lijun-chen-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. I personally very much like sonar-resolve as feature, so looking forward to see everything merged and released.

I left some comments. Curious to hear what you think.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

* CPP-7901 Make SonarResolve parser case-insensitive

* CPP-7901 Fix SonarResolveTest constructor usage

* CPP-7902 Support multiple justification delimiters in SonarResolve

* CPP-7902 Support multiple justification delimiters in SonarResolve

* CPP-7902 Fix SonarResolveTest constructor usage
@sonarqube-next
Copy link
Copy Markdown

Copy link
Copy Markdown

@sonar-review-alpha sonar-review-alpha bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

🗣️ Give feedback

Copy link
Copy Markdown

@lijun-chen-sonarsource lijun-chen-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mostafa-mohammed-sonarsource mostafa-mohammed-sonarsource merged commit 5dd8cd4 into master Apr 14, 2026
7 checks passed
@mostafa-mohammed-sonarsource mostafa-mohammed-sonarsource deleted the mm/CPP-7789-add-SonarResolve branch April 14, 2026 09:35
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