Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SONARJAVA-5232 S1192 Relax duplicate string check for Throwable arguments #5036

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomasz-tylenda-sonarsource
Copy link
Contributor

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource commented Feb 18, 2025

SONARJAVA-5232

Previously #4984 and #5006

Copy link

github-actions bot commented Mar 4, 2025

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

@github-actions github-actions bot added the stale label Mar 4, 2025
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource marked this pull request as draft March 4, 2025 08:07
@github-actions github-actions bot removed the stale label Mar 5, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good overall but before merging, it would make sense to:

  • Document the last suggested case with exceptions constructed and then thrown
  • Squash the PR into a single commit
  • Rewrite the commit message to be very explicit in the fact that this change is a relaxation of the existing rule

This change addresses false positives reported on SonarCloud.
Throwable arguments are intended for human readability,
making constant extraction unnecessary. Therefore, we no longer
report duplication in these cases.

This change also accounts for corner cases where a string used
in a Throwable has an existing constant or is also used
in another context.
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource force-pushed the tt-S1192-exceptions-with-constants branch from c74a3fe to a60893c Compare March 6, 2025 12:27
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource changed the title SONARJAVA-5232 S1192 Do not consider arguments in Throwables when checking for duplicated strings SONARJAVA-5232 S1192 Relax duplicate string check for Throwable arguments Mar 6, 2025
@tomasz-tylenda-sonarsource
Copy link
Contributor Author

This looks good overall but before merging, it would make sense to:

  • Document the last suggested case with exceptions constructed and then thrown
  • Squash the PR into a single commit
  • Rewrite the commit message to be very explicit in the fact that this change is a relaxation of the existing rule

Done.

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