From e2273a3d8557963a77c3270d926d4199385d6646 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 24 Sep 2014 19:27:31 +0400 Subject: [PATCH] Update JetExceptionFilter behavior for top level closures Now JetExceptionFilter can determine the source file looking at the class FQ name for sure --- .../jet/codegen/binding/CodegenBinding.java | 2 +- .../codegen/binding/PsiCodegenPredictor.java | 47 ----------------- .../plugin/filters/JetExceptionFilter.java | 8 ++- .../jet/plugin/util/DebuggerUtils.java | 38 ++++++-------- .../filters/JetExceptionFilterTest.java | 52 +++++++++++++++---- 5 files changed, 62 insertions(+), 85 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/binding/CodegenBinding.java b/compiler/backend/src/org/jetbrains/jet/codegen/binding/CodegenBinding.java index 06279cf32a9..e0b748a5378 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/binding/CodegenBinding.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/binding/CodegenBinding.java @@ -52,7 +52,7 @@ public class CodegenBinding { public static final WritableSlice CLASS_FOR_SCRIPT = Slices.createSimpleSlice(); - public static final WritableSlice ASM_TYPE = Slices.createCollectiveSlice(); + public static final WritableSlice ASM_TYPE = Slices.createSimpleSlice(); public static final WritableSlice ENUM_ENTRY_CLASS_NEED_SUBCLASS = Slices.createSimpleSetSlice(); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/binding/PsiCodegenPredictor.java b/compiler/backend/src/org/jetbrains/jet/codegen/binding/PsiCodegenPredictor.java index e38a1e76ad2..8043fc8ac92 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/binding/PsiCodegenPredictor.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/binding/PsiCodegenPredictor.java @@ -16,30 +16,18 @@ package org.jetbrains.jet.codegen.binding; -import com.intellij.openapi.project.Project; import com.intellij.psi.PsiElement; import com.intellij.psi.util.PsiTreeUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.codegen.AsmUtil; -import org.jetbrains.jet.codegen.ClassBuilderFactories; -import org.jetbrains.jet.codegen.state.GenerationState; -import org.jetbrains.jet.lang.descriptors.ClassDescriptor; import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.descriptors.ModuleDescriptor; import org.jetbrains.jet.lang.psi.*; -import org.jetbrains.jet.lang.resolve.BindingContext; -import org.jetbrains.jet.lang.resolve.BindingTrace; -import org.jetbrains.jet.lang.resolve.DescriptorToSourceUtils; import org.jetbrains.jet.lang.resolve.java.JvmAbi; -import org.jetbrains.jet.lang.resolve.java.JvmClassName; import org.jetbrains.jet.lang.resolve.kotlin.PackagePartClassUtils; import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.org.objectweb.asm.Type; -import java.util.ArrayList; -import java.util.Collection; - import static org.jetbrains.jet.lang.resolve.DescriptorToSourceUtils.descriptorToDeclaration; public final class PsiCodegenPredictor { @@ -126,39 +114,4 @@ public final class PsiCodegenPredictor { return parentInternalName + (parentDeclaration == null ? "/" : "$") + name.asString(); } - - @Nullable - public static JetFile getFileForPackagePartName(@NotNull Collection allPackageFiles, @NotNull JvmClassName className) { - for (JetFile file : allPackageFiles) { - String internalName = PackagePartClassUtils.getPackagePartInternalName(file); - JvmClassName jvmClassName = JvmClassName.byInternalName(internalName); - if (jvmClassName.equals(className)) { - return file; - } - } - return null; - } - - @Nullable - public static JetFile getFileForCodegenNamedClass( - @NotNull ModuleDescriptor module, - @NotNull BindingContext context, - @NotNull Collection allPackageFiles, - @NotNull String classInternalName - ) { - Project project = allPackageFiles.iterator().next().getProject(); - GenerationState state = new GenerationState(project, ClassBuilderFactories.THROW_EXCEPTION, module, context, - new ArrayList(allPackageFiles)); - state.beforeCompile(); - - BindingTrace trace = state.getBindingTrace(); - for (ClassDescriptor classDescriptor : trace.getKeys(CodegenBinding.ASM_TYPE)) { - Type type = trace.get(CodegenBinding.ASM_TYPE, classDescriptor); - if (type != null && classInternalName.equals(type.getInternalName())) { - return DescriptorToSourceUtils.getContainingFile(classDescriptor); - } - } - - return null; - } } diff --git a/idea/src/org/jetbrains/jet/plugin/filters/JetExceptionFilter.java b/idea/src/org/jetbrains/jet/plugin/filters/JetExceptionFilter.java index d104579431b..7c7c332571c 100644 --- a/idea/src/org/jetbrains/jet/plugin/filters/JetExceptionFilter.java +++ b/idea/src/org/jetbrains/jet/plugin/filters/JetExceptionFilter.java @@ -58,10 +58,8 @@ public class JetExceptionFilter implements Filter { // fullyQualifiedName is of format "package.Class$Inner" String fullyQualifiedName = element.getClassName(); - // All classes except package classes and package parts are handled correctly in the default ExceptionFilter - if (!isPackageClassOrSubClass(fullyQualifiedName)) { - return null; - } + // All classes except package classes, package parts and top level closures are handled correctly in the default ExceptionFilter + if (!isPackageClassOrPackagePartPrefixedName(fullyQualifiedName)) return null; String internalName = fullyQualifiedName.replace('.', '/'); JvmClassName jvmClassName = JvmClassName.byInternalName(internalName); @@ -75,7 +73,7 @@ public class JetExceptionFilter implements Filter { return new OpenFileHyperlinkInfo(project, virtualFile, element.getLineNumber() - 1); } - private static boolean isPackageClassOrSubClass(String fqName) { + private static boolean isPackageClassOrPackagePartPrefixedName(String fqName) { if (fqName.equals(PackageClassUtils.getPackageClassName(FqName.ROOT))) { return true; } diff --git a/idea/src/org/jetbrains/jet/plugin/util/DebuggerUtils.java b/idea/src/org/jetbrains/jet/plugin/util/DebuggerUtils.java index bdbb0536141..e4283be72ae 100644 --- a/idea/src/org/jetbrains/jet/plugin/util/DebuggerUtils.java +++ b/idea/src/org/jetbrains/jet/plugin/util/DebuggerUtils.java @@ -26,18 +26,14 @@ import kotlin.Function1; import kotlin.KotlinPackage; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.jet.analyzer.AnalyzeExhaust; -import org.jetbrains.jet.codegen.binding.PsiCodegenPredictor; import org.jetbrains.jet.lang.psi.JetElement; import org.jetbrains.jet.lang.psi.JetFile; import org.jetbrains.jet.lang.resolve.java.JvmClassName; -import org.jetbrains.jet.lang.resolve.name.FqName; -import org.jetbrains.jet.plugin.caches.resolve.ResolvePackage; +import org.jetbrains.jet.lang.resolve.kotlin.PackagePartClassUtils; import org.jetbrains.jet.plugin.codeInsight.CodeInsightUtils; import org.jetbrains.jet.plugin.debugger.DebuggerPackage; import java.util.Collection; -import java.util.List; import static org.jetbrains.jet.plugin.stubindex.PackageIndexUtil.findFilesWithExactPackage; @@ -53,10 +49,7 @@ public class DebuggerUtils { @NotNull final String fileName, final int lineNumber ) { - - FqName packageFqName = getPackageFqNameForClass(className); - - Collection filesInPackage = findFilesWithExactPackage(packageFqName, searchScope, project); + Collection filesInPackage = findFilesWithExactPackage(className.getPackageFqName(), searchScope, project); Collection filesWithExactName = Collections2.filter(filesInPackage, new Predicate() { @Override public boolean apply(@Nullable JetFile file) { @@ -70,7 +63,7 @@ public class DebuggerUtils { return filesWithExactName.iterator().next(); } - JetFile file = PsiCodegenPredictor.getFileForPackagePartName(filesWithExactName, className); + JetFile file = getFileForPackagePartPrefixedName(filesWithExactName, className.getInternalName()); if (file != null) { return file; } @@ -95,19 +88,20 @@ public class DebuggerUtils { })); } - // In the rare case that there's more than one file with this name in this package, - // we may actually need to analyze the project in order to find a file which produces this class - // TODO: this code is not entirely correct, because it takes a session for only one file - AnalyzeExhaust analyzeExhaust = ResolvePackage.getAnalysisResultsForElements(filesWithExactName); - - return PsiCodegenPredictor.getFileForCodegenNamedClass(analyzeExhaust.getModuleDescriptor(), analyzeExhaust.getBindingContext(), - filesWithExactName, className.getInternalName()); + return null; } - @NotNull - private static FqName getPackageFqNameForClass(@NotNull JvmClassName className) { - String internalName = className.getInternalName(); - int lastSlash = internalName.lastIndexOf('/'); - return lastSlash == -1 ? FqName.ROOT : new FqName(internalName.substring(0, lastSlash).replace('/', '.')); + @Nullable + private static JetFile getFileForPackagePartPrefixedName( + @NotNull Collection allPackageFiles, + @NotNull String classInternalName + ) { + for (JetFile file : allPackageFiles) { + String packagePartInternalName = PackagePartClassUtils.getPackagePartInternalName(file); + if (classInternalName.startsWith(packagePartInternalName)) { + return file; + } + } + return null; } } diff --git a/idea/tests/org/jetbrains/jet/plugin/filters/JetExceptionFilterTest.java b/idea/tests/org/jetbrains/jet/plugin/filters/JetExceptionFilterTest.java index 8c5a5eab0c3..a26f0ee0078 100644 --- a/idea/tests/org/jetbrains/jet/plugin/filters/JetExceptionFilterTest.java +++ b/idea/tests/org/jetbrains/jet/plugin/filters/JetExceptionFilterTest.java @@ -28,14 +28,18 @@ import com.intellij.psi.PsiDocumentManager; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.refactoring.MultiFileTestCase; import com.intellij.testFramework.PsiTestUtil; +import kotlin.Function1; import org.jetbrains.annotations.NotNull; -import org.jetbrains.jet.lang.resolve.java.PackageClassUtils; import org.jetbrains.jet.lang.resolve.name.FqName; import org.jetbrains.jet.plugin.PluginTestCaseBase; import org.jetbrains.jet.utils.UtilsPackage; import java.io.File; +import static org.jetbrains.jet.lang.resolve.java.PackageClassUtils.getPackageClassFqName; +import static org.jetbrains.jet.lang.resolve.java.PackageClassUtils.getPackageClassName; +import static org.jetbrains.jet.lang.resolve.kotlin.PackagePartClassUtils.getPackagePartFqName; + public class JetExceptionFilterTest extends MultiFileTestCase { private VirtualFile rootDir; @@ -82,7 +86,7 @@ public class JetExceptionFilterTest extends MultiFileTestCase { return "\tat " + element + "\n"; } - private void doTest(@NotNull String fileName, @NotNull String className, int lineNumber) { + private void doTest(@NotNull String fileName, int lineNumber, @NotNull Function1 className) { if (rootDir == null) { configure(); } @@ -90,7 +94,10 @@ public class JetExceptionFilterTest extends MultiFileTestCase { Filter filter = new JetExceptionFilterFactory().create(GlobalSearchScope.allScope(myProject)); - String line = createStackTraceElementLine(fileName, className, lineNumber); + VirtualFile expectedFile = VfsUtilCore.findRelativeFile(fileName, rootDir); + assertNotNull(expectedFile); + + String line = createStackTraceElementLine(fileName, className.invoke(expectedFile), lineNumber); Filter.Result result = filter.applyFilter(line, 0); assertNotNull(result); @@ -100,8 +107,6 @@ public class JetExceptionFilterTest extends MultiFileTestCase { OpenFileDescriptor descriptor = ((OpenFileHyperlinkInfo) info).getDescriptor(); assertNotNull(descriptor); - VirtualFile expectedFile = VfsUtilCore.findRelativeFile(fileName, rootDir); - assertNotNull(expectedFile); assertEquals(expectedFile, descriptor.getFile()); Document document = FileDocumentManager.getInstance().getDocument(expectedFile); @@ -111,17 +116,44 @@ public class JetExceptionFilterTest extends MultiFileTestCase { } public void testSimple() { - doTest("simple.kt", PackageClassUtils.getPackageClassName(FqName.ROOT), 2); + doTest("simple.kt", 2, new Function1() { + @Override + public String invoke(VirtualFile file) { + return getPackageClassName(FqName.ROOT); + } + }); } public void testKt2489() { - doTest("a.kt", PackageClassUtils.getPackageClassName(FqName.ROOT) + "$a$f$1", 3); - doTest("main.kt", PackageClassUtils.getPackageClassName(FqName.ROOT) + "$main$f$1", 3); + final FqName packageClassFqName = getPackageClassFqName(FqName.ROOT); + doTest("a.kt", 3, new Function1() { + @Override + public String invoke(VirtualFile file) { + return getPackagePartFqName(packageClassFqName, file) + "$a$f$1"; + } + }); + doTest("main.kt", 3, new Function1() { + @Override + public String invoke(VirtualFile file) { + return getPackagePartFqName(packageClassFqName, file) + "$main$f$1"; + } + }); } public void testMultiSameName() { + final FqName packageClassFqName = getPackageClassFqName(new FqName("multiSameName")); // The order and the exact names do matter here - doTest("1/foo.kt", "multiSameName." + PackageClassUtils.getPackageClassName(new FqName("multiSameName")) + "$foo$f$1", 4); - doTest("2/foo.kt", "multiSameName." + PackageClassUtils.getPackageClassName(new FqName("multiSameName")) + "$foo$f$2", 4); + doTest("1/foo.kt", 4, new Function1() { + @Override + public String invoke(VirtualFile file) { + return getPackagePartFqName(packageClassFqName, file) + "$foo$f$1"; + } + }); + doTest("2/foo.kt", 4, new Function1() { + @Override + public String invoke(VirtualFile file) { + return getPackagePartFqName(packageClassFqName, file) + "$foo$f$1"; + } + }); } }