Skip to content

SONARJAVA-6090 Detect usage of io in rule S106#5435

Merged
rombirli merged 7 commits intomasterfrom
rombirli/SONARJAVA-6090-detect-usage-of-io-in-S106
Feb 11, 2026
Merged

SONARJAVA-6090 Detect usage of io in rule S106#5435
rombirli merged 7 commits intomasterfrom
rombirli/SONARJAVA-6090-detect-usage-of-io-in-S106

Conversation

@rombirli
Copy link
Copy Markdown
Contributor

@rombirli rombirli commented Feb 4, 2026

No description provided.

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

hashicorp-vault-sonar-prod Bot commented Feb 4, 2026

SONARJAVA-6090

Copy link
Copy Markdown
Contributor

@asya-vorobeva asya-vorobeva left a comment

Choose a reason for hiding this comment

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

Also makes sense to enrich rule description to mention IO.

Comment thread java-checks/src/main/java/org/sonar/java/checks/SystemOutOrErrUsageCheck.java Outdated
@rombirli rombirli force-pushed the rombirli/SONARJAVA-6090-detect-usage-of-io-in-S106 branch from b018bac to 03705e4 Compare February 5, 2026 10:59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but please wait for Asya's review.

Copy link
Copy Markdown
Contributor

@asya-vorobeva asya-vorobeva left a comment

Choose a reason for hiding this comment

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

I think we can remove in line 49 a check
tree instanceof MemberSelectExpressionTree mset. (which is always met)
Also please don't forget about updating metadata for the rule here.

@rombirli
Copy link
Copy Markdown
Contributor Author

I think we can remove in line 49 a check
tree instanceof MemberSelectExpressionTree mset. (which is always met)

I’d prefer to keep this as is for a couple of reasons:

  • Scope Creep: This change wasn't part of my PR; it was recently introduced by @tomasz-tylenda-sonarsource in b9dcc06. I'd like to keep the current PR changes minimal and focused.
  • Best Practices: I find using pattern matching for instanceof to be a cleaner and safer way to handle variable casting compared to the traditional approach.

Since this is largely a matter of personal style preference and the code is already functional, can we leave it unchanged?

@rombirli
Copy link
Copy Markdown
Contributor Author

Also please don't forget about updating metadata for the rule here.

done in aa43ca2

@asya-vorobeva
Copy link
Copy Markdown
Contributor

asya-vorobeva commented Feb 10, 2026

I think we can remove in line 49 a check
tree instanceof MemberSelectExpressionTree mset. (which is always met)

I’d prefer to keep this as is for a couple of reasons:

  • Scope Creep: This change wasn't part of my PR; it was recently introduced by @tomasz-tylenda-sonarsource in b9dcc06. I'd like to keep the current PR changes minimal and focused.
  • Best Practices: I find using pattern matching for instanceof to be a cleaner and safer way to handle variable casting compared to the traditional approach.

Since this is largely a matter of personal style preference and the code is already functional, can we leave it unchanged?

As for me,

 else if (!isCompactSourceFile) {
      visitMemberSelectExpression((MemberSelectExpressionTree) tree);
 }

looks much more cleaner. But as you wish. (best practices are not dogmatic thing, depends on the situation)

About your first point, please believe me: it's much more better to leave the code in the file that you're working with better than before. Sort of cumulative changes, good way to fight with tech debt. :)

Copy link
Copy Markdown
Contributor

@asya-vorobeva asya-vorobeva left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqube-next
Copy link
Copy Markdown

@rombirli rombirli merged commit 25ed53b into master Feb 11, 2026
16 checks passed
@rombirli rombirli deleted the rombirli/SONARJAVA-6090-detect-usage-of-io-in-S106 branch February 11, 2026 14:30
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.

3 participants