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

EmptyFunctionBlock false positive: overridden function with comment in body #1684

Closed
technoir42 opened this issue Jun 8, 2019 · 2 comments · Fixed by #1690
Closed

EmptyFunctionBlock false positive: overridden function with comment in body #1684

technoir42 opened this issue Jun 8, 2019 · 2 comments · Fixed by #1690

Comments

@technoir42
Copy link

detekt 1.0.0-RC15

This gets reported by EmptyFunctionBlock as violation:

override fun fetch() {
    // No-op
}

There is ignoreOverriddenFunctions but I don't want to ignore all overridden functions, just ones with a comment in their body.

@rock3r
Copy link
Contributor

rock3r commented Jun 8, 2019

I have the same. An example that triggers it:

private interface AnimationEndListener : Animator.AnimatorListener {
    override fun onAnimationStart(animation: Animator) {
        // No-op
    }
    override fun onAnimationCancel(animation: Animator) {
        // No-op
    }
    override fun onAnimationRepeat(animation: Animator) {
        // No-op
    }
}

It looks like what the docs say: "This rule will not report functions overriding others.", is false, or more likely, the rule is bugged.

@technoir42 I think that config option is for the opposite case, that is, open functions that are meant to be overridden by subclasses.

cortinico added a commit to cortinico/detekt that referenced this issue Jun 12, 2019
This rule had a bug in reporting functions with override and empty
blocks (with or without comments). I'm fixing it and adding a couple of
tests to make sure it works properly.

Fixes detekt#1684
@cortinico
Copy link
Member

Yup I was able to reproduce it.
Apparently the logic use to check if the body had a comment or not was buggy. Moreover as @rock3r pointed out, the documentation was really confusing. Here the fix #1690

cortinico added a commit to cortinico/detekt that referenced this issue Jun 13, 2019
This rule had a bug in reporting functions with override and empty
blocks (with or without comments). I'm fixing it and adding a couple of
tests to make sure it works properly.

Fixes detekt#1684
Bugs and False Positives automation moved this from To do to Done Jun 17, 2019
arturbosch pushed a commit that referenced this issue Jun 17, 2019
* Fixed bug reporting false positives with EmptyFunctionBlock

This rule had a bug in reporting functions with override and empty
blocks (with or without comments). I'm fixing it and adding a couple of
tests to make sure it works properly.

Fixes #1684

* Clarify the documentation for the EmptyFunctionBlock rule

* Running the fixed EmptyFunctionBlock on the codebase

* Update EmptyFunctionBlockSpec to use hasLocationStrings

* Introduce the `hasExactlyLocationStrings` assertion method.
@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

4 participants