Skip to content

SONARJAVA-4634 Test for duplicates in credential method json file #5132

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

romainbrenguier
Copy link
Contributor

@romainbrenguier romainbrenguier commented May 7, 2025

SONARJAVA-4634

Adding a test for duplicated entries found a few cases where we could have false positives because of wildcard usage.

@romainbrenguier romainbrenguier changed the title SONARJAVA-5464 Test for duplicates in credential method json file SONARJAVA-4634 Test for duplicates in credential method json file May 7, 2025
@romainbrenguier romainbrenguier force-pushed the romain/test-credential-methods branch from 629a216 to 9799aad Compare May 7, 2025 14:25
To ensure full coverage of the changes.
@romainbrenguier romainbrenguier marked this pull request as ready for review May 7, 2025 14:52
@@ -7438,7 +7438,7 @@
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","int","java.lang.String","int","java.lang.String"],"indices":[4]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","int","java.lang.String","java.lang.String","int"],"indices":[5]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","int","java.lang.String","java.lang.String","int","java.lang.String"],"indices":[5]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","java.lang.String"],"indices":[3]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","org.apache.commons.pool2.impl.GenericObjectPoolConfig","java.lang.String"],"indices":[3]},

Choose a reason for hiding this comment

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

I think the entry with * that was removed here was covering both line 7441 and line 7443, maybe instead of replacing this entry we could remove line 7443 as it's redundant?
I'm not sure if the intent of these entries is to prevent or to fix, the * approach would cover possible future new signatures, while being more specific with the naming allows to intervene only when needed.
I think we might want an opinion from Hendrik here, as he's the main contributor to this list

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