From 730a5d1a8841b703259ddf4653e54d3e31d7afcf Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 1 Jul 2022 16:47:35 +0200 Subject: [PATCH] FIR: introduce Java vs Kotlin class conflicts checker #KT-44441 Fixed --- .../api/fir/sessions/LLFirSessionFactory.kt | 9 +- .../jvm/checkers/JvmDeclarationCheckers.kt | 3 +- .../declaration/FirJvmConflictsChecker.kt | 24 +++++ .../fir/session/ComponentsContainers.kt | 4 - .../kotlin/fir/session/FirSessionFactory.kt | 4 +- .../kotlin/fir/java/JavaSymbolProvider.kt | 3 + .../overrideWithTypeUseInClasspath.fir.kt | 89 +++++++++++++++++++ .../jsr305/overrideWithTypeUseInClasspath.kt | 2 +- ...rideWithTypeUseInClasspathWithArray.fir.kt | 89 +++++++++++++++++++ ...overrideWithTypeUseInClasspathWithArray.kt | 1 - .../j+k/brokenCode/classDuplicates.fir.kt | 13 +++ .../tests/j+k/brokenCode/classDuplicates.kt | 1 - 12 files changed, 231 insertions(+), 11 deletions(-) create mode 100644 compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmConflictsChecker.kt create mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.fir.kt create mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.fir.kt create mode 100644 compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.fir.kt diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirSessionFactory.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirSessionFactory.kt index 7717b0b82d2..9efd60cb6db 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirSessionFactory.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirSessionFactory.kt @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.fir.analysis.checkersComponent import org.jetbrains.kotlin.fir.analysis.extensions.additionalCheckers import org.jetbrains.kotlin.fir.backend.jvm.FirJvmTypeMapper import org.jetbrains.kotlin.fir.extensions.* +import org.jetbrains.kotlin.fir.java.JavaSymbolProvider import org.jetbrains.kotlin.fir.resolve.providers.FirDependenciesSymbolProvider import org.jetbrains.kotlin.fir.resolve.providers.FirProvider import org.jetbrains.kotlin.fir.resolve.providers.FirSymbolProvider @@ -120,6 +121,7 @@ internal object LLFirSessionFactory { } } + val javaSymbolProvider = createJavaSymbolProvider(this, moduleData, project, contentScope) register( FirSymbolProvider::class, LLFirModuleWithDependenciesSymbolProvider( @@ -128,10 +130,11 @@ internal object LLFirSessionFactory { providers = listOfNotNull( provider.symbolProvider, switchableExtensionDeclarationsSymbolProvider, - createJavaSymbolProvider(this, moduleData, project, contentScope) + javaSymbolProvider, ), ) ) + register(JavaSymbolProvider::class, javaSymbolProvider) register(FirDependenciesSymbolProvider::class, dependencyProvider) register(FirJvmTypeMapper::class, FirJvmTypeMapper(this)) @@ -210,6 +213,7 @@ internal object LLFirSessionFactory { ) } + val javaSymbolProvider = createJavaSymbolProvider(this, moduleData, project, contentScope) register( FirSymbolProvider::class, LLFirModuleWithDependenciesSymbolProvider( @@ -217,10 +221,11 @@ internal object LLFirSessionFactory { dependencyProvider, providers = listOf( provider.symbolProvider, - createJavaSymbolProvider(this, moduleData, project, contentScope), + javaSymbolProvider, ), ) ) + register(JavaSymbolProvider::class, javaSymbolProvider) register(FirDependenciesSymbolProvider::class, dependencyProvider) register(FirJvmTypeMapper::class, FirJvmTypeMapper(this)) diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmDeclarationCheckers.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmDeclarationCheckers.kt index 09b460ba6b4..32a7e96b921 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmDeclarationCheckers.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/JvmDeclarationCheckers.kt @@ -28,7 +28,8 @@ object JvmDeclarationCheckers : DeclarationCheckers() { override val regularClassCheckers: Set get() = setOf( FirJvmRecordChecker, - FirJvmInlineApplicabilityChecker + FirJvmInlineApplicabilityChecker, + FirJvmConflictsChecker, ) override val propertyCheckers: Set diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmConflictsChecker.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmConflictsChecker.kt new file mode 100644 index 00000000000..95584a76814 --- /dev/null +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirJvmConflictsChecker.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2010-2022 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.jvm.checkers.declaration + +import org.jetbrains.kotlin.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirRegularClassChecker +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.declarations.FirRegularClass +import org.jetbrains.kotlin.fir.declarations.utils.classId +import org.jetbrains.kotlin.fir.java.javaSymbolProvider + +object FirJvmConflictsChecker : FirRegularClassChecker() { + override fun check(declaration: FirRegularClass, context: CheckerContext, reporter: DiagnosticReporter) { + val javaSymbol = context.session.javaSymbolProvider.getClassLikeSymbolByClassId(declaration.classId) ?: return + reporter.reportOn( + declaration.source, FirErrors.PACKAGE_OR_CLASSIFIER_REDECLARATION, listOf(declaration.symbol, javaSymbol), context + ) + } +} diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/ComponentsContainers.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/ComponentsContainers.kt index 91eea9c61bc..3f5286f2018 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/ComponentsContainers.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/ComponentsContainers.kt @@ -5,9 +5,6 @@ package org.jetbrains.kotlin.fir.session -import com.intellij.psi.PsiFile -import org.jetbrains.kotlin.KtNodeTypes -import org.jetbrains.kotlin.KtPsiSourceElement import org.jetbrains.kotlin.KtSourceElement import org.jetbrains.kotlin.config.JvmAnalysisFlags import org.jetbrains.kotlin.config.LanguageVersionSettings @@ -30,7 +27,6 @@ import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory import org.jetbrains.kotlin.fir.resolve.calls.FirSyntheticNamesProvider import org.jetbrains.kotlin.fir.resolve.calls.jvm.JvmCallConflictResolverFactory import org.jetbrains.kotlin.fir.resolve.inference.InferenceComponents -import org.jetbrains.kotlin.fir.resolve.providers.firProvider import org.jetbrains.kotlin.fir.resolve.providers.impl.FirQualifierResolverImpl import org.jetbrains.kotlin.fir.resolve.providers.impl.FirTypeResolverImpl import org.jetbrains.kotlin.fir.resolve.transformers.FirPhaseCheckingPhaseManager diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirSessionFactory.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirSessionFactory.kt index 741a2e99cbe..907ac734059 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirSessionFactory.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirSessionFactory.kt @@ -197,6 +197,7 @@ object FirSessionFactory { val dependenciesSymbolProvider = FirDependenciesSymbolProviderImpl(this) val generatedSymbolsProvider = FirSwitchableExtensionDeclarationsSymbolProvider.create(this) + val javaSymbolProvider = JavaSymbolProvider(this, projectEnvironment.getFirJavaFacade(this, moduleData, javaSourcesScope)) register( FirSymbolProvider::class, FirCompositeSymbolProvider( @@ -206,12 +207,13 @@ object FirSessionFactory { *(incrementalCompilationContext?.previousFirSessionsSymbolProviders?.toTypedArray() ?: emptyArray()), symbolProviderForBinariesFromIncrementalCompilation, generatedSymbolsProvider, - JavaSymbolProvider(this, projectEnvironment.getFirJavaFacade(this, moduleData, javaSourcesScope)), + javaSymbolProvider, dependenciesSymbolProvider, optionalAnnotationClassesProviderForBinariesFromIncrementalCompilation, ) ) ) + register(JavaSymbolProvider::class, javaSymbolProvider) generatedSymbolsProvider?.let { register(FirSwitchableExtensionDeclarationsSymbolProvider::class, it) } diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaSymbolProvider.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaSymbolProvider.kt index c29e3542105..c3467ce7882 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaSymbolProvider.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaSymbolProvider.kt @@ -57,3 +57,6 @@ class JavaSymbolProvider( @OptIn(FirSymbolProviderInternals::class) override fun getTopLevelPropertySymbolsTo(destination: MutableList, packageFqName: FqName, name: Name) {} } + +val FirSession.javaSymbolProvider: JavaSymbolProvider by FirSession.sessionComponentAccessor() + diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.fir.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.fir.kt new file mode 100644 index 00000000000..403b8158fac --- /dev/null +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.fir.kt @@ -0,0 +1,89 @@ +// SOURCE_RETENTION_ANNOTATIONS +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// JSR305_GLOBAL_REPORT: strict + +// FILE: spr/Nullable.java +package spr; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierNickname; +import javax.annotation.meta.When; + +@Target({ElementType.TYPE_USE}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull(when = When.MAYBE) +@TypeQualifierNickname +public @interface Nullable { +} + +// FILE: spr/NotNull.java +package spr; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierNickname; +import javax.annotation.meta.When; + +@Target({ElementType.METHOD, ElementType.PARAMETER}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull +@TypeQualifierNickname +public @interface NotNull { +} + + +// FILE: J1.java + +import spr.*; + +public interface J1 { + @Nullable + T getFoo(); +} + +// FILE: J2.java + +import spr.*; + +public class J2 implements J1 { + @NotNull + public String getFoo() { return ""; } +} + + +// FILE: Nonnull.kt +package spr +import javax.annotation.meta.TypeQualifierNickname +import javax.annotation.Nonnull; + +// This is a crucial part: NotNull defined in Java has no TYPE_USE target, so it has been applied only to the method +// While on use-site (main.kt) spr.NotNull shall be resolved to Kotlin version with TYPE among target +// So, even while our annotation is type use, we still should consider situation when it's applied to the method/parameter, but not to type +@Target( + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.VALUE_PARAMETER, + AnnotationTarget.TYPE, +) +@Retention(AnnotationRetention.RUNTIME) +@Nonnull +@TypeQualifierNickname +annotation class NotNull + +// FILE: main.kt + +fun bar(j2: J2) { + j2.foo.length // `j2.foo` is not nullable +} diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.kt index d8664f51ecb..c7fb2b2dc19 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.kt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspath.kt @@ -1,4 +1,4 @@ -// FIR_IDENTICAL +// SOURCE_RETENTION_ANNOTATIONS // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER // JSR305_GLOBAL_REPORT: strict diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.fir.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.fir.kt new file mode 100644 index 00000000000..92a6fbb19ad --- /dev/null +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.fir.kt @@ -0,0 +1,89 @@ +// SOURCE_RETENTION_ANNOTATIONS +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// JSR305_GLOBAL_REPORT: strict +// !LANGUAGE: +TypeEnhancementImprovementsInStrictMode + +// FILE: spr/Nullable.java +package spr; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierNickname; +import javax.annotation.meta.When; + +@Target({ElementType.TYPE_USE}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull(when = When.MAYBE) +@TypeQualifierNickname +public @interface Nullable { +} + +// FILE: spr/NotNull.java +package spr; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierNickname; +import javax.annotation.meta.When; + +@Target({ElementType.METHOD, ElementType.PARAMETER}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull +@TypeQualifierNickname +public @interface NotNull { +} + + +// FILE: J1.java + +import spr.*; + +public interface J1 { + T @Nullable [] getFoo(); +} + +// FILE: J2.java + +import spr.*; + +public class J2 implements J1 { + @NotNull + public String[] getFoo() { return null; } +} + + +// FILE: Nonnull.kt +package spr +import javax.annotation.meta.TypeQualifierNickname +import javax.annotation.Nonnull; + +// This is a crucial part: NotNull defined in Java has no TYPE_USE target, so it has been applied only to the method +// While on use-site (main.kt) spr.NotNull shall be resolved to Kotlin version with TYPE among target +// So, even while our annotation is type use, we still should consider situation when it's applied to the method/parameter, but not to type +@Target( + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.VALUE_PARAMETER, + AnnotationTarget.TYPE, +) +@Retention(AnnotationRetention.RUNTIME) +@Nonnull +@TypeQualifierNickname +annotation class NotNull + +// FILE: main.kt + +fun bar(j2: J2) { + j2.foo.get(0).length // `j2.foo` is not nullable +} diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.kt index aec1f7b8505..6a4504718b1 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.kt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jsr305/overrideWithTypeUseInClasspathWithArray.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL // SOURCE_RETENTION_ANNOTATIONS // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER // JSR305_GLOBAL_REPORT: strict diff --git a/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.fir.kt b/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.fir.kt new file mode 100644 index 00000000000..8adb2a06223 --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.fir.kt @@ -0,0 +1,13 @@ +// FILE: Hello.kt +private class Hello() +{ + val a = 4 +} + +fun test() { + // no exception is thrown (see KT-3897) + Hello().a +} + +// FILE: Hello.java +public class Hello {} diff --git a/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.kt b/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.kt index 0626e104442..928b78be8ea 100644 --- a/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.kt +++ b/compiler/testData/diagnostics/tests/j+k/brokenCode/classDuplicates.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL // FILE: Hello.kt private class Hello() {