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

NoNameShadowing false positive in kotlin-dsl #5328

Closed
eygraber opened this issue Sep 21, 2022 · 9 comments
Closed

NoNameShadowing false positive in kotlin-dsl #5328

eygraber opened this issue Sep 21, 2022 · 9 comments
Labels

Comments

@eygraber
Copy link
Contributor

I've been getting NoNameShadowing violations in a project that uses kotlin-dsl where the name being shadowed is implicit lambda parameter 'it'. I think it might be because of the lambda getting converted into an Action because there's no other it in scope.

public fun Project.registerDetektTask(
  name: String,
  sourceSet: KotlinSourceSet
): TaskProvider<Detekt> = tasks.register("detekt${name.capitalize()}", Detekt::class.java) {
  val detekt = detekt
  detekt.basePath?.let { basePath = it } // NoNameShadowing
}
@eygraber eygraber added the bug label Sep 21, 2022
@BraisGabin
Copy link
Member

I assume that the lambda of register has a parameter it and that's the issue. Can that be the case?

@eygraber
Copy link
Contributor Author

I believe it does, but I think kotlin-dsl applies a compiler plugin that converts it into a lambda with receiver. So there is no it and the compiler won't let you assign a name to the parameter (because it doesn't exist anymore).

@BraisGabin
Copy link
Member

I see the problem... But I don't think detekt should take care of this strange behaviour were a compiler plugin is involved. If those functions would annotated with something we could use that to ignore them but that's not the case here.

I think that here you should use a @Suppress or write it like this:

  detekt.basePath?.let { b -> basePath = b } // NoNameShadowing

@eygraber
Copy link
Contributor Author

I ended up specifying a name to get rid of the violation, but it could be annoying if there was a lot of them.

If detekt used a compiler plugin, would it make it easier to avoid this issue (or would it solve it outright)?

@BraisGabin
Copy link
Member

I imagine that with a compiler plugin this could be spotted.

Closing for the reasons I wrote before.

@TWiStErRob
Copy link
Member

TWiStErRob commented Oct 9, 2022

+2

I had the same issue with a different rules: just throwing my case here to maybe help the case of getting support for this in some form. I encountered two weird reports, one from Deprecated and another from NestedScopeFunctions. Only the Deprecated case is detailed here, as it was a bigger WTF. The below code is only focusing on the problem, other parts removed, also copied some context from external classes near the code for more visibility:

image

(😅 It's really meta to be having a problem with setting up Detekt Gradle plugin and getting reports from Detekt on it.)

@eygraber
Copy link
Contributor Author

eygraber commented Nov 3, 2022

Ran into this again with Deprecation and NoNameShadowing in a different projects that had a lot of violations. Was very annoying to address.

@BraisGabin
Copy link
Member

BraisGabin commented Nov 5, 2022

#5492 should fix this issue (or at least give us the option to fix it). And I understand that this is annoying but we are facing a limitation on our current engine. That's the reason of #5492.

@eygraber
Copy link
Contributor Author

eygraber commented Nov 6, 2022

Very excited for that. I'm ready to test 😅

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

No branches or pull requests

3 participants