SONARJAVA-6446 Refactor merging list of arguments to MethodMatchers by tomasz-tylenda-sonarsource · Pull Request #5657 · SonarSource/sonar-java

tomasz-tylenda-sonarsource · 2026-06-05T08:41:21Z

Summary by Gitar

New utility method:
- Added StringUtils.flatten to concatenate String, String[], and Collection<String> arguments into a single String[] array.
Refactoring:
- Simplified MethodMatchers definitions in UnitTestUtils by replacing complex Stream.concat logic with the new flatten helper.
Testing:
- Added StringUtilsTest.testFlatten to verify argument merging and exception handling for unsupported types.

_{This will update automatically on new commits.}

hashicorp-vault-sonar-prod · 2026-06-05T08:41:40Z

gitar-bot · 2026-06-05T08:57:25Z

+  public static String[] stringArgs(Object ... args) {
+    List<String> result = new ArrayList<>();
+    for (Object arg : args) {
+      if (arg instanceof String s) {
+        result.add(s);
+      } else if (arg instanceof String[] arr) {
+        Collections.addAll(result, arr);
+      } else if (arg instanceof Collection<?> col) {
+        for (Object o : col) {
+          if (o instanceof String s) {
+            result.add(s);
+          } else {
+            throw new IllegalArgumentException("Unsupported collection element type: " + o.getClass());
+          }
+        }


💡 Edge Case: stringArgs throws NPE on null argument instead of IllegalArgumentException

In StringUtils.stringArgs(Object... args), a null element in args (or a null element inside a passed Collection) does not match any instanceof branch and falls through to throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()). Since arg is null, arg.getClass() throws a NullPointerException rather than the intended IllegalArgumentException, and the message is never produced. All current call sites pass non-null string literals and constants, so this is not triggered today, but the method is public and the failure mode is misleading. Consider handling null explicitly, e.g. if (arg == null) throw new IllegalArgumentException("Unsupported argument type: null"); (and similarly for collection elements).

_{Was this helpful? React with 👍 / 👎}

dorian-burihabwa-sonarsource

The change looks OK to me but I would rather we keep the code simpler and make it evolve if we feel a need for a dedicated type of exception message when we encounter the issue in a collection or a regular argument

…Utils.java Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com>

sonarqube-next · 2026-06-05T15:34:42Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

gitar-bot · 2026-06-05T15:45:10Z

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Simplifies MethodMatchers argument list merging by introducing a new StringUtils helper method. Note that stringArgs currently throws an NPE on null inputs instead of the expected IllegalArgumentException.

💡 Edge Case: stringArgs throws NPE on null argument instead of IllegalArgumentException

📄 java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java:53-67

In StringUtils.stringArgs(Object... args), a null element in args (or a null element inside a passed Collection) does not match any instanceof branch and falls through to throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()). Since arg is null, arg.getClass() throws a NullPointerException rather than the intended IllegalArgumentException, and the message is never produced. All current call sites pass non-null string literals and constants, so this is not triggered today, but the method is public and the failure mode is misleading. Consider handling null explicitly, e.g. if (arg == null) throw new IllegalArgumentException("Unsupported argument type: null"); (and similarly for collection elements).

🤖 Prompt for agents

Code Review: Simplifies MethodMatchers argument list merging by introducing a new StringUtils helper method. Note that stringArgs currently throws an NPE on null inputs instead of the expected IllegalArgumentException.

1. 💡 Edge Case: stringArgs throws NPE on null argument instead of IllegalArgumentException
   Files: java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java:53-67

   In `StringUtils.stringArgs(Object... args)`, a `null` element in `args` (or a `null` element inside a passed `Collection`) does not match any `instanceof` branch and falls through to `throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass())`. Since `arg` is null, `arg.getClass()` throws a `NullPointerException` rather than the intended `IllegalArgumentException`, and the message is never produced. All current call sites pass non-null string literals and constants, so this is not triggered today, but the method is `public` and the failure mode is misleading. Consider handling null explicitly, e.g. `if (arg == null) throw new IllegalArgumentException("Unsupported argument type: null");` (and similarly for collection elements).

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 08:41 — with GitHub Actions Inactive

hashicorp-vault-sonar-prod Bot changed the title ~~Refactor merging list of arguments to MethodMatchers~~ SONARJAVA-6446 Refactor merging list of arguments to MethodMatchers Jun 5, 2026

tomasz-tylenda-sonarsource force-pushed the tt/string-varargs branch from 849d446 to 2ccd493 Compare June 5, 2026 08:56

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 08:56 — with GitHub Actions Inactive

gitar-bot Bot reviewed Jun 5, 2026

View reviewed changes

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 09:35 — with GitHub Actions Inactive

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 09:46 — with GitHub Actions Inactive

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 09:50 — with GitHub Actions Inactive

Refactor merging list of arguments to MethodMatchers

16dd947

tomasz-tylenda-sonarsource force-pushed the tt/string-varargs branch from 3e6eb59 to 16dd947 Compare June 5, 2026 12:45

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 12:45 — with GitHub Actions Inactive

tomasz-tylenda-sonarsource requested a review from dorian-burihabwa-sonarsource June 5, 2026 14:22

dorian-burihabwa-sonarsource approved these changes Jun 5, 2026

View reviewed changes

Comment thread java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java Outdated

Comment thread java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java Outdated

Comment thread java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java Outdated

Update java-checks/src/main/java/org/sonar/java/checks/helpers/String…

3ef5550

…Utils.java Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com>

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 15:13 — with GitHub Actions Inactive

Update java-checks/src/main/java/org/sonar/java/checks/helpers/String…

f79f243

…Utils.java Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com>

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 15:13 — with GitHub Actions Inactive

review

e92f334

tomasz-tylenda-sonarsource temporarily deployed to sca-checking June 5, 2026 15:23 — with GitHub Actions Inactive

amend javadoc

7e2f744

tomasz-tylenda-sonarsource deployed to sca-checking June 5, 2026 15:26 — with GitHub Actions Active

tomasz-tylenda-sonarsource enabled auto-merge (squash) June 5, 2026 15:35

tomasz-tylenda-sonarsource merged commit 7d839d4 into master Jun 5, 2026
15 checks passed

tomasz-tylenda-sonarsource deleted the tt/string-varargs branch June 5, 2026 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SONARJAVA-6446 Refactor merging list of arguments to MethodMatchers#5657

SONARJAVA-6446 Refactor merging list of arguments to MethodMatchers#5657
tomasz-tylenda-sonarsource merged 5 commits into
masterfrom
tt/string-varargs

tomasz-tylenda-sonarsource commented Jun 5, 2026 •

edited by gitar-bot Bot

Loading

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jun 5, 2026 •

edited by atlassian Bot

Loading

Uh oh!

gitar-bot Bot Jun 5, 2026

Uh oh!

dorian-burihabwa-sonarsource left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqube-next Bot commented Jun 5, 2026

Uh oh!

Uh oh!

gitar-bot Bot commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tomasz-tylenda-sonarsource commented Jun 5, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jun 5, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitar-bot Bot Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

dorian-burihabwa-sonarsource left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqube-next Bot commented Jun 5, 2026

Quality Gate passed

Uh oh!

Uh oh!

gitar-bot Bot commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tomasz-tylenda-sonarsource commented Jun 5, 2026 •

edited by gitar-bot Bot

Loading

hashicorp-vault-sonar-prod Bot commented Jun 5, 2026 •

edited by atlassian Bot

Loading