From bb92655ecdbbdb07d351a9b7b37627df00df3daa Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 11 Sep 2012 16:03:33 +0400 Subject: [PATCH] Prohibit creation of JetLightClass on built-ins If we allow JetLightClass to be created on built-in types, a normal codegen would be launched, with a mapping from Kotlin to Java types enabled, every time IDE requests built-in classes. Codegen would then try to make something meaningless, e.g. create a class for a primitive type (since our jet.Boolean is mapped to primitive boolean). This would result in different exceptions when navigating to built-in library from IDE. Add a helpful error message to ClassFileFactory if we ever again produce classes for primitive types. --- .../jet/codegen/ClassFileFactory.java | 8 +++-- .../lang/types/lang/JetStandardClasses.java | 9 ++++++ .../jet/asJava/JavaElementFinder.java | 22 +++++++++++--- .../jetbrains/jet/asJava/JetLightClass.java | 29 +++++++++++++++---- .../highlighter/JetLineMarkerProvider.java | 13 +++++++-- .../run/JetJUnitConfigurationProducer.java | 29 ++++++++++--------- .../search/KotlinDefinitionsSearcher.java | 14 +++++---- .../KotlinDirectInheritorsSearcher.java | 5 +++- .../search/KotlinReferencesSearcher.java | 12 +++++--- .../lightClassIsNotCreatedForBuiltins.kt | 3 ++ .../plugin/javaFacade/JetJavaFacadeTest.java | 24 +++++++++++++++ 11 files changed, 129 insertions(+), 39 deletions(-) create mode 100644 idea/testData/javaFacade/lightClassIsNotCreatedForBuiltins.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ClassFileFactory.java b/compiler/backend/src/org/jetbrains/jet/codegen/ClassFileFactory.java index 4cb61dcbff9..820959a410e 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ClassFileFactory.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ClassFileFactory.java @@ -17,6 +17,7 @@ package org.jetbrains.jet.codegen; import org.jetbrains.annotations.NotNull; +import org.jetbrains.asm4.Type; import org.jetbrains.jet.codegen.state.GenerationState; import org.jetbrains.jet.codegen.state.GenerationStateAware; import org.jetbrains.jet.codegen.state.JetTypeMapperMode; @@ -112,8 +113,11 @@ public final class ClassFileFactory extends GenerationStateAware { } public ClassBuilder forClassImplementation(ClassDescriptor aClass) { - return newVisitor( - state.getTypeMapper().mapType(aClass.getDefaultType(), JetTypeMapperMode.IMPL).getInternalName() + ".class"); + Type type = state.getTypeMapper().mapType(aClass.getDefaultType(), JetTypeMapperMode.IMPL); + if (CodegenUtil.isPrimitive(type)) { + throw new IllegalStateException("Codegen for primitive type is not possible: " + aClass); + } + return newVisitor(type.getInternalName() + ".class"); } public ClassBuilder forNamespacepart(String name) { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/lang/JetStandardClasses.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/lang/JetStandardClasses.java index d5c2debf82e..e7ff6f56e0c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/lang/JetStandardClasses.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/lang/JetStandardClasses.java @@ -542,4 +542,13 @@ public class JetStandardClasses { public static Collection getAllStandardClasses() { return STANDARD_CLASSES.getAllDescriptors(); } + + public static boolean isStandardClass(@NotNull FqName fqName) { + for (DeclarationDescriptor descriptor : getAllStandardClasses()) { + if (DescriptorUtils.getFQName(descriptor).equals(fqName.toUnsafe())) { + return true; + } + } + return false; + } } diff --git a/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JavaElementFinder.java b/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JavaElementFinder.java index 07b3adc0e98..d678a28e9d1 100644 --- a/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JavaElementFinder.java +++ b/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JavaElementFinder.java @@ -118,7 +118,10 @@ public class JavaElementFinder extends PsiElementFinder implements JavaPsiFacade if (packageName != null && qualifiedName.getFqName().startsWith(packageName.getFqName())) { if (qualifiedName.equals(QualifiedNamesUtil.combine(packageName, Name.identifier(JvmAbi.PACKAGE_CLASS))) && NamespaceCodegen.shouldGenerateNSClass(Arrays.asList(file))) { - answer.add(new JetLightClass(psiManager, file, qualifiedName)); + JetLightClass lightClass = JetLightClass.create(psiManager, file, qualifiedName); + if (lightClass != null) { + answer.add(lightClass); + } } else { for (JetDeclaration declaration : file.getDeclarations()) { @@ -137,7 +140,10 @@ public class JavaElementFinder extends PsiElementFinder implements JavaPsiFacade FqName fqn = QualifiedNamesUtil.combine(containerFqn, Name.identifier(localName)); if (qualifiedName.equals(fqn)) { if (!(declaration instanceof JetEnumEntry)) { - answer.add(new JetLightClass(psiManager, file, qualifiedName)); + JetLightClass lightClass = JetLightClass.create(psiManager, file, qualifiedName); + if (lightClass != null) { + answer.add(lightClass); + } } } else if (QualifiedNamesUtil.isSubpackageOf(qualifiedName, fqn)) { @@ -231,12 +237,20 @@ public class JavaElementFinder extends PsiElementFinder implements JavaPsiFacade FqName packageFQN = new FqName(psiPackage.getQualifiedName()); for (JetFile file : filesInScope) { if (packageFQN.equals(JetPsiUtil.getFQName(file))) { - answer.add(new JetLightClass(psiManager, file, QualifiedNamesUtil.combine(packageFQN, Name.identifier(JvmAbi.PACKAGE_CLASS)))); + JetLightClass lightClass = JetLightClass + .create(psiManager, file, QualifiedNamesUtil.combine(packageFQN, Name.identifier(JvmAbi.PACKAGE_CLASS))); + if (lightClass != null) { + answer.add(lightClass); + } for (JetDeclaration declaration : file.getDeclarations()) { if (declaration instanceof JetClassOrObject) { String localName = getLocalName(declaration); if (localName != null) { - answer.add(new JetLightClass(psiManager, file, QualifiedNamesUtil.combine(packageFQN, Name.identifier(localName)))); + JetLightClass aClass = JetLightClass.create(psiManager, file, + QualifiedNamesUtil.combine(packageFQN, Name.identifier(localName))); + if (aClass != null) { + answer.add(aClass); + } } } } diff --git a/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JetLightClass.java b/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JetLightClass.java index c822decdca1..6be528e7de9 100644 --- a/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JetLightClass.java +++ b/compiler/jet.as.java.psi/src/org/jetbrains/jet/asJava/JetLightClass.java @@ -21,7 +21,6 @@ package org.jetbrains.jet.asJava; import com.intellij.navigation.ItemPresentation; import com.intellij.navigation.ItemPresentationProviders; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Key; @@ -36,8 +35,12 @@ import com.intellij.psi.stubs.StubElement; import com.intellij.psi.util.*; import com.intellij.util.containers.Stack; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.analyzer.AnalyzeExhaust; -import org.jetbrains.jet.codegen.*; +import org.jetbrains.jet.codegen.ClassBuilder; +import org.jetbrains.jet.codegen.ClassBuilderFactory; +import org.jetbrains.jet.codegen.ClassBuilderMode; +import org.jetbrains.jet.codegen.CompilationErrorHandler; import org.jetbrains.jet.codegen.state.GenerationState; import org.jetbrains.jet.codegen.state.GenerationStrategy; import org.jetbrains.jet.lang.BuiltinsScopeExtensionMode; @@ -49,6 +52,7 @@ import org.jetbrains.jet.lang.resolve.java.AnalyzerFacadeForJVM; import org.jetbrains.jet.lang.resolve.java.JetFilesProvider; import org.jetbrains.jet.lang.resolve.java.JetJavaMirrorMarker; import org.jetbrains.jet.lang.resolve.name.FqName; +import org.jetbrains.jet.lang.types.lang.JetStandardClasses; import org.jetbrains.jet.plugin.JetLanguage; import javax.swing.*; @@ -61,20 +65,28 @@ public class JetLightClass extends AbstractLightClass implements JetJavaMirrorMa private final FqName qualifiedName; private PsiClass delegate; - public JetLightClass(PsiManager manager, JetFile file, FqName qualifiedName) { + private JetLightClass(PsiManager manager, JetFile file, FqName qualifiedName) { super(manager, JetLanguage.INSTANCE); this.file = file; this.qualifiedName = qualifiedName; } + @Nullable + public static JetLightClass create(PsiManager manager, JetFile file, FqName qualifiedName) { + return JetStandardClasses.isStandardClass(qualifiedName) ? null : new JetLightClass(manager, file, qualifiedName); + } + @Override public String getName() { return qualifiedName.shortName().getName(); } + @NotNull @Override public PsiElement copy() { - return new JetLightClass(getManager(), file, qualifiedName); + JetLightClass result = create(getManager(), file, qualifiedName); + assert result != null; + return result; } @NotNull @@ -233,13 +245,20 @@ public class JetLightClass extends AbstractLightClass implements JetJavaMirrorMa return true; } + @Nullable public static JetLightClass wrapDelegate(JetClass jetClass) { - return new JetLightClass(jetClass.getManager(), (JetFile) jetClass.getContainingFile(), JetPsiUtil.getFQName(jetClass)); + if (jetClass == null) return null; + return create(jetClass.getManager(), (JetFile) jetClass.getContainingFile(), JetPsiUtil.getFQName(jetClass)); } + @Nullable public static PsiMethod wrapMethod(JetFunction function) { JetClass containingClass = PsiTreeUtil.getParentOfType(function, JetClass.class); JetLightClass wrapper = wrapDelegate(containingClass); + if (wrapper == null) { + return null; + } + for (PsiMethod method : wrapper.getMethods()) { if (method instanceof PsiCompiledElement && ((PsiCompiledElement) method).getMirror() == function) { return method; diff --git a/idea/src/org/jetbrains/jet/plugin/highlighter/JetLineMarkerProvider.java b/idea/src/org/jetbrains/jet/plugin/highlighter/JetLineMarkerProvider.java index 176a4b3a6a5..3270dd41b2c 100644 --- a/idea/src/org/jetbrains/jet/plugin/highlighter/JetLineMarkerProvider.java +++ b/idea/src/org/jetbrains/jet/plugin/highlighter/JetLineMarkerProvider.java @@ -82,8 +82,11 @@ public class JetLineMarkerProvider implements LineMarkerProvider { return null; } } - final PsiClass lightClass = JetLightClass.wrapDelegate((JetClass) element).getDelegate(); - final PsiElement[] children = lightClass.getChildren(); + JetLightClass lightClass = JetLightClass.wrapDelegate((JetClass) element); + if (lightClass == null) { + return null; + } + final PsiElement[] children = lightClass.getDelegate().getChildren(); return children.length > 0 ? children[0] : null; } @@ -251,7 +254,11 @@ public class JetLineMarkerProvider implements LineMarkerProvider { if (!element.hasModifier(JetTokens.OPEN_KEYWORD)) { return; } - PsiClass inheritor = ClassInheritorsSearch.search(JetLightClass.wrapDelegate(element), false).findFirst(); + JetLightClass lightClass = JetLightClass.wrapDelegate(element); + if (lightClass == null) { + return; + } + PsiClass inheritor = ClassInheritorsSearch.search(lightClass, false).findFirst(); if (inheritor != null) { final PsiElement nameIdentifier = element.getNameIdentifier(); PsiElement anchor = nameIdentifier != null ? nameIdentifier : element; diff --git a/idea/src/org/jetbrains/jet/plugin/run/JetJUnitConfigurationProducer.java b/idea/src/org/jetbrains/jet/plugin/run/JetJUnitConfigurationProducer.java index c220a9b3e7f..ae371407b47 100644 --- a/idea/src/org/jetbrains/jet/plugin/run/JetJUnitConfigurationProducer.java +++ b/idea/src/org/jetbrains/jet/plugin/run/JetJUnitConfigurationProducer.java @@ -68,24 +68,25 @@ public class JetJUnitConfigurationProducer extends RuntimeConfigurationProducer if (owner instanceof JetClass) { JetLightClass delegate = JetLightClass.wrapDelegate((JetClass) owner); - for (PsiMethod method : delegate.getMethods()) { - if (method.getNavigationElement() == function) { - Location methodLocation = PsiLocation.fromPsiElement(method); - if (JUnitUtil.isTestMethod(methodLocation, false)) { - RunnerAndConfigurationSettings settings = cloneTemplateConfiguration(context.getProject(), context); - final JUnitConfiguration configuration = (JUnitConfiguration) settings.getConfiguration(); + if (delegate != null) { + for (PsiMethod method : delegate.getMethods()) { + if (method.getNavigationElement() == function) { + Location methodLocation = PsiLocation.fromPsiElement(method); + if (JUnitUtil.isTestMethod(methodLocation, false)) { + RunnerAndConfigurationSettings settings = cloneTemplateConfiguration(context.getProject(), context); + final JUnitConfiguration configuration = (JUnitConfiguration) settings.getConfiguration(); - final Module originalModule = configuration.getConfigurationModule().getModule(); - configuration.beMethodConfiguration(methodLocation); - configuration.restoreOriginalModule(originalModule); - JavaRunConfigurationExtensionManager.getInstance().extendCreatedConfiguration(configuration, location); + final Module originalModule = configuration.getConfigurationModule().getModule(); + configuration.beMethodConfiguration(methodLocation); + configuration.restoreOriginalModule(originalModule); + JavaRunConfigurationExtensionManager.getInstance().extendCreatedConfiguration(configuration, location); - return settings; + return settings; + } + break; } - break; } } - } } @@ -99,7 +100,7 @@ public class JetJUnitConfigurationProducer extends RuntimeConfigurationProducer myElement = jetClass; PsiClass delegate = JetLightClass.wrapDelegate(jetClass); - if (JUnitUtil.isTestClass(delegate)) { + if (delegate != null && JUnitUtil.isTestClass(delegate)) { RunnerAndConfigurationSettings settings = cloneTemplateConfiguration(context.getProject(), context); final JUnitConfiguration configuration = (JUnitConfiguration) settings.getConfiguration(); diff --git a/idea/src/org/jetbrains/jet/plugin/search/KotlinDefinitionsSearcher.java b/idea/src/org/jetbrains/jet/plugin/search/KotlinDefinitionsSearcher.java index 1552c124204..b5cf6f007e2 100644 --- a/idea/src/org/jetbrains/jet/plugin/search/KotlinDefinitionsSearcher.java +++ b/idea/src/org/jetbrains/jet/plugin/search/KotlinDefinitionsSearcher.java @@ -42,12 +42,14 @@ public class KotlinDefinitionsSearcher extends QueryExecutorBase query = ClassInheritorsSearch.search(psiClass, true); - query.forEach(new Processor() { - @Override - public boolean process(final PsiClass clazz) { - return consumer.process(clazz); - } - }); + if (psiClass != null) { + query.forEach(new Processor() { + @Override + public boolean process(final PsiClass clazz) { + return consumer.process(clazz); + } + }); + } } } } diff --git a/idea/src/org/jetbrains/jet/plugin/search/KotlinDirectInheritorsSearcher.java b/idea/src/org/jetbrains/jet/plugin/search/KotlinDirectInheritorsSearcher.java index 1db94c8b4db..b90505f4c47 100644 --- a/idea/src/org/jetbrains/jet/plugin/search/KotlinDirectInheritorsSearcher.java +++ b/idea/src/org/jetbrains/jet/plugin/search/KotlinDirectInheritorsSearcher.java @@ -82,7 +82,10 @@ public class KotlinDirectInheritorsSearcher extends QueryExecutorBaseInt = 42 +} diff --git a/idea/tests/org/jetbrains/jet/plugin/javaFacade/JetJavaFacadeTest.java b/idea/tests/org/jetbrains/jet/plugin/javaFacade/JetJavaFacadeTest.java index 3226c1feb24..1254d9b551f 100644 --- a/idea/tests/org/jetbrains/jet/plugin/javaFacade/JetJavaFacadeTest.java +++ b/idea/tests/org/jetbrains/jet/plugin/javaFacade/JetJavaFacadeTest.java @@ -21,6 +21,12 @@ import com.intellij.psi.search.GlobalSearchScope; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; import org.jetbrains.annotations.NotNull; +import org.jetbrains.jet.asJava.JetLightClass; +import org.jetbrains.jet.lang.psi.JetClass; +import org.jetbrains.jet.lang.psi.JetFile; +import org.jetbrains.jet.lang.psi.JetPsiUtil; +import org.jetbrains.jet.lang.resolve.name.FqName; +import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.jet.plugin.JetLightProjectDescriptor; import org.jetbrains.jet.plugin.PluginTestCaseBase; @@ -74,4 +80,22 @@ public class JetJavaFacadeTest extends LightCodeInsightFixtureTestCase { assertEquals("java.io.PrintStream", methods[0].getReturnType().getCanonicalText()); } + + public void testLightClassIsNotCreatedForBuiltins() throws Exception { + myFixture.configureByFile(getTestName(true) + ".kt"); + + PsiReference reference = myFixture.getFile().findReferenceAt(myFixture.getCaretOffset()); + assert reference != null; + PsiElement element = reference.resolve(); + assertInstanceOf(element, JetClass.class); + JetClass aClass = (JetClass) element; + + JetLightClass createdByWrapDelegate = JetLightClass.wrapDelegate(aClass); + assertNull(createdByWrapDelegate); + + JetLightClass createdByFactory = JetLightClass.create(element.getManager(), + (JetFile) element.getContainingFile(), + JetPsiUtil.getFQName(aClass)); + assertNull(createdByFactory); + } }