From 01354c8ce58a1a04ca85cb2e2f5178c9c3bf5cfd Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Wed, 21 Jun 2023 17:31:03 +0200 Subject: [PATCH] K2: Fix false-positive resolution to j.l.String constructor In K1 (see LazyImportScope), default start import with different priority worked as follows: - if something is found in HIGH, don't look at LOW - otherwise, look at LOW That, in particular, helped to avoid looking into JDK mirroring classes' constructors like when resolving calls like String(...) because we just don't look into j.l.String while already found kotlin.String The change inside FirTowerResolveTask.kt is not made accidentally: - At first, it's more or less obviously a bug fix because tower group for hide-members candidate with implicit receiver should take into account the tower level of the receiver itself. - The change is attached to this commit because otherwise the test at compiler/testData/diagnostics/testsWithStdLib/kt55503.kt starts failing. The bug was hidden because previously after finding a successful `Sequence.forEach` candidate for the inner receiver (at the default HIGH star import scope), resolver was continuing to look into default LOW star import scope where it's frozen forever because we had the better/closer candidate anyway. But after this change with merging default star imports into the same tower leve, resolver was continuing its job, enumerating implicit receivers, finding List there and noticing that there's a TopPrioritized hide-member candidates for them (erroneously ignoring it has a worse/more far receiver). ^KT-51670 Fixed --- .../problems/stringConstructors.fir.txt | 4 +- .../problems/stringConstructors.kt | 5 +- .../impl/FirDefaultStarImportingScope.kt | 102 +++++++++--------- ...FirSingleLevelDefaultStarImportingScope.kt | 69 ++++++++++++ .../calls/tower/FirTowerResolveTask.kt | 2 +- .../kotlin/fir/scopes/ImportingScopes.kt | 19 ++-- .../tests/j+k/javaLangStringCtr.fir.kt | 2 +- .../platformTypes/javaMappedCtors.fir.kt | 12 +-- 8 files changed, 143 insertions(+), 72 deletions(-) create mode 100644 compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirSingleLevelDefaultStarImportingScope.kt diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.fir.txt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.fir.txt index 81c30b6e33d..e56ea0c2d37 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.fir.txt @@ -1,8 +1,8 @@ FILE: stringConstructors.kt public final val sRef: R|kotlin/reflect/KFunction0| = ::R|kotlin/String.String| public get(): R|kotlin/reflect/KFunction0| - public final val s: R|java/lang/String| = R|java/lang/String.String|(R|kotlin/byteArrayOf|(vararg(Byte(1), Byte(2), Byte(3))), Int(4), Int(5), Int(6)) - public get(): R|java/lang/String| + public final val s: R|kotlin/String| = R|kotlin/text/String#|(R|kotlin/byteArrayOf|(vararg(Byte(1), Byte(2), Byte(3))), Int(4), Int(5), Int(6)) + public get(): R|kotlin/String| public final val s2: R|java/lang/String| = Q|java/lang|.R|java/lang/String.String|(R|kotlin/byteArrayOf|(vararg(Byte(1), Byte(2), Byte(3))), Int(4), Int(5), Int(6)) public get(): R|java/lang/String| public final val s3: R|kotlin/String| = R|kotlin/text/String|(R|kotlin/byteArrayOf|(vararg(Byte(1), Byte(2), Byte(3))), Int(4), Int(5)) diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.kt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.kt index f533ad769c3..fbd3a0fa1f6 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.kt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/stringConstructors.kt @@ -2,10 +2,7 @@ val sRef = ::String -// Note: FE 1.0 resolves this to kotlin.text.String() synthetic call with type mismatch on the last argument -// May be we (FIR) should migrate to the same behavior -// Other properties in this file are resolved similarly in FE 1.0 & FIR -val s = String(byteArrayOf(1, 2, 3), 4, 5, 6) +val s = String(byteArrayOf(1, 2, 3), 4, 5, 6) val s2 = java.lang.String(byteArrayOf(1, 2, 3), 4, 5, 6) val s3 = String(byteArrayOf(1, 2, 3), 4, 5) diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirDefaultStarImportingScope.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirDefaultStarImportingScope.kt index 60dc25f5560..5bc7967aebf 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirDefaultStarImportingScope.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirDefaultStarImportingScope.kt @@ -1,67 +1,69 @@ /* - * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2023 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.scopes.impl -import org.jetbrains.kotlin.config.AnalysisFlags -import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl -import org.jetbrains.kotlin.fir.FirSession -import org.jetbrains.kotlin.fir.declarations.builder.buildImport -import org.jetbrains.kotlin.fir.declarations.builder.buildResolvedImport -import org.jetbrains.kotlin.fir.languageVersionSettings -import org.jetbrains.kotlin.fir.moduleData -import org.jetbrains.kotlin.fir.resolve.ScopeSession -import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirVariableSymbol -import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor +import org.jetbrains.kotlin.fir.scopes.FirScope +import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.name.Name +/** + * Works as composite scope that only looks to the second scope if the first one is empty. + * It's necessary, e.g. to make sure we don't look for java.lang.String classifier/constructors once we've found kotlin.String one. + * @see org.jetbrains.kotlin.resolve.lazy.LazyImportScope that produces similar semantics + */ class FirDefaultStarImportingScope( - session: FirSession, - scopeSession: ScopeSession, - priority: DefaultImportPriority, - excludedImportNames: Set -) : FirAbstractStarImportingScope( - session, scopeSession, - lookupInFir = session.languageVersionSettings.getFlag(AnalysisFlags.allowKotlinPackage), - excludedImportNames -) { - // TODO: put languageVersionSettings into FirSession? - override val starImports = run { - val analyzerServices = session.moduleData.analyzerServices - val allDefaultImports = priority.getAllDefaultImports(analyzerServices, LanguageVersionSettingsImpl.DEFAULT) - allDefaultImports - ?.filter { it.isAllUnder } - ?.map { - buildResolvedImport { - delegate = buildImport { - importedFqName = it.fqName - isAllUnder = true - } - packageFqName = it.fqName - } - } ?: emptyList() + private val first: FirSingleLevelDefaultStarImportingScope, + private val second: FirSingleLevelDefaultStarImportingScope, +) : FirScope(), DefaultStarImportingScopeMarker { + override fun processClassifiersByNameWithSubstitution(name: Name, processor: (FirClassifierSymbol<*>, ConeSubstitutor) -> Unit) { + var wasFoundAny = false + first.processClassifiersByNameWithSubstitution(name) { symbol, substitutor -> + wasFoundAny = true + processor(symbol, substitutor) + } + + if (!wasFoundAny) { + second.processClassifiersByNameWithSubstitution(name, processor) + } + } + + private fun > processSymbolsByName( + name: Name, + processingFactory: FirScope.(Name, (S) -> Unit) -> Unit, + processor: (S) -> Unit, + ) { + var wasFoundAny = false + first.processingFactory(name) { + wasFoundAny = true + processor(it) + } + + if (!wasFoundAny) { + second.processingFactory(name, processor) + } } override fun processFunctionsByName(name: Name, processor: (FirNamedFunctionSymbol) -> Unit) { - if (name.isSpecial || name.identifier.isNotEmpty()) { - for (import in starImports) { - for (symbol in provider.getTopLevelFunctionSymbols(import.packageFqName, name)) { - processor(symbol) - } - } - } + processSymbolsByName(name, FirScope::processFunctionsByName, processor) } override fun processPropertiesByName(name: Name, processor: (FirVariableSymbol<*>) -> Unit) { - if (name.isSpecial || name.identifier.isNotEmpty()) { - for (import in starImports) { - for (symbol in provider.getTopLevelPropertySymbols(import.packageFqName, name)) { - processor(symbol) - } - } + processSymbolsByName(name, FirScope::processPropertiesByName, processor) + } + + override fun processDeclaredConstructors(processor: (FirConstructorSymbol) -> Unit) { + var wasFoundAny = false + first.processDeclaredConstructors { symbol -> + wasFoundAny = true + processor(symbol) + } + + if (!wasFoundAny) { + second.processDeclaredConstructors(processor) } } -} +} \ No newline at end of file diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirSingleLevelDefaultStarImportingScope.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirSingleLevelDefaultStarImportingScope.kt new file mode 100644 index 00000000000..816f5b8d33f --- /dev/null +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirSingleLevelDefaultStarImportingScope.kt @@ -0,0 +1,69 @@ +/* + * 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.scopes.impl + +import org.jetbrains.kotlin.config.AnalysisFlags +import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl +import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.declarations.builder.buildImport +import org.jetbrains.kotlin.fir.declarations.builder.buildResolvedImport +import org.jetbrains.kotlin.fir.languageVersionSettings +import org.jetbrains.kotlin.fir.moduleData +import org.jetbrains.kotlin.fir.resolve.ScopeSession +import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirVariableSymbol +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.name.Name + +interface DefaultStarImportingScopeMarker + +class FirSingleLevelDefaultStarImportingScope( + session: FirSession, + scopeSession: ScopeSession, + priority: DefaultImportPriority, + excludedImportNames: Set +) : FirAbstractStarImportingScope( + session, scopeSession, + lookupInFir = session.languageVersionSettings.getFlag(AnalysisFlags.allowKotlinPackage), + excludedImportNames +), DefaultStarImportingScopeMarker { + // TODO: put languageVersionSettings into FirSession? + override val starImports = run { + val analyzerServices = session.moduleData.analyzerServices + val allDefaultImports = priority.getAllDefaultImports(analyzerServices, LanguageVersionSettingsImpl.DEFAULT) + allDefaultImports + ?.filter { it.isAllUnder } + ?.map { + buildResolvedImport { + delegate = buildImport { + importedFqName = it.fqName + isAllUnder = true + } + packageFqName = it.fqName + } + } ?: emptyList() + } + + override fun processFunctionsByName(name: Name, processor: (FirNamedFunctionSymbol) -> Unit) { + if (name.isSpecial || name.identifier.isNotEmpty()) { + for (import in starImports) { + for (symbol in provider.getTopLevelFunctionSymbols(import.packageFqName, name)) { + processor(symbol) + } + } + } + } + + override fun processPropertiesByName(name: Name, processor: (FirVariableSymbol<*>) -> Unit) { + if (name.isSpecial || name.identifier.isNotEmpty()) { + for (import in starImports) { + for (symbol in provider.getTopLevelPropertySymbols(import.packageFqName, name)) { + processor(symbol) + } + } + } + } +} diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolveTask.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolveTask.kt index 91964c31752..509f7856c48 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolveTask.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolveTask.kt @@ -421,7 +421,7 @@ internal open class FirTowerResolveTask( skipSynthetics: Boolean, ) { processExtensionsThatHideMembers( - info, receiver, TowerGroup.EmptyRoot, + info, receiver, parentGroup, ExplicitReceiverKind.NO_EXPLICIT_RECEIVER, ) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/ImportingScopes.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/ImportingScopes.kt index 276c2f18cb9..1e23910453e 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/ImportingScopes.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/ImportingScopes.kt @@ -16,7 +16,7 @@ import org.jetbrains.kotlin.fir.symbols.lazyResolveToPhase import org.jetbrains.kotlin.name.FqName private val ALL_IMPORTS = scopeSessionKey() -private val DEFAULT_STAR_IMPORT = scopeSessionKey() +private val DEFAULT_STAR_IMPORT = scopeSessionKey() private val DEFAULT_SIMPLE_IMPORT = scopeSessionKey() private data class DefaultStarImportKey(val priority: DefaultImportPriority, val excludedImportNames: Set) @@ -48,12 +48,15 @@ private fun doCreateImportingScopes( } return listOf( - scopeSession.getOrBuild(DefaultStarImportKey(DefaultImportPriority.LOW, excludedImportNames), DEFAULT_STAR_IMPORT) { - FirDefaultStarImportingScope(session, scopeSession, DefaultImportPriority.LOW, excludedImportNames) - }, - scopeSession.getOrBuild(DefaultStarImportKey(DefaultImportPriority.HIGH, excludedImportNames), DEFAULT_STAR_IMPORT) { - FirDefaultStarImportingScope(session, scopeSession, DefaultImportPriority.HIGH, excludedImportNames) - }, + FirDefaultStarImportingScope( + scopeSession.getOrBuild(DefaultStarImportKey(DefaultImportPriority.HIGH, excludedImportNames), DEFAULT_STAR_IMPORT) { + FirSingleLevelDefaultStarImportingScope(session, scopeSession, DefaultImportPriority.HIGH, excludedImportNames) + }, + scopeSession.getOrBuild(DefaultStarImportKey(DefaultImportPriority.LOW, excludedImportNames), DEFAULT_STAR_IMPORT) { + FirSingleLevelDefaultStarImportingScope(session, scopeSession, DefaultImportPriority.LOW, excludedImportNames) + }, + ), + scopeSession.getOrBuild(DefaultImportPriority.KOTLIN_THROWS, DEFAULT_SIMPLE_IMPORT) { FirDefaultSimpleImportingScope(session, scopeSession, priority = DefaultImportPriority.KOTLIN_THROWS) }, @@ -79,4 +82,4 @@ private fun doCreateImportingScopes( ) } -private class ListStorageFirScope(val result: List) : FirScope() +private class ListStorageFirScope(val result: List) : FirScope() \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/j+k/javaLangStringCtr.fir.kt b/compiler/testData/diagnostics/tests/j+k/javaLangStringCtr.fir.kt index c8bc864cde3..b20b68bea96 100644 --- a/compiler/testData/diagnostics/tests/j+k/javaLangStringCtr.fir.kt +++ b/compiler/testData/diagnostics/tests/j+k/javaLangStringCtr.fir.kt @@ -1,4 +1,4 @@ // WITH_STDLIB // ISSUE: KT-51670 -val s = String(byteArrayOf(1, 2, 3), 4, 5, 6) +val s = String(byteArrayOf(1, 2, 3), 4, 5, 6) diff --git a/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt index 0139afd7672..933ec862217 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt @@ -11,7 +11,7 @@ typealias ADouble = Double fun main() { ABoolean(false) - Boolean(false) + Boolean(false) AChar('c') Char('c') @@ -20,17 +20,17 @@ fun main() { Int(42) ALong(42) - Long(42) + Long(42) AShort(42) - Short(42) + Short(42) AByte(42) - Byte(42) + Byte(42) AFloat(4.2f) - Float(4.2f) + Float(4.2f) ADouble(4.2) - Double(4.2) + Double(4.2) }