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

StringLiteralDuplication potential false positive with interpolated strings #7271

Open
TWiStErRob opened this issue May 8, 2024 · 3 comments

Comments

@TWiStErRob
Copy link
Member

Expected Behavior

No finding.

Observed Behavior

Test.kt:76:21 (see cursor in screenshot): Multiple occurrences of the same string literal within a single file detected. Prefer extracting the string literal into a property or constant. [StringLiteralDuplication]

Steps to Reproduce

image

Context

As you can see in the screenshot above, the dynamic test names are being detected as duplicate. This would be fine, because at first glance all the "cycle start-end" strings "look" the same, but if you look at the start/end variables, you can see they all have different types. At this point, I'm unsure if this detection is valid, because there would be no function that could be extracted to simplify the code.

Your Environment

  • Version of detekt used: 1.23.6
  • Version of Gradle used (if applicable): 8.7
  • Gradle scan link (add --scan option when running the gradle task): N/A
  • Operating System and version: N/A
  • Link to your project (if it's a public repository): N/A
@BraisGabin
Copy link
Member

At this point, I'm unsure if this detection is valid, because there would be no function that could be extracted to simplify the code.

A function that receives Any. You only care about the toString of those classes.

Anyway, should this rule check strings with variables inside it? I'm not sure.

@TWiStErRob
Copy link
Member Author

🤦🏻‍♂️ that makes so much sense. So this is a viable option:

fun cycle(start: Any, end: Any): String =
    "cycle ${start}-${end}"

I guess your observation will hold true for any expression inside an interpolated string. So extracting a function with String or Any params is a viable way to fix the duplication.

Could this be just a doc update with a hint then? (TBH, I haven't read the docs of this rule yet, might be already there...)

@BraisGabin
Copy link
Member

Could this be just a doc update with a hint then?

No sure about what could we write there for this case but sure, any improvement on the docs is more than welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants