From c818a7f2f15b29b78a7027c0c86f6b13113980b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 7 Dec 2023 11:52:50 +0100 Subject: [PATCH] Fix #413: Handle inner classes based on content, not names. Previously, we assumed that if there is a file `Foo` and a file `Foo$Bar`, that the latter must be an inner class (or companion) of `Foo`, and that it must therefore be ignored. As #413 shows, and as the new `JavaDefined$Evil` test shows, this is not always valid. The only true test of a class being top-level can only be found in its content, either through the `Scala` attribute or the `InnerClasses` attribute. We completely rewrite `Loaders`. We now systematically try to load every file that is requested, except when we *already* know that it is a Java inner class. This means that we will read the contents of more files than before, but still each file at most once in the common case. A file *may* be read multiple times if it is a Java non-top-level class that gets requested by its name as if it were a top-level class. The loading will fail but we have to keep the file in case it is loaded later on as an inner class. When reading *all* the declarations of a package, we always try outer classes before potential inner classes so that no double reading happens. --- .../src/main/scala/tastyquery/Symbols.scala | 11 +- .../scala/tastyquery/reader/Loaders.scala | 392 ++++++++---------- .../reader/classfiles/ClassfileParser.scala | 66 +-- .../reader/classfiles/ClassfileReader.scala | 33 +- .../src/test/scala/tastyquery/TypeSuite.scala | 14 + .../scala/javadefined/JavaDefined$Evil.java | 4 + 6 files changed, 272 insertions(+), 248 deletions(-) create mode 100644 test-sources/src/main/scala/javadefined/JavaDefined$Evil.java diff --git a/tasty-query/shared/src/main/scala/tastyquery/Symbols.scala b/tasty-query/shared/src/main/scala/tastyquery/Symbols.scala index fd052df4..839fe86c 100644 --- a/tasty-query/shared/src/main/scala/tastyquery/Symbols.scala +++ b/tasty-query/shared/src/main/scala/tastyquery/Symbols.scala @@ -1832,6 +1832,9 @@ object Symbols { /** Is this the root package? */ final def isRootPackage: Boolean = owner == null + /** Is this the empty package? */ + private[tastyquery] def isEmptyPackage: Boolean = specialKind == SpecialKind.empty + /** Is this the scala package? */ private[tastyquery] def isScalaPackage: Boolean = specialKind == SpecialKind.scala @@ -1961,9 +1964,10 @@ object Symbols { private[Symbols] object SpecialKind: inline val None = 0 inline val root = 1 - inline val scala = 2 - inline val java = 3 - inline val javaLang = 4 + inline val empty = 2 + inline val scala = 3 + inline val java = 4 + inline val javaLang = 5 end SpecialKind private def computeSpecialKind(name: SimpleName, owner: PackageSymbol | Null): SpecialKind = @@ -1972,6 +1976,7 @@ object Symbols { (owner.specialKind: @switch) match case SpecialKind.root => name match + case nme.EmptyPackageName => SpecialKind.empty case nme.scalaPackageName => SpecialKind.scala case nme.javaPackageName => SpecialKind.java case _ => SpecialKind.None diff --git a/tasty-query/shared/src/main/scala/tastyquery/reader/Loaders.scala b/tasty-query/shared/src/main/scala/tastyquery/reader/Loaders.scala index 8f38f8f7..314d1e89 100644 --- a/tasty-query/shared/src/main/scala/tastyquery/reader/Loaders.scala +++ b/tasty-query/shared/src/main/scala/tastyquery/reader/Loaders.scala @@ -18,33 +18,154 @@ import tastyquery.reader.tasties.TastyUnpickler private[tastyquery] object Loaders { - class MissingTopLevelTasty(root: Loader.Root) extends Exception(s"Missing TASTy for $root") - class UnexpectedTasty(root: Loader.Root) extends Exception(s"Unexpected TASTy for $root") + private final class PackageLoadingInfo(val pkg: PackageSymbol, initPackageData: List[PackageData]): + private lazy val dataByBinaryName = + val localRoots = mutable.HashMap.empty[String, ClassData] + for packageData <- initPackageData do + for classData <- packageData.listAllClassDatas() do + // duplicate roots from later classpath entries are ignored + localRoots.getOrElseUpdate(classData.binaryName, classData) + end for + localRoots + end dataByBinaryName - object Loader: - private[Loaders] case class Root private[Loaders] (pkg: PackageSymbol, rootName: SimpleName): - override def toString(): String = - if pkg.isRootPackage || pkg.name == nme.EmptyPackageName then rootName.toString - else pkg.displayFullName + "." + rootName.toString - end Loader + private val topLevelTastys = mutable.HashMap.empty[String, List[Tree]] - class Loader(val classpath: Classpath) { + private type LoadedFiles = mutable.HashSet[String] - given Resolver = Resolver() + def topLevelTastyFor(rootName: String): Option[List[Tree]] = + topLevelTastys.get(rootName) + + def loadAllRoots()(using ReaderContext, Resolver): Unit = + // Sort for determinism, and to make sure that outer classes always come before their inner classes + val allNames = dataByBinaryName.keysIterator.toList.sorted + + loadingRoots { loadedFiles => + for rootName <- allNames do tryLoadRoot(rootName, loadedFiles) + } + + // Upon success of loading all roots, we can throw away everything, even potential inner Java classes + dataByBinaryName.clear() + end loadAllRoots + + def loadAllPackageObjectRoots()(using ReaderContext, Resolver): Unit = + def isPackageObjectBinaryName(name: String): Boolean = + name == "package" || name.endsWith("$package") + + // Sort for determinism, and to make sure that outer classes always come before their inner classes + val candidateNames = dataByBinaryName.keysIterator.filter(isPackageObjectBinaryName(_)).toList.sorted + + loadingRoots { loadedFiles => + for rootName <- candidateNames do tryLoadRoot(rootName, loadedFiles) + } + end loadAllPackageObjectRoots + + def loadOneRoot(binaryName: String)(using ReaderContext, Resolver): Boolean = + loadingRoots { loadedFiles => + tryLoadRoot(binaryName, loadedFiles) + } + end loadOneRoot + + private def loadingRoots[A](op: LoadedFiles => A): A = + val loadedFiles = mutable.HashSet.empty[String] + val result = op(loadedFiles) + + // Upon success, remove all the loaded files so that we do not attempt to load them again later + dataByBinaryName --= loadedFiles + + result + end loadingRoots + + private def tryLoadRoot(binaryName: String, loadedFiles: LoadedFiles)(using ReaderContext, Resolver): Boolean = + dataByBinaryName.get(binaryName) match + case None => + false + case Some(classData) => + // Avoid reading inner classes that we already loaded through their outer classes. + if loadedFiles.add(binaryName) then + if classData.hasTastyFile then + doLoadTasty(classData) + true + else if doLoadClassFile(classData, loadedFiles) then true + else + // Oops, maybe we will need this one later; removing it from loadedFiles + loadedFiles -= binaryName + false + else false + end tryLoadRoot + + private lazy val fullBinaryNamePrefix: String = + if pkg.isEmptyPackage then "" + else pkg.fullName.path.mkString("", "/", "/") + + def doLoadClassFile(classData: ClassData, loadedFiles: LoadedFiles)(using ReaderContext, Resolver): Boolean = + val structure = ClassfileReader.readStructure(pkg, classData) + val kind = ClassfileParser.detectClassKind(structure) + kind match + case ClassKind.Scala2 => + ClassfileParser.loadScala2Class(structure) + true + case ClassKind.Java => + val innerDecls = ClassfileParser.loadJavaClass(pkg, termName(classData.binaryName), structure) + doLoadJavaInnerClasses(innerDecls, loadedFiles) + true + case ClassKind.TASTy => + throw TastyFormatException(s"Missing TASTy file for class ${classData.binaryName} in package $pkg") + case ClassKind.ScalaArtifact => + // Always ignore; we can consider it loaded because nobody will ever need it + true + case ClassKind.JavaInnerOrArtifact => + // Ignore at the top-level, but we cannot throw it away because it might be needed as an inner class + false + end doLoadClassFile + + private def doLoadJavaInnerClasses(explore: List[InnerClassDecl], loadedFiles: LoadedFiles)( + using ReaderContext, + Resolver + ): Unit = + explore match + case inner :: rest => + val innerSimpleName = inner.innerSimpleName + val localBinaryName = inner.innerBinaryName.name.stripPrefix(fullBinaryNamePrefix) + dataByBinaryName.get(localBinaryName) match + case Some(innerData) if !loadedFiles.contains(localBinaryName) => + loadedFiles += localBinaryName + val structure = ClassfileReader.readStructure(inner.owner, innerData) + val innerDecls = ClassfileParser.loadJavaClass(inner.owner, innerSimpleName, structure) + doLoadJavaInnerClasses(rest ::: innerDecls, loadedFiles) + case _ => + throw ClassfileFormatException( + s"Inner class $localBinaryName not found in package $pkg for owner ${inner.owner}" + ) + case Nil => + () + end doLoadJavaInnerClasses + + private def doLoadTasty(classData: ClassData)(using ReaderContext): Unit = + val unpickler = TastyUnpickler(classData.readTastyFileBytes()) + val debugPath = classData.toString() + val trees = unpickler + .unpickle( + debugPath, + TastyUnpickler.TreeSectionUnpickler( + unpickler.unpickle(debugPath, new TastyUnpickler.PositionSectionUnpickler) + ) + ) + .get + .unpickle() + topLevelTastys += classData.binaryName -> trees + end doLoadTasty + end PackageLoadingInfo - private enum Entry: - case ClassAndTasty(classData: ClassData) - case TastyOnly(classData: ClassData) - case ClassOnly(classData: ClassData, inners: List[ClassData]) + class Loader(val classpath: Classpath) { + given Resolver = Resolver() - private type ByEntryMap = Map[ClasspathEntry, IArray[(PackageSymbol, IArray[SimpleName])]] + private type ByEntryMap = Map[ClasspathEntry, IArray[(PackageSymbol, IArray[String])]] - private var searched = false - private var packages: Map[PackageSymbol, List[PackageData]] = compiletime.uninitialized + private var initialized = false + private var packages: Map[PackageSymbol, PackageLoadingInfo] = compiletime.uninitialized private var _hasGenericTuples: Boolean = compiletime.uninitialized private var byEntry: ByEntryMap | Null = null - private val roots: mutable.Map[PackageSymbol, mutable.Map[SimpleName, Entry]] = mutable.HashMap.empty - private var topLevelTastys: Map[Loader.Root, List[Tree]] = Map.empty private def toPackageName(dotSeparated: String): PackageFullName = val parts = @@ -52,138 +173,35 @@ private[tastyquery] object Loaders { else dotSeparated.split('.').toList.map(termName(_)) PackageFullName(parts) - private def topLevelSymbolNameToRootName(name: Name): SimpleName = name match + private def topLevelSymbolNameToRootName(name: Name): String = name match case name: TypeName => topLevelSymbolNameToRootName(name.toTermName) case ObjectClassName(objName) => - objName + topLevelSymbolNameToRootName(objName) case name: SimpleName => - name + NameTransformer.encode(name.name) case _ => throw IllegalStateException(s"Invalid top-level symbol name ${name.toDebugString}") end topLevelSymbolNameToRootName + private def rootNameToTopLevelTermSymbolName(rootName: String): SimpleName = + termName(NameTransformer.decode(rootName)) + /** If this is a root symbol, lookup possible top level tasty trees associated with it. */ private[tastyquery] def topLevelTasty(rootSymbol: Symbol)(using Context): Option[List[Tree]] = rootSymbol.owner match case pkg: PackageSymbol => val rootName = topLevelSymbolNameToRootName(rootSymbol.name) - val root = Loader.Root(pkg, rootName) - topLevelTastys.get(root) + packages.get(pkg).flatMap(_.topLevelTastyFor(rootName)) case _ => None - /** Completes a root by reading the corresponding entry. - * - * Zero to many new declarations can be added to `root.pkg` as a result. - * - * In any case, no new declarations can ever be found for the given root - * after this method. - */ - private def completeRoot(root: Loader.Root, entry: Entry)(using ctx: Context): Unit = - doCompleteRoot(root, entry)(using ReaderContext(ctx)) - - private def doCompleteRoot(root: Loader.Root, entry: Entry)(using ReaderContext): Unit = - def innerClassLookup(nested: List[ClassData]): Map[SimpleName, ClassData] = - val mkBinaryName: String => SimpleName = - if root.pkg == rctx.EmptyPackage then termName(_) - else - val pre = root.pkg.fullName.path.mkString("/") - bin => termName(s"$pre/$bin") - nested.iterator.map(c => mkBinaryName(c.binaryName) -> c).toMap - end innerClassLookup - - def inspectClass(root: Loader.Root, classData: ClassData, entry: Entry): Unit = - val structure = ClassfileReader.readStructure(root.pkg, classData) - val kind = ClassfileParser.detectClassKind(structure) - kind match - case ClassKind.Scala2 => - ClassfileParser.loadScala2Class(structure) - case ClassKind.Java => - entry match - case Entry.ClassOnly(_, nested) => - val lookup = innerClassLookup(nested) - val innerDecls = ClassfileParser.loadJavaClass(root.pkg, root.rootName, structure, lookup) - enterInners(innerDecls, lookup) - case _ => throw UnexpectedTasty(root) - case ClassKind.TASTy => - entry match - case Entry.ClassAndTasty(classData) => - // TODO: verify UUID of tasty matches classfile, then parse symbols - enterTasty(root, classData) - case _ => throw MissingTopLevelTasty(root) - case ClassKind.Artifact => - () // no initialisation step to take - end inspectClass - - def enterTasty(root: Loader.Root, classData: ClassData): Unit = - val unpickler = TastyUnpickler(classData.readTastyFileBytes()) - val debugPath = classData.toString() - val trees = unpickler - .unpickle( - debugPath, - TastyUnpickler.TreeSectionUnpickler( - unpickler.unpickle(debugPath, new TastyUnpickler.PositionSectionUnpickler) - ) - ) - .get - .unpickle() - topLevelTastys += root -> trees - end enterTasty - - def enterInners(explore: List[InnerClassDecl], lookup: Map[SimpleName, ClassData])(using Resolver): Unit = - def unexpectedClassKind(kind: ClassKind, inner: InnerClassDecl) = - val fullName = s"${root.pkg.fullName.path.mkString("/")}/${inner.classData.binaryName}" - ClassfileFormatException(s"Unexpected class kind $kind for inner class $fullName") - - explore match - case inner :: rest => - val structure = ClassfileReader.readStructure(inner.owner, inner.classData) - val innerDecls = ClassfileParser.loadJavaClass(inner.owner, inner.name, structure, lookup) - enterInners(rest ::: innerDecls, lookup) - case Nil => - () - end enterInners - - entry match - case entry: Entry.ClassOnly => - // Tested in `TypeSuite` - aka Java and Scala 2 dependencies - inspectClass(root, entry.classData, entry) - case entry: Entry.ClassAndTasty => - // Tested in `TypeSuite` - read Tasty file that may reference Java and Scala 2 dependencies - // maybe we do not need to parse the class, however the classfile could be missing the TASTY attribute. - inspectClass(root, entry.classData, entry) - case entry: Entry.TastyOnly => - // Tested in `SymbolSuite`, `ReadTreeSuite`, these do not need to see class files. - enterTasty(root, entry.classData) - end doCompleteRoot - /** Loads all the roots of the given `pkg`. */ private[tastyquery] def loadAllRoots(pkg: PackageSymbol)(using Context): Unit = - scanPackage(pkg) - roots.get(pkg) match - case Some(entries) => - val allNames = - entries.keysIterator.toList.sortBy(_.name) // sort for determinism - for rootName <- allNames do doLoadRoot(pkg, entries, rootName) - // Upon success, we won't need anything from that package anymore - roots -= pkg - case None => - () - end loadAllRoots + for loadingInfo <- packages.get(pkg) do loadingInfo.loadAllRoots()(using ReaderContext(ctx)) /** Loads all the roots of the given `pkg` that could be package objects. */ private[tastyquery] def loadAllPackageObjectRoots(pkg: PackageSymbol)(using Context): Unit = - scanPackage(pkg) - roots.get(pkg) match - case Some(entries) => - val candidateNames = - entries.keysIterator.filter(_.isPackageObjectName).toList.sortBy(_.name) // sort for determinism - for rootName <- candidateNames do doLoadRoot(pkg, entries, rootName) - // Upon success, we won't need any of the loaded entries anymore - entries --= candidateNames - case None => - () - end loadAllPackageObjectRoots + for loadingInfo <- packages.get(pkg) do loadingInfo.loadAllPackageObjectRoots()(using ReaderContext(ctx)) /** Loads the root of the given `pkg` that would define `name`, if there is one such root. * @@ -199,82 +217,24 @@ private[tastyquery] object Loaders { * `true` if a root was loaded, `false` otherwise. */ private[tastyquery] def loadRoot(pkg: PackageSymbol, name: Name)(using Context): Boolean = - scanPackage(pkg) - roots.get(pkg) match - case Some(entries) => + packages.get(pkg) match + case Some(loadingInfo) => val rootName = topLevelSymbolNameToRootName(name) - val result = doLoadRoot(pkg, entries, rootName) - if result then - // Upon success, we won't need that particular entry anymore - entries -= rootName - result + loadingInfo.loadOneRoot(rootName)(using ReaderContext(ctx)) case None => false end loadRoot - private def doLoadRoot(pkg: PackageSymbol, pkgEntries: mutable.Map[SimpleName, Entry], rootName: SimpleName)( - using Context - ): Boolean = - pkgEntries.get(rootName) match - case Some(entry) => - completeRoot(Loader.Root(pkg, rootName), entry) - true - case None => - false - end doLoadRoot - - private def foreachEntry(data: PackageData)(f: (SimpleName, Entry) => Unit): Unit = - def binaryNameToRootName(binaryName: String): SimpleName = - termName(NameTransformer.decode(binaryName)) - - val allClassDatas = data.listAllClassDatas() - val nestedPrefixes = allClassDatas.map(_.binaryName + "$") - - for cData <- allClassDatas do - val binaryName = cData.binaryName - - if cData.hasTastyFile then - // #263 If there is a `.tasty` file, it is necessarily top-level - val entry = - if cData.hasClassFile then Entry.ClassAndTasty(cData) - else Entry.TastyOnly(cData) - f(binaryNameToRootName(binaryName), entry) - else - /* Otherwise, it can be Scala 2 or Java. In that case, we must - * only process top-level classes. We must include nested class - * data regardless, because we cannot tell whether it is Java - * or Scala 2 here. - */ - val isTopLevel = !nestedPrefixes.exists(binaryName.startsWith(_)) - if isTopLevel then - val nestedPrefix = binaryName + "$" - val nestedData = allClassDatas.filter(_.binaryName.startsWith(nestedPrefix)) - f(binaryNameToRootName(binaryName), Entry.ClassOnly(cData, nestedData)) - end foreachEntry - - private def scanPackage(pkg: PackageSymbol)(using Context): Unit = { - require(searched) - packages.get(pkg) match { - case Some(datas) => - val localRoots = mutable.HashMap.empty[SimpleName, Entry] - for data <- datas do - foreachEntry(data)(localRoots.getOrElseUpdate) // duplicate roots from later classpath entries are ignored - roots(pkg) = localRoots - packages -= pkg - case _ => // probably a synthetic package that only has other packages as members. (i.e. `package java`) - } - } - def lookupByEntry(src: ClasspathEntry)(using Context): Option[Iterable[TermOrTypeSymbol]] = - - def lookupRoots(pkg: PackageSymbol, rootNames: IArray[SimpleName]) = + def lookupRoots(pkg: PackageSymbol, rootNames: IArray[String]) = val buf = IArray.newBuilder[TermOrTypeSymbol] def lookup(n: Name) = for case t: TermOrTypeSymbol <- pkg.getDecl(n) do buf += t for rootName <- rootNames do - lookup(rootName) - lookup(rootName.toTypeName) - lookup(rootName.withObjectSuffix.toTypeName) + val possibleTermName = rootNameToTopLevelTermSymbolName(rootName) + lookup(possibleTermName) + lookup(possibleTermName.toTypeName) + lookup(possibleTermName.withObjectSuffix.toTypeName) buf.result() def computeLookup(map: ByEntryMap) = @@ -291,6 +251,9 @@ private[tastyquery] object Loaders { end lookupByEntry def initPackages()(using ctx: Context): Unit = + if initialized then throw IllegalStateException(s"Classloader was already initialized") + + initialized = true def loadPackages(): List[(PackageSymbol, PackageData)] = val localPackages = mutable.HashMap.empty[String, PackageSymbol] @@ -301,21 +264,21 @@ private[tastyquery] object Loaders { ) end loadPackages - if !searched then - searched = true - packages = loadPackages().groupMap((pkg, _) => pkg)((_, data) => data) - _hasGenericTuples = - packages.get(defn.scalaPackage).exists(_.exists(_.getClassDataByBinaryName("$times$colon").isDefined)) + val rawMap = loadPackages().groupBy(_._1) + packages = rawMap.map((pkg, pairs) => pkg -> new PackageLoadingInfo(pkg, pairs.map(_._2))) + _hasGenericTuples = rawMap + .get(defn.scalaPackage) + .exists(_.exists { (pkg, data) => + data.getClassDataByBinaryName("$times$colon").isDefined + }) end initPackages def hasGenericTuples: Boolean = _hasGenericTuples private def computeByEntry()(using Context): ByEntryMap = - require(searched) - val localByEntry = - mutable.HashMap.empty[ClasspathEntry, mutable.HashMap[PackageSymbol, mutable.HashSet[SimpleName]]] - val localSeen = mutable.HashMap.empty[PackageSymbol, mutable.HashSet[SimpleName]] + mutable.HashMap.empty[ClasspathEntry, mutable.HashMap[PackageSymbol, mutable.HashSet[String]]] + val localSeen = mutable.HashMap.empty[PackageSymbol, mutable.HashSet[String]] val localPackages = mutable.HashMap.empty[String, PackageSymbol] def lookupPackage(pkgName: String): PackageSymbol = @@ -326,16 +289,21 @@ private[tastyquery] object Loaders { pkgData <- entry.listAllPackages() do val pkg = lookupPackage(pkgData.dotSeparatedName) - foreachEntry(pkgData)((name, _) => - if !localSeen.getOrElseUpdate(pkg, mutable.HashSet.empty).contains(name) then - // only enter here the first time we see this name in this package - localSeen(pkg).add(name) + val localSeenForThisPackage = localSeen.getOrElseUpdate(pkg, mutable.HashSet.empty) + + for classData <- pkgData.listAllClassDatas() do + val localBinaryName = classData.binaryName + + // only enter here the first time we see this name in this package, + // i.e., exclude names that we found in this package in earlier class path entries + if localSeenForThisPackage.add(localBinaryName) then localByEntry .getOrElseUpdate(entry, mutable.HashMap.empty) .getOrElseUpdate(pkg, mutable.HashSet.empty) - .add(name) - ) + .add(localBinaryName) + end for end for + localByEntry.view .mapValues(entries => IArray.from(entries.view.mapValues(IArray.from))) .toMap diff --git a/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileParser.scala b/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileParser.scala index 6260ef87..3b88112d 100644 --- a/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileParser.scala +++ b/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileParser.scala @@ -33,11 +33,11 @@ private[reader] object ClassfileParser { inline def resolver(using resolver: Resolver): resolver.type = resolver enum ClassKind: - case Scala2, Java, TASTy, Artifact + case Scala2, Java, TASTy, ScalaArtifact, JavaInnerOrArtifact - case class InnerClassRef(name: SimpleName, outer: SimpleName, isStatic: Boolean) + case class InnerClassRef(innerSimpleName: SimpleName, outerBinaryName: SimpleName, isStatic: Boolean) - case class InnerClassDecl(classData: ClassData, name: SimpleName, owner: DeclaringSymbol) + case class InnerClassDecl(innerSimpleName: SimpleName, owner: DeclaringSymbol, innerBinaryName: SimpleName) class Resolver: private val refs = mutable.HashMap.empty[SimpleName, TypeRef] @@ -86,25 +86,25 @@ private[reader] object ClassfileParser { cls: ClassSymbol, moduleClass: ClassSymbol, structure: Structure, - lookup: Map[SimpleName, ClassData], innerClasses: Forked[DataStream] ): InnerClasses = import structure.given - def missingClass(binaryName: SimpleName) = - ClassfileFormatException(s"Inner class $binaryName not found, keys: ${lookup.keys.toList}") - - def lookupDeclaration(isStatic: Boolean, name: SimpleName, binaryName: SimpleName): InnerClassDecl = - val data = lookup.getOrElse(binaryName, throw missingClass(binaryName)) - InnerClassDecl(data, name, if isStatic then moduleClass else cls) - val refsBuf = Map.newBuilder[SimpleName, InnerClassRef] val declsBuf = List.newBuilder[InnerClassDecl] innerClasses.use { - ClassfileReader.readInnerClasses { (name, innerBinaryName, outerBinaryName, flags) => - val isStatic = flags.is(Flags.Static) - refsBuf += innerBinaryName -> InnerClassRef(name, outerBinaryName, isStatic) - if outerBinaryName == structure.binaryName then declsBuf += lookupDeclaration(isStatic, name, innerBinaryName) + ClassfileReader.readInnerClasses { (innerBinaryName, outerBinaryNameOpt, innerSimpleNameOpt, flags) => + // We don't care about local, anonymous or synthetic classes + if outerBinaryNameOpt.isDefined && innerSimpleNameOpt.isDefined && !flags.is(Synthetic) then + val outerBinaryName = outerBinaryNameOpt.get + val innerSimpleName = innerSimpleNameOpt.get + + val isStatic = flags.is(Flags.Static) + refsBuf += innerBinaryName -> InnerClassRef(innerSimpleName, outerBinaryName, isStatic) + + if outerBinaryName == structure.binaryName then + val owner = if isStatic then moduleClass else cls + declsBuf += InnerClassDecl(innerSimpleName, owner, innerBinaryName) } } InnerClasses(refsBuf.result(), declsBuf.result()) @@ -144,12 +144,10 @@ private[reader] object ClassfileParser { Unpickler.loadInfo(sigBytes) } - def loadJavaClass( - classOwner: DeclaringSymbol, - name: SimpleName, - structure: Structure, - innerLookup: Map[SimpleName, ClassData] - )(using ReaderContext, Resolver): List[InnerClassDecl] = { + def loadJavaClass(classOwner: DeclaringSymbol, name: SimpleName, structure: Structure)( + using ReaderContext, + Resolver + ): List[InnerClassDecl] = { import structure.given val attributes = structure.attributes.use(ClassfileReader.readAttributeMap()) @@ -187,7 +185,7 @@ private[reader] object ClassfileParser { val innerClassesStrict: InnerClasses = attributes.get(attr.InnerClasses) match case None => InnerClasses.Empty - case Some(stream) => InnerClasses.parse(cls, moduleClass, structure, innerLookup, stream) + case Some(stream) => InnerClasses.parse(cls, moduleClass, structure, stream) given InnerClasses = innerClassesStrict /* Does this class contain signature-polymorphic methods? @@ -498,6 +496,8 @@ private[reader] object ClassfileParser { def detectClassKind(structure: Structure): ClassKind = import structure.given + var innerClassesStream: Forked[DataStream] | Null = null + var result: ClassKind = ClassKind.Java // if we do not find anything special, it will be Java structure.attributes.use { ClassfileReader.scanAttributes { @@ -505,16 +505,34 @@ private[reader] object ClassfileParser { result = ClassKind.Scala2 true case attr.Scala => - result = ClassKind.Artifact + result = ClassKind.ScalaArtifact false // keep going; there might be a ScalaSig or TASTY later on case attr.TASTY => result = ClassKind.TASTy true + case attr.InnerClasses => + innerClassesStream = data.fork + false case _ => false } } - result + + if result == ClassKind.Java && innerClassesStream != null then + if containsSelfInnerClassDecl(structure.binaryName, innerClassesStream.nn) then ClassKind.JavaInnerOrArtifact + else ClassKind.Java + else result end detectClassKind + private def containsSelfInnerClassDecl(binaryName: SimpleName, innerClassesStream: Forked[DataStream])( + using ConstantPool + ): Boolean = + innerClassesStream.use { + var result = false + ClassfileReader.readInnerClasses { (innerFullBinaryName, _, _, _) => + result ||= (innerFullBinaryName == binaryName) + } + result + } + end containsSelfInnerClassDecl } diff --git a/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileReader.scala b/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileReader.scala index 197b0d23..a5cda111 100644 --- a/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileReader.scala +++ b/tasty-query/shared/src/main/scala/tastyquery/reader/classfiles/ClassfileReader.scala @@ -154,6 +154,11 @@ private[reader] object ClassfileReader { private[ClassfileReader] def idx(i: Int): Index = Indexing.idx(i) + /** An `Index` in this constant pool, or `None` if 0. */ + private[ClassfileReader] def optIdx(i: Int): Option[Index] = + if i == 0 then None + else Some(idx(i)) + private[ClassfileReader] def add(info: ConstantInfo): Boolean = { infos(index) = info def debug() = { @@ -428,22 +433,32 @@ private[reader] object ClassfileReader { } end readAnnotationArgument + /** Reads the content of the `InnerClasses` attribute. + * + * Calls `op` for each entry with the following arguments: + * + * - `innerBinaryName`: the fully-qualified binary name of the inner class. + * - `outerBinaryName`: the declaring outer class of the inner class, if + * it exists, i.e., if it is not top-level, local, or anymous. + * - `innerSimpleName`: the simple name of the inner class, as found in the + * source code, if it is not anonymous. + * - `innerAccessFlags`: the access flags of the inner class. + */ def readInnerClasses( - op: (SimpleName, SimpleName, SimpleName, FlagSet) => Unit + op: (SimpleName, Option[SimpleName], Option[SimpleName], FlagSet) => Unit )(using ds: DataStream, pool: ConstantPool): Unit = { val numberOfClasses = data.readU2() loop(numberOfClasses) { val innerClassIdx = pool.idx(data.readU2()) - val outerClassId = data.readU2() // 0 if a local/anonymous class - val innerNameId = data.readU2() // 0 if anonymous + val outerClassIdx = pool.optIdx(data.readU2()) // 0 if a local/anonymous class + val innerNameIdx = pool.optIdx(data.readU2()) // 0 if anonymous val accessFlags = readAccessFlags().toFlags - // We don't care about local, anonymous or synthetic classes - if outerClassId != 0 && innerNameId != 0 && !accessFlags.is(Synthetic) then - val innerClass = pool.utf8(pool.cls(innerClassIdx).nameIdx) - val innerName = pool.utf8(pool.idx(innerNameId)) - val outerClass = pool.utf8(pool.cls(pool.idx(outerClassId)).nameIdx) - op(innerName, innerClass, outerClass, accessFlags) + val innerBinaryName = pool.utf8(pool.cls(innerClassIdx).nameIdx) + val outerBinaryName = outerClassIdx.map(idx => pool.utf8(pool.cls(idx).nameIdx)) + val innerSimpleName = innerNameIdx.map(pool.utf8(_)) + + op(innerBinaryName, outerBinaryName, innerSimpleName, accessFlags) } } diff --git a/tasty-query/shared/src/test/scala/tastyquery/TypeSuite.scala b/tasty-query/shared/src/test/scala/tastyquery/TypeSuite.scala index b3f44ff4..71e008f3 100644 --- a/tasty-query/shared/src/test/scala/tastyquery/TypeSuite.scala +++ b/tasty-query/shared/src/test/scala/tastyquery/TypeSuite.scala @@ -3647,4 +3647,18 @@ class TypeSuite extends UnrestrictedUnpicklingSuite { RecursiveMatchTypeSym.boundsAsSeenFrom(staticRef.prefix) staticRef.underlying } + + testWithContext("sister-classes-with-names-that-look-like-inner-classes-issue-413") { + // Can find a top-level class that looks like an inner class + val JavaDefinedEvilClass = ctx.findTopLevelClass("javadefined.JavaDefined$Evil") + assert(clue(JavaDefinedEvilClass.owner).isPackage) + assert(clue(JavaDefinedEvilClass.name) == typeName("JavaDefined$Evil")) + + // Cannot confuse an inner class for a top-level class + intercept[MemberNotFoundException](ctx.findTopLevelClass("javadefined.GenericJavaClass$MyInner")) + + // After that, can still find the real inner class (it wasn't thrown away) + val GenericJavaClass = ctx.findTopLevelClass("javadefined.GenericJavaClass") + assert(clue(GenericJavaClass.getDecl(typeName("MyInner"))).exists(_.isClass)) + } } diff --git a/test-sources/src/main/scala/javadefined/JavaDefined$Evil.java b/test-sources/src/main/scala/javadefined/JavaDefined$Evil.java new file mode 100644 index 00000000..010f2abc --- /dev/null +++ b/test-sources/src/main/scala/javadefined/JavaDefined$Evil.java @@ -0,0 +1,4 @@ +package javadefined; + +public class JavaDefined$Evil { +}