From e8e38d90ff3806395d6c1e7cf73edf3b9e8a6e08 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 6 Sep 2017 15:59:26 +0300 Subject: [PATCH] Validate JvmPackageName annotation value and placement - do not allow it to be used together with JvmMultifileClass (otherwise implementation becomes complex) - do not allow to declare classes in a JvmPackageName-annotated file (similarly, the implementation of this would be much harder in the compiler, and there would need to be special support in the IDE) - check that the value is a valid FQ name - do not allow root package just in case --- .../kotlin/fileClasses/JvmFileClassUtil.kt | 2 +- .../jvm/checkers/annotationCheckers.kt | 49 ++++++++++++++----- .../diagnostics/DefaultErrorMessagesJvm.java | 5 ++ .../resolve/jvm/diagnostics/ErrorsJvm.java | 5 ++ .../jvmPackageName/incorrectJvmPackageName.kt | 34 +++++++++++++ .../incorrectJvmPackageName.txt | 32 ++++++++++++ .../DiagnosticsTestWithStdLibGenerated.java | 15 ++++++ ...ticsTestWithStdLibUsingJavacGenerated.java | 15 ++++++ 8 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.txt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/fileClasses/JvmFileClassUtil.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/fileClasses/JvmFileClassUtil.kt index 9613f3f2e38..bf2b4960b8f 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/fileClasses/JvmFileClassUtil.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/fileClasses/JvmFileClassUtil.kt @@ -33,7 +33,7 @@ object JvmFileClassUtil { val JVM_MULTIFILE_CLASS: FqName = FqName("kotlin.jvm.JvmMultifileClass") val JVM_MULTIFILE_CLASS_SHORT = JVM_MULTIFILE_CLASS.shortName().asString() - private val JVM_PACKAGE_NAME: FqName = FqName("kotlin.jvm.JvmPackageName") + val JVM_PACKAGE_NAME: FqName = FqName("kotlin.jvm.JvmPackageName") private val JVM_PACKAGE_NAME_SHORT = JVM_PACKAGE_NAME.shortName().asString() private const val MULTIFILE_PART_NAME_DELIMITER = "__" diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/annotationCheckers.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/annotationCheckers.kt index fb54a818704..cb1007eff58 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/annotationCheckers.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/annotationCheckers.kt @@ -22,15 +22,13 @@ import org.jetbrains.kotlin.descriptors.annotations.KotlinRetention import org.jetbrains.kotlin.descriptors.annotations.KotlinTarget import org.jetbrains.kotlin.fileClasses.JvmFileClassUtil import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.name.isValidJavaFqName +import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.AdditionalAnnotationChecker import org.jetbrains.kotlin.resolve.AnnotationChecker import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingTrace -import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass -import org.jetbrains.kotlin.resolve.descriptorUtil.classId -import org.jetbrains.kotlin.resolve.descriptorUtil.getAnnotationRetention -import org.jetbrains.kotlin.resolve.descriptorUtil.isRepeatableAnnotation +import org.jetbrains.kotlin.resolve.descriptorUtil.* import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm object RepeatableAnnotationChecker: AdditionalAnnotationChecker { @@ -60,7 +58,7 @@ object RepeatableAnnotationChecker: AdditionalAnnotationChecker { object FileClassAnnotationsChecker: AdditionalAnnotationChecker { // JvmName & JvmMultifileClass annotations are applicable to multi-file class parts regardless of their retention. - private val ALWAYS_APPLICABLE = hashSetOf(JvmFileClassUtil.JVM_NAME, JvmFileClassUtil.JVM_MULTIFILE_CLASS) + private val alwaysApplicable = hashSetOf(JvmFileClassUtil.JVM_NAME, JvmFileClassUtil.JVM_MULTIFILE_CLASS) override fun checkEntries(entries: List, actualTargets: List, trace: BindingTrace) { val fileAnnotationsToCheck = arrayListOf>() @@ -74,13 +72,40 @@ object FileClassAnnotationsChecker: AdditionalAnnotationChecker { fileAnnotationsToCheck.add(Pair(entry, classDescriptor)) } - if (!fileAnnotationsToCheck.any { it.second.classId?.asSingleFqName() == JvmFileClassUtil.JVM_MULTIFILE_CLASS }) return + val isMultifileClass = fileAnnotationsToCheck.any { it.second.fqNameSafe == JvmFileClassUtil.JVM_MULTIFILE_CLASS } - for ((entry, classDescriptor) in fileAnnotationsToCheck) { - val classFqName = classDescriptor.classId!!.asSingleFqName() - if (classFqName in ALWAYS_APPLICABLE) continue - if (classDescriptor.getAnnotationRetention() != KotlinRetention.SOURCE) { - trace.report(ErrorsJvm.ANNOTATION_IS_NOT_APPLICABLE_TO_MULTIFILE_CLASSES.on(entry, classFqName)) + if (isMultifileClass) { + for ((entry, classDescriptor) in fileAnnotationsToCheck) { + val classFqName = classDescriptor.fqNameSafe + if (classFqName in alwaysApplicable) continue + if (classDescriptor.getAnnotationRetention() != KotlinRetention.SOURCE) { + trace.report(ErrorsJvm.ANNOTATION_IS_NOT_APPLICABLE_TO_MULTIFILE_CLASSES.on(entry, classFqName)) + } + if (classFqName == JvmFileClassUtil.JVM_PACKAGE_NAME) { + trace.report(ErrorsJvm.JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_MULTIFILE_CLASSES.on(entry)) + } + } + } + else { + for ((entry, classDescriptor) in fileAnnotationsToCheck) { + if (classDescriptor.fqNameSafe != JvmFileClassUtil.JVM_PACKAGE_NAME) continue + + val argumentExpression = entry.valueArguments.firstOrNull()?.getArgumentExpression() ?: continue + val stringTemplateEntries = (argumentExpression as? KtStringTemplateExpression)?.entries ?: continue + if (stringTemplateEntries.size > 1) continue + + val value = (stringTemplateEntries.singleOrNull() as? KtLiteralStringTemplateEntry)?.text + if (value == null) { + trace.report(ErrorsJvm.JVM_PACKAGE_NAME_CANNOT_BE_EMPTY.on(entry)) + } + else if (!isValidJavaFqName(value)) { + trace.report(ErrorsJvm.JVM_PACKAGE_NAME_MUST_BE_VALID_NAME.on(entry)) + } + else if (entry.containingKtFile.declarations.any { + it !is KtFunction && it !is KtProperty && it !is KtTypeAlias + }) { + trace.report(ErrorsJvm.JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_FILES_WITH_CLASSES.on(entry)) + } } } } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index 0e9e612cc3b..4cf269a0030 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -73,6 +73,11 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(NON_SOURCE_REPEATED_ANNOTATION, "Only annotations with SOURCE retention can be repeated on JVM version before 1.8"); MAP.put(ANNOTATION_IS_NOT_APPLICABLE_TO_MULTIFILE_CLASSES, "Annotation ''@{0}'' is not applicable to the multi-file classes", TO_STRING); + MAP.put(JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_MULTIFILE_CLASSES, "''@JvmPackageName'' annotation is not supported in multi-file classes"); + MAP.put(JVM_PACKAGE_NAME_CANNOT_BE_EMPTY, "''@JvmPackageName'' annotation value cannot be empty"); + MAP.put(JVM_PACKAGE_NAME_MUST_BE_VALID_NAME, "''@JvmPackageName'' annotation value must be a valid dot-qualified name of a package"); + MAP.put(JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_FILES_WITH_CLASSES, "''@JvmPackageName'' annotation is not supported for files with class declarations"); + MAP.put(NO_REFLECTION_IN_CLASS_PATH, "Call uses reflection API which is not found in compilation classpath. " + "Make sure you have kotlin-reflect.jar in the classpath"); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index 283bc7a1c05..7d6c2c9840a 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -73,6 +73,11 @@ public interface ErrorsJvm { DiagnosticFactory0 NON_SOURCE_REPEATED_ANNOTATION = DiagnosticFactory0.create(ERROR); DiagnosticFactory1 ANNOTATION_IS_NOT_APPLICABLE_TO_MULTIFILE_CLASSES = DiagnosticFactory1.create(ERROR); + DiagnosticFactory0 JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_MULTIFILE_CLASSES = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 JVM_PACKAGE_NAME_CANNOT_BE_EMPTY = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 JVM_PACKAGE_NAME_MUST_BE_VALID_NAME = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 JVM_PACKAGE_NAME_NOT_SUPPORTED_IN_FILES_WITH_CLASSES = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 INTERFACE_CANT_CALL_DEFAULT_METHOD_VIA_SUPER = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 SUBCLASS_CANT_CALL_COMPANION_PROTECTED_NON_STATIC = DiagnosticFactory0.create(ERROR); diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt new file mode 100644 index 00000000000..e739a7e7461 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt @@ -0,0 +1,34 @@ +// !DIAGNOSTICS: -INVISIBLE_MEMBER -INVISIBLE_REFERENCE +// !API_VERSION: 1.2 + +// FILE: a.kt +@file:JvmPackageName("a") +@file:JvmMultifileClass +package a +fun a() {} + +// FILE: b.kt +@file:JvmPackageName("") +package b +fun b() {} + +// FILE: c.kt +@file:JvmPackageName("invalid-fq-name") +package c +fun c() {} + +// FILE: d.kt +@file:JvmPackageName("d") +package d +class D +fun d() {} + +// FILE: e.kt +@file:JvmPackageName(42) +package e +fun e() {} + +// FILE: f.kt +@file:JvmPackageName(f) +package f +const val name = "f" diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.txt new file mode 100644 index 00000000000..628e6376875 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.txt @@ -0,0 +1,32 @@ +package + +package a { + public fun a(): kotlin.Unit +} + +package b { + public fun b(): kotlin.Unit +} + +package c { + public fun c(): kotlin.Unit +} + +package d { + public fun d(): kotlin.Unit + + public final class D { + public constructor D() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +package e { + public fun e(): kotlin.Unit +} + +package f { + public const val name: kotlin.String = "f" +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 42de2143933..0b982460af8 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -539,6 +539,21 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW } } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class JvmPackageName extends AbstractDiagnosticsTestWithStdLib { + public void testAllFilesPresentInJvmPackageName() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("incorrectJvmPackageName.kt") + public void testIncorrectJvmPackageName() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java index e8eb40d96a3..e1325e81638 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -539,6 +539,21 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno } } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class JvmPackageName extends AbstractDiagnosticsTestWithStdLibUsingJavac { + public void testAllFilesPresentInJvmPackageName() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("incorrectJvmPackageName.kt") + public void testIncorrectJvmPackageName() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmPackageName/incorrectJvmPackageName.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)