From 806aa7d4c1364156fa8927c3089e47377fa8a7a4 Mon Sep 17 00:00:00 2001 From: takahirom Date: Tue, 15 Aug 2017 12:45:38 +0300 Subject: [PATCH] Add inspection for redundant overrides that only calls the super method So #KT-19428 Fixed --- .../KotlinRedundantOverride.html | 5 + idea/src/META-INF/plugin.xml | 10 +- .../KotlinRedundantOverrideInspection.kt | 91 +++++++++++++++++++ .../redundantOverride/.inspection | 1 + .../redundantOverride/annotated.kt | 15 +++ .../redundantOverride/arguments.kt | 10 ++ .../redundantOverride/arguments.kt.after | 7 ++ .../redundantOverride/argumentsReplaced.kt | 11 +++ .../redundantOverride/basic.kt | 10 ++ .../redundantOverride/basic.kt.after | 7 ++ .../callDifferentSuperMethod.kt | 15 +++ .../redundantOverride/notCallSuper.kt | 12 +++ .../overrideModifireFinal.kt | 13 +++ .../overrideModifireVisibility.kt | 13 +++ .../singleExpressionFunction.kt | 8 ++ .../singleExpressionFunction.kt.after | 7 ++ .../redundantOverride/useGenericsSuper.kt | 14 +++ .../LocalInspectionTestGenerated.java | 69 ++++++++++++++ 18 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 idea/resources/inspectionDescriptions/KotlinRedundantOverride.html create mode 100644 idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/.inspection create mode 100644 idea/testData/inspectionsLocal/redundantOverride/annotated.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/arguments.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/arguments.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantOverride/argumentsReplaced.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/basic.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/basic.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/overrideModifireFinal.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt diff --git a/idea/resources/inspectionDescriptions/KotlinRedundantOverride.html b/idea/resources/inspectionDescriptions/KotlinRedundantOverride.html new file mode 100644 index 00000000000..f84f8e7b235 --- /dev/null +++ b/idea/resources/inspectionDescriptions/KotlinRedundantOverride.html @@ -0,0 +1,5 @@ + + +This inspection reports redundant 'override' function which can be omitted. + + \ No newline at end of file diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 1d1f10961f0..0f73159e30c 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2520,7 +2520,6 @@ displayName="Local variable naming convention" level="WEAK WARNING"/> - + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt new file mode 100644 index 00000000000..0d63b018452 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt @@ -0,0 +1,91 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.* +import com.intellij.openapi.project.Project +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression + +class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) = + object : KtVisitorVoid() { + override fun visitNamedFunction(function: KtNamedFunction) { + super.visitNamedFunction(function) + if (!function.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return + if (MODIFIER_EXCLUDE_OVERRIDE.any { function.hasModifier(it) }) return + if (function.annotationEntries.isNotEmpty()) return + + val bodyExpression = function.bodyExpression ?: return + val qualifiedExpression = when (bodyExpression) { + is KtDotQualifiedExpression -> bodyExpression + is KtBlockExpression -> { + val body = bodyExpression.statements.singleOrNull() + when (body) { + is KtReturnExpression -> body.returnedExpression + is KtDotQualifiedExpression -> body.takeIf { + function.typeReference.let { it == null || it.text == "Unit" } + } + else -> null + } + + } + else -> null + } as? KtDotQualifiedExpression ?: return + + val superExpression = qualifiedExpression.receiverExpression as? KtSuperExpression ?: return + if (superExpression.superTypeQualifier != null) return + + val superCallElement = qualifiedExpression.selectorExpression as? KtCallElement ?: return + if (!isSameFunctionName(superCallElement, function)) return + if (!isSameArguments(superCallElement, function)) return + + holder.registerProblem(function, + "Redundant override", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + RedundantOverrideFix()) + } + } + + private fun isSameArguments(superCallElement: KtCallElement, function: KtNamedFunction): Boolean { + val arguments = superCallElement.valueArguments + val parameters = function.valueParameters + if (arguments.size != parameters.size) return false + return arguments.zip(parameters).all { (argument, parameter) -> + argument.getArgumentExpression()?.text == parameter.name + } + } + + private fun isSameFunctionName(superSelectorExpression: KtCallElement, function: KtNamedFunction): Boolean { + val superCallMethodName = superSelectorExpression.getCallNameExpression()?.text ?: return false + return function.name == superCallMethodName + } + + private class RedundantOverrideFix : LocalQuickFix { + override fun getName() = "Remove redundant overrides" + override fun getFamilyName() = name + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + descriptor.psiElement.delete() + } + } + + companion object { + private val MODIFIER_EXCLUDE_OVERRIDE = KtTokens.MODIFIER_KEYWORDS_ARRAY.asList() - KtTokens.OVERRIDE_KEYWORD + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/.inspection b/idea/testData/inspectionsLocal/redundantOverride/.inspection new file mode 100644 index 00000000000..ee94eea81ba --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.KotlinRedundantOverrideInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/annotated.kt b/idea/testData/inspectionsLocal/redundantOverride/annotated.kt new file mode 100644 index 00000000000..6d92fd70e61 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/annotated.kt @@ -0,0 +1,15 @@ +// PROBLEM: none +@Target(AnnotationTarget.FUNCTION) +annotation class Annotation + +open class Foo { + open fun simple() { + } +} + +class Bar : Foo() { + @Annotation override fun simple() { + super.simple() + } +} + diff --git a/idea/testData/inspectionsLocal/redundantOverride/arguments.kt b/idea/testData/inspectionsLocal/redundantOverride/arguments.kt new file mode 100644 index 00000000000..5c569b0aa6e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/arguments.kt @@ -0,0 +1,10 @@ +open class Foo { + open fun arguments(arg1: Int, arg2: Long) { + } +} + +class Bar : Foo() { + override fun arguments(arg1: Int, arg2: Long) { + super.arguments(arg1, arg2) + } +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/arguments.kt.after b/idea/testData/inspectionsLocal/redundantOverride/arguments.kt.after new file mode 100644 index 00000000000..6a2c2330813 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/arguments.kt.after @@ -0,0 +1,7 @@ +open class Foo { + open fun arguments(arg1: Int, arg2: Long) { + } +} + +class Bar : Foo() { +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/argumentsReplaced.kt b/idea/testData/inspectionsLocal/redundantOverride/argumentsReplaced.kt new file mode 100644 index 00000000000..9782aa12f68 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/argumentsReplaced.kt @@ -0,0 +1,11 @@ +// PROBLEM: none +open class Foo { + open fun arguments(arg1: Int, arg2: Int) { + } +} + +class Bar : Foo() { + override fun arguments(arg1: Int, arg2: Int) { + super.arguments(arg2, arg1) + } +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/basic.kt b/idea/testData/inspectionsLocal/redundantOverride/basic.kt new file mode 100644 index 00000000000..fa3fd15fcfc --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/basic.kt @@ -0,0 +1,10 @@ +open class Foo { + open fun simple() { + } +} + +class Bar : Foo() { + override fun simple() { + super.simple() + } +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/basic.kt.after b/idea/testData/inspectionsLocal/redundantOverride/basic.kt.after new file mode 100644 index 00000000000..0731ec2a389 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/basic.kt.after @@ -0,0 +1,7 @@ +open class Foo { + open fun simple() { + } +} + +class Bar : Foo() { +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt b/idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt new file mode 100644 index 00000000000..bbef9e675fb --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt @@ -0,0 +1,15 @@ +// PROBLEM: none + +open class Foo { + open fun simple() { + } + + open fun callDifferentSuperMethod() { + } +} + +class Bar : Foo() { + override fun callDifferentSuperMethod() { + super.simple() + } +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt b/idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt new file mode 100644 index 00000000000..afd9180cbc4 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt @@ -0,0 +1,12 @@ +// PROBLEM: none + +open class Foo { + open fun simple() { + } +} + +class Bar : Foo() { + override fun simple() { + 1 + 1; + } +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/overrideModifireFinal.kt b/idea/testData/inspectionsLocal/redundantOverride/overrideModifireFinal.kt new file mode 100644 index 00000000000..ab4f7cf223d --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/overrideModifireFinal.kt @@ -0,0 +1,13 @@ +// PROBLEM: none + +open class Foo { + open fun simple() { + } +} + +class Bar : Foo() { + final override fun simple() { + super.simple() + } +} + diff --git a/idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt b/idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt new file mode 100644 index 00000000000..663aa0e198e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt @@ -0,0 +1,13 @@ +// PROBLEM: none + +open class Foo { + protected open fun simple() { + } +} + +class Bar : Foo() { + public override fun simple() { + super.simple() + } +} + diff --git a/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt b/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt new file mode 100644 index 00000000000..1893122c6d4 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt @@ -0,0 +1,8 @@ +open class Foo { + open fun singleExpression() { + } +} + +class Bar : Foo() { + override fun singleExpression() = super.singleExpression() +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt.after b/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt.after new file mode 100644 index 00000000000..4f254f03444 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt.after @@ -0,0 +1,7 @@ +open class Foo { + open fun singleExpression() { + } +} + +class Bar : Foo() { +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt b/idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt new file mode 100644 index 00000000000..78013ce8c4e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt @@ -0,0 +1,14 @@ +// PROBLEM: none + +interface First { + fun foo() = 2 +} +interface Second { + fun foo() = 3 +} + +class Diamond : First, Second { + override fun foo(): Int { + return super.foo() + } +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 12a87032338..6b4d053e4b6 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -1326,6 +1326,75 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/redundantOverride") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class RedundantOverride extends AbstractLocalInspectionTest { + public void testAllFilesPresentInRedundantOverride() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/redundantOverride"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("annotated.kt") + public void testAnnotated() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/annotated.kt"); + doTest(fileName); + } + + @TestMetadata("arguments.kt") + public void testArguments() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/arguments.kt"); + doTest(fileName); + } + + @TestMetadata("argumentsReplaced.kt") + public void testArgumentsReplaced() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/argumentsReplaced.kt"); + doTest(fileName); + } + + @TestMetadata("basic.kt") + public void testBasic() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/basic.kt"); + doTest(fileName); + } + + @TestMetadata("callDifferentSuperMethod.kt") + public void testCallDifferentSuperMethod() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt"); + doTest(fileName); + } + + @TestMetadata("notCallSuper.kt") + public void testNotCallSuper() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt"); + doTest(fileName); + } + + @TestMetadata("overrideModifireFinal.kt") + public void testOverrideModifireFinal() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/overrideModifireFinal.kt"); + doTest(fileName); + } + + @TestMetadata("overrideModifireVisibility.kt") + public void testOverrideModifireVisibility() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt"); + doTest(fileName); + } + + @TestMetadata("singleExpressionFunction.kt") + public void testSingleExpressionFunction() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt"); + doTest(fileName); + } + + @TestMetadata("useGenericsSuper.kt") + public void testUseGenericsSuper() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/inspectionsLocal/removeRedundantSpreadOperator") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)