From fb5fb449930968858337b0e597df9734712ae66e Mon Sep 17 00:00:00 2001 From: "vladislavf7@gmail.com" Date: Tue, 14 Jul 2020 16:07:51 +0300 Subject: [PATCH] [FIR] Add RedundantModalityModifierChecker --- .../RedundantModalityModifierChecker.kt | 60 ++++++++++ .../RedundantModalityModifierChecker.txt | 103 ++++++++++++++++++ .../ExtendedFirDiagnosticsTestGenerated.java | 5 + .../fir/analysis/checkers/FirHelpers.kt | 42 +++++++ .../declaration/DefaultDeclarationCheckers.kt | 4 +- .../RedundantModalityModifierChecker.kt | 36 ++++++ .../fir/analysis/diagnostics/FirErrors.kt | 1 + 7 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.kt create mode 100644 compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.txt create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierChecker.kt diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.kt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.kt new file mode 100644 index 00000000000..a29cc746cbd --- /dev/null +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.kt @@ -0,0 +1,60 @@ +// Abstract +abstract class Base { + // Redundant final + final fun foo() {} + // Abstract + abstract fun bar() + // Open + open val gav = 42 +} + +class FinalDerived : Base() { + // Redundant final + override final fun bar() {} + // Non-final member in final class + override open val gav = 13 +} +// Open +open class OpenDerived : Base() { + // Final + override final fun bar() {} + // Redundant open + override open val gav = 13 +} +// Redundant final +final class Final +// Interface +interface Interface { + // Redundant + abstract fun foo() + // Redundant + private final fun bar() {} + // Redundant + open val gav: Int + get() = 42 +} +// Derived interface +interface Derived : Interface { + // Redundant + override open fun foo() {} + // Redundant + final class Nested +} +// Derived abstract class +abstract class AbstractDerived1(override final val gav: Int) : Interface { + // Redundant + override open fun foo() {} +} +// Derived abstract class +abstract class AbstractDerived2 : Interface { + // Final + override final fun foo() {} + // Redundant + override open val gav = 13 +} +// Redundant abstract interface +abstract interface AbstractInterface +// Redundant final object +final object FinalObject +// Open interface +open interface OpenInterface diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.txt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.txt new file mode 100644 index 00000000000..56077911f63 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.txt @@ -0,0 +1,103 @@ +FILE: RedundantModalityModifierChecker.kt + public abstract class Base : R|kotlin/Any| { + public constructor(): R|Base| { + super() + } + + public final fun foo(): R|kotlin/Unit| { + } + + public abstract fun bar(): R|kotlin/Unit| + + public open val gav: R|kotlin/Int| = Int(42) + public get(): R|kotlin/Int| + + } + public final class FinalDerived : R|Base| { + public constructor(): R|FinalDerived| { + super() + } + + public final override fun bar(): R|kotlin/Unit| { + } + + public open override val gav: R|kotlin/Int| = Int(13) + public get(): R|kotlin/Int| + + } + public open class OpenDerived : R|Base| { + public constructor(): R|OpenDerived| { + super() + } + + public final override fun bar(): R|kotlin/Unit| { + } + + public open override val gav: R|kotlin/Int| = Int(13) + public get(): R|kotlin/Int| + + } + public final class Final : R|kotlin/Any| { + public constructor(): R|Final| { + super() + } + + } + public abstract interface Interface : R|kotlin/Any| { + public abstract fun foo(): R|kotlin/Unit| + + private final fun bar(): R|kotlin/Unit| { + } + + public open val gav: R|kotlin/Int| + public get(): R|kotlin/Int| { + ^ Int(42) + } + + } + public abstract interface Derived : R|Interface| { + public open override fun foo(): R|kotlin/Unit| { + } + + public final class Nested : R|kotlin/Any| { + public constructor(): R|Derived.Nested| { + super() + } + + } + + } + public abstract class AbstractDerived1 : R|Interface| { + public constructor(gav: R|kotlin/Int|): R|AbstractDerived1| { + super() + } + + public final override val gav: R|kotlin/Int| = R|/gav| + public get(): R|kotlin/Int| + + public open override fun foo(): R|kotlin/Unit| { + } + + } + public abstract class AbstractDerived2 : R|Interface| { + public constructor(): R|AbstractDerived2| { + super() + } + + public final override fun foo(): R|kotlin/Unit| { + } + + public open override val gav: R|kotlin/Int| = Int(13) + public get(): R|kotlin/Int| + + } + public abstract interface AbstractInterface : R|kotlin/Any| { + } + public final object FinalObject : R|kotlin/Any| { + private constructor(): R|FinalObject| { + super() + } + + } + public open interface OpenInterface : R|kotlin/Any| { + } diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/ExtendedFirDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/ExtendedFirDiagnosticsTestGenerated.java index 7a19623cb0c..a4ff8ffedc5 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/ExtendedFirDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/ExtendedFirDiagnosticsTestGenerated.java @@ -28,6 +28,11 @@ public class ExtendedFirDiagnosticsTestGenerated extends AbstractExtendedFirDiag KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/extendedCheckers"), Pattern.compile("^([^.]+)\\.kt$"), null, true); } + @TestMetadata("RedundantModalityModifierChecker.kt") + public void testRedundantModalityModifierChecker() throws Exception { + runTest("compiler/fir/analysis-tests/testData/extendedCheckers/RedundantModalityModifierChecker.kt"); + } + @TestMetadata("RedundantReturnUnitTypeChecker.kt") public void testRedundantReturnUnitTypeChecker() throws Exception { runTest("compiler/fir/analysis-tests/testData/extendedCheckers/RedundantReturnUnitTypeChecker.kt"); diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt index a3eaeb92644..56633113373 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt @@ -12,6 +12,7 @@ import org.jetbrains.kotlin.fir.FirSymbolOwner import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression +import org.jetbrains.kotlin.fir.expressions.impl.FirSingleExpressionBlock import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference import org.jetbrains.kotlin.fir.resolve.firSymbolProvider import org.jetbrains.kotlin.fir.resolve.toSymbol @@ -26,6 +27,8 @@ import org.jetbrains.kotlin.fir.types.ConeClassLikeType import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.fir.types.FirResolvedTypeRef import org.jetbrains.kotlin.fir.types.FirTypeRef +import org.jetbrains.kotlin.lexer.KtModifierKeywordToken +import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtModifierList import org.jetbrains.kotlin.psi.psiUtil.toVisibility import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType @@ -230,3 +233,42 @@ fun FirClass<*>.modality(): Modality? { else -> Modality.FINAL } } + +/** + * returns implicit modality by FirMemberDeclaration + */ +fun FirMemberDeclaration.implicitModality(context: CheckerContext): KtModifierKeywordToken { + if (this is FirRegularClass && (this.classKind == ClassKind.CLASS || this.classKind == ClassKind.OBJECT)) { + if (this.classKind == ClassKind.INTERFACE) return KtTokens.ABSTRACT_KEYWORD + return KtTokens.FINAL_KEYWORD + } + + val klass = context.findClosestClassOrObject() ?: return KtTokens.FINAL_KEYWORD + val modifiers = this.modifierListOrNull() ?: return KtTokens.FINAL_KEYWORD + if (modifiers.hasModifier(KtTokens.OVERRIDE_KEYWORD)) { + val klassModifiers = klass.modifierListOrNull() + if (klassModifiers != null && klassModifiers.run { + hasModifier(KtTokens.ABSTRACT_KEYWORD) || hasModifier(KtTokens.OPEN_KEYWORD) || hasModifier(KtTokens.SEALED_KEYWORD) + }) { + return KtTokens.OPEN_KEYWORD + } + } + + if ( + klass is FirRegularClass + && klass.classKind == ClassKind.INTERFACE + && !modifiers.hasModifier(KtTokens.PRIVATE_KEYWORD) + ) { + return if (this.hasBody()) KtTokens.OPEN_KEYWORD else KtTokens.ABSTRACT_KEYWORD + } + + return KtTokens.FINAL_KEYWORD +} + +private fun FirDeclaration.modifierListOrNull() = (this.source.getModifierList() as? FirPsiModifierList)?.modifierList + +private fun FirDeclaration.hasBody(): Boolean = when (this) { + is FirSimpleFunction -> this.body != null && this.body !is FirSingleExpressionBlock + is FirProperty -> this.setter != null || this.getter != null + else -> false +} \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DefaultDeclarationCheckers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DefaultDeclarationCheckers.kt index 12dd507aa34..6551350f478 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DefaultDeclarationCheckers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/DefaultDeclarationCheckers.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration +import org.jetbrains.kotlin.fir.analysis.checkers.extended.RedundantModalityModifierChecker import org.jetbrains.kotlin.fir.analysis.checkers.extended.RedundantReturnUnitType import org.jetbrains.kotlin.fir.analysis.checkers.extended.RedundantVisibilityModifierChecker @@ -18,7 +19,8 @@ object CommonDeclarationCheckers : DeclarationCheckers() { override val memberDeclarationCheckers: List = listOf( FirInfixFunctionDeclarationChecker, - FirExposedVisibilityChecker + FirExposedVisibilityChecker, + RedundantModalityModifierChecker ) override val constructorCheckers: List = listOf( diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierChecker.kt new file mode 100644 index 00000000000..beb8d9d749b --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierChecker.kt @@ -0,0 +1,36 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.analysis.checkers.extended + +import com.intellij.psi.impl.source.tree.LeafPsiElement +import org.jetbrains.kotlin.fir.FirFakeSourceElement +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirMemberDeclarationChecker +import org.jetbrains.kotlin.fir.analysis.checkers.implicitModality +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.REDUNDANT_MODALITY_MODIFIER +import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration +import org.jetbrains.kotlin.fir.psi +import org.jetbrains.kotlin.fir.toFirPsiSourceElement +import org.jetbrains.kotlin.lexer.KtModifierKeywordToken +import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.psi.psiUtil.modalityModifier + +object RedundantModalityModifierChecker : FirMemberDeclarationChecker() { + override val isExtended = true + + override fun check(declaration: FirMemberDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { + if (declaration.source is FirFakeSourceElement<*>) return + + val modalityModifier = (declaration.psi as? KtDeclaration)?.modalityModifier() ?: return + val modality = (modalityModifier as? LeafPsiElement)?.elementType as? KtModifierKeywordToken ?: return + val implicitModality = declaration.implicitModality(context) + + if (modality != implicitModality) return + + reporter.report(modalityModifier.toFirPsiSourceElement(), REDUNDANT_MODALITY_MODIFIER) + } +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 2440c50a664..6eeb88e88a0 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -84,6 +84,7 @@ object FirErrors { // Extended checkers group val REDUNDANT_VISIBILITY_MODIFIER by warning0() + val REDUNDANT_MODALITY_MODIFIER by warning0() val REDUNDANT_RETURN_UNIT_TYPE by warning0() }