From 5da4d31d37bc101015d88dbce13caab81e34305c Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Mon, 15 Jan 2024 15:38:03 +0100 Subject: [PATCH] [JS FIR] Rework JS_NAME_CLASH diagnostic for constructors - Fix a false positive JS_NAME_CLASH report on constructors from different classes but with the same JsName. - Allow the same JsName for class constructors and other top-level declarations. This behavior differs from K1, but it is correct since there is no real name clash in the generated JS code. ^KT-64867 Fixed --- .../declaration/FirJsNameClashClassMembersChecker.kt | 2 ++ .../FirJsNameClashFileTopLevelDeclarationsChecker.kt | 5 ----- .../name/topLevelMethodAndJsNameConstructor.fir.kt | 11 +++++++++++ .../name/topLevelMethodAndJsNameConstructor.kt | 5 ++++- 4 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.fir.kt diff --git a/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashClassMembersChecker.kt b/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashClassMembersChecker.kt index 111f439442b..dec5ec15822 100644 --- a/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashClassMembersChecker.kt +++ b/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashClassMembersChecker.kt @@ -17,6 +17,7 @@ import org.jetbrains.kotlin.fir.analysis.js.checkers.FirJsStableName import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirClassLikeDeclaration import org.jetbrains.kotlin.fir.declarations.FirDeclarationOrigin +import org.jetbrains.kotlin.fir.declarations.constructors import org.jetbrains.kotlin.fir.declarations.utils.isFinal import org.jetbrains.kotlin.fir.scopes.* import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol @@ -118,6 +119,7 @@ object FirJsNameClashClassMembersChecker : FirClassChecker() { val scope = declaration.symbol.unsubstitutedScope(context) + scope.processDeclaredConstructors(allSymbols::add) addAllSymbolsFrom(scope.collectAllFunctions()) addAllSymbolsFrom(scope.collectAllProperties()) diff --git a/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashFileTopLevelDeclarationsChecker.kt b/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashFileTopLevelDeclarationsChecker.kt index f9e0880657f..e24ae33115c 100644 --- a/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashFileTopLevelDeclarationsChecker.kt +++ b/compiler/fir/checkers/checkers.js/src/org/jetbrains/kotlin/fir/analysis/js/checkers/declaration/FirJsNameClashFileTopLevelDeclarationsChecker.kt @@ -35,11 +35,6 @@ object FirJsNameClashFileTopLevelDeclarationsChecker : FirFileChecker() { val topLevelDeclarationsWithStableName = mutableMapOf>() for (topLevelDeclaration in declaration.declarations) { topLevelDeclarationsWithStableName.addStableName(topLevelDeclaration.symbol, context) - if (topLevelDeclaration is FirClass) { - for (classConstructor in topLevelDeclaration.constructors(context.session)) { - topLevelDeclarationsWithStableName.addStableName(classConstructor, context) - } - } } for ((name, stableNames) in topLevelDeclarationsWithStableName.entries) { for (symbol in stableNames) { diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.fir.kt b/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.fir.kt new file mode 100644 index 00000000000..489f4d76b8c --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.fir.kt @@ -0,0 +1,11 @@ +// FIR_DIFFERENCE +// K1 performs a check for legacy JS BE; however, this is not relevant for IR BE because in the generated JS code, +// there is no name clash between the constructor with JsName and the other top-level declarations with the same JsName. +// Keep K1 as it is, but for K2, implement a more relevant check without the clash. +package foo + +class A(val x: String) { + @JsName("aa") constructor(x: Int) : this("int $x") +} + +fun aa() {} diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.kt b/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.kt index 5c98482653d..263f0ac0768 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/name/topLevelMethodAndJsNameConstructor.kt @@ -1,4 +1,7 @@ -// FIR_IDENTICAL +// FIR_DIFFERENCE +// K1 performs a check for legacy JS BE; however, this is not relevant for IR BE because in the generated JS code, +// there is no name clash between the constructor with JsName and the other top-level declarations with the same JsName. +// Keep K1 as it is, but for K2, implement a more relevant check without the clash. package foo class A(val x: String) {