Fixes after review

This commit is contained in:
Ilya Chernikov
2018-05-28 15:55:33 +02:00
parent 6218b2bcf6
commit 6fdb8746de
13 changed files with 63 additions and 68 deletions
@@ -27,7 +27,6 @@ import org.jetbrains.kotlin.compiler.plugin.*
import org.jetbrains.kotlin.config.CompilerConfiguration
import java.io.File
import java.net.URL
import java.net.URLClassLoader
import java.util.*
object PluginCliParser {
@@ -41,7 +41,7 @@ open class KotlinScriptDefinitionFromAnnotatedTemplate(
val environment: Map<String, Any?>? = null,
val templateClasspath: List<File> = emptyList()
) : KotlinScriptDefinition(template) {
val scriptFilePattern by lazy {
val scriptFilePattern by lazy(LazyThreadSafetyMode.PUBLICATION) {
val pattern =
takeUnlessError {
val ann = template.annotations.firstIsInstanceOrNull<kotlin.script.templates.ScriptTemplateDefinition>()
@@ -52,7 +52,7 @@ open class KotlinScriptDefinitionFromAnnotatedTemplate(
Regex(pattern)
}
override val dependencyResolver: DependenciesResolver by lazy {
override val dependencyResolver: DependenciesResolver by lazy(LazyThreadSafetyMode.PUBLICATION) {
resolverFromAnnotation(template) ?:
resolverFromLegacyAnnotation(template) ?:
DependenciesResolver.NoDependencies
@@ -99,12 +99,12 @@ open class KotlinScriptDefinitionFromAnnotatedTemplate(
}
}
private val samWithReceiverAnnotations: List<String>? by lazy {
private val samWithReceiverAnnotations: List<String>? by lazy(LazyThreadSafetyMode.PUBLICATION) {
takeUnlessError { template.annotations.firstIsInstanceOrNull<kotlin.script.extensions.SamWithReceiverAnnotations>()?.annotations?.toList() }
?: takeUnlessError { template.annotations.firstIsInstanceOrNull<org.jetbrains.kotlin.script.SamWithReceiverAnnotations>()?.annotations?.toList() }
}
override val acceptedAnnotations: List<KClass<out Annotation>> by lazy {
override val acceptedAnnotations: List<KClass<out Annotation>> by lazy(LazyThreadSafetyMode.PUBLICATION) {
fun sameSignature(left: KFunction<*>, right: KFunction<*>): Boolean =
left.name == right.name &&
@@ -134,7 +134,7 @@ open class KotlinScriptDefinitionFromAnnotatedTemplate(
}
}
override val scriptExpectedLocations: List<ScriptExpectedLocation> by lazy {
override val scriptExpectedLocations: List<ScriptExpectedLocation> by lazy(LazyThreadSafetyMode.PUBLICATION) {
takeUnlessError {
template.annotations.firstIsInstanceOrNull<ScriptExpectedLocations>()
}?.value?.toList() ?: super.scriptExpectedLocations
@@ -153,7 +153,7 @@ open class KotlinScriptDefinitionFromAnnotatedTemplate(
override val annotationsForSamWithReceivers: List<String>
get() = samWithReceiverAnnotations ?: super.annotationsForSamWithReceivers
override val additionalCompilerArguments: Iterable<String>? by lazy {
override val additionalCompilerArguments: Iterable<String>? by lazy(LazyThreadSafetyMode.PUBLICATION) {
takeUnlessError {
template.annotations.firstIsInstanceOrNull<kotlin.script.templates.ScriptTemplateAdditionalCompilerArguments>()?.let {
val res = it.provider.primaryConstructor?.call(it.arguments.asIterable())
@@ -58,8 +58,8 @@ class ScriptContentLoader(private val project: Project) {
class BasicScriptContents(virtualFile: VirtualFile, getAnnotations: () -> Iterable<Annotation>) : ScriptContents {
override val file: File = File(virtualFile.path)
override val annotations: Iterable<Annotation> by lazy { getAnnotations() }
override val text: CharSequence? by lazy { virtualFile.inputStream.reader(charset = virtualFile.charset).readText() }
override val annotations: Iterable<Annotation> by lazy(LazyThreadSafetyMode.PUBLICATION) { getAnnotations() }
override val text: CharSequence? by lazy(LazyThreadSafetyMode.PUBLICATION) { virtualFile.inputStream.reader(charset = virtualFile.charset).readText() }
}
fun loadContentsAndResolveDependencies(
@@ -8,13 +8,27 @@ package kotlin.script.experimental.api
import kotlin.reflect.KClass
import kotlin.reflect.KType
class KotlinType(
/**
* A Kotlin type representation for using in the scripting API
*/
class KotlinType private constructor(
val typeName: String,
val fromClass: KClass<*>? = null
val fromClass: KClass<*>?
// TODO: copy properties from KType
) {
// TODO: implement other approach for non-class types
constructor(type: KType) : this((type.classifier as KClass<*>).qualifiedName!!, type.classifier as KClass<*>)
/**
* Constructs KotlinType from fully-qualified [qualifiedTypeName] in a dot-separated form, e.g. "org.acme.Outer.Inner"
*/
constructor(qualifiedTypeName: String) : this(qualifiedTypeName, null)
/**
* Constructs KotlinType from reflected [kclass]
*/
constructor(kclass: KClass<*>) : this(kclass.qualifiedName!!, kclass)
// TODO: implement other approach for non-class types
/**
* Constructs KotlinType from reflected [ktype]
*/
constructor(type: KType) : this(type.classifier as KClass<*>)
}
@@ -15,7 +15,7 @@ private const val ILLEGAL_CONFIG_ANN_ARG =
open class AnnotationsBasedCompilationConfigurator(val environment: ScriptingEnvironment) : ScriptCompilationConfigurator {
override val defaultConfiguration by lazy {
override val defaultConfiguration by lazy(LazyThreadSafetyMode.PUBLICATION) {
val baseClass = environment.getScriptBaseClass(this)
val cfg = baseClass.annotations.filterIsInstance(KotlinScriptDefaultCompilationConfiguration::class.java).flatMap { ann ->
val params = try {
@@ -19,7 +19,7 @@ private const val ERROR_MSG_PREFIX = "Unable to construct script definition: "
open class ScriptDefinitionFromAnnotatedBaseClass(val environment: ScriptingEnvironment) : ScriptDefinition {
private val getScriptingClass = environment.getOrNull(ScriptingEnvironmentProperties.getScriptingClass)
?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting 'getClass' parameter in the scripting environment")
?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting 'getScriptingClass' parameter in the scripting environment")
private val baseClass: KClass<*> = run {
val baseClassType = environment.getOrNull(ScriptingEnvironmentProperties.baseClass)
@@ -33,13 +33,7 @@ open class ChainedPropertyBag private constructor(private val parent: ChainedPro
inline operator fun <reified T> get(key: TypedKey<T>): T = getRaw(key) as T
fun <T> getRaw(key: TypedKey<T>): Any? =
when {
data.containsKey(key) -> data[key]
parent != null -> parent.getRaw(key)
key.defaultValue != null -> key.defaultValue
else -> throw IllegalArgumentException("Unknown key $key")
}
fun <T> getRaw(key: TypedKey<T>): Any? = getOrNullRaw(key) ?: throw IllegalArgumentException("Unknown key $key")
inline fun <reified T> getOrNull(key: TypedKey<T>): T? = getOrNullRaw(key)?.let { it as T }
@@ -13,6 +13,7 @@ class JvmGetScriptingClass : GetScriptingClass {
private var dependencies: List<ScriptDependency>? = null
private var classLoader: ClassLoader? = null
private var baseClassLoaderIsInitialized = false
private var baseClassLoader: ClassLoader? = null
@Synchronized
@@ -20,10 +21,11 @@ class JvmGetScriptingClass : GetScriptingClass {
// checking if class already loaded in the same context
val contextClassloader = contextClass.java.classLoader
if (classType.fromClass != null) {
if (classType.fromClass!!.java.classLoader == null) return classType.fromClass!! // root classloader
val actualClassLoadersChain = generateSequence(classType.fromClass!!.java.classLoader) { it.parent }
if (actualClassLoadersChain.any { it == contextClassloader }) return classType.fromClass!!
val fromClass = classType.fromClass
if (fromClass != null) {
if (fromClass.java.classLoader == null) return fromClass // root classloader
val actualClassLoadersChain = generateSequence(contextClassloader) { it.parent }
if (actualClassLoadersChain.any { it == fromClass.java.classLoader }) return fromClass
}
val newDeps = environment.getOrNull(ScriptingEnvironmentProperties.configurationDependencies)
@@ -33,9 +35,10 @@ class JvmGetScriptingClass : GetScriptingClass {
if (newDeps != dependencies) throw IllegalArgumentException("scripting environment dependencies changed")
}
if (baseClassLoader == null) {
if (!baseClassLoaderIsInitialized) {
baseClassLoader = contextClassloader
} else {
baseClassLoaderIsInitialized = true
} else if (baseClassLoader != null) {
val baseClassLoadersChain = generateSequence(baseClassLoader) { it.parent }
if (baseClassLoadersChain.none { it == contextClassloader }) throw IllegalArgumentException("scripting class instantiation context changed")
}
@@ -26,7 +26,7 @@ abstract class KotlinScriptDefinitionAdapterFromNewAPIBase : KotlinScriptDefinit
protected abstract val scriptFileExtensionWithDot: String
open val baseClass: KClass<*> by lazy {
open val baseClass: KClass<*> by lazy(LazyThreadSafetyMode.PUBLICATION) {
getScriptingClass(scriptDefinition.compilationConfigurator.defaultConfiguration[ScriptingEnvironmentProperties.baseClass])
}
@@ -48,33 +48,30 @@ abstract class KotlinScriptDefinitionAdapterFromNewAPIBase : KotlinScriptDefinit
override val annotationsForSamWithReceivers: List<String>
get() = emptyList()
override val dependencyResolver: DependenciesResolver by lazy {
override val dependencyResolver: DependenciesResolver by lazy(LazyThreadSafetyMode.PUBLICATION) {
BridgeDependenciesResolver(scriptDefinition.compilationConfigurator)
}
override val acceptedAnnotations: List<KClass<out Annotation>> by lazy {
val annNames =
scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.refineConfigurationOnAnnotations)
?: emptyList()
annNames.map { getScriptingClass(it) as KClass<out Annotation> }
override val acceptedAnnotations: List<KClass<out Annotation>> by lazy(LazyThreadSafetyMode.PUBLICATION) {
scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.refineConfigurationOnAnnotations)
.orEmpty()
.map { getScriptingClass(it) as KClass<out Annotation> }
}
override val implicitReceivers: List<KType> by lazy {
val rcNames =
scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.scriptImplicitReceivers)
?: emptyList()
rcNames.map { getScriptingClass(it).starProjectedType }
override val implicitReceivers: List<KType> by lazy(LazyThreadSafetyMode.PUBLICATION) {
scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.scriptImplicitReceivers)
.orEmpty()
.map { getScriptingClass(it).starProjectedType }
}
override val environmentVariables: List<Pair<String, KType>> by lazy {
override val environmentVariables: List<Pair<String, KType>> by lazy(LazyThreadSafetyMode.PUBLICATION) {
scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.contextVariables)
?.map { (k, v) -> k to getScriptingClass(v).starProjectedType }
?: emptyList()
?.map { (k, v) -> k to getScriptingClass(v).starProjectedType }.orEmpty()
}
override val additionalCompilerArguments: List<String>
get() = scriptDefinition.compilationConfigurator.defaultConfiguration.getOrNull(ScriptCompileConfigurationProperties.compilerOptions)
?: emptyList()
.orEmpty()
override val scriptExpectedLocations: List<ScriptExpectedLocation> =
listOf(
@@ -82,7 +79,7 @@ abstract class KotlinScriptDefinitionAdapterFromNewAPIBase : KotlinScriptDefinit
ScriptExpectedLocation.TestsOnly
)
private val scriptingClassGetter by lazy {
private val scriptingClassGetter by lazy(LazyThreadSafetyMode.PUBLICATION) {
scriptDefinition.properties.getOrNull(ScriptingEnvironmentProperties.getScriptingClass)
?: throw IllegalArgumentException("Expecting 'getScriptingClass' property in the scripting environment")
}
@@ -34,7 +34,7 @@ class LazyScriptDefinitionFromDiscoveredClass internal constructor(
messageCollector: MessageCollector
) : this(loadAnnotationsFromClass(classBytes), className, classpath, messageCollector)
override val scriptDefinition: ScriptDefinition by lazy {
override val scriptDefinition: ScriptDefinition by lazy(LazyThreadSafetyMode.PUBLICATION) {
messageCollector.report(
CompilerMessageSeverity.LOGGING,
"Configure scripting: loading script definition class $className using classpath $classpath\n. ${Thread.currentThread().stackTrace}"
@@ -59,7 +59,7 @@ class LazyScriptDefinitionFromDiscoveredClass internal constructor(
}
}
override val scriptFileExtensionWithDot: String by lazy {
override val scriptFileExtensionWithDot: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
val ext = annotationsFromAsm.find { it.name == KotlinScriptFileExtension::class.simpleName!! }?.args?.first()
?: scriptDefinition.properties.let {
it.getOrNull(ScriptDefinitionProperties.fileExtension) ?: "kts"
@@ -67,7 +67,7 @@ class LazyScriptDefinitionFromDiscoveredClass internal constructor(
".$ext"
}
override val name: String by lazy {
override val name: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
annotationsFromAsm.find { it.name == KotlinScript::class.simpleName!! }?.args?.first()
?: super.name
}
@@ -52,7 +52,7 @@ internal fun discoverScriptTemplatesInClasspath(
messageCollector: MessageCollector
): Sequence<KotlinScriptDefinition> = buildSequence {
// TODO: try to find a way to reduce classpath (and classloader) to minimal one needed to load script definition and its dependencies
val classLoader by lazy {
val classLoader by lazy(LazyThreadSafetyMode.PUBLICATION) {
URLClassLoader(classpath.map { it.toURI().toURL() }.toTypedArray(), baseClassLoader)
}
for (dep in classpath) {
@@ -89,10 +89,10 @@ internal fun discoverScriptTemplatesInClasspath(
dep.isDirectory -> {
val dir = File(dep, SCRIPT_DEFINITION_MARKERS_PATH)
if (dir.isDirectory) {
val templateClasspath by lazy {
val templateClasspath by lazy(LazyThreadSafetyMode.PUBLICATION) {
listOf(dep) + defaultScriptDefinitionClasspath
}
val classLoader by lazy {
val classLoader by lazy(LazyThreadSafetyMode.PUBLICATION) {
URLClassLoader(templateClasspath.map { it.toURI().toURL() }.toTypedArray(), baseClassLoader)
}
dir.listFiles().forEach { templateClassNmae ->
@@ -154,10 +154,10 @@ internal fun loadScriptTemplatesFromClasspath(
}
// then searching the remaining templates in the supplied classpath
if (templatesLeftToFind.isNotEmpty()) {
val templateClasspath by lazy {
val templateClasspath by lazy(LazyThreadSafetyMode.PUBLICATION) {
classpath + dependenciesClasspath
}
val classLoader by lazy {
val classLoader by lazy(LazyThreadSafetyMode.PUBLICATION) {
URLClassLoader(templateClasspath.map { it.toURI().toURL() }.toTypedArray(), baseClassLoader)
}
for (dep in classpath) {
@@ -5,12 +5,7 @@
package org.jetbrains.kotlin.scripting.compiler.plugin
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.org.objectweb.asm.AnnotationVisitor
import org.jetbrains.org.objectweb.asm.ClassReader
import org.jetbrains.org.objectweb.asm.ClassVisitor
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.*
internal class BinAnnData(
val name: String,
@@ -25,7 +20,7 @@ private class TemplateAnnotationVisitor(val anns: ArrayList<BinAnnData> = arrayL
private class TemplateClassVisitor(val annVisitor: TemplateAnnotationVisitor) : ClassVisitor(Opcodes.ASM5) {
override fun visitAnnotation(desc: String, visible: Boolean): AnnotationVisitor {
val shortName = jvmDescToClassId(desc).shortClassName.asString()
val shortName = Type.getType(desc).internalName.substringAfterLast("/")
if (shortName.startsWith("KotlinScript")) {
annVisitor.anns.add(BinAnnData(shortName))
}
@@ -33,13 +28,6 @@ private class TemplateClassVisitor(val annVisitor: TemplateAnnotationVisitor) :
}
}
private fun jvmDescToClassId(desc: String): ClassId {
assert(desc.startsWith("L") && desc.endsWith(";")) { "Not a JVM descriptor: $desc" }
val name = desc.substring(1, desc.length - 1)
val cid = ClassId.topLevel(FqName(name.replace('/', '.')))
return cid
}
internal fun loadAnnotationsFromClass(fileContents: ByteArray): ArrayList<BinAnnData> {
val visitor =
@@ -36,7 +36,7 @@ class ScriptingCompilerPluginTest : TestCaseWithTmpdir() {
const val TEST_DATA_DIR = "plugins/scripting/scripting-cli/testData"
}
private val kotlinPaths: KotlinPaths by lazy {
private val kotlinPaths: KotlinPaths by lazy(LazyThreadSafetyMode.PUBLICATION) {
val paths = PathUtil.kotlinPathsForDistDirectory
TestCase.assertTrue("Lib directory doesn't exist. Run 'ant dist'", paths.libPath.absoluteFile.isDirectory)
paths