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

ignoreAnnotated not respected on UnnecessaryAbstractClass #4356

Closed
ZacSweers opened this issue Dec 3, 2021 · 7 comments · Fixed by #4570
Closed

ignoreAnnotated not respected on UnnecessaryAbstractClass #4356

ZacSweers opened this issue Dec 3, 2021 · 7 comments · Fixed by #4570
Labels

Comments

@ZacSweers
Copy link
Contributor

ZacSweers commented Dec 3, 2021

Expected Behavior

After migrating to ignoreAnnotated per deprecation warning messages, it's no longer respecting the annotations we specify

Detekt 1.18.0

UnnecessaryAbstractClass:
  active: true
  excludeAnnotatedClasses: ['dagger.Module']

Warns

Property 'style>UnnecessaryAbstractClass>excludeAnnotatedClasses' is deprecated. Use `ignoreAnnotated` instead.

Detekt 1.19.0

UnnecessaryAbstractClass:
  active: true
  ignoreAnnotated: ['dagger.Module']

Observed Behavior

It incorrectly warns about classes annotated with dagger.Module

Steps to Reproduce

Here's an example module

@Module
abstract class CallsBaseModule {

  @Binds
  abstract fun CallPresenterImpl.provideCallPresenter(): CallContract.CallPresenter

  @Binds
  abstract fun CallServiceProviderImpl.bindCallServiceProvider(): CallServiceProvider
}

Context

It is a regression

Your Environment

  • Version of detekt used: 1.19.0
  • Version of Gradle used (if applicable): N/A
  • Gradle scan link (add --scan option when running the gradle task): N/A
  • Operating System and version: macOS 11
  • Link to your project (if it's a public repository): N/A
@ZacSweers ZacSweers added the bug label Dec 3, 2021
@severn-everett
Copy link
Contributor

I was unable to reproduce the issue in a POC project with the following CallBaseModule.kt file:

package com.severett.kotlinpoc.model

import dagger.Module

@Module
abstract class CallsBaseModule

This used Detekt 1.19.0 with the following configuration:

  UnnecessaryAbstractClass:
    active: true
    ignoreAnnotated: ['dagger.Module']

In addition, the project uses Kotlin 1.6.0 and Gradle 7.3.1.

Would you be able to give a more detailed description of the code base layout and the Detekt configuration file in full?

@leinardi
Copy link

leinardi commented Dec 5, 2021

Hi @severn-everett, my project is also affected by this issue. You should be able to reproduce it checking out the branch detekt-1.19.0: https://github.com/leinardi/Forlago/tree/detekt-1.19.0

if you run ./gradlew detekt you should get this output:

> Task :modules:core-navigation:detekt FAILED
style - 5min debt
        UnnecessaryAbstractClass - [NavigatorModule] at /home/leinardi/Workspace/github/Forlago/modules/core-navigation/src/main/kotlin/com/leinardi/forlago/core/navigation/NavigatorModule.kt:24:1

Overall debt: 5min


FAILURE: Build failed with an exception.

@BraisGabin
Copy link
Member

I think that this issue issue is virtually the same as: #4355 (comment) and the same "workaround" should work.

@leinardi
Copy link

leinardi commented Dec 5, 2021

As a "workaround" you can use the name of the annotation instead of the full qualified name. Or enable type solving.

@BraisGabin yep, I can confirm that changing

    ignoreAnnotated:
      - 'dagger.Module'

to

    ignoreAnnotated:
      - 'Module'

is a valid workaround.

@severn-everett
Copy link
Contributor

@leinardi Your project is for Android, whereas I only have JVM-based projects on my machine. I'm wondering if this could be an issue specific with Android; would you have this issue if you tried setting it up in a JVM-based project?

@leinardi
Copy link

leinardi commented Dec 5, 2021

I'm pretty sure that Detekt runs on JVM even if the project is an Android project: every Gradle task that runs on my local machine does not run on the Android Runtime (ART) but on my local JVM. The ART is used only on emulators and real Android devices.
If you are worried about JVM specific issues, I'm currently running Detekt on JVM 11:

$ java -version
openjdk version "11.0.12" 2021-07-20
OpenJDK Runtime Environment (build 11.0.12+7-Ubuntu-0ubuntu3)
OpenJDK 64-Bit Server VM (build 11.0.12+7-Ubuntu-0ubuntu3, mixed mode, sharing)

@cortinico you are also an Android developer, do you think this could be Android specific?

@severn-everett
Copy link
Contributor

I realized that UnnecessaryAbstractClass wouldn't ding my example abstract class CallsBaseModule in any case; once I added brackets, I was able to reproduce the issue.

alvindizon added a commit to alvindizon/Tampisaw that referenced this issue Aug 7, 2022
- Using 'dagger.Module' triggers UnnecessaryAbstractClass, replacing it
  with 'Module' fixes the warning
- See: detekt/detekt#4356
Signed-off-by: Alvin Dizon <alvin.dizon91@gmail.com>
alvindizon added a commit to alvindizon/Tampisaw that referenced this issue Aug 7, 2022
- Using 'dagger.Module' triggers UnnecessaryAbstractClass, replacing it
  with 'Module' fixes the warning
- See: detekt/detekt#4356
Signed-off-by: Alvin Dizon <alvin.dizon91@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants