From a419112c25d5858c4879041e10aac5fcbe0028fe Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 3 Jan 2019 18:16:02 +0100 Subject: [PATCH] Refactor class lookup methods in KotlinBuiltIns Remove external usages of get*Nullable methods, inline/remove other methods to simplify the API facade --- .../kotlin/codegen/range/RangeValues.kt | 3 +- .../kotlin/resolve/ExpectedResolveData.java | 36 +++---- .../kotlin/builtins/KotlinBuiltIns.java | 94 ++++++------------- 3 files changed, 46 insertions(+), 87 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/range/RangeValues.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/range/RangeValues.kt index 9e15755ac33..08d6abdff3a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/range/RangeValues.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/range/RangeValues.kt @@ -22,7 +22,6 @@ import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.descriptors.impl.SyntheticFieldDescriptor -import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall @@ -89,7 +88,7 @@ private fun isSubtypeOfString(type: KotlinType, builtIns: KotlinBuiltIns) = KotlinTypeChecker.DEFAULT.isSubtypeOf(type, builtIns.stringType) private fun isSubtypeOfCharSequence(type: KotlinType, builtIns: KotlinBuiltIns) = - KotlinTypeChecker.DEFAULT.isSubtypeOf(type, builtIns.getBuiltInClassByName(Name.identifier("CharSequence")).defaultType) + KotlinTypeChecker.DEFAULT.isSubtypeOf(type, builtIns.charSequence.defaultType) private fun getResolvedCallForRangeExpression( bindingContext: BindingContext, diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/resolve/ExpectedResolveData.java b/compiler/tests-common/tests/org/jetbrains/kotlin/resolve/ExpectedResolveData.java index eaf762deed0..ace832312b1 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/resolve/ExpectedResolveData.java +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/resolve/ExpectedResolveData.java @@ -21,12 +21,14 @@ import com.intellij.psi.PsiQualifiedNamedElement; import com.intellij.psi.util.PsiTreeUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.kotlin.builtins.DefaultBuiltIns; +import org.jetbrains.kotlin.analyzer.AnalysisResult; +import org.jetbrains.kotlin.builtins.KotlinBuiltIns; import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.diagnostics.Diagnostic; import org.jetbrains.kotlin.diagnostics.Errors; import org.jetbrains.kotlin.diagnostics.PsiDiagnosticUtils; +import org.jetbrains.kotlin.incremental.components.NoLookupLocation; import org.jetbrains.kotlin.name.FqName; import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.*; @@ -126,16 +128,18 @@ public abstract class ExpectedResolveData { protected abstract KtFile createKtFile(String fileName, String text); - protected static BindingContext analyze(List files, KotlinCoreEnvironment environment) { + protected static AnalysisResult analyze(List files, KotlinCoreEnvironment environment) { if (files.isEmpty()) { - System.err.println("Suspicious: no files"); - return BindingContext.EMPTY; + throw new AssertionError("Suspicious: no files"); } - return JvmResolveUtil.analyze(files, environment).getBindingContext(); + return JvmResolveUtil.analyze(files, environment); } - public final void checkResult(BindingContext bindingContext) { + public final void checkResult(@NotNull AnalysisResult result) { + BindingContext bindingContext = result.getBindingContext(); + ModuleDescriptor module = result.getModuleDescriptor(); + Set unresolvedReferences = new HashSet<>(); for (Diagnostic diagnostic : bindingContext.getDiagnostics()) { if (Errors.UNRESOLVED_REFERENCE_DIAGNOSTICS.contains(diagnostic.getFactory())) { @@ -236,7 +240,7 @@ public abstract class ExpectedResolveData { KotlinType actualType = bindingContext.get(BindingContext.TYPE, typeReference); assertNotNull("Type " + name + " not resolved for reference " + name, actualType); - ClassifierDescriptor expectedClass = getBuiltinClass(name.substring(STANDARD_PREFIX.length())); + ClassDescriptor expectedClass = getBuiltinClass(module, name.substring(STANDARD_PREFIX.length())); assertTypeConstructorEquals("Type resolution mismatch: ", expectedClass.getTypeConstructor(), actualType.getConstructor()); continue; } @@ -292,8 +296,7 @@ public abstract class ExpectedResolveData { TypeConstructor expectedTypeConstructor; if (typeName.startsWith(STANDARD_PREFIX)) { String name = typeName.substring(STANDARD_PREFIX.length()); - ClassifierDescriptor expectedClass = getBuiltinClass(name); - expectedTypeConstructor = expectedClass.getTypeConstructor(); + expectedTypeConstructor = getBuiltinClass(module, name).getTypeConstructor(); } else { Position declarationPosition = declarationToPosition.get(typeName); @@ -339,15 +342,12 @@ public abstract class ExpectedResolveData { } @NotNull - public static ClassifierDescriptor getBuiltinClass(String nameOrFqName) { - ClassifierDescriptor expectedClass; - - if (nameOrFqName.indexOf('.') >= 0) { - expectedClass = DefaultBuiltIns.getInstance().getBuiltInClassByFqNameNullable(FqName.fromSegments(Arrays.asList(nameOrFqName.split("\\.")))); - } - else { - expectedClass = DefaultBuiltIns.getInstance().getBuiltInClassByNameNullable(Name.identifier(nameOrFqName)); - } + private static ClassDescriptor getBuiltinClass(@NotNull ModuleDescriptor module, @NotNull String nameOrFqName) { + FqName fqName = + nameOrFqName.contains(".") + ? new FqName(nameOrFqName) + : KotlinBuiltIns.BUILT_INS_PACKAGE_FQ_NAME.child(Name.identifier(nameOrFqName)); + ClassDescriptor expectedClass = DescriptorUtilKt.resolveClassByFqName(module, fqName, NoLookupLocation.FROM_TEST); assertNotNull("Expected class not found: " + nameOrFqName, expectedClass); return expectedClass; diff --git a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java index 907e88860f0..d4c94b0be22 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java +++ b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java @@ -85,12 +85,12 @@ public abstract class KotlinBuiltIns { Map nameToFragment = new LinkedHashMap(); PackageFragmentDescriptor kotlin = createPackage(provider, nameToFragment, BUILT_INS_PACKAGE_FQ_NAME); createPackage(provider, null, COROUTINES_PACKAGE_FQ_NAME_RELEASE); - PackageFragmentDescriptor kotlinCollections = createPackage(provider, nameToFragment, COLLECTIONS_PACKAGE_FQ_NAME); + createPackage(provider, nameToFragment, COLLECTIONS_PACKAGE_FQ_NAME); createPackage(provider, nameToFragment, RANGES_PACKAGE_FQ_NAME); createPackage(provider, nameToFragment, ANNOTATION_PACKAGE_FQ_NAME); Set allImportedByDefault = new LinkedHashSet(nameToFragment.values()); - return new PackageFragments(kotlin, kotlinCollections, allImportedByDefault); + return new PackageFragments(kotlin, allImportedByDefault); } }); @@ -117,7 +117,15 @@ public abstract class KotlinBuiltIns { this.builtInClassesByName = storageManager.createMemoizedFunction(new Function1() { @Override public ClassDescriptor invoke(Name name) { - return getBuiltInClassByName(name, getBuiltInsPackageFragment()); + ClassifierDescriptor classifier = + getBuiltInsPackageFragment().getMemberScope().getContributedClassifier(name, NoLookupLocation.FROM_BUILTINS); + if (classifier == null) { + throw new AssertionError("Built-in class " + BUILT_INS_PACKAGE_FQ_NAME.child(name) + " is not found"); + } + if (!(classifier instanceof ClassDescriptor)) { + throw new AssertionError("Must be a class descriptor " + name + ", but was " + classifier); + } + return (ClassDescriptor) classifier; } }); } @@ -220,16 +228,13 @@ public abstract class KotlinBuiltIns { private static class PackageFragments { public final PackageFragmentDescriptor builtInsPackageFragment; - public final PackageFragmentDescriptor collectionsPackageFragment; public final Set allImportedByDefaultBuiltInsPackageFragments; private PackageFragments( @NotNull PackageFragmentDescriptor builtInsPackageFragment, - @NotNull PackageFragmentDescriptor collectionsPackageFragment, @NotNull Set allImportedByDefaultBuiltInsPackageFragments ) { this.builtInsPackageFragment = builtInsPackageFragment; - this.collectionsPackageFragment = collectionsPackageFragment; this.allImportedByDefaultBuiltInsPackageFragments = allImportedByDefaultBuiltInsPackageFragments; } } @@ -408,56 +413,16 @@ public abstract class KotlinBuiltIns { return packageFragments.invoke().builtInsPackageFragment.getMemberScope(); } - @NotNull - public ClassDescriptor getBuiltInClassByName(@NotNull Name simpleName) { - return builtInClassesByName.invoke(simpleName); - } - - @NotNull - private static ClassDescriptor getBuiltInClassByName(@NotNull Name simpleName, @NotNull PackageFragmentDescriptor packageFragment) { - ClassDescriptor classDescriptor = getBuiltInClassByNameNullable(simpleName, packageFragment); - if (classDescriptor == null) { - throw new AssertionError("Built-in class " + packageFragment.getFqName().child(simpleName).asString() + " is not found"); - } - return classDescriptor; - } - - @Nullable - public ClassDescriptor getBuiltInClassByNameNullable(@NotNull Name simpleName) { - return getBuiltInClassByNameNullable(simpleName, getBuiltInsPackageFragment()); - } - - @Nullable - public ClassDescriptor getBuiltInClassByFqNameNullable(@NotNull FqName fqName) { - return DescriptorUtilKt.resolveClassByFqName(builtInsModule, fqName, NoLookupLocation.FROM_BUILTINS); - } - @NotNull public ClassDescriptor getBuiltInClassByFqName(@NotNull FqName fqName) { - ClassDescriptor descriptor = getBuiltInClassByFqNameNullable(fqName); + ClassDescriptor descriptor = DescriptorUtilKt.resolveClassByFqName(builtInsModule, fqName, NoLookupLocation.FROM_BUILTINS); assert descriptor != null : "Can't find built-in class " + fqName; return descriptor; } - @Nullable - private static ClassDescriptor getBuiltInClassByNameNullable(@NotNull Name simpleName, @NotNull PackageFragmentDescriptor packageFragment) { - ClassifierDescriptor classifier = packageFragment.getMemberScope().getContributedClassifier( - simpleName, - NoLookupLocation.FROM_BUILTINS); - - assert classifier == null || - classifier instanceof ClassDescriptor : "Must be a class descriptor " + simpleName + ", but was " + classifier; - return (ClassDescriptor) classifier; - } - @NotNull private ClassDescriptor getBuiltInClassByName(@NotNull String simpleName) { - return getBuiltInClassByName(Name.identifier(simpleName)); - } - - @NotNull - private static ClassDescriptor getBuiltInClassByName(@NotNull String simpleName, PackageFragmentDescriptor packageFragment) { - return getBuiltInClassByName(Name.identifier(simpleName), packageFragment); + return builtInClassesByName.invoke(Name.identifier(simpleName)); } @NotNull @@ -591,69 +556,64 @@ public abstract class KotlinBuiltIns { return getBuiltInClassByFqName(FQ_NAMES.kClass.toSafe()); } - @NotNull - private ClassDescriptor getCollectionClassByName(@NotNull String simpleName) { - return getBuiltInClassByName(simpleName, packageFragments.invoke().collectionsPackageFragment); - } - @NotNull public ClassDescriptor getIterator() { - return getCollectionClassByName("Iterator"); + return getBuiltInClassByFqName(FQ_NAMES.iterator); } @NotNull public ClassDescriptor getIterable() { - return getCollectionClassByName("Iterable"); + return getBuiltInClassByFqName(FQ_NAMES.iterable); } @NotNull public ClassDescriptor getMutableIterable() { - return getCollectionClassByName("MutableIterable"); + return getBuiltInClassByFqName(FQ_NAMES.mutableIterable); } @NotNull public ClassDescriptor getMutableIterator() { - return getCollectionClassByName("MutableIterator"); + return getBuiltInClassByFqName(FQ_NAMES.mutableIterator); } @NotNull public ClassDescriptor getCollection() { - return getCollectionClassByName("Collection"); + return getBuiltInClassByFqName(FQ_NAMES.collection); } @NotNull public ClassDescriptor getMutableCollection() { - return getCollectionClassByName("MutableCollection"); + return getBuiltInClassByFqName(FQ_NAMES.mutableCollection); } @NotNull public ClassDescriptor getList() { - return getCollectionClassByName("List"); + return getBuiltInClassByFqName(FQ_NAMES.list); } @NotNull public ClassDescriptor getMutableList() { - return getCollectionClassByName("MutableList"); + return getBuiltInClassByFqName(FQ_NAMES.mutableList); } @NotNull public ClassDescriptor getSet() { - return getCollectionClassByName("Set"); + return getBuiltInClassByFqName(FQ_NAMES.set); } @NotNull public ClassDescriptor getMutableSet() { - return getCollectionClassByName("MutableSet"); + return getBuiltInClassByFqName(FQ_NAMES.mutableSet); } @NotNull public ClassDescriptor getMap() { - return getCollectionClassByName("Map"); + return getBuiltInClassByFqName(FQ_NAMES.map); } @NotNull public ClassDescriptor getMutableMap() { - return getCollectionClassByName("MutableMap"); + return getBuiltInClassByFqName(FQ_NAMES.mutableMap); } @NotNull @@ -672,12 +632,12 @@ public abstract class KotlinBuiltIns { @NotNull public ClassDescriptor getListIterator() { - return getCollectionClassByName("ListIterator"); + return getBuiltInClassByFqName(FQ_NAMES.listIterator); } @NotNull public ClassDescriptor getMutableListIterator() { - return getCollectionClassByName("MutableListIterator"); + return getBuiltInClassByFqName(FQ_NAMES.mutableListIterator); } @NotNull