Fix reflection-based operations on compiler arguments after conversion

2017-07-27 15:20:57 +03:00
parent 50599c933f
commit 2984a5a19f
10 changed files with 162 additions and 95 deletions
@@ -16,13 +16,21 @@

 package org.jetbrains.kotlin.compilerRunner;

+import com.intellij.util.containers.ContainerUtil;
+import kotlin.jvm.JvmClassMappingKt;
+import kotlin.reflect.KClass;
+import kotlin.reflect.KProperty1;
+import kotlin.reflect.KVisibility;
+import kotlin.reflect.full.KClasses;
+import kotlin.reflect.jvm.ReflectJvmMapping;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.kotlin.cli.common.arguments.Argument;
 import org.jetbrains.kotlin.cli.common.arguments.CommonToolArguments;
 import org.jetbrains.kotlin.cli.common.arguments.ParseCommandLineArgumentsKt;
 import org.jetbrains.kotlin.utils.StringsKt;

-import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -33,39 +41,35 @@ public class ArgumentUtils {

    @NotNull
    public static List<String> convertArgumentsToStringList(@NotNull CommonToolArguments arguments)
-            throws InstantiationException, IllegalAccessException {
+            throws InstantiationException, IllegalAccessException, InvocationTargetException {
        List<String> result = new ArrayList<>();
-        convertArgumentsToStringList(arguments, arguments.getClass().newInstance(), arguments.getClass(), result);
+        Class<? extends CommonToolArguments> argumentsClass = arguments.getClass();
+        convertArgumentsToStringList(arguments, argumentsClass.newInstance(), JvmClassMappingKt.getKotlinClass(argumentsClass), result);
        result.addAll(arguments.getFreeArgs());
        return result;
    }

+    @SuppressWarnings("unchecked")
    private static void convertArgumentsToStringList(
            @NotNull CommonToolArguments arguments,
            @NotNull CommonToolArguments defaultArguments,
-            @NotNull Class<?> clazz,
+            @NotNull KClass<?> clazz,
            @NotNull List<String> result
-    ) throws IllegalAccessException, InstantiationException {
-        for (Field field : clazz.getDeclaredFields()) {
-            Argument argument = field.getAnnotation(Argument.class);
+    ) throws IllegalAccessException, InstantiationException, InvocationTargetException {
+        for (KProperty1 property : KClasses.getMemberProperties(clazz)) {
+            Argument argument = ContainerUtil.findInstance(property.getAnnotations(), Argument.class);
            if (argument == null) continue;

-            Object value;
-            Object defaultValue;
-            try {
-                value = field.get(arguments);
-                defaultValue = field.get(defaultArguments);
-            }
-            catch (IllegalAccessException ignored) {
-                // skip this field
-                continue;
-            }
+            if (property.getVisibility() != KVisibility.PUBLIC) continue;
+
+            Object value = property.get(arguments);
+            Object defaultValue = property.get(defaultArguments);

            if (value == null || Objects.equals(value, defaultValue)) continue;

-            Class<?> fieldType = field.getType();
+            Type propertyJavaType = ReflectJvmMapping.getJavaType(property.getReturnType());

-            if (fieldType.isArray()) {
+            if (propertyJavaType instanceof Class && ((Class) propertyJavaType).isArray()) {
                Object[] values = (Object[]) value;
                if (values.length == 0) continue;
                value = StringsKt.join(Arrays.asList(values), ",");
@@ -73,7 +77,7 @@ public class ArgumentUtils {

            result.add(argument.value());

-            if (fieldType == boolean.class || fieldType == Boolean.class) continue;
+            if (propertyJavaType == boolean.class || propertyJavaType == Boolean.class) continue;

            if (ParseCommandLineArgumentsKt.isAdvanced(argument)) {
                result.set(result.size() - 1, argument.value() + "=" + value.toString());
@@ -82,10 +86,5 @@ public class ArgumentUtils {
                result.add(value.toString());
            }
        }
-
-        Class<?> superClazz = clazz.getSuperclass();
-        if (superClazz != null) {
-            convertArgumentsToStringList(arguments, defaultArguments, superClazz, result);
-        }
    }
 }
@@ -16,29 +16,47 @@

 package org.jetbrains.kotlin.cli.common.arguments

-import java.lang.reflect.Field
-import java.lang.reflect.Modifier
 import java.util.*
+import kotlin.reflect.KClass
+import kotlin.reflect.KMutableProperty1
+import kotlin.reflect.KProperty1
+import kotlin.reflect.KVisibility
+import kotlin.reflect.full.declaredMemberProperties
+import kotlin.reflect.full.findAnnotation
+import kotlin.reflect.full.memberProperties

-fun <T : Any> copyBean(bean: T) = copyFields(bean, bean::class.java.newInstance(), true, collectFieldsToCopy(bean::class.java, false))
+@Suppress("UNCHECKED_CAST")
+fun <T : Any> copyBean(bean: T) =
+        copyProperties(bean, bean::class.java.newInstance()!!, true, collectProperties(bean::class as KClass<T>, false))

 fun <From : Any, To : From> mergeBeans(from: From, to: To): To {
    // TODO: rewrite when updated version of com.intellij.util.xmlb is available on TeamCity
-    return copyFields(from, to, false, collectFieldsToCopy(from::class.java, false))
+    @Suppress("UNCHECKED_CAST")
+    return copyProperties(from, to, false, collectProperties(from::class as KClass<From>, false))
 }

-fun <From : Any, To : Any> copyInheritedFields(from: From, to: To) = copyFields(from, to, true, collectFieldsToCopy(from::class.java, true))
+@Suppress("UNCHECKED_CAST")
+fun <From : Any, To : Any> copyInheritedFields(from: From, to: To) =
+        copyProperties(from, to, true, collectProperties(from::class as KClass<From>, true))

-fun <From : Any, To : Any> copyFieldsSatisfying(from: From, to: To, predicate: (Field) -> Boolean) =
-        copyFields(from, to, true, collectFieldsToCopy(from::class.java, false).filter(predicate))
+@Suppress("UNCHECKED_CAST")
+fun <From : Any, To : Any> copyFieldsSatisfying(from: From, to: To, predicate: (KProperty1<From, Any?>) -> Boolean) =
+        copyProperties(from, to, true, collectProperties(from::class as KClass<From>, false).filter(predicate))

-private fun <From : Any, To : Any> copyFields(from: From, to: To, deepCopyWhenNeeded: Boolean, fieldsToCopy: List<Field>): To {
+private fun <From : Any, To : Any> copyProperties(
+        from: From,
+        to: To,
+        deepCopyWhenNeeded: Boolean,
+        propertiesToCopy: List<KProperty1<From, Any?>>
+): To {
    if (from == to) return to

-    for (fromField in fieldsToCopy) {
-        val toField = to::class.java.getField(fromField.name)
-        val fromValue = fromField.get(from)
-        toField.set(to, if (deepCopyWhenNeeded) fromValue?.copyValueIfNeeded() else fromValue)
+    for (fromProperty in propertiesToCopy) {
+        @Suppress("UNCHECKED_CAST")
+        val toProperty = to::class.memberProperties.firstOrNull { it.name == fromProperty.name } as? KMutableProperty1<To, Any?>
+                         ?: continue
+        val fromValue = fromProperty.get(from)
+        toProperty.set(to, if (deepCopyWhenNeeded) fromValue?.copyValueIfNeeded() else fromValue)
    }
    return to
 }
@@ -72,19 +90,12 @@ private fun Any.copyValueIfNeeded(): Any {
    }
 }

-fun collectFieldsToCopy(clazz: Class<*>, inheritedOnly: Boolean): List<Field> {
-    val fromFields = ArrayList<Field>()
-
-    var currentClass: Class<*>? = if (inheritedOnly) clazz.superclass else clazz
-    while (currentClass != null) {
-        for (field in currentClass.declaredFields) {
-            val modifiers = field.modifiers
-            if (!Modifier.isStatic(modifiers) && Modifier.isPublic(modifiers) && !Modifier.isTransient(modifiers)) {
-                fromFields.add(field)
-            }
-        }
-        currentClass = currentClass.superclass
+fun <T : Any> collectProperties(kClass: KClass<T>, inheritedOnly: Boolean): List<KProperty1<T, Any?>> {
+    val properties = ArrayList(kClass.memberProperties)
+    if (inheritedOnly) {
+        properties.removeAll(kClass.declaredMemberProperties)
    }
-
-    return fromFields
-}
+    return properties.filter {
+        it.visibility == KVisibility.PUBLIC && it.findAnnotation<Transient>() == null
+    }
+}
@@ -17,7 +17,9 @@
 package org.jetbrains.kotlin.cli.common.arguments

 import com.intellij.util.SmartList
-import java.lang.reflect.Field
+import kotlin.reflect.KMutableProperty1
+import kotlin.reflect.full.findAnnotation
+import kotlin.reflect.full.memberProperties

 annotation class Argument(
        val value: String,
@@ -54,11 +56,13 @@ data class ArgumentParseErrors(

 // Parses arguments into the passed [result] object. Errors related to the parsing will be collected into [CommonToolArguments.errors].
 fun <A : CommonToolArguments> parseCommandLineArguments(args: List<String>, result: A) {
-    data class ArgumentField(val field: Field, val argument: Argument)
+    data class ArgumentField(val property: KMutableProperty1<A, Any?>, val argument: Argument)

-    val fields = result::class.java.fields.mapNotNull { field ->
-        val argument = field.getAnnotation(Argument::class.java)
-        if (argument != null) ArgumentField(field, argument) else null
+    @Suppress("UNCHECKED_CAST")
+    val properties = result::class.memberProperties.mapNotNull { property ->
+        if (property !is KMutableProperty1<*, *>) return@mapNotNull null
+        val argument = property.findAnnotation<Argument>() ?: return@mapNotNull null
+        ArgumentField(property as KMutableProperty1<A, Any?>, argument)
    }

    val errors = result.errors
@@ -72,7 +76,7 @@ fun <A : CommonToolArguments> parseCommandLineArguments(args: List<String>, resu
        }

        if (argument.value == arg) {
-            if (argument.isAdvanced && field.type != Boolean::class.java) {
+            if (argument.isAdvanced && property.returnType.classifier != Boolean::class) {
                errors.extraArgumentsPassedInObsoleteForm.add(arg)
            }
            return true
@@ -95,7 +99,7 @@ fun <A : CommonToolArguments> parseCommandLineArguments(args: List<String>, resu
            continue
        }

-        val argumentField = fields.firstOrNull { it.matches(arg) }
+        val argumentField = properties.firstOrNull { it.matches(arg) }
        if (argumentField == null) {
            when {
                arg.startsWith(ADVANCED_ARGUMENT_PREFIX) -> errors.unknownExtraFlags.add(arg)
@@ -105,9 +109,9 @@ fun <A : CommonToolArguments> parseCommandLineArguments(args: List<String>, resu
            continue
        }

-        val (field, argument) = argumentField
+        val (property, argument) = argumentField
        val value: Any = when {
-            field.type == Boolean::class.java -> true
+            argumentField.property.returnType.classifier == Boolean::class -> true
            argument.isAdvanced && arg.startsWith(argument.value + "=") -> {
                arg.substring(argument.value.length + 1)
            }
@@ -120,24 +124,25 @@ fun <A : CommonToolArguments> parseCommandLineArguments(args: List<String>, resu
            }
        }

-        if (!field.type.isArray && !visitedArgs.add(argument.value) && value is String && field.get(result) != value) {
+        if (argumentField.property.returnType.classifier?.javaClass?.isArray == false
+            && !visitedArgs.add(argument.value) && value is String && property.get(result) != value) {
            errors.duplicateArguments.put(argument.value, value)
        }

-        updateField(field, result, value, argument.delimiter)
+        updateField(property, result, value, argument.delimiter)
    }
 }

-private fun <A : CommonToolArguments> updateField(field: Field, result: A, value: Any, delimiter: String) {
-    when (field.type) {
-        Boolean::class.java, String::class.java -> field.set(result, value)
-        Array<String>::class.java -> {
+private fun <A : CommonToolArguments> updateField(property: KMutableProperty1<A, Any?>, result: A, value: Any, delimiter: String) {
+    when (property.returnType.classifier) {
+        Boolean::class, String::class -> property.set(result, value)
+        Array<String>::class -> {
            val newElements = (value as String).split(delimiter).toTypedArray()
            @Suppress("UNCHECKED_CAST")
-            val oldValue = field.get(result) as Array<String>?
-            field.set(result, if (oldValue != null) arrayOf(*oldValue, *newElements) else newElements)
+            val oldValue = property.get(result) as Array<String>?
+            property.set(result, if (oldValue != null) arrayOf(*oldValue, *newElements) else newElements)
        }
-        else -> throw IllegalStateException("Unsupported argument type: ${field.type}")
+        else -> throw IllegalStateException("Unsupported argument type: ${property.returnType}")
    }
 }

@@ -22,7 +22,7 @@ import org.jetbrains.kotlin.cli.common.arguments.Argument;
 import org.jetbrains.kotlin.cli.common.arguments.CommonToolArguments;
 import org.jetbrains.kotlin.cli.common.arguments.ParseCommandLineArgumentsKt;

-import java.lang.reflect.Field;
+import java.lang.reflect.Method;

 public class Usage {
    // The magic number 29 corresponds to the similar padding width in javac and scalac command line compilers
@@ -34,8 +34,8 @@ public class Usage {
        appendln(sb, "Usage: " + tool.executableScriptFileName() + " <options> <source files>");
        appendln(sb, "where " + (arguments.getExtraHelp() ? "advanced" : "possible") + " options include:");
        for (Class<?> clazz = arguments.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
-            for (Field field : clazz.getDeclaredFields()) {
-                fieldUsage(sb, field, arguments.getExtraHelp());
+            for (Method method : clazz.getDeclaredMethods()) {
+                propertyUsage(sb, method, arguments.getExtraHelp());
            }
        }

@@ -47,8 +47,8 @@ public class Usage {
        return sb.toString();
    }

-    private static void fieldUsage(@NotNull StringBuilder sb, @NotNull Field field, boolean extraHelp) {
-        Argument argument = field.getAnnotation(Argument.class);
+    private static void propertyUsage(@NotNull StringBuilder sb, @NotNull Method method, boolean extraHelp) {
+        Argument argument = method.getAnnotation(Argument.class);
        if (argument == null) return;

        if (extraHelp != ParseCommandLineArgumentsKt.isAdvanced(argument)) return;
@@ -24,6 +24,7 @@ import java.io.PrintStream
 import kotlin.reflect.KAnnotatedElement
 import kotlin.reflect.KProperty1
 import kotlin.reflect.full.declaredMemberProperties
+import kotlin.reflect.full.withNullability

 // Additional properties that should be included in interface
@Suppress("unused")
@@ -271,7 +272,8 @@ private val KProperty1<*, *>.gradleDefaultValue: String

 private val KProperty1<*, *>.gradleReturnType: String
        get() {
-            var type = returnType.toString().substringBeforeLast("!")
+            // Set nullability based on Gradle default value
+            var type = returnType.withNullability(false).toString().substringBeforeLast("!")
            if (gradleDefaultValue == "null") {
                type += "?"
            }
@@ -23,6 +23,7 @@ import com.intellij.util.xmlb.XmlSerializer
 import org.jdom.Element
 import org.jetbrains.kotlin.cli.common.arguments.*
 import org.jetbrains.kotlin.config.SettingConstants
+import kotlin.reflect.KClass

 abstract class BaseKotlinCompilerSettings<T : Any> protected constructor() : PersistentStateComponent<Element>, Cloneable {
    @Suppress("LeakingThis")
@@ -40,9 +41,10 @@ abstract class BaseKotlinCompilerSettings<T : Any> protected constructor() : Per
    }

    protected fun validateInheritedFieldsUnchanged(settings: T) {
-        val inheritedFields = collectFieldsToCopy(settings.javaClass, true)
+        @Suppress("UNCHECKED_CAST")
+        val inheritedProperties = collectProperties<T>(settings::class as KClass<T>, true)
        val defaultInstance = createSettings()
-        val invalidFields = inheritedFields.filter { it.get(settings) != it.get(defaultInstance) }
+        val invalidFields = inheritedProperties.filter { it.get(settings) != it.get(defaultInstance) }
        if (invalidFields.isNotEmpty()) {
            throw IllegalArgumentException("Following fields are expected to be left unchanged in ${settings.javaClass}: ${invalidFields.joinToString { it.name }}")
        }
@@ -17,11 +17,18 @@
 package org.jetbrains.kotlin.config

 import com.intellij.util.PathUtil
+import com.intellij.util.xmlb.SerializationFilter
 import com.intellij.util.xmlb.SkipDefaultsSerializationFilter
 import com.intellij.util.xmlb.XmlSerializer
 import org.jdom.DataConversionException
 import org.jdom.Element
 import org.jetbrains.kotlin.cli.common.arguments.*
+import org.jetbrains.kotlin.load.java.JvmAbi
+import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName
+import org.jetbrains.kotlin.name.Name
+import java.lang.reflect.Modifier
+import kotlin.reflect.KClass
+import kotlin.reflect.full.superclasses

 fun Element.getOption(name: String) = getChildren("option").firstOrNull { it.getAttribute("name").value == name }

@@ -166,6 +173,50 @@ fun CompilerSettings.convertPathsToSystemIndependent() {
    outputDirectoryForJsLibraryFiles = PathUtil.toSystemIndependentName(outputDirectoryForJsLibraryFiles)
 }

+private fun KClass<*>.superClass() = superclasses.firstOrNull { !it.java.isInterface }
+
+private fun Class<*>.computeNormalPropertyOrdering(): Map<String, Int> {
+    val result = LinkedHashMap<String, Int>()
+    var count = 0
+    generateSequence(this) { it.superclass }.forEach { clazz ->
+        for (method in clazz.declaredMethods) {
+            if (method.modifiers and Modifier.STATIC != 0) continue
+
+            val name = method.name
+            if (!JvmAbi.isGetterName(name)) continue
+
+            val propertyName = propertyNameByGetMethodName(Name.identifier(name))?.asString() ?: continue
+
+            result[propertyName] = count++
+        }
+    }
+    return result
+}
+
+private val allNormalOrderings = HashMap<Class<*>, Map<String, Int>>()
+
+private val Class<*>.normalOrdering
+    get() = allNormalOrderings.getOrPut(this) { computeNormalPropertyOrdering() }
+
+// Replacing fields with delegated properties leads to unexpected reordering of entries in facet configuration XML
+// It happens due to XmlSerializer using different orderings for field- and method-based accessors
+// This code restores the original ordering
+private fun Element.restoreNormalOrdering(bean: Any) {
+    val normalOrdering = bean.javaClass.normalOrdering
+    val elementsToReorder = this.getContent<Element> { it is Element && it.getAttribute("name")?.value in normalOrdering }
+    elementsToReorder
+            .sortedBy { normalOrdering[it.getAttribute("name")?.value!!] }
+            .forEachIndexed { index, element -> elementsToReorder[index] = element.clone() }
+}
+
+private fun buildChildElement(element: Element, tag: String, bean: Any, filter: SerializationFilter) {
+    Element(tag).apply {
+        XmlSerializer.serializeInto(bean, this, filter)
+        restoreNormalOrdering(bean)
+        element.addContent(this)
+    }
+}
+
 private fun KotlinFacetSettings.writeLatestConfig(element: Element) {
    val filter = SkipDefaultsSerializationFilter()

@@ -177,17 +228,11 @@ private fun KotlinFacetSettings.writeLatestConfig(element: Element) {
    }
    compilerSettings?.let { copyBean(it) }?.let {
        it.convertPathsToSystemIndependent()
-        Element("compilerSettings").apply {
-            XmlSerializer.serializeInto(it, this, filter)
-            element.addContent(this)
-        }
+        buildChildElement(element, "compilerSettings", it, filter)
    }
    compilerArguments?.let { copyBean(it) }?.let {
        it.convertPathsToSystemIndependent()
-        Element("compilerArguments").apply {
-            XmlSerializer.serializeInto(it, this, filter)
-            element.addContent(this)
-        }
+        buildChildElement(element, "compilerArguments", it, filter)
    }
 }

@@ -68,5 +68,6 @@
    <orderEntry type="module" module-name="frontend.script" />
    <orderEntry type="module" module-name="idea-maven" />
    <orderEntry type="module" module-name="backend.jvm" />
+    <orderEntry type="library" name="kotlin-reflect" level="project" />
  </component>
 </module>
@@ -31,11 +31,11 @@ import org.jetbrains.kotlin.config.TargetPlatformKind
 import org.jetbrains.kotlin.config.createCompilerArguments
 import org.jetbrains.kotlin.config.splitArgumentString
 import org.jetbrains.kotlin.idea.compiler.configuration.*
-import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
 import java.awt.BorderLayout
 import javax.swing.*
 import javax.swing.border.EmptyBorder
 import javax.swing.event.DocumentEvent
+import kotlin.reflect.full.findAnnotation

 class KotlinFacetEditorGeneralTab(
        private val configuration: KotlinFacetConfiguration,
@@ -172,14 +172,15 @@ class KotlinFacetEditorGeneralTab(
                is TargetPlatformKind.JavaScript -> jsUIExposedFields
                else -> commonUIExposedFields
            }
-            val fieldsToCheck = collectFieldsToCopy(argumentClass, false).filter { it.name in fieldNamesToCheck }
+
+            val propertiesToCheck = collectProperties(argumentClass.kotlin, false).filter { it.name in fieldNamesToCheck }
            val overridingArguments = ArrayList<String>()
            val redundantArguments = ArrayList<String>()
-            for (field in fieldsToCheck) {
-                val additionalValue = field[additionalArguments]
-                if (additionalValue != field[emptyArguments]) {
-                    val argumentInfo = field.annotations.firstIsInstanceOrNull<Argument>() ?: continue
-                    val addTo = if (additionalValue != field[primaryArguments]) overridingArguments else redundantArguments
+            for (property in propertiesToCheck) {
+                val additionalValue = property.get(additionalArguments)
+                if (additionalValue != property.get(emptyArguments)) {
+                    val argumentInfo = property.findAnnotation<Argument>() ?: continue
+                    val addTo = if (additionalValue != property.get(primaryArguments)) overridingArguments else redundantArguments
                    addTo += "<strong>" + argumentInfo.value.first() + "</strong>"
                }
            }
@@ -35,7 +35,7 @@ import org.jetbrains.kotlin.idea.compiler.configuration.KotlinCommonCompilerArgu
 import org.jetbrains.kotlin.idea.compiler.configuration.KotlinCompilerSettings
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.idea.versions.*
-import java.lang.reflect.Field
+import kotlin.reflect.KProperty1

 private fun getDefaultTargetPlatform(module: Module, rootModel: ModuleRootModel?): TargetPlatformKind<*> {
    if (getRuntimeLibraryVersions(module, rootModel, TargetPlatformKind.JavaScript).isNotEmpty()) {
@@ -221,7 +221,8 @@ fun parseCompilerArgumentsToFacet(
        val primaryFields = compilerArguments.primaryFields
        val ignoredFields = compilerArguments.ignoredFields

-        fun exposeAsAdditionalArgument(field: Field) = field.name !in primaryFields && field.get(compilerArguments) != field.get(defaultCompilerArguments)
+        fun exposeAsAdditionalArgument(property: KProperty1<CommonCompilerArguments, Any?>) =
+                property.name !in primaryFields && property.get(compilerArguments) != property.get(defaultCompilerArguments)

        val additionalArgumentsString = with(compilerArguments::class.java.newInstance()) {
            copyFieldsSatisfying(compilerArguments, this) { exposeAsAdditionalArgument(it) && it.name !in ignoredFields }