FIR checker/IDE: Add checker and quickfix for WRONG_MODIFIER_TARGET.

There are only simple checks for `external` and `const` for now. The
rest of the checks (see ModifiersChecker in FE1.0) will be added later.
This commit is contained in:
Mark Punzalan
2021-03-03 20:34:03 +00:00
committed by Ilya Kirillov
parent ebfc431733
commit c3d2ce0c1f
22 changed files with 138 additions and 20 deletions
@@ -160,6 +160,10 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
parameter<KtModifierKeywordToken>("modifier2")
}
val REDUNDANT_OPEN_IN_INTERFACE by warning<FirSourceElement, KtModifierListOwner>(PositioningStrategy.OPEN_MODIFIER)
val WRONG_MODIFIER_TARGET by error<FirSourceElement, PsiElement> {
parameter<KtModifierKeywordToken>("modifier")
parameter<String>("target")
}
}
val INLINE_CLASSES by object : DiagnosticGroup("Inline classes") {
@@ -145,6 +145,7 @@ object FirErrors {
val DEPRECATED_MODIFIER_PAIR by error2<FirSourceElement, PsiElement, KtModifierKeywordToken, KtModifierKeywordToken>()
val INCOMPATIBLE_MODIFIERS by error2<FirSourceElement, PsiElement, KtModifierKeywordToken, KtModifierKeywordToken>()
val REDUNDANT_OPEN_IN_INTERFACE by warning0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.OPEN_MODIFIER)
val WRONG_MODIFIER_TARGET by error2<FirSourceElement, PsiElement, KtModifierKeywordToken, String>()
// Inline classes
val INLINE_CLASS_NOT_TOP_LEVEL by error0<FirSourceElement, KtDeclaration>(SourceElementPositioningStrategies.INLINE_OR_VALUE_MODIFIER)
@@ -0,0 +1,36 @@
/*
* Copyright 2010-2021 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.declaration
import org.jetbrains.kotlin.fir.FirFakeSourceElementKind
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.lexer.KtTokens
object FirConstDeclarationChecker : FirBasicDeclarationChecker() {
override fun check(declaration: FirDeclaration, context: CheckerContext, reporter: DiagnosticReporter) {
if (declaration !is FirVariable<*>) return
val source = declaration.source ?: return
if (source.kind is FirFakeSourceElementKind) return
val modifierList = with(FirModifierList) { source.getModifierList() }
val constModifier = modifierList?.modifiers?.firstOrNull { it.token == KtTokens.CONST_KEYWORD } ?: return
if (declaration.isVar) {
reporter.reportOn(constModifier.source, FirErrors.WRONG_MODIFIER_TARGET, constModifier.token, "vars", context)
}
// TODO: Implement checkers for these errors (see ConstModifierChecker in FE1.0):
// - CONST_VAL_NOT_TOP_LEVEL_OR_OBJECT
// - CONST_VAL_WITH_DELEGATE
// - CONST_VAL_WITH_GETTER
// - TYPE_CANT_BE_USED_FOR_CONST_VAL
// - CONST_VAL_WITHOUT_INITIALIZER
// - CONST_VAL_WITH_NON_CONST_INITIALIZER
}
}
@@ -0,0 +1,39 @@
/*
* Copyright 2010-2021 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.declaration
import org.jetbrains.kotlin.fir.FirFakeSourceElementKind
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.lexer.KtTokens
object FirExternalDeclarationChecker : FirBasicDeclarationChecker() {
override fun check(declaration: FirDeclaration, context: CheckerContext, reporter: DiagnosticReporter) {
val source = declaration.source ?: return
if (source.kind is FirFakeSourceElementKind) return
val modifierList = with(FirModifierList) { source.getModifierList() }
val externalModifier = modifierList?.modifiers?.firstOrNull { it.token == KtTokens.EXTERNAL_KEYWORD } ?: return
// WRONG_MODIFIER_TARGET on external constructor is intentionally NOT covered in this checker.
if (declaration !is FirFunction<*>) {
val target = when (declaration) {
is FirProperty -> "property"
is FirRegularClass -> "class"
else -> "non-function declaration"
}
reporter.reportOn(externalModifier.source, FirErrors.WRONG_MODIFIER_TARGET, externalModifier.token, target, context)
}
// TODO: Implement checkers for these JVM-specific errors (see ExternalFunChecker in FE1.0):
// - EXTERNAL_DECLARATION_IN_INTERFACE
// - EXTERNAL_DECLARATION_CANNOT_BE_ABSTRACT
// - EXTERNAL_DECLARATION_CANNOT_HAVE_BODY
// - EXTERNAL_DECLARATION_CANNOT_BE_INLINED
}
}
@@ -214,6 +214,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_ANNOTATION_PA
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_OVERRIDDEN_BY_VAL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_TYPE_MISMATCH_ON_OVERRIDE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_INVOCATION_KIND
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_MODIFIER_TARGET
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_NUMBER_OF_TYPE_ARGUMENTS
@Suppress("unused")
@@ -354,6 +355,7 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
map.put(DEPRECATED_MODIFIER_PAIR, "Modifier ''{0}'' is deprecated in presence of ''{1}''", TO_STRING, TO_STRING)
map.put(INCOMPATIBLE_MODIFIERS, "Modifier ''{0}'' is incompatible with ''{1}''", TO_STRING, TO_STRING)
map.put(REDUNDANT_OPEN_IN_INTERFACE, "Modifier 'open' is redundant for abstract interface members")
map.put(WRONG_MODIFIER_TARGET, "Modifier ''{0}'' is not applicable to ''{1}''", TO_STRING, TO_STRING)
// Applicability
map.put(NONE_APPLICABLE, "None of the following functions are applicable: {0}", SYMBOLS)
@@ -18,6 +18,8 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
FirModifierChecker,
FirConflictsChecker,
FirConflictingProjectionChecker,
FirExternalDeclarationChecker,
FirConstDeclarationChecker,
)
override val memberDeclarationCheckers: Set<FirMemberDeclarationChecker> = setOf(
@@ -4,7 +4,7 @@ import kotlin.reflect.KProperty
const val topLevel: Int = 0
const val topLevelInferred = 1
const var topLeveLVar: Int = 2
<!WRONG_MODIFIER_TARGET!>const<!> var topLeveLVar: Int = 2
private val privateTopLevel = 3
@@ -12,6 +12,6 @@ const val arrayConst: Array<String> = arrayOf("1")
const val intArrayConst: IntArray = intArrayOf()
const val unresolvedConst1 = <!UNRESOLVED_REFERENCE!>Unresolved<!>
const var unresolvedConst2 = <!UNRESOLVED_REFERENCE!>Unresolved<!>
<!WRONG_MODIFIER_TARGET!>const<!> var unresolvedConst2 = <!UNRESOLVED_REFERENCE!>Unresolved<!>
const val unresolvedConst3 = <!PROPERTY_INITIALIZER_NO_BACKING_FIELD, UNRESOLVED_REFERENCE!>Unresolved<!>
get() = 10
@@ -26,7 +26,7 @@ val allProhibited: (<!INCOMPATIBLE_MODIFIERS, REDUNDANT_MODIFIER!>abstract<!>
<!INCOMPATIBLE_MODIFIERS!>crossinline<!>
<!INCOMPATIBLE_MODIFIERS!>data<!>
enum
external
<!WRONG_MODIFIER_TARGET!>external<!>
<!INCOMPATIBLE_MODIFIERS!>final<!>
<!INCOMPATIBLE_MODIFIERS!>in<!>
<!INCOMPATIBLE_MODIFIERS!>inline<!>
@@ -39,3 +39,5 @@ fun test() {
}
}
<!WRONG_MODIFIER_TARGET!>external<!> <!REPEATED_MODIFIER!>external<!> val i = 0
<!WRONG_MODIFIER_TARGET!>const<!> <!REPEATED_MODIFIER!>const<!> var x = 0
@@ -39,3 +39,5 @@ fun test() {
}
}
<!WRONG_MODIFIER_TARGET!>external <!REPEATED_MODIFIER!>external<!> val i = 0<!>
<!WRONG_MODIFIER_TARGET!>const<!> <!REPEATED_MODIFIER!>const<!> var x = 0
@@ -1,6 +1,8 @@
package
public val bar: kotlin.Int = 1
public external val i: kotlin.Int = 0
public const var x: kotlin.Int
public fun foo(): kotlin.Int
public fun test(): kotlin.Unit
@@ -1,9 +1,9 @@
external class A
<!WRONG_MODIFIER_TARGET!>external<!> class A
external val foo: Int = 23
<!WRONG_MODIFIER_TARGET!>external<!> val foo: Int = 23
class B {
external class A
<!WRONG_MODIFIER_TARGET!>external<!> class A
external val foo: Int = 23
<!WRONG_MODIFIER_TARGET!>external<!> val foo: Int = 23
}
@@ -48,6 +48,11 @@ class MainKtQuickFixRegistrar : KtQuickFixRegistrar() {
KtFirDiagnostic.NestedClassNotAllowed::class,
AddModifierFix.addInnerModifier
)
registerPsiQuickFixes(
KtFirDiagnostic.WrongModifierTarget::class,
RemoveModifierFix.removeNonRedundantModifier,
ChangeVariableMutabilityFix.CONST_VAL_FACTORY
)
}
private val overrides = KtQuickFixesListBuilder.registerPsiQuickFix {
@@ -628,6 +628,14 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.WRONG_MODIFIER_TARGET) { firDiagnostic ->
WrongModifierTargetImpl(
firDiagnostic.a,
firDiagnostic.b,
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.NONE_APPLICABLE) { firDiagnostic ->
NoneApplicableImpl(
firDiagnostic.a.map { abstractFirBasedSymbol ->
@@ -451,6 +451,12 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
override val diagnosticClass get() = ValueClassCannotBeCloneable::class
}
abstract class WrongModifierTarget : KtFirDiagnostic<PsiElement>() {
override val diagnosticClass get() = WrongModifierTarget::class
abstract val modifier: KtModifierKeywordToken
abstract val target: String
}
abstract class NoneApplicable : KtFirDiagnostic<PsiElement>() {
override val diagnosticClass get() = NoneApplicable::class
abstract val candidates: List<KtSymbol>
@@ -725,6 +725,15 @@ internal class ValueClassCannotBeCloneableImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class WrongModifierTargetImpl(
override val modifier: KtModifierKeywordToken,
override val target: String,
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.WrongModifierTarget(), KtAbstractFirDiagnostic<PsiElement> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class NoneApplicableImpl(
override val candidates: List<KtSymbol>,
firDiagnostic: FirPsiDiagnostic<*>,
@@ -9,6 +9,7 @@ import com.intellij.openapi.editor.Editor
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.idea.KotlinBundle
import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
@@ -74,6 +75,13 @@ class ChangeVariableMutabilityFix(
}
}
val CONST_VAL_FACTORY: QuickFixesPsiBasedFactory<PsiElement> =
quickFixesPsiBasedFactory { psiElement: PsiElement ->
if (psiElement.node.elementType as? KtModifierKeywordToken != KtTokens.CONST_KEYWORD) return@quickFixesPsiBasedFactory emptyList()
val property = psiElement.getStrictParentOfType<KtProperty>() ?: return@quickFixesPsiBasedFactory emptyList()
listOf(ChangeVariableMutabilityFix(property, makeVar = false))
}
val MUST_BE_INITIALIZED_FACTORY: QuickFixesPsiBasedFactory<PsiElement> =
quickFixesPsiBasedFactory { psiElement: PsiElement ->
val property = psiElement as? KtProperty ?: return@quickFixesPsiBasedFactory emptyList()
@@ -39,16 +39,6 @@ class ReassignmentActionFactory(val factory: DiagnosticFactory1<*, DeclarationDe
}
}
// TODO: Move this to idea-fir-independent
object ConstValFactory : KotlinSingleIntentionActionFactory() {
override fun createAction(diagnostic: Diagnostic): IntentionAction? {
val (modifier, element) = Errors.WRONG_MODIFIER_TARGET.cast(diagnostic).run { a to psiElement }
if (modifier != KtTokens.CONST_KEYWORD) return null
val property = element.getStrictParentOfType<KtProperty>() ?: return null
return ChangeVariableMutabilityFix(property, makeVar = false)
}
}
object DelegatedPropertyValFactory : KotlinSingleIntentionActionFactory() {
override fun createAction(diagnostic: Diagnostic): IntentionAction? {
val element = Errors.DELEGATE_SPECIAL_FUNCTION_MISSING.cast(diagnostic).psiElement
@@ -137,7 +137,7 @@ class QuickFixRegistrar : QuickFixContributor {
RemoveModifierFix.removeNonRedundantModifier
)
REDUNDANT_MODIFIER_IN_GETTER.registerFactory(RemoveModifierFix.removeRedundantModifier)
WRONG_MODIFIER_TARGET.registerFactory(RemoveModifierFix.removeNonRedundantModifier, ConstValFactory)
WRONG_MODIFIER_TARGET.registerFactory(RemoveModifierFix.removeNonRedundantModifier, ChangeVariableMutabilityFix.CONST_VAL_FACTORY)
DEPRECATED_MODIFIER.registerFactory(ReplaceModifierFix)
REDUNDANT_MODIFIER_FOR_TARGET.registerFactory(RemoveModifierFix.removeNonRedundantModifier)
WRONG_MODIFIER_CONTAINING_DECLARATION.registerFactory(RemoveModifierFix.removeNonRedundantModifier)
@@ -1,4 +1,5 @@
// "Change to val" "true"
object Test {
<caret>const var foo = "123"
}
}
/* FIR_COMPARISON */
@@ -1,4 +1,5 @@
// "Change to val" "true"
object Test {
<caret>const val foo = "123"
}
}
/* FIR_COMPARISON */