Skip to content

Commit

Permalink
[FIR] Introduce PARAMETER_NAME_CHANGED_ON_OVERRIDE diagnostic
Browse files Browse the repository at this point in the history
^KT-71982 Fixed
  • Loading branch information
KvanTTT authored and Space Cloud committed Dec 11, 2024
1 parent a55c9f5 commit 459e346
Show file tree
Hide file tree
Showing 38 changed files with 139 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3410,6 +3410,14 @@ internal val KT_DIAGNOSTIC_CONVERTER = KaDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.PARAMETER_NAME_CHANGED_ON_OVERRIDE) { firDiagnostic ->
ParameterNameChangedOnOverrideImpl(
firSymbolBuilder.classifierBuilder.buildClassLikeSymbol(firDiagnostic.a),
firSymbolBuilder.buildSymbol(firDiagnostic.b),
firDiagnostic as KtPsiDiagnostic,
token,
)
}
add(FirErrors.MANY_COMPANION_OBJECTS) { firDiagnostic ->
ManyCompanionObjectsImpl(
firDiagnostic as KtPsiDiagnostic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2410,6 +2410,12 @@ sealed interface KaFirDiagnostic<PSI : PsiElement> : KaDiagnosticWithPsi<PSI> {
val overriddenContainer: KaClassLikeSymbol
}

interface ParameterNameChangedOnOverride : KaFirDiagnostic<KtParameter> {
override val diagnosticClass get() = ParameterNameChangedOnOverride::class
val superType: KaClassLikeSymbol
val conflictingParameter: KaSymbol
}

interface ManyCompanionObjects : KaFirDiagnostic<KtObjectDeclaration> {
override val diagnosticClass get() = ManyCompanionObjects::class
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2883,6 +2883,13 @@ internal class VirtualMemberHiddenImpl(
token: KaLifetimeToken,
) : KaAbstractFirDiagnostic<KtNamedDeclaration>(firDiagnostic, token), KaFirDiagnostic.VirtualMemberHidden

internal class ParameterNameChangedOnOverrideImpl(
override val superType: KaClassLikeSymbol,
override val conflictingParameter: KaSymbol,
firDiagnostic: KtPsiDiagnostic,
token: KaLifetimeToken,
) : KaAbstractFirDiagnostic<KtParameter>(firDiagnostic, token), KaFirDiagnostic.ParameterNameChangedOnOverride

internal class ManyCompanionObjectsImpl(
firDiagnostic: KtPsiDiagnostic,
token: KaLifetimeToken,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.intellij.openapi.Disposable
import org.jetbrains.kotlin.analysis.api.standalone.base.projectStructure.AnalysisApiServiceRegistrar
import org.jetbrains.kotlin.test.services.TestServices

@Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE") // Use `testServices` name instead of `data`
abstract class AnalysisApiTestServiceRegistrar : AnalysisApiServiceRegistrar<TestServices> {
override fun registerApplicationServices(application: MockApplication, testServices: TestServices) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ FILE: lambdaArgInScopeFunction.kt
private final val name: R|kotlin/String| = R|<local>/name|
private get(): R|kotlin/String|

public open override operator fun compareTo(that: R|KotlinClass|): R|kotlin/Int| {
^compareTo this@R|/KotlinClass|.R|/KotlinClass.name|.R|kotlin/String.compareTo|(R|<local>/that|.R|/KotlinClass.name|)
public open override operator fun compareTo(other: R|KotlinClass|): R|kotlin/Int| {
^compareTo this@R|/KotlinClass|.R|/KotlinClass.name|.R|kotlin/String.compareTo|(R|<local>/other|.R|/KotlinClass.name|)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// ISSUE: KT-37070

class KotlinClass(private val name: String) : Comparable<KotlinClass> {
override operator fun compareTo(that: KotlinClass): Int {
return name.compareTo(that.name)
override operator fun compareTo(other: KotlinClass): Int {
return name.compareTo(other.name)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ FILE: KotlinClass.kt
private final val name: R|kotlin/String| = R|<local>/name|
private get(): R|kotlin/String|

public open override operator fun compareTo(that: R|KotlinClass|): R|kotlin/Int| {
^compareTo this@R|/KotlinClass|.R|/KotlinClass.name|.R|kotlin/String.compareTo|(R|<local>/that|.R|/KotlinClass.name|)
public open override operator fun compareTo(other: R|KotlinClass|): R|kotlin/Int| {
^compareTo this@R|/KotlinClass|.R|/KotlinClass.name|.R|kotlin/String.compareTo|(R|<local>/other|.R|/KotlinClass.name|)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class BooCase1() {
// TESTCASE NUMBER: 2

class KotlinClass(private val name: String) : Comparable<KotlinClass> {
override operator fun compareTo(that: KotlinClass): Int {
return name.compareTo(that.name)
override operator fun compareTo(other: KotlinClass): Int {
return name.compareTo(other.name)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,10 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") {
parameter<FirCallableSymbol<*>>("declared")
parameter<FirRegularClassSymbol>("overriddenContainer")
}
val PARAMETER_NAME_CHANGED_ON_OVERRIDE by warning<KtParameter>(PositioningStrategy.NAME_IDENTIFIER) {
parameter<FirRegularClassSymbol>("superType")
parameter<FirValueParameterSymbol>("conflictingParameter")
}
}

val REDECLARATIONS by object : DiagnosticGroup("Redeclarations") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ object FirErrors {
val NON_FINAL_MEMBER_IN_FINAL_CLASS: KtDiagnosticFactory0 = KtDiagnosticFactory0("NON_FINAL_MEMBER_IN_FINAL_CLASS", WARNING, SourceElementPositioningStrategies.OPEN_MODIFIER, KtNamedDeclaration::class)
val NON_FINAL_MEMBER_IN_OBJECT: KtDiagnosticFactory0 = KtDiagnosticFactory0("NON_FINAL_MEMBER_IN_OBJECT", WARNING, SourceElementPositioningStrategies.OPEN_MODIFIER, KtNamedDeclaration::class)
val VIRTUAL_MEMBER_HIDDEN: KtDiagnosticFactory2<FirCallableSymbol<*>, FirRegularClassSymbol> = KtDiagnosticFactory2("VIRTUAL_MEMBER_HIDDEN", ERROR, SourceElementPositioningStrategies.DECLARATION_NAME, KtNamedDeclaration::class)
val PARAMETER_NAME_CHANGED_ON_OVERRIDE: KtDiagnosticFactory2<FirRegularClassSymbol, FirValueParameterSymbol> = KtDiagnosticFactory2("PARAMETER_NAME_CHANGED_ON_OVERRIDE", WARNING, SourceElementPositioningStrategies.NAME_IDENTIFIER, KtParameter::class)

// Redeclarations
val MANY_COMPANION_OBJECTS: KtDiagnosticFactory0 = KtDiagnosticFactory0("MANY_COMPANION_OBJECTS", ERROR, SourceElementPositioningStrategies.COMPANION_OBJECT, KtObjectDeclaration::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,29 @@ import org.jetbrains.kotlin.diagnostics.reportOn
import org.jetbrains.kotlin.fir.FirElement
import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.getContainingClassSymbol
import org.jetbrains.kotlin.fir.analysis.checkers.isValueClass
import org.jetbrains.kotlin.fir.analysis.checkers.leastUpperBound
import org.jetbrains.kotlin.fir.analysis.checkers.unsubstitutedScope
import org.jetbrains.kotlin.fir.analysis.checkers.valOrVarKeyword
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.containingClassLookupTag
import org.jetbrains.kotlin.fir.declarations.FirAnonymousFunction
import org.jetbrains.kotlin.fir.declarations.FirConstructor
import org.jetbrains.kotlin.fir.declarations.FirFunction
import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction
import org.jetbrains.kotlin.fir.declarations.FirValueParameter
import org.jetbrains.kotlin.fir.declarations.utils.hasStableParameterNames
import org.jetbrains.kotlin.fir.diagnostics.ConeSimpleDiagnostic
import org.jetbrains.kotlin.fir.diagnostics.DiagnosticKind
import org.jetbrains.kotlin.fir.expressions.FirPropertyAccessExpression
import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression
import org.jetbrains.kotlin.fir.references.toResolvedValueParameterSymbol
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
import org.jetbrains.kotlin.fir.resolve.toRegularClassSymbol
import org.jetbrains.kotlin.fir.scopes.getDirectOverriddenFunctions
import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol
import org.jetbrains.kotlin.fir.types.*
import org.jetbrains.kotlin.fir.visitors.FirVisitorVoid

Expand All @@ -36,6 +44,7 @@ object FirFunctionParameterChecker : FirFunctionChecker(MppCheckerKind.Common) {
checkParameterTypes(declaration, context, reporter)
checkUninitializedParameter(declaration, context, reporter)
checkValOrVarParameter(declaration, context, reporter)
checkParameterNameChangedOnOverride(declaration, context, reporter)
}

private fun checkParameterTypes(function: FirFunction, context: CheckerContext, reporter: DiagnosticReporter) {
Expand Down Expand Up @@ -145,4 +154,33 @@ object FirFunctionParameterChecker : FirFunctionChecker(MppCheckerKind.Common) {
}
}
}

private fun checkParameterNameChangedOnOverride(
function: FirFunction,
context: CheckerContext,
reporter: DiagnosticReporter,
) {
if (function !is FirSimpleFunction || !function.hasStableParameterNames) return

val currentScope =
function.symbol.containingClassLookupTag()?.toRegularClassSymbol(context.session)?.unsubstitutedScope(context) ?: return
val overriddenFunctions = currentScope.getDirectOverriddenFunctions(function.symbol)

for (overriddenFunction in overriddenFunctions) {
if (!overriddenFunction.resolvedStatus.hasStableParameterNames) continue

val valueParameterPairs = function.symbol.valueParameterSymbols.zip(overriddenFunction.valueParameterSymbols)
for ((currentValueParameter, overriddenValueParameter) in valueParameterPairs) {
if (currentValueParameter.name != overriddenValueParameter.name) {
reporter.reportOn(
currentValueParameter.source,
FirErrors.PARAMETER_NAME_CHANGED_ON_OVERRIDE,
overriddenFunction.getContainingClassSymbol() as FirRegularClassSymbol,
overriddenValueParameter,
context,
)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSUPPORTED_CONTE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONTEXT_PARAMETER_WITHOUT_NAME
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONTEXT_PARAMETER_WITH_DEFAULT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.GENERIC_QUALIFIER_ON_CONSTRUCTOR_CALL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PARAMETER_NAME_CHANGED_ON_OVERRIDE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSUPPORTED_FEATURE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSUPPORTED_INHERITANCE_FROM_JAVA_MEMBER_REFERENCING_KOTLIN_FUNCTION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSUPPORTED_SEALED_FUN_INTERFACE
Expand Down Expand Up @@ -2035,6 +2036,13 @@ object FirErrorsDefaultMessages : BaseDiagnosticRendererFactory() {
VIRTUAL_MEMBER_HIDDEN, "''{0}'' hides member of supertype ''{1}'' and needs an ''override'' modifier.", DECLARATION_NAME,
DECLARATION_NAME
)
map.put(
PARAMETER_NAME_CHANGED_ON_OVERRIDE,
"The corresponding parameter in the supertype ''{0}'' is named ''{1}''. " +
"This may cause problems when calling this function with named arguments.",
DECLARATION_NAME,
VARIABLE_NAME,
)
map.put(
DATA_CLASS_OVERRIDE_CONFLICT,
"Function ''{0}'' generated for the data class conflicts with the supertype member {1}.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class FirJavaValueParameter @FirImplementationDetail constructor(
override fun replaceSetter(newSetter: FirPropertyAccessor?) {
}

override fun replaceContextParameters(newContextParameter: List<FirValueParameter>) {
override fun replaceContextParameters(newContextParameters: List<FirValueParameter>) {
error("Body cannot be replaced for FirJavaValueParameter")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ expect open class Foo3 {
// FILE: jvm.kt

actual open class Foo1 : Base() {
override fun <!ACTUAL_WITHOUT_EXPECT!>foo<!>(paramNameChanged: Int) {}
override fun <!ACTUAL_WITHOUT_EXPECT!>foo<!>(<!PARAMETER_NAME_CHANGED_ON_OVERRIDE!>paramNameChanged<!>: Int) {}
}

actual typealias Foo2 = Foo2Java
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface C : A, B { // Warning here, this is correct, C.foo has no named parame
}

interface D : C {
override fun foo(d1: Int, d2: Double)
override fun foo(<!PARAMETER_NAME_CHANGED_ON_OVERRIDE!>d1<!>: Int, <!PARAMETER_NAME_CHANGED_ON_OVERRIDE!>d2<!>: Double)
}

fun test1(d: D) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// RUN_PIPELINE_TILL: BACKEND
interface A {
fun b(a : Int)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ interface D {
interface E : C, D

interface F : C, D {
override fun foo(a : Int) {
override fun foo(<!PARAMETER_NAME_CHANGED_ON_OVERRIDE!>a<!> : Int) {
throw UnsupportedOperationException()
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// RUN_PIPELINE_TILL: BACKEND
// FILE: Super.kt

Expand Down
8 changes: 4 additions & 4 deletions core/compiler.common/src/org/jetbrains/kotlin/name/FqName.kt
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ class FqName {
return fqName.toString()
}

override fun equals(o: Any?): Boolean {
if (this === o) return true
if (o !is FqName) return false
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is FqName) return false

if (fqName != o.fqName) return false
if (fqName != other.fqName) return false

return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ class FqNameUnsafe {
return if (isRoot) ROOT_NAME.asString() else fqName
}

override fun equals(o: Any?): Boolean {
if (this === o) return true
if (o !is FqNameUnsafe) return false
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is FqNameUnsafe) return false

if (fqName != o.fqName) return false
if (fqName != other.fqName) return false

return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class ClassStabilityTransformer(
private val unstableClassesWarning: MutableSet<ClassDescriptor>? = if (!context.platform.isJvm()) mutableSetOf() else null


override fun lower(module: IrModuleFragment) {
module.transformChildrenVoid(this)
override fun lower(irModule: IrModuleFragment) {
irModule.transformChildrenVoid(this)

if (!context.platform.isJvm() && !unstableClassesWarning.isNullOrEmpty()) {
val classIds = unstableClassesWarning.mapTo(mutableSetOf()) { it.fqNameSafe.toString() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
import org.jetbrains.kotlin.ir.expressions.IrCall
import org.jetbrains.kotlin.ir.expressions.IrExpression
import org.jetbrains.kotlin.ir.symbols.impl.IrSimpleFunctionSymbolImpl
import org.jetbrains.kotlin.ir.util.DeepCopySymbolRemapper
import org.jetbrains.kotlin.ir.util.addChild
import org.jetbrains.kotlin.ir.util.copyAnnotationsFrom
import org.jetbrains.kotlin.ir.util.createThisReceiverParameter
Expand Down Expand Up @@ -91,8 +90,8 @@ class ComposableDefaultParamLowering(
) : AbstractComposeLowering(context, metrics, stabilityInferencer, featureFlags) {
private val originalToTransformed = mutableMapOf<IrSimpleFunction, IrSimpleFunction>()

override fun lower(module: IrModuleFragment) {
module.transformChildrenVoid()
override fun lower(irModule: IrModuleFragment) {
irModule.transformChildrenVoid()
}

override fun visitSimpleFunction(declaration: IrSimpleFunction): IrStatement {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class ComposableFunInterfaceLowering(private val context: IrPluginContext) :
IrElementTransformerVoidWithContext(),
ModuleLoweringPass {

override fun lower(module: IrModuleFragment) {
override fun lower(irModule: IrModuleFragment) {
if (context.platform.isJvm()) {
module.transformChildrenVoid(this)
irModule.transformChildrenVoid(this)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,11 @@ class ComposableFunctionBodyTransformer(

private var inlineLambdaInfo = ComposeInlineLambdaLocator(context)

override fun lower(module: IrModuleFragment) {
inlineLambdaInfo.scan(module)
module.transformChildrenVoid(this)
override fun lower(irModule: IrModuleFragment) {
inlineLambdaInfo.scan(irModule)
irModule.transformChildrenVoid(this)
applySourceFixups()
module.patchDeclarationParents()
irModule.patchDeclarationParents()
}

override fun lower(irFile: IrFile) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,14 @@ class ComposableTargetAnnotationsTransformer(
}
)

override fun lower(module: IrModuleFragment) {
override fun lower(irModule: IrModuleFragment) {
// Only transform if the attributes being inferred are in the runtime
if (
ComposableTargetClass != null &&
ComposableInferredTargetClass != null &&
ComposableOpenTargetClass != null
) {
module.transformChildrenVoid(this)
irModule.transformChildrenVoid(this)
}
}

Expand Down
Loading

0 comments on commit 459e346

Please sign in to comment.