From 4dd0a25eca9b522a0447735e15322509e2a7bba4 Mon Sep 17 00:00:00 2001 From: Stanislav Ruban Date: Mon, 11 Mar 2024 00:00:15 +0200 Subject: [PATCH] [FIR][checkers][Wasm] Refine WRONG_JS_INTEROP_TYPE reporting This commit improves four aspects of WRONG_JS_INTEROP_TYPE error reporting: 1) more precise source code ranges are preferred when possible (e.g. value parameter type instead of the entire value parameter, explicit return type instead of the entire declaration, etc.) 2) only relevant parameter and return types of function types are reported as wrong (to prevent confusion with the "function types are supported" part of the error message) 3) WRONG_JS_INTEROP_TYPE errors are now deduplicated in cases where more than one such error was previously reported because of compiler-generated declarations 4) error messages were slightly proofread and contain slightly more information now --- .../wasm/FirWasmErrorsDefaultMessages.kt | 2 +- .../FirWasmJsInteropTypesChecker.kt | 160 +++++------- .../complexCasesWithJsInteropTypes.fir.kt | 10 +- .../jsInterop/wrongJsInteropTypes.fir.kt | 236 +++++++++--------- 4 files changed, 193 insertions(+), 215 deletions(-) diff --git a/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/diagnostics/wasm/FirWasmErrorsDefaultMessages.kt b/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/diagnostics/wasm/FirWasmErrorsDefaultMessages.kt index 806b3b53371..ca8fd44c2b4 100644 --- a/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/diagnostics/wasm/FirWasmErrorsDefaultMessages.kt +++ b/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/diagnostics/wasm/FirWasmErrorsDefaultMessages.kt @@ -57,7 +57,7 @@ object FirWasmErrorsDefaultMessages : BaseDiagnosticRendererFactory() { map.put(CALL_TO_DEFINED_EXTERNALLY_FROM_NON_EXTERNAL_DECLARATION, "This property can only be used from external declarations.") map.put( WRONG_JS_INTEROP_TYPE, - "Type ''{0}'' cannot be used in {1}. Only external, primitive, string and function types are supported in Kotlin/Wasm JS interop.", + "Type ''{0}'' cannot be used as {1}. Only external, primitive, string, and function types are supported in Kotlin/Wasm JS interop.", FirDiagnosticRenderers.RENDER_TYPE, TO_STRING ) map.put( diff --git a/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/wasm/checkers/declaration/FirWasmJsInteropTypesChecker.kt b/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/wasm/checkers/declaration/FirWasmJsInteropTypesChecker.kt index c409e634bf4..6d6e44b9034 100644 --- a/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/wasm/checkers/declaration/FirWasmJsInteropTypesChecker.kt +++ b/compiler/fir/checkers/checkers.wasm/src/org/jetbrains/kotlin/fir/analysis/wasm/checkers/declaration/FirWasmJsInteropTypesChecker.kt @@ -5,10 +5,11 @@ package org.jetbrains.kotlin.fir.analysis.wasm.checkers.declaration +import org.jetbrains.kotlin.KtFakeSourceElement import org.jetbrains.kotlin.KtSourceElement import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.diagnostics.reportOn -import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.FirFunctionTypeParameter import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirBasicDeclarationChecker @@ -17,7 +18,6 @@ import org.jetbrains.kotlin.fir.analysis.wasm.checkers.hasValidJsCodeBody import org.jetbrains.kotlin.fir.analysis.wasm.checkers.isJsExportedDeclaration import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.isEffectivelyExternal -import org.jetbrains.kotlin.fir.declarations.utils.isEnumClass import org.jetbrains.kotlin.fir.resolve.fullyExpandedType import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.name.WasmStandardClassIds @@ -46,114 +46,92 @@ object FirWasmJsInteropTypesChecker : FirBasicDeclarationChecker(MppCheckerKind. return } - // Skip enums to avoid reporting errors for synthetic static members with unsupported interop types. - // External enums errors are reported in a separate checker. - if (context.containingDeclarations.any { it is FirClass && it.isEnumClass }) { - return + // filter out compiler-generated declarations (data/enum methods, property accessors, primary constructors, etc.) to prevent + // 1) reporting excessive diagnostics + // (e.g. on generated methods of external enum classes (which are handled by another checker)) + // 2) reporting duplicate diagnostics + // (e.g. on properties with generated accessors and type parameters of classes with generated primary constructors) + if (declaration.source is KtFakeSourceElement) return + + fun ConeKotlinType.isSupportedInJsInterop(position: Position): Boolean { + if (isUnit || isNothing) { + // Unit and Nothing are supported in return type positions and unsupported in other type positions + return position == Position.RETURN_TYPE || position == Position.FUNCTION_TYPE_RETURN_TYPE + } + + // primitive types, unsigned types, and String are supported (regardless of nullability) + if (isPrimitiveOrNullablePrimitive) return true + if (isUnsignedTypeOrNullableUnsignedType) return true + if (isString || isNullableString) return true + + // type parameters' upper bounds should be checked separately on declaration-site + if (this is ConeTypeParameterType) return true + + // function types themselves are supported + // (and their parameter and return types should be checked separately) + if (isBasicFunctionType(session)) return true + + // aside from the aforementioned cases, only external types are supported + return toRegularClassSymbol(session)?.isEffectivelyExternal(session) == true } - fun ConeKotlinType.checkJsInteropType( - typePositionDescription: String, - source: KtSourceElement?, - isInFunctionReturnPosition: Boolean = false, - ) { - if (!isTypeSupportedInJsInterop(this, isInFunctionReturnPosition, session)) { + fun FirTypeRef.checkSupportInJsInterop(position: Position, fallbackSource: KtSourceElement?) { + val type = coneType.let { + val unexpandedType = if (position == Position.VARARG_VALUE_PARAMETER_TYPE) it.varargElementType() else it + unexpandedType.fullyExpandedType(session) + } + + if (!type.isSupportedInJsInterop(position)) { reporter.reportOn( - source, + source ?: fallbackSource, FirWasmErrors.WRONG_JS_INTEROP_TYPE, - this, - typePositionDescription, + type, + position.description, context ) + return + } + + // although function types themselves are supported, their parameter and return types should be checked separately + val functionTypeRef = (this as? FirResolvedTypeRef)?.delegatedTypeRef as? FirFunctionTypeRef ?: return + with(functionTypeRef) { + parameters.map(FirFunctionTypeParameter::returnTypeRef).forEach { parameterTypeRef -> + parameterTypeRef.checkSupportInJsInterop(Position.FUNCTION_TYPE_PARAMETER_TYPE, fallbackSource = functionTypeRef.source) + } + returnTypeRef.checkSupportInJsInterop(Position.FUNCTION_TYPE_RETURN_TYPE, fallbackSource = functionTypeRef.source) } } - fun FirTypeParameterRef.checkJsInteropTypeParameter() { - for (upperBound in this.symbol.resolvedBounds) { - upperBound.type.checkJsInteropType( - "JS interop type parameter upper bound", - upperBound.source ?: this.source - ) - } - } - - if (declaration is FirMemberDeclaration) { + if (declaration is FirTypeParameterRefsOwner) { for (typeParameter in declaration.typeParameters) { - typeParameter.checkJsInteropTypeParameter() + for (upperBound in typeParameter.symbol.resolvedBounds) { + upperBound.checkSupportInJsInterop(Position.TYPE_PARAMETER_UPPER_BOUND, fallbackSource = typeParameter.source) + } } } when (declaration) { is FirProperty -> { - declaration.returnTypeRef.coneType.checkJsInteropType( - "JS interop property", - declaration.source - ) + declaration.returnTypeRef.checkSupportInJsInterop(Position.PROPERTY_TYPE, fallbackSource = declaration.source) } is FirFunction -> { - for (parameter in declaration.valueParameters) { - val type = parameter.returnTypeRef.coneType - val varargElementTypeOrType = if (parameter.isVararg) type.varargElementType() else type - varargElementTypeOrType.checkJsInteropType( - "JS interop function parameter", - parameter.source - ) + for (valueParameter in declaration.valueParameters) { + val position = if (valueParameter.isVararg) Position.VARARG_VALUE_PARAMETER_TYPE else Position.VALUE_PARAMETER_TYPE + valueParameter.returnTypeRef.checkSupportInJsInterop(position, fallbackSource = valueParameter.source) } - declaration.returnTypeRef.coneType.checkJsInteropType( - "JS interop function return", - declaration.source, - isInFunctionReturnPosition = true - ) + declaration.returnTypeRef.checkSupportInJsInterop(Position.RETURN_TYPE, fallbackSource = declaration.source) } - else -> {} + else -> Unit } } -} - -private fun isTypeSupportedInJsInterop( - unexpandedType: ConeKotlinType, - isInFunctionReturnPosition: Boolean, - session: FirSession, -): Boolean { - val type = unexpandedType.fullyExpandedType(session) - - if (type.isUnit || type.isNothing) { - return isInFunctionReturnPosition - } - - val nonNullable = type.withNullability(ConeNullability.NOT_NULL, session.typeContext) - - if (nonNullable.isPrimitive || nonNullable.isUnsignedType || nonNullable.isString) { - return true - } - - // Interop type parameters upper bounds should be checked - // on declaration site separately - if (nonNullable is ConeTypeParameterType) { - return true - } - - val regularClassSymbol = nonNullable.toRegularClassSymbol(session) - if (regularClassSymbol?.isEffectivelyExternal(session) == true) { - return true - } - - if (type.isBasicFunctionType(session)) { - val arguments = type.typeArguments - for (i in 0 until arguments.lastIndex) { - val argType = arguments[i].type ?: return false - if (!isTypeSupportedInJsInterop(argType, isInFunctionReturnPosition = false, session)) { - return false - } - } - - val returnType = arguments.last().type ?: return false - return isTypeSupportedInJsInterop( - returnType, - isInFunctionReturnPosition = true, - session - ) - } - - return false + + private enum class Position(val description: String) { + TYPE_PARAMETER_UPPER_BOUND("upper bound of JS interop type parameter"), + PROPERTY_TYPE("type of JS interop property"), + VALUE_PARAMETER_TYPE("value parameter type of JS interop function"), + VARARG_VALUE_PARAMETER_TYPE("value parameter type of JS interop function"), + RETURN_TYPE("return type of JS interop function"), + FUNCTION_TYPE_PARAMETER_TYPE("parameter type of JS interop function type"), + FUNCTION_TYPE_RETURN_TYPE("return type of JS interop function type"), + } } diff --git a/compiler/testData/diagnostics/wasmTests/jsInterop/complexCasesWithJsInteropTypes.fir.kt b/compiler/testData/diagnostics/wasmTests/jsInterop/complexCasesWithJsInteropTypes.fir.kt index 04e3d5f5b72..36ac6bb1ee9 100644 --- a/compiler/testData/diagnostics/wasmTests/jsInterop/complexCasesWithJsInteropTypes.fir.kt +++ b/compiler/testData/diagnostics/wasmTests/jsInterop/complexCasesWithJsInteropTypes.fir.kt @@ -17,12 +17,12 @@ external fun complexFunctionTypesWithNullableTypes( ) external fun complexFunctionTypesWithWrongTypes( - f1: ((Any?, Byte?, Unit?, Int?, CharSequence?, Float?, IntArray?, Char?, String?, EI?, EC?, EO?) -> Unit)?, - f2: ((Boolean?) -> ((Any?) -> ((Float?) -> ((CharSequence?) -> EI)?)?)?)?, - f3: ((((((((Boolean?) -> Unit?)?) -> Float?)?) -> IntArray?)?) -> EI?)?, + f1: ((Any?, Byte?, Unit?, Int?, CharSequence?, Float?, IntArray?, Char?, String?, EI?, EC?, EO?) -> Unit)?, + f2: ((Boolean?) -> ((Any?) -> ((Float?) -> ((CharSequence?) -> EI)?)?)?)?, + f3: ((((((((Boolean?) -> Unit?)?) -> Float?)?) -> IntArray?)?) -> EI?)?, ) external fun typeParameterWithUpperBoundsWithDifferentJsInteropCorrectness(arg: T): T where T : EI, T : Any -fun jsCodeFunctionWithBlockBody(x: Any): Any { js("return x;") } -val jsCodeProperty: Any = js("1") +fun jsCodeFunctionWithBlockBody(x: Any): Any { js("return x;") } +val jsCodeProperty: Any = js("1") diff --git a/compiler/testData/diagnostics/wasmTests/jsInterop/wrongJsInteropTypes.fir.kt b/compiler/testData/diagnostics/wasmTests/jsInterop/wrongJsInteropTypes.fir.kt index e05ed4fcb7f..478d6c200d6 100644 --- a/compiler/testData/diagnostics/wasmTests/jsInterop/wrongJsInteropTypes.fir.kt +++ b/compiler/testData/diagnostics/wasmTests/jsInterop/wrongJsInteropTypes.fir.kt @@ -9,76 +9,76 @@ object UserDefinedObject external fun wrongJsInteropTypes( // Unit and Nothing - unit: Unit, - nothing: Nothing, + unit: Unit, + nothing: Nothing, // built-in types - any: Any, - number: Number, - charSequence: CharSequence, - specializedArray: IntArray, - pair: Pair, - list: List, - genericArray: Array, + any: Any, + number: Number, + charSequence: CharSequence, + specializedArray: IntArray, + pair: Pair, + list: List, + genericArray: Array, // user-defined types - userDefinedInterface: UserDefinedInterface, - userDefinedOpenClass: UserDefinedOpenClass, - userDefinedObject: UserDefinedObject, + userDefinedInterface: UserDefinedInterface, + userDefinedOpenClass: UserDefinedOpenClass, + userDefinedObject: UserDefinedObject, ) external fun wrongNullableJsInteropTypes( // Unit and Nothing - unit: Unit?, - nothing: Nothing?, + unit: Unit?, + nothing: Nothing?, // built-in types - any: Any?, - number: Number?, - charSequence: CharSequence?, - specializedArray: IntArray?, - pair: Pair?, - list: List?, - genericArray: Array?, + any: Any?, + number: Number?, + charSequence: CharSequence?, + specializedArray: IntArray?, + pair: Pair?, + list: List?, + genericArray: Array?, // user-defined types - userDefinedInterface: UserDefinedInterface?, - userDefinedOpenClass: UserDefinedOpenClass?, - userDefinedObject: UserDefinedObject?, + userDefinedInterface: UserDefinedInterface?, + userDefinedOpenClass: UserDefinedOpenClass?, + userDefinedObject: UserDefinedObject?, ) // wrong JS interop types as return types // built-in types -external fun anyAsReturnType(): Any -external fun numberAsReturnType(): Number -external fun charSequenceAsReturnType(): CharSequence -external fun specializedArrayAsReturnType(): IntArray -external fun pairAsReturnType(): Pair -external fun listAsReturnType(): List -external fun genericArrayAsReturnType(): Array +external fun anyAsReturnType(): Any +external fun numberAsReturnType(): Number +external fun charSequenceAsReturnType(): CharSequence +external fun specializedArrayAsReturnType(): IntArray +external fun pairAsReturnType(): Pair +external fun listAsReturnType(): List +external fun genericArrayAsReturnType(): Array // user-defined types -external fun userDefinedInterfaceAsReturnType(): UserDefinedInterface -external fun userDefinedOpenClassAsReturnType(): UserDefinedOpenClass -external fun userDefinedObjectAsReturnType(): UserDefinedObject +external fun userDefinedInterfaceAsReturnType(): UserDefinedInterface +external fun userDefinedOpenClassAsReturnType(): UserDefinedOpenClass +external fun userDefinedObjectAsReturnType(): UserDefinedObject // wrong JS interop types as vararg parameter types // Unit (Nothing is not allowed in vararg parameters) -external fun unitAsVarargParameterType(vararg args: Unit) +external fun unitAsVarargParameterType(vararg args: Unit) // built-in types -external fun anyAsVarargParameterType(vararg args: Any) -external fun numberAsVarargParameterType(vararg args: Number) -external fun charSequenceAsVarargParameterType(vararg args: CharSequence) -external fun specializedArrayAsVarargParameterType(vararg args: IntArray) -external fun pairAsVarargParameterType(vararg args: Pair) -external fun listAsVarargParameterType(vararg args: List) -external fun genericArrayAsVarargParameterType(vararg args: Array) +external fun anyAsVarargParameterType(vararg args: Any) +external fun numberAsVarargParameterType(vararg args: Number) +external fun charSequenceAsVarargParameterType(vararg args: CharSequence) +external fun specializedArrayAsVarargParameterType(vararg args: IntArray) +external fun pairAsVarargParameterType(vararg args: Pair) +external fun listAsVarargParameterType(vararg args: List) +external fun genericArrayAsVarargParameterType(vararg args: Array) // user-defined types -external fun userDefinedInterfaceAsVarargParameterType(vararg args: UserDefinedInterface) -external fun userDefinedOpenClassAsVarargParameterType(vararg args: UserDefinedOpenClass) -external fun userDefinedObjectAsVarargParameterType(vararg args: UserDefinedObject) +external fun userDefinedInterfaceAsVarargParameterType(vararg args: UserDefinedInterface) +external fun userDefinedOpenClassAsVarargParameterType(vararg args: UserDefinedOpenClass) +external fun userDefinedObjectAsVarargParameterType(vararg args: UserDefinedObject) external fun < @@ -115,101 +115,101 @@ external fun < external class WrongJsInteropTypesAsClassTypeParameterUpperBounds< // Unit (Nothing as an upper bound results in an empty intersection type) - TUnit: Unit, + TUnit: Unit, // built-in types - TAny: Any, - TNumber: Number, - TCharSequence: CharSequence, - TSpecializedArray: IntArray, - TPair: Pair, - TList: List, - TGenericArray: Array, + TAny: Any, + TNumber: Number, + TCharSequence: CharSequence, + TSpecializedArray: IntArray, + TPair: Pair, + TList: List, + TGenericArray: Array, // user-defined types - TUserDefinedInterface: UserDefinedInterface, - TUserDefinedOpenClass: UserDefinedOpenClass, - TUserDefinedObject: UserDefinedObject, + TUserDefinedInterface: UserDefinedInterface, + TUserDefinedOpenClass: UserDefinedOpenClass, + TUserDefinedObject: UserDefinedObject, // type parameter with implicit upper bound - TTypeParameterWithImplicitUpperBound + TTypeParameterWithImplicitUpperBound > // wrong JS interop types as property types // Unit and Nothing -external val unitProperty: Unit -external val nothingProperty: Nothing +external val unitProperty: Unit +external val nothingProperty: Nothing // built-in types -external val anyProperty: Any -external val numberProperty: Number -external val charSequenceProperty: CharSequence -external val specializedArrayProperty: IntArray -external val pairProperty: Pair -external val listProperty: List -external val genericArrayProperty: Array +external val anyProperty: Any +external val numberProperty: Number +external val charSequenceProperty: CharSequence +external val specializedArrayProperty: IntArray +external val pairProperty: Pair +external val listProperty: List +external val genericArrayProperty: Array // user-defined types -external val userDefinedInterfaceProperty: UserDefinedInterface -external val userDefinedOpenClassProperty: UserDefinedOpenClass -external val userDefinedObjectProperty: UserDefinedObject +external val userDefinedInterfaceProperty: UserDefinedInterface +external val userDefinedOpenClassProperty: UserDefinedOpenClass +external val userDefinedObjectProperty: UserDefinedObject external fun wrongJsInteropTypesAsFunctionTypeParameterTypes( - unitAndNothing: (Unit, Nothing) -> Unit, - builtInTypes: (Any, Number, CharSequence, IntArray, Pair, List, Array) -> Unit, - userDefinedTypes: (UserDefinedInterface, UserDefinedOpenClass, UserDefinedObject) -> Unit, + unitAndNothing: (Unit, Nothing) -> Unit, + builtInTypes: (Any, Number, CharSequence, IntArray, Pair, List, Array) -> Unit, + userDefinedTypes: (UserDefinedInterface, UserDefinedOpenClass, UserDefinedObject) -> Unit, ) external fun wrongJsInteropTypesAsFunctionTypeReturnTypes( // built-in types - any: () -> Any, - number: () -> Number, - charSequence: () -> CharSequence, - specializedArray: () -> IntArray, - pair: () -> Pair, - list: () -> List, - genericArray: () -> Array, + any: () -> Any, + number: () -> Number, + charSequence: () -> CharSequence, + specializedArray: () -> IntArray, + pair: () -> Pair, + list: () -> List, + genericArray: () -> Array, // user-defined types - userDefinedInterface: () -> UserDefinedInterface, - userDefinedOpenClass: () -> UserDefinedOpenClass, - userDefinedObject: () -> UserDefinedObject, + userDefinedInterface: () -> UserDefinedInterface, + userDefinedOpenClass: () -> UserDefinedOpenClass, + userDefinedObject: () -> UserDefinedObject, ) fun wrongJsInteropTypesInJsCodeFunction( // Unit and Nothing - unit: Unit, - nothing: Nothing, + unit: Unit, + nothing: Nothing, // built-in types - any: Any, - number: Number, - charSequence: CharSequence, - specializedArray: IntArray, - pair: Pair, - list: List, - genericArray: Array, + any: Any, + number: Number, + charSequence: CharSequence, + specializedArray: IntArray, + pair: Pair, + list: List, + genericArray: Array, // user-defined types - userDefinedInterface: UserDefinedInterface, - userDefinedOpenClass: UserDefinedOpenClass, - userDefinedObject: UserDefinedObject, + userDefinedInterface: UserDefinedInterface, + userDefinedOpenClass: UserDefinedOpenClass, + userDefinedObject: UserDefinedObject, ): Nothing = js("42") @JsExport fun wrongJsInteropTypesInJsExportFunction( // Unit and Nothing - unit: Unit, - nothing: Nothing, + unit: Unit, + nothing: Nothing, // built-in types - any: Any, - number: Number, - charSequence: CharSequence, - specializedArray: IntArray, - pair: Pair, - list: List, - genericArray: Array, + any: Any, + number: Number, + charSequence: CharSequence, + specializedArray: IntArray, + pair: Pair, + list: List, + genericArray: Array, // user-defined types - userDefinedInterface: UserDefinedInterface, - userDefinedOpenClass: UserDefinedOpenClass, - userDefinedObject: UserDefinedObject, + userDefinedInterface: UserDefinedInterface, + userDefinedOpenClass: UserDefinedOpenClass, + userDefinedObject: UserDefinedObject, ) {} @@ -230,18 +230,18 @@ typealias AliasedUserDefinedObject = UserDefinedObject external fun aliasedWrongJsInteropTypes( // Unit and Nothing - unit: AliasedUnit, - nothing: AliasedNothing, + unit: AliasedUnit, + nothing: AliasedNothing, // built-in types - any: AliasedAny, - number: AliasedNumber, - charSequence: AliasedCharSequence, - specializedArray: AliasedSpecializedArray, - pair: AliasedPair, - list: AliasedList, - genericArray: AliasedGenericArray, + any: AliasedAny, + number: AliasedNumber, + charSequence: AliasedCharSequence, + specializedArray: AliasedSpecializedArray, + pair: AliasedPair, + list: AliasedList, + genericArray: AliasedGenericArray, // user-defined types - userDefinedInterface: AliasedUserDefinedInterface, - userDefinedOpenClass: AliasedUserDefinedOpenClass, - userDefinedObject: AliasedUserDefinedObject, + userDefinedInterface: AliasedUserDefinedInterface, + userDefinedOpenClass: AliasedUserDefinedOpenClass, + userDefinedObject: AliasedUserDefinedObject, )