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

UnnecessaryAbstractClass false positive for abstractproperties #2526

Closed
WonderCsabo opened this issue Mar 27, 2020 · 4 comments · Fixed by #2533
Closed

UnnecessaryAbstractClass false positive for abstractproperties #2526

WonderCsabo opened this issue Mar 27, 2020 · 4 comments · Fixed by #2533

Comments

@WonderCsabo
Copy link

Expected Behavior

UnnecessaryAbstractClass should allow an abstract class with abstract properties

Observed Behavior

UnnecessaryAbstractClass fails on an abstract class with abstract properties

Steps to Reproduce

abstract class ChildViewModel : ViewModel() {

    var parentViewModel: BaseViewModel? = null

    public override fun onCleared() {
        viewModelScope.cancel()
        super.onCleared()
    }
}

abstract class BaseTransferWidgetViewModel : ChildViewModel() {

    abstract val items: LiveData<List<RowViewModel<*>>>

    abstract val widgetListMaxSize: Int

    abstract val selectItemCommand: CommandFactory<RowViewModel<*>>

    abstract val showMoreCommand: Command
}

UnnecessaryAbstractClass fails on BaseTransferWidgetViewModel. It allowed it in detekt 1.7.0

Your Environment

  • Version of detekt used: 1.7.1
  • Version of Gradle used (if applicable): 6.2.2
  • Operating System and version: macOS 10.14.6
@BraisGabin
Copy link
Member

🤔 BaseTransferWidgetViewModel could be an interface safely. But maybe that's a bit opinated: "composition over inheritance". I'm not sure if that's what this rule is looking for.

@schalkms
Copy link
Member

This is a bug. Thanks for reporting this.
This came in with a bug fix in v1.7.1. Now detekt also considers base classes, when reporting an abstract class without any concrete members. Unfortunately, this case slipped through.
I have a fix in the queue.

@schalkms schalkms removed the bug label Mar 27, 2020
@arturbosch arturbosch added this to the 1.7.2 milestone Mar 28, 2020
arturbosch pushed a commit that referenced this issue Mar 28, 2020
An abstract class with concrete members derived from a base class is not reported by this rule, since there are actually concrete members created in the inheritance tree.

Closes #2526
@sakthi-logn
Copy link

sakthi-logn commented Feb 23, 2023

Why does Detekt report this ParentClass as an UnnecessaryAbstractClass ?

abstract class ParentClass {
    abstract val type: String
}
data class ChildOne(val prop1: Int) {
   override val type = "ChildOne"
}
data class ChildTwo(val prop2: Double) {
   override val type = "ChildTwo"
}

Edit: just improve formatting.

@BraisGabin
Copy link
Member

I think that the rule tells that it could be replaced by an interface and that's true and the reason yout code is reported:

interface ParentClass {
    val type: String
}

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

Successfully merging a pull request may close this issue.

5 participants