diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt new file mode 100644 index 00000000000..571bf5853af --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt @@ -0,0 +1,52 @@ +public class C { + public val foo: Int = 0 + + public fun bar() {} + +} + +open class D { + protected open fun willRemainProtected() { + } + + protected open fun willBecomePublic() { + } +} + +class E : D() { + protected override fun willRemainProtected() { + } + + public override fun willBecomePublic() { + } +} + +enum class F private constructor(val x: Int) { + FIRST(42) +} + +sealed class G constructor(val y: Int) { + private constructor(): this(42) + + object H : G() +} + +interface I { + fun bar() +} + +fun f() { + val i = object : I { + internal var foo = 0 + override fun bar() {} + } + i.foo = 1 + + class LocalClass { + internal var foo = 0 + } + LocalClass().foo = 1 + public var baz = 0 +} + +public var baz = 0 diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java index d0e0dedef1a..724661a30f4 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java @@ -1414,6 +1414,24 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { } } + @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class ExtendedCheckers extends AbstractFirDiagnosticsTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + public void testAllFilesPresentInExtendedCheckers() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/extendedCheckers"), Pattern.compile("^([^.]+)\\.kt$"), null, true); + } + + @TestMetadata("RedundantVisibilityModifierChecker.kt") + public void testRedundantVisibilityModifierChecker() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); + } + } + @TestMetadata("compiler/fir/analysis-tests/testData/resolve/fromBuilder") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java index 29228cbf02c..195a27c3b4f 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java @@ -1414,6 +1414,24 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos } } + @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class ExtendedCheckers extends AbstractFirDiagnosticsWithLightTreeTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + public void testAllFilesPresentInExtendedCheckers() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/extendedCheckers"), Pattern.compile("^([^.]+)\\.kt$"), null, true); + } + + @TestMetadata("RedundantVisibilityModifierChecker.kt") + public void testRedundantVisibilityModifierChecker() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); + } + } + @TestMetadata("compiler/fir/analysis-tests/testData/resolve/fromBuilder") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) 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 c8c1bebfa7d..a3eaeb92644 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 @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.fir.analysis.checkers import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.FirSymbolOwner import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext @@ -15,12 +16,19 @@ import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference import org.jetbrains.kotlin.fir.resolve.firSymbolProvider import org.jetbrains.kotlin.fir.resolve.toSymbol import org.jetbrains.kotlin.fir.resolve.transformers.firClassLike +import org.jetbrains.kotlin.fir.scopes.FirTypeScope +import org.jetbrains.kotlin.fir.scopes.ProcessorAction +import org.jetbrains.kotlin.fir.scopes.unsubstitutedScope import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol 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.psi.KtModifierList +import org.jetbrains.kotlin.psi.psiUtil.toVisibility +import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType import org.jetbrains.kotlin.utils.addToStdlib.safeAs /** @@ -185,3 +193,40 @@ fun CheckerContext.findClosestClassOrObject(): FirClass<*>? { return null } + +/** + * Returns the list of functions that overridden by given + */ +fun FirSimpleFunction.overriddenFunctions( + containingClass: FirClass<*>, + context: CheckerContext +): List> { + val firTypeScope = containingClass.unsubstitutedScope( + context.sessionHolder.session, + context.sessionHolder.scopeSession + ) as FirTypeScope + + val overriddenFunctions = mutableListOf>() + firTypeScope.processFunctionsByName(symbol.fir.name) { } + firTypeScope.processOverriddenFunctions(symbol) { + overriddenFunctions.add(it) + ProcessorAction.NEXT + } + + return overriddenFunctions +} + +/** + * Returns the visibility by given KtModifierList + */ +fun KtModifierList?.getVisibility() = this?.visibilityModifierType()?.toVisibility() + +/** + * Returns the modality of the class + */ +fun FirClass<*>.modality(): Modality? { + return when (this) { + is FirRegularClass -> modality + else -> Modality.FINAL + } +} 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 97da78334dc..f2e43683602 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,10 +5,13 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration +import org.jetbrains.kotlin.fir.analysis.checkers.extended.RedundantVisibilityModifierChecker + object CommonDeclarationCheckers : DeclarationCheckers() { override val declarationCheckers: List = listOf( FirAnnotationClassDeclarationChecker, - FirModifierChecker + FirModifierChecker, + RedundantVisibilityModifierChecker ) override val memberDeclarationCheckers: List = listOf( diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt new file mode 100644 index 00000000000..09c1ee84594 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt @@ -0,0 +1,107 @@ +/* + * 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 org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.descriptors.Visibility +import org.jetbrains.kotlin.fir.FirFakeSourceElement +import org.jetbrains.kotlin.fir.analysis.checkers.* +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirBasicDeclarationChecker +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.psi +import org.jetbrains.kotlin.fir.toFirPsiSourceElement +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtModifierList +import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier +import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType + +object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { + override fun check(declaration: FirDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { + if (declaration is FirPropertyAccessor && declaration.isGetter) return + if (declaration !is FirMemberDeclaration) return + if (declaration is FirConstructor && declaration.source is FirFakeSourceElement<*>) return + + val modifierList = (declaration.source.getModifierList() as? FirPsiModifierList)?.modifierList ?: return + val visibilityModifier = modifierList.getVisibility() + val implicitVisibility = declaration.implicitVisibility(context) + val containingMemberDeclaration = context.containingDeclarations.getOrNull(1) as? FirMemberDeclaration + + val redundantVisibility = when { + visibilityModifier == implicitVisibility -> implicitVisibility + visibilityModifier == Visibilities.INTERNAL + && containingMemberDeclaration?.visibility == Visibilities.PRIVATE + || declaration.visibility == Visibilities.INTERNAL -> Visibilities.INTERNAL + else -> null + } ?: return + + if (redundantVisibility == Visibilities.PUBLIC + && declaration is FirProperty + && declaration.isVar + && declaration.setter?.visibility.let { it != null && it != Visibilities.PUBLIC } + ) return + + val source = modifierList.visibilityModifier()?.toFirPsiSourceElement() + reporter.report(source, FirErrors.REDUNDANT_VISIBILITY_MODIFIER) + } + + private fun FirDeclaration.implicitVisibility(context: CheckerContext): Visibility { + return when { + this is FirPropertyAccessor && isSetter && status.isOverride -> { + this.visibility + } + + this is FirConstructor -> { + val clazz = this.getContainingClass(context) + if ( + clazz is FirClass + && (clazz.classKind == ClassKind.ENUM_CLASS || clazz.modality() == Modality.SEALED) + ) { + return Visibilities.PRIVATE + } else { + Visibilities.DEFAULT_VISIBILITY + } + } + + this is FirSimpleFunction + && context.containingDeclarations.getOrNull(1) is FirClass<*> + && this.isOverride -> findFunctionVisibility(this, context) + + else -> Visibilities.DEFAULT_VISIBILITY + } + } + + private fun findFunctionVisibility(function: FirSimpleFunction, context: CheckerContext): Visibility { + val currentClass = context.findClosestClassOrObject() ?: return Visibilities.UNKNOWN + val overriddenFunctions = function.overriddenFunctions(currentClass, context) + var visibility: Visibility = Visibilities.PRIVATE + for (func in overriddenFunctions) { + val currentVisibility = func.fir.visibility() + if (currentVisibility != null) { + val compareResult = Visibilities.compare(currentVisibility, visibility) + if (compareResult != null && compareResult > 0) { + visibility = currentVisibility + } + } + } + + return visibility + } + + private fun FirFunction<*>.visibility(): Visibility? { + (symbol.fir as? FirMemberDeclaration)?.visibility?.let { + return it + } + (symbol.fir as? FirPropertyAccessor)?.visibility?.let { + return it + } + return null + } +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/ReportHelper.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/ReportHelper.kt new file mode 100644 index 00000000000..c5d248cd440 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/ReportHelper.kt @@ -0,0 +1,18 @@ +/* + * 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.PsiElement +import org.jetbrains.kotlin.fir.FirSourceElement +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticFactory0 + +inline fun DiagnosticReporter.report( + source: T?, + factory: FirDiagnosticFactory0 +) { + source?.let { report(factory.on(it)) } +} \ No newline at end of file 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 4bb3c5754d1..24e974a88d7 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 @@ -13,6 +13,7 @@ import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.fir.DeclarationWithRelation import org.jetbrains.kotlin.fir.FirEffectiveVisibility import org.jetbrains.kotlin.fir.FirSourceElement +import org.jetbrains.kotlin.fir.declarations.FirDeclaration import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol import org.jetbrains.kotlin.fir.types.ConeKotlinType @@ -48,6 +49,7 @@ object FirErrors { val SUPERCLASS_NOT_ACCESSIBLE_FROM_INTERFACE by error0() val ABSTRACT_SUPER_CALL by error0() val QUALIFIED_SUPERTYPE_EXTENDED_BY_OTHER_SUPERTYPE by error0() + val REDUNDANT_VISIBILITY_MODIFIER by error0() val INAPPLICABLE_INFIX_MODIFIER by existing(Errors.INAPPLICABLE_INFIX_MODIFIER) val CONSTRUCTOR_IN_OBJECT by existing(Errors.CONSTRUCTOR_IN_OBJECT)