Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assorted code improvement #1076

Merged
merged 6 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ class ExtractAPI[GlobalType <: Global](
makeParameter(simpleName(s), tp, tp.typeSymbol, s)
}

// paramSym is only for 2.8 and is to determine if the parameter has a default
def makeParameter(
name: String,
tpe: Type,
Expand Down Expand Up @@ -692,9 +691,7 @@ class ExtractAPI[GlobalType <: Global](
private def tparamID(s: Symbol): String =
existentialRenamings.renaming(s) match {
case Some(rename) =>
// can't use debuglog because it doesn't exist in Scala 2.9.x
if (settings.debug.value)
log("Renaming existential type variable " + s.fullName + " to " + rename)
debuglog(s"Renaming existential type variable ${s.fullName} to $rename")
rename
case None =>
s.fullName
Expand Down Expand Up @@ -832,13 +829,6 @@ class ExtractAPI[GlobalType <: Global](
private def staticAnnotations(annotations: List[AnnotationInfo]): List[AnnotationInfo] =
if (annotations == Nil) Nil
else {
// compat stub for 2.8/2.9
class IsStatic(ann: AnnotationInfo) {
def isStatic: Boolean =
ann.atp.typeSymbol isNonBottomSubClass definitions.StaticAnnotationClass
}
implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann)

// `isStub` for scala/bug#11679: annotations of inherited members may be absent from the compile time
// classpath so avoid calling `isNonBottomSubClass` on these stub symbols which would trigger an error.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public interface MultipleOutput extends Output {
*/
public OutputGroup[] getOutputGroups();

@Deprecated
@Override
public default Optional<File> getSingleOutput() {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public default Path getOutputDirectoryAsPath() {
return getOutputDirectory().toPath();
}

@Deprecated
@Override
public default Optional<File> getSingleOutput() {
return Optional.of(getOutputDirectory());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import xsbti.api
import xsbti.api.SafeLazyProxy
import collection.mutable
import sbt.io.IO
import sbt.util.Logger

object ClassToAPI {
def apply(c: Seq[Class[_]]): Seq[api.ClassLike] = process(c)._1
Expand Down Expand Up @@ -219,7 +220,7 @@ object ClassToAPI {

/** TODO: over time, ClassToAPI should switch the majority of access to the classfile parser */
private[this] def classFileForClass(c: Class[_]): ClassFile =
classfile.Parser.apply(IO.classfileLocation(c))
classfile.Parser.apply(IO.classfileLocation(c), Logger.Null)

@inline private[this] def lzyS[T <: AnyRef](t: T): xsbti.api.Lazy[T] = SafeLazyProxy.strict(t)
@inline final def lzy[T <: AnyRef](t: => T): xsbti.api.Lazy[T] = SafeLazyProxy(t)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private[sbt] object JavaAnalyze {
// as class->class dependencies that must be mapped back to source->class dependencies using the source+class assignment
for {
newClass <- newClasses
classFile = Parser(newClass)
classFile = Parser(newClass, log)
_ <- classFile.sourceFile orElse guessSourceName(newClass.getFileName.toString)
source <- guessSourcePath(sourceMap, classFile, log)
binaryClassName = classFile.className
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,25 @@ import sbt.internal.io.ErrorHandling

import scala.annotation.switch
import sbt.io.Using
import sbt.util.Logger

// Translation of jdepend.framework.ClassFileParser by Mike Clark, Clarkware Consulting, Inc.
// BSD Licensed
// Loosely based on jdepend.framework.ClassFileParser by Mike Clark, Clarkware Consulting, Inc.
// (BSD license at time of initial port; MIT license as of 2022)
//
// Note that unlike the rest of sbt, some things might be null.

import Constants._

private[sbt] object Parser {
def apply(file: Path): ClassFile =
Using.bufferedInputStream(Files.newInputStream(file))(parse(file.toString)).right.get

def apply(file: File): ClassFile =
Using.fileInputStream(file)(parse(file.toString)).right.get
def apply(file: Path, log: Logger): ClassFile =
Using.bufferedInputStream(Files.newInputStream(file))(parse(file.toString, log)).right.get

def apply(url: URL): ClassFile =
usingUrlInputStreamWithoutCaching(url)(parse(url.toString)).right.get
def apply(file: File, log: Logger): ClassFile =
Using.fileInputStream(file)(parse(file.toString, log)).right.get

def apply(url: URL, log: Logger): ClassFile =
usingUrlInputStreamWithoutCaching(url)(parse(url.toString, log)).right.get

// JarURLConnection with caching enabled will never close the jar
private val usingUrlInputStreamWithoutCaching = Using.resource((u: URL) =>
Expand All @@ -50,9 +52,9 @@ private[sbt] object Parser {
}
)

private def parse(readableName: String)(is: InputStream): Either[String, ClassFile] =
Right(parseImpl(readableName, is))
private def parseImpl(readableName: String, is: InputStream): ClassFile = {
private def parse(readableName: String, log: Logger)(is: InputStream): Either[String, ClassFile] =
Right(parseImpl(readableName, log, is))
private def parseImpl(readableName: String, log: Logger, is: InputStream): ClassFile = {
val in = new DataInputStream(is)
assume(in.readInt() == JavaMagic, "Invalid class file: " + readableName)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ object JavaCompilerForUnitTesting {
val classloader = new URLClassLoader(Array(classesDir.toURI.toURL))

val logger = ConsoleLogger()
// logger.setLevel(sbt.util.Level.Debug)

// we pass extractParents as readAPI. In fact, Analyze expect readAPI to do both things:
// - extract api representation out of Class (and saved it via a side effect)
Expand Down Expand Up @@ -109,7 +110,10 @@ object JavaCompilerForUnitTesting {
private val extractParents: Seq[Class[_]] => Set[(String, String)] = { classes =>
def canonicalNames(p: (Class[_], Class[_])): (String, String) =
p._1.getCanonicalName -> p._2.getCanonicalName
val parents = classes.map(c => c -> c.getSuperclass)
val parents =
classes
.map(c => c -> c.getSuperclass)
.filterNot(_._2 == null) // may be null for an interface
val parentInterfaces = classes.flatMap(c => c.getInterfaces.map(i => c -> i))
(parents ++ parentInterfaces).map(canonicalNames).toSet
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@ package internal
package inc
package classfile

import sbt.internal.util.ConsoleLogger

import org.scalacheck._
import Prop._

object ParserSpecification extends Properties("Parser") {

val log = ConsoleLogger()

property("able to parse all relevant classes") = Prop.forAll(classes) { (c: Class[_]) =>
Parser(sbt.io.IO.classfileLocation(c)) ne null
Parser(sbt.io.IO.classfileLocation(c), log) ne null
}

implicit def classes: Gen[Class[_]] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,18 @@ class IncHandler(directory: Path, cacheDir: Path, scriptedLog: ManagedLogger, co
new AnalyzingCompiler(instance, bridgeProvider, classpath, unit, IncHandler.classLoaderCache)
}

// hopefully the meaning of the commands can be understood by looking at examples.
// the `check-recompilations` test is a good one for seeing how `checkDependencies`
// and `checkRecompilations` work.
lazy val commands: Map[String, IncCommand] = Map(
noArgs("compile") { case (p, i) => p.compile(i).map(_ => ()) },
noArgs("clean") { case (p, _) => p.clean() },
onArgs("checkIterations") {
case (p, x :: Nil, i) => p.checkNumberOfCompilerIterations(i, x.toInt)
},
// note that this can only tell us the *last* round a class got compiled in.
// it can't tell us *every* round something got compiled in, since only
// still-extant classfiles are available for inspection
onArgs("checkRecompilations") {
case (p, step :: classNames, i) => p.checkRecompilations(i, step.toInt, classNames)
},
Expand Down
36 changes: 36 additions & 0 deletions zinc/src/sbt-test/source-dependencies/check-recompilations/test
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
# this was the first checkRecompilations test ever written. it's a good
# one for understanding how checkDependencies and checkRecompilations
# are used.

# the first compile command causes round 0 to happen

> compile

# every class gets compiled.

> checkRecompilations 0 A B C D

# now zinc knows the dependency relations between the classes:
# ┌─┐┌─┐
# │D││B│
# └┬┘└┬┘
# ┌▽┐ │
# │C│ │
# └┬┘ │
# ┌▽──▽┐
# │A │
# └────┘
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already forgot which tool I used to make this :-)


> checkDependencies A:
> checkDependencies B: A
> checkDependencies C: A
> checkDependencies D: C

# next, we swap in changed code

$ copy-file changes/A.scala A.scala

# the second compile command causes rounds 1, 2, and 3 to happen

> compile

# checkRecompilations only knows the *last* round in which something
# got compiled. it doesn't know *all* the rounds when something got
# compiled. so that's why `checkRecompilations 0` is now empty.
# after rounds 1, 2, and 3 have all finished, none of the classfiles
# from round 0 are still extant.

> checkRecompilations 0
> checkRecompilations 1 A
> checkRecompilations 2 B C
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@

# introduces first compile iteration
> compile
> checkRecompilations 0 A A2 B C
$ copy-file changes/A1.scala src/main/scala/A.scala
# second iteration and third iteration (due to invalidation of C)
> compile
> checkRecompilations 1 A A.AA A2
> checkRecompilations 2 C
$ copy-file changes/A2.scala src/main/scala/A.scala
# fourth iteration
> compile
Expand Down