From 3226093fa6a75fc3086df488087a6848eb804d1b Mon Sep 17 00:00:00 2001 From: Evgeny Gerashchenko Date: Wed, 27 Aug 2014 18:15:05 +0400 Subject: [PATCH] Fixed redeclaration from incremental compilation. Added workaround for JPS bug. --- .../compiler/KotlinToJVMBytecodeCompiler.java | 34 ++++++++++++++++--- .../testData/cli/jvm/duplicateSource.args | 2 ++ compiler/testData/cli/jvm/duplicateSource.kt | 0 compiler/testData/cli/jvm/duplicateSource.out | 2 ++ compiler/testData/cli/jvm/duplicateSource.xml | 6 ++++ .../org/jetbrains/jet/cli/CliCommonTest.java | 5 +++ .../cli/KotlincExecutableTestGenerated.java | 5 +++ .../jps/build/KotlinSourceFileCollector.java | 5 +++ .../build/IncrementalJpsTestGenerated.java | 22 +++++++++++- .../circularDependency/simple/build.log | 15 ++++++++ .../simple/dependencies.txt | 2 ++ .../circularDependency/simple/module1_a.kt | 9 +++++ .../circularDependency/simple/module2_b.kt | 7 ++++ .../simple/module2_b.kt.new | 11 ++++++ 14 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 compiler/testData/cli/jvm/duplicateSource.args create mode 100644 compiler/testData/cli/jvm/duplicateSource.kt create mode 100644 compiler/testData/cli/jvm/duplicateSource.out create mode 100644 compiler/testData/cli/jvm/duplicateSource.xml create mode 100644 jps-plugin/testData/incremental/circularDependency/simple/build.log create mode 100644 jps-plugin/testData/incremental/circularDependency/simple/dependencies.txt create mode 100644 jps-plugin/testData/incremental/circularDependency/simple/module1_a.kt create mode 100644 jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt create mode 100644 jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt.new diff --git a/compiler/cli/src/org/jetbrains/jet/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.java b/compiler/cli/src/org/jetbrains/jet/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.java index 52d9a8c177e..2832ada89d5 100644 --- a/compiler/cli/src/org/jetbrains/jet/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.java +++ b/compiler/cli/src/org/jetbrains/jet/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.java @@ -19,6 +19,7 @@ package org.jetbrains.jet.cli.jvm.compiler; import com.google.common.base.Predicates; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.intellij.openapi.Disposable; import com.intellij.openapi.util.Disposer; import com.intellij.psi.PsiFile; @@ -36,6 +37,8 @@ import org.jetbrains.jet.cli.common.CLIConfigurationKeys; import org.jetbrains.jet.cli.common.CompilerPlugin; import org.jetbrains.jet.cli.common.CompilerPluginContext; import org.jetbrains.jet.cli.common.messages.AnalyzerWithCompilerReport; +import org.jetbrains.jet.cli.common.messages.CompilerMessageLocation; +import org.jetbrains.jet.cli.common.messages.CompilerMessageSeverity; import org.jetbrains.jet.cli.common.messages.MessageCollector; import org.jetbrains.jet.cli.jvm.JVMConfigurationKeys; import org.jetbrains.jet.codegen.*; @@ -51,8 +54,8 @@ import org.jetbrains.jet.lang.resolve.AnalyzerScriptParameter; import org.jetbrains.jet.lang.resolve.BindingTrace; import org.jetbrains.jet.lang.resolve.BindingTraceContext; import org.jetbrains.jet.lang.resolve.ScriptNameUtil; -import org.jetbrains.jet.lang.resolve.java.TopDownAnalyzerFacadeForJVM; import org.jetbrains.jet.lang.resolve.java.PackageClassUtils; +import org.jetbrains.jet.lang.resolve.java.TopDownAnalyzerFacadeForJVM; import org.jetbrains.jet.lang.resolve.kotlin.incremental.IncrementalPackage; import org.jetbrains.jet.lang.resolve.kotlin.incremental.cache.IncrementalCache; import org.jetbrains.jet.lang.resolve.kotlin.incremental.cache.IncrementalCacheProvider; @@ -63,9 +66,7 @@ import org.jetbrains.jet.utils.KotlinPaths; import java.io.File; import java.net.URL; import java.net.URLClassLoader; -import java.util.Collection; -import java.util.List; -import java.util.Map; +import java.util.*; public class KotlinToJVMBytecodeCompiler { @@ -159,8 +160,11 @@ public class KotlinToJVMBytecodeCompiler { @NotNull File directory ) { CompilerConfiguration configuration = base.copy(); + + List sourceRoots = Lists.newArrayList(); + for (Module module : chunk) { - configuration.addAll(CommonConfigurationKeys.SOURCE_ROOTS_KEY, getAbsolutePaths(directory, module)); + sourceRoots.addAll(getAbsolutePaths(directory, module)); for (String classpathRoot : module.getClasspathRoots()) { configuration.add(JVMConfigurationKeys.CLASSPATH_KEY, new File(classpathRoot)); @@ -173,9 +177,29 @@ public class KotlinToJVMBytecodeCompiler { configuration.add(JVMConfigurationKeys.MODULE_IDS, module.getModuleName()); } + MessageCollector messageCollector = configuration.get(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY); + assert messageCollector != null : "messageCollector should be set: " + base; + configuration.put(CommonConfigurationKeys.SOURCE_ROOTS_KEY, checkForDuplicatePaths(messageCollector, sourceRoots)); + return configuration; } + private static List checkForDuplicatePaths(@NotNull MessageCollector messageCollector, @NotNull List sourceRoots) { + Set uniqueSourceRoots = Sets.newLinkedHashSet(); + + for (String sourceRoot : sourceRoots) { + if (!uniqueSourceRoots.add(sourceRoot)) { + messageCollector.report( + CompilerMessageSeverity.WARNING, + "Duplicate source roots: " + sourceRoot, + CompilerMessageLocation.NO_LOCATION + ); + } + } + + return new ArrayList(uniqueSourceRoots); + } + @Nullable private static FqName findMainClass(@NotNull GenerationState generationState, @NotNull List files) { MainFunctionDetector mainFunctionDetector = new MainFunctionDetector(generationState.getBindingContext()); diff --git a/compiler/testData/cli/jvm/duplicateSource.args b/compiler/testData/cli/jvm/duplicateSource.args new file mode 100644 index 00000000000..5d876a0701e --- /dev/null +++ b/compiler/testData/cli/jvm/duplicateSource.args @@ -0,0 +1,2 @@ +-module +$TESTDATA_DIR$/duplicateSource.xml diff --git a/compiler/testData/cli/jvm/duplicateSource.kt b/compiler/testData/cli/jvm/duplicateSource.kt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/compiler/testData/cli/jvm/duplicateSource.out b/compiler/testData/cli/jvm/duplicateSource.out new file mode 100644 index 00000000000..6c63ae17dc9 --- /dev/null +++ b/compiler/testData/cli/jvm/duplicateSource.out @@ -0,0 +1,2 @@ +WARNING: Duplicate source roots: $TESTDATA_DIR$/duplicateSource.kt +OK \ No newline at end of file diff --git a/compiler/testData/cli/jvm/duplicateSource.xml b/compiler/testData/cli/jvm/duplicateSource.xml new file mode 100644 index 00000000000..0f352053623 --- /dev/null +++ b/compiler/testData/cli/jvm/duplicateSource.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/cli/CliCommonTest.java b/compiler/tests/org/jetbrains/jet/cli/CliCommonTest.java index 20999a6f6a5..7d3f939cfbc 100644 --- a/compiler/tests/org/jetbrains/jet/cli/CliCommonTest.java +++ b/compiler/tests/org/jetbrains/jet/cli/CliCommonTest.java @@ -41,6 +41,11 @@ public class CliCommonTest extends CliBaseTest { Assert.assertTrue(new File(tmpdir.getTmpDir(), PackageClassUtils.getPackageClassName(FqName.ROOT) + ".class").isFile()); } + @Test + public void duplicateSource() throws Exception { + executeCompilerCompareOutputJVM(); + } + @Test public void diagnosticsOrder() throws Exception { executeCompilerCompareOutputJVM(); diff --git a/compiler/tests/org/jetbrains/jet/cli/KotlincExecutableTestGenerated.java b/compiler/tests/org/jetbrains/jet/cli/KotlincExecutableTestGenerated.java index 205361af5bb..82f931f19f1 100644 --- a/compiler/tests/org/jetbrains/jet/cli/KotlincExecutableTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/cli/KotlincExecutableTestGenerated.java @@ -51,6 +51,11 @@ public class KotlincExecutableTestGenerated extends AbstractKotlincExecutableTes doJvmTest("compiler/testData/cli/jvm/diagnosticsOrder.args"); } + @TestMetadata("duplicateSource.args") + public void testDuplicateSource() throws Exception { + doJvmTest("compiler/testData/cli/jvm/duplicateSource.args"); + } + @TestMetadata("extraHelp.args") public void testExtraHelp() throws Exception { doJvmTest("compiler/testData/cli/jvm/extraHelp.args"); diff --git a/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinSourceFileCollector.java b/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinSourceFileCollector.java index c1e77ab0d3d..402086b3e03 100644 --- a/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinSourceFileCollector.java +++ b/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinSourceFileCollector.java @@ -49,6 +49,11 @@ public class KotlinSourceFileCollector { dirtyFilesHolder.processDirtyFiles(new FileProcessor() { @Override public boolean apply(ModuleBuildTarget target, File file, JavaSourceRootDescriptor root) throws IOException { + //TODO this is a workaround for bug in JPS: the latter erroneously calls process with invalid parameters + if (!root.getTarget().equals(target)) { + return true; + } + if (isKotlinSourceFile(file)) { result.putValue(target, file); } diff --git a/jps-plugin/test/org/jetbrains/jet/jps/build/IncrementalJpsTestGenerated.java b/jps-plugin/test/org/jetbrains/jet/jps/build/IncrementalJpsTestGenerated.java index 58756f3e9ce..d1f2f8d6972 100644 --- a/jps-plugin/test/org/jetbrains/jet/jps/build/IncrementalJpsTestGenerated.java +++ b/jps-plugin/test/org/jetbrains/jet/jps/build/IncrementalJpsTestGenerated.java @@ -28,12 +28,31 @@ import java.util.regex.Pattern; /** This class is generated by {@link org.jetbrains.jet.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */ @SuppressWarnings("all") @TestMetadata("jps-plugin/testData/incremental") -@InnerTestClasses({IncrementalJpsTestGenerated.PureKotlin.class}) +@InnerTestClasses({IncrementalJpsTestGenerated.CircularDependency.class, IncrementalJpsTestGenerated.PureKotlin.class}) public class IncrementalJpsTestGenerated extends AbstractIncrementalJpsTest { public void testAllFilesPresentInIncremental() throws Exception { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("jps-plugin/testData/incremental"), Pattern.compile("^([^\\.]+)$"), true); } + @TestMetadata("jps-plugin/testData/incremental/circularDependency") + @InnerTestClasses({}) + public static class CircularDependency extends AbstractIncrementalJpsTest { + public void testAllFilesPresentInCircularDependency() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("jps-plugin/testData/incremental/circularDependency"), Pattern.compile("^([^\\.]+)$"), true); + } + + @TestMetadata("simple") + public void testSimple() throws Exception { + doTest("jps-plugin/testData/incremental/circularDependency/simple/"); + } + + public static Test innerSuite() { + TestSuite suite = new TestSuite("CircularDependency"); + suite.addTestSuite(CircularDependency.class); + return suite; + } + } + @TestMetadata("jps-plugin/testData/incremental/pureKotlin") @InnerTestClasses({}) public static class PureKotlin extends AbstractIncrementalJpsTest { @@ -231,6 +250,7 @@ public class IncrementalJpsTestGenerated extends AbstractIncrementalJpsTest { public static Test suite() { TestSuite suite = new TestSuite("IncrementalJpsTestGenerated"); suite.addTestSuite(IncrementalJpsTestGenerated.class); + suite.addTest(CircularDependency.innerSuite()); suite.addTest(PureKotlin.innerSuite()); return suite; } diff --git a/jps-plugin/testData/incremental/circularDependency/simple/build.log b/jps-plugin/testData/incremental/circularDependency/simple/build.log new file mode 100644 index 00000000000..ee01130e93f --- /dev/null +++ b/jps-plugin/testData/incremental/circularDependency/simple/build.log @@ -0,0 +1,15 @@ +Cleaning output files: +out/production/module2/b/BPackage-module2_b-*.class +out/production/module2/b/BPackage.class +End of files +Compiling files: +module2/src/module2_b.kt +End of files +Cleaning output files: +out/production/module1/a/A.class +out/production/module1/a/APackage-module1_a-*.class +out/production/module1/a/APackage.class +End of files +Compiling files: +module1/src/module1_a.kt +End of files \ No newline at end of file diff --git a/jps-plugin/testData/incremental/circularDependency/simple/dependencies.txt b/jps-plugin/testData/incremental/circularDependency/simple/dependencies.txt new file mode 100644 index 00000000000..02d5c8ca1a7 --- /dev/null +++ b/jps-plugin/testData/incremental/circularDependency/simple/dependencies.txt @@ -0,0 +1,2 @@ +module1->module2 +module2->module1 diff --git a/jps-plugin/testData/incremental/circularDependency/simple/module1_a.kt b/jps-plugin/testData/incremental/circularDependency/simple/module1_a.kt new file mode 100644 index 00000000000..5f77590e624 --- /dev/null +++ b/jps-plugin/testData/incremental/circularDependency/simple/module1_a.kt @@ -0,0 +1,9 @@ +package a + +class A + +fun a() { + // TODO: this call is compiled via package facade or package part depending on if callee comes from compiled or source + // Must be uncommented when modules are supported in compiler + //b.b() +} diff --git a/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt b/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt new file mode 100644 index 00000000000..164222697b7 --- /dev/null +++ b/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt @@ -0,0 +1,7 @@ +package b + +fun b() { + // TODO: this call is compiled via package facade or package part depending on if callee comes from compiled or source + // Must be uncommented when modules are supported in compiler + // a.a() +} diff --git a/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt.new b/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt.new new file mode 100644 index 00000000000..b6a3f3fd029 --- /dev/null +++ b/jps-plugin/testData/incremental/circularDependency/simple/module2_b.kt.new @@ -0,0 +1,11 @@ +package b + +fun b() { + // TODO: this call is compiled via package facade or package part depending on if callee comes from compiled or source + // Must be uncommented when modules are supported in compiler + // a.a() +} + +fun bb() { + +}