diff --git a/README.md b/README.md index b55599a5..d38d8642 100644 --- a/README.md +++ b/README.md @@ -437,6 +437,10 @@ Generate JVM bytecode to assert that a method is called on the correct IDEA thre 4. `com.intellij.util.concurrency.annotations.RequiresReadLockAbsence` 5. `com.intellij.util.concurrency.annotations.RequiresWriteLock` +This is the same instrumentation that IntelliJ IDEA applies to its own codebase. Note that, matching IntelliJ IDEA's +behavior, `@RequiresReadLock` generates a call to `ThreadingAssertions.softAssertReadAccess`, which logs an error +instead of throwing an exception when read access is missing. + See: [IntelliJ IDEA ThreadingAssertions.java](https://github.com/JetBrains/intellij-community/blob/5758eb99b4a1971ebe75cda755693cc930949465/platform/core-api/src/com/intellij/util/concurrency/ThreadingAssertions.java) #### `instrumentNotNullAnnotations :: SettingKey[Boolean]` diff --git a/build.sbt b/build.sbt index 79890b5f..df030d48 100644 --- a/build.sbt +++ b/build.sbt @@ -134,8 +134,9 @@ lazy val ideaSupport = (project in file("ideaSupport")) "io.get-coursier" %% "coursier" % "2.1.24", "commons-io" % "commons-io" % "2.22.0", - // Used to compile the test fixtures of the @NotNull instrumentation tests - "org.jetbrains" % "annotations" % "26.1.0" % Test + // Used by the sources copied from IntelliJ IDEA in the threadingModelHelper package + // and to compile the test fixtures of the @NotNull instrumentation tests + "org.jetbrains" % "annotations" % "26.1.0" ), ) diff --git a/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator.java b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator.java new file mode 100644 index 00000000..5350c6d6 --- /dev/null +++ b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator.java @@ -0,0 +1,17 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copied from IntelliJ IDEA Community Edition (commit 7a35a7d7fe64), original package org.jetbrains.jps.devkit.threadingModelHelper. +// The only changes are the package name and the ASM package (org.jetbrains.org.objectweb.asm -> org.objectweb.asm). +package org.jetbrains.sbtidea.instrumentation.threadingModelHelper; + +import org.jetbrains.annotations.ApiStatus; +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.MethodVisitor; + +@ApiStatus.Internal +public interface TMHAssertionGenerator { + boolean isMyAnnotation(String annotationDescriptor); + + AnnotationVisitor getAnnotationChecker(int api, Runnable onShouldGenerateAssertion); + + void generateAssertion(MethodVisitor writer, int methodStartLineNumber); +} diff --git a/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator2.java b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator2.java new file mode 100644 index 00000000..fc7dabd6 --- /dev/null +++ b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHAssertionGenerator2.java @@ -0,0 +1,112 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copied from IntelliJ IDEA Community Edition (commit 7a35a7d7fe64), original package org.jetbrains.jps.devkit.threadingModelHelper. +// The only changes are the package name and the ASM package (org.jetbrains.org.objectweb.asm -> org.objectweb.asm). +package org.jetbrains.sbtidea.instrumentation.threadingModelHelper; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; + +import java.util.Set; + +@ApiStatus.Internal +public final class TMHAssertionGenerator2 implements TMHAssertionGenerator { + + private static final String THREAD_ASSERTIONS_CLASS_NAME = "com/intellij/util/concurrency/ThreadingAssertions"; + private static final String GENERATE_ASSERTION_PARAMETER = "generateAssertion"; + + private final String myThreadAssertionsClassName; + private final Type myAnnotationClass; + private final String myAssertionMethodName; + + TMHAssertionGenerator2(String threadAssertionsClassName, Type annotationClass, String assertionMethodName) { + myThreadAssertionsClassName = threadAssertionsClassName; + myAnnotationClass = annotationClass; + myAssertionMethodName = assertionMethodName; + } + + @Override + public boolean isMyAnnotation(String annotationDescriptor) { + return myAnnotationClass.getDescriptor().equals(annotationDescriptor); + } + + @Override + public AnnotationVisitor getAnnotationChecker(int api, Runnable onShouldGenerateAssertion) { + return new AnnotationChecker(api, onShouldGenerateAssertion); + } + + @Override + public void generateAssertion(MethodVisitor writer, int methodStartLineNumber) { + if (methodStartLineNumber != -1) { + Label generatedCodeStart = new Label(); + writer.visitLabel(generatedCodeStart); + writer.visitLineNumber(methodStartLineNumber, generatedCodeStart); + } + writer.visitMethodInsn( + Opcodes.INVOKESTATIC, + myThreadAssertionsClassName, + myAssertionMethodName, + "()V", + false + ); + } + + static class AnnotationChecker extends AnnotationVisitor { + private boolean myShouldGenerateAssertion = true; + private final Runnable myOnShouldGenerateAssertion; + + private AnnotationChecker(int api, Runnable onShouldGenerateAssertion) { + super(api); + myOnShouldGenerateAssertion = onShouldGenerateAssertion; + } + + @Override + public void visit(String annotationParameterName, Object value) { + if (GENERATE_ASSERTION_PARAMETER.equals(annotationParameterName) && Boolean.FALSE.equals(value)) { + myShouldGenerateAssertion = false; + } + } + + @Override + public void visitEnd() { + if (myShouldGenerateAssertion) { + myOnShouldGenerateAssertion.run(); + } + } + } + + // TODO avoid hardcoding annotation names + static @NotNull Set generators() { + return GENERATORS; + } + + private static final Set GENERATORS = generators( + THREAD_ASSERTIONS_CLASS_NAME, + "com/intellij/util/concurrency/annotations" + ); + + public static @NotNull Set generators( + @NotNull String threadAssertionsClassName, + @NotNull String packageString + ) { + return Set.of( + generator(threadAssertionsClassName, packageString + "/RequiresEdt", "assertEventDispatchThread"), + generator(threadAssertionsClassName, packageString + "/RequiresBackgroundThread", "assertBackgroundThread"), + generator(threadAssertionsClassName, packageString + "/RequiresReadLock", "softAssertReadAccess"), + generator(threadAssertionsClassName, packageString + "/RequiresReadLockAbsence", "assertNoReadAccess"), + generator(threadAssertionsClassName, packageString + "/RequiresWriteLock", "assertWriteAccess") + ); + } + + private static @NotNull TMHAssertionGenerator generator( + @NotNull String threadAssertionsClassName, + @NotNull String annotationClassName, + @NotNull String assertionMethodName + ) { + return new TMHAssertionGenerator2(threadAssertionsClassName, Type.getType("L" + annotationClassName + ";"), assertionMethodName); + } +} diff --git a/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHInstrumenter.java b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHInstrumenter.java new file mode 100644 index 00000000..268c42f6 --- /dev/null +++ b/ideaSupport/src/main/java/org/jetbrains/sbtidea/instrumentation/threadingModelHelper/TMHInstrumenter.java @@ -0,0 +1,134 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copied from IntelliJ IDEA Community Edition (commit 7a35a7d7fe64), original package org.jetbrains.jps.devkit.threadingModelHelper. +// The only changes are the package name, the ASM package (org.jetbrains.org.objectweb.asm -> org.objectweb.asm), +// Opcodes.API_VERSION (a JetBrains ASM addition) -> Opcodes.ASM9 and the import of FailSafeMethodVisitor +// (originally com.intellij.compiler.instrumentation.FailSafeMethodVisitor). +package org.jetbrains.sbtidea.instrumentation.threadingModelHelper; + +import org.jetbrains.sbtidea.instrumentation.notNullVerification.FailSafeMethodVisitor; +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +public final class TMHInstrumenter { + public static boolean instrument(ClassReader classReader, + ClassVisitor classWriter, + Set generators, + boolean generateLineNumbers) { + AnnotatedMethodsCollector collector = new AnnotatedMethodsCollector(generators); + int options = ClassReader.SKIP_FRAMES; + if (!generateLineNumbers) { + options |= ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG; + } + classReader.accept(collector, options); + if (collector.annotatedMethods.isEmpty()) { + return false; + } + Instrumenter instrumenter = new Instrumenter(classWriter, collector.annotatedMethods); + classReader.accept(instrumenter, 0); + return true; + } + + private static final class AnnotatedMethodsCollector extends ClassVisitor { + final Set assertionGenerators; + final Map annotatedMethods = new HashMap<>(); + + AnnotatedMethodsCollector(Set assertionGenerators) { + super(Opcodes.ASM9); + this.assertionGenerators = assertionGenerators; + } + + @Override + public MethodVisitor visitMethod(int access, final String name, final String methodDescriptor, String signature, String[] exceptions) { + return new MethodVisitor(Opcodes.ASM9) { + private final MethodKey methodKey = new MethodKey(name, methodDescriptor); + private boolean annotated = false; + private boolean firstLineNumberVisited = false; + + @Override + public AnnotationVisitor visitAnnotation(String annotationDescriptor, boolean visible) { + for (TMHAssertionGenerator assertionGenerator : assertionGenerators) { + if (assertionGenerator.isMyAnnotation(annotationDescriptor)) { + return assertionGenerator.getAnnotationChecker(Opcodes.ASM9, () -> { + annotatedMethods.put(methodKey, new InstrumentationInfo(assertionGenerator)); + annotated = true; + }); + } + } + return super.visitAnnotation(annotationDescriptor, visible); + } + + @Override + public void visitLineNumber(int line, Label start) { + super.visitLineNumber(line, start); + if (annotated && !firstLineNumberVisited) { + annotatedMethods.get(methodKey).methodStartLineNumber = line; + firstLineNumberVisited = true; + } + } + }; + } + } + + private static final class Instrumenter extends ClassVisitor { + private final Map myAnnotatedMethods; + + Instrumenter(ClassVisitor writer, Map annotatedMethods) { + super(Opcodes.ASM9, writer); + myAnnotatedMethods = annotatedMethods; + } + + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + InstrumentationInfo instrumentationInfo = myAnnotatedMethods.get(new MethodKey(name, descriptor)); + if (instrumentationInfo == null) { + return super.visitMethod(access, name, descriptor, signature, exceptions); + } + return new FailSafeMethodVisitor(Opcodes.ASM9, super.visitMethod(access, name, descriptor, signature, exceptions)) { + @Override + public void visitCode() { + instrumentationInfo.assertionGenerator.generateAssertion(mv, instrumentationInfo.methodStartLineNumber); + super.visitCode(); + } + }; + } + } + + private static final class MethodKey { + final String name; + final String descriptor; + + private MethodKey(String name, String descriptor) { + this.name = name; + this.descriptor = descriptor; + } + + @Override + public int hashCode() { + int result = 1; + result = 31 * result + name.hashCode(); + result = 31 * result + descriptor.hashCode(); + return result; + } + + @Override + public boolean equals(Object obj) { + return obj == this || + obj instanceof MethodKey && ((MethodKey)obj).name.equals(name) && ((MethodKey)obj).descriptor.equals(descriptor); + } + } + + private static final class InstrumentationInfo { + final TMHAssertionGenerator assertionGenerator; + int methodStartLineNumber = -1; + + private InstrumentationInfo(TMHAssertionGenerator generator) {assertionGenerator = generator;} + } +} diff --git a/ideaSupport/src/main/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenter.scala b/ideaSupport/src/main/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenter.scala index 62804f24..43c35a8b 100644 --- a/ideaSupport/src/main/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenter.scala +++ b/ideaSupport/src/main/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenter.scala @@ -1,106 +1,32 @@ package org.jetbrains.sbtidea.instrumentation -import org.objectweb.asm.* +import org.jetbrains.sbtidea.instrumentation.notNullVerification.FailSafeClassReader +import org.jetbrains.sbtidea.instrumentation.threadingModelHelper.{TMHAssertionGenerator, TMHAssertionGenerator2, TMHInstrumenter} +import org.objectweb.asm.ClassWriter import java.nio.file.{Files, Path} +import java.util private object ThreadingAnnotationInstrumenter { + private val Generators: util.Set[? <: TMHAssertionGenerator] = TMHAssertionGenerator2.generators( + "com/intellij/util/concurrency/ThreadingAssertions", + "com/intellij/util/concurrency/annotations" + ) + def instrument(classFile: Path): Unit = { val bytes = Files.readAllBytes(classFile) - val reader = new ClassReader(bytes) - if (requiresInstrumentation(reader)) { - doInstrumentation(classFile, reader) - } - } - - private def requiresInstrumentation(reader: ClassReader): Boolean = { - val searcher = new AnnotationSearcher() - reader.accept(searcher, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES) - searcher.requiresInstrumentation - } - - private def doInstrumentation(classFile: Path, reader: ClassReader): Unit = { + val reader = new FailSafeClassReader(bytes) + // The generated assertion is a zero-argument static ()V call at method entry, + // which changes neither stack map frames nor the maximum stack size. val writer = new ClassWriter(reader, 0) - val instrumenter = new Instrumenter(writer) - reader.accept(instrumenter, 0) - Files.write(classFile, writer.toByteArray) - } - - private val GenerateAssertion = "generateAssertion" - - private val ThreadingAssertionsSignature = "com/intellij/util/concurrency/ThreadingAssertions" - - private val AnnotationClasses: Map[String, String] = - Map( - "RequiresBackgroundThread" -> "assertBackgroundThread", - "RequiresEdt" -> "assertEventDispatchThread", - "RequiresReadLock" -> "assertReadAccess", - "RequiresReadLockAbsence" -> "assertNoReadAccess", - "RequiresWriteLock" -> "assertWriteAccess" - ).map { case (annotation, method) => - val fqcn = s"com.intellij.util.concurrency.annotations.$annotation" - val signature = fqcn.replace('.', '/') - val descriptor = s"L$signature;" - (descriptor, method) - } - - private final class AnnotationSearcher extends ClassVisitor(Opcodes.ASM9) { - var requiresInstrumentation: Boolean = false - - override def visitMethod(access: Int, name: String, descriptor: String, signature: String, exceptions: Array[String]): MethodVisitor = - new MethodVisitor(Opcodes.ASM9) { - override def visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor = { - if (!requiresInstrumentation) { - requiresInstrumentation = AnnotationClasses.contains(descriptor) - } - null - } - } - } - - private final class Instrumenter(visitor: ClassVisitor) extends ClassVisitor(Opcodes.ASM9, visitor) { - override def visitMethod(access: Int, name: String, descriptor: String, signature: String, exceptions: Array[String]): MethodVisitor = { - val methodVisitor = super.visitMethod(access, name, descriptor, signature, exceptions) - new MethodInstrumenter(methodVisitor) - } - } - - private final class MethodInstrumenter(visitor: MethodVisitor) extends MethodVisitor(Opcodes.ASM9, visitor) { - private var assertionMethod: Option[String] = None - - override def visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor = { - val annotationVisitor = super.visitAnnotation(descriptor, visible) - if (AnnotationClasses.contains(descriptor)) { - new AnnotationChecker(annotationVisitor, () => { - assertionMethod = AnnotationClasses.get(descriptor) - }) - } else annotationVisitor - } - - override def visitCode(): Unit = { - assertionMethod.foreach { method => - super.visitMethodInsn(Opcodes.INVOKESTATIC, ThreadingAssertionsSignature, method, "()V", false) - } - super.visitCode() - } - } - - private final class AnnotationChecker(visitor: AnnotationVisitor, onShouldGenerateAssertion: () => Unit) extends AnnotationVisitor(Opcodes.ASM9, visitor) { - private var shouldGenerateAssertion: Boolean = true - - override def visit(name: String, value: Any): Unit = { - super.visit(name, value) - if (name == GenerateAssertion && value == java.lang.Boolean.FALSE) { - shouldGenerateAssertion = false - } - } - - override def visitEnd(): Unit = { - super.visitEnd() - if (shouldGenerateAssertion) { - onShouldGenerateAssertion() - } + // Line number generation matches IDE-driven JPS builds. While the standalone JPS builder defaults + // "tmh.generate.line.numbers" to false, the DevKit plugin injects -Dtmh.generate.line.numbers=true into every + // build process it spawns (the registry key defaults to true in intellij.devkit.core.xml). The generated + // assertion is annotated with the line number of the start of the method, producing better stack traces when + // the assertion throws. + if (TMHInstrumenter.instrument(reader, writer, Generators, /*generateLineNumbers =*/ true)) { + Files.write(classFile, writer.toByteArray) } } } diff --git a/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/InstrumenterTestHarness.scala b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/InstrumenterTestHarness.scala new file mode 100644 index 00000000..265f6f9f --- /dev/null +++ b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/InstrumenterTestHarness.scala @@ -0,0 +1,59 @@ +package org.jetbrains.sbtidea.instrumentation + +import org.scalatest.Assertions + +import java.lang.reflect.InvocationTargetException +import java.net.URLClassLoader +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Path, Paths} +import javax.tools.ToolProvider +import scala.collection.JavaConverters.asScalaIteratorConverter + +/** + * Shared harness for the instrumenter tests: compiles Java fixture sources at test runtime, + * lets the test instrument the resulting class files and loads them in a throwaway + * classloader. Loading and invoking the instrumented classes also runs the JVM bytecode + * verifier over the rewritten methods. + */ +trait InstrumenterTestHarness { this: Assertions => + + protected def annotationsJar: Path = + Paths.get(classOf[org.jetbrains.annotations.NotNull].getProtectionDomain.getCodeSource.getLocation.toURI) + + protected def compileFixture(sources: Map[String, String], debugInfo: Boolean = false): Path = { + val dir = Files.createTempDirectory("instrumentation-test") + val javaFiles = sources.map { case (relativePath, content) => + val file = dir.resolve(relativePath) + Files.createDirectories(file.getParent) + Files.write(file, content.getBytes(StandardCharsets.UTF_8)) + file.toString + }.toSeq + val args = Seq("-classpath", annotationsJar.toString, "-d", dir.toString) ++ + (if (debugInfo) Seq("-g") else Seq.empty) ++ + javaFiles + val exitCode = ToolProvider.getSystemJavaCompiler.run(null, null, null, args: _*) + assert(exitCode == 0, s"javac exited with code $exitCode") + dir + } + + protected def classFiles(dir: Path): Seq[Path] = { + val stream = Files.walk(dir) + try stream.iterator().asScala.filter(_.toString.endsWith(".class")).toList + finally stream.close() + } + + protected def classLoader(dir: Path): ClassLoader = + new URLClassLoader(Array(dir.toUri.toURL), getClass.getClassLoader) + + protected def loadClass(dir: Path, name: String): Class[_] = + classLoader(dir).loadClass(name) + + protected def invokeStatic(clazz: Class[_], methodName: String, args: AnyRef*): AnyRef = { + val method = clazz.getMethods.find(_.getName == methodName) + .getOrElse(fail(s"Method $methodName not found in ${clazz.getName}")) + method.invoke(null, args: _*) + } + + protected def interceptCause(clazz: Class[_], methodName: String, args: AnyRef*): Throwable = + intercept[InvocationTargetException](invokeStatic(clazz, methodName, args: _*)).getCause +} diff --git a/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/NotNullInstrumenterTest.scala b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/NotNullInstrumenterTest.scala index 6e4cc3a8..2d65bea4 100644 --- a/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/NotNullInstrumenterTest.scala +++ b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/NotNullInstrumenterTest.scala @@ -3,20 +3,15 @@ package org.jetbrains.sbtidea.instrumentation import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers -import java.lang.reflect.InvocationTargetException import java.net.URLClassLoader -import java.nio.charset.StandardCharsets -import java.nio.file.{Files, Path, Paths} -import javax.tools.ToolProvider -import scala.collection.JavaConverters.asScalaIteratorConverter +import java.nio.file.{Files, Path} /** * Modeled on IntelliJ IDEA's `com.intellij.java.compiler.notNullVerification.NotNullVerifyingInstrumenterTest`: * compile small annotated Java fixtures at test runtime, instrument the class files, load them - * in a throwaway classloader and assert on the thrown exceptions. Loading and invoking the - * instrumented classes also runs the JVM bytecode verifier over the rewritten methods. + * in a throwaway classloader and assert on the thrown exceptions. */ -class NotNullInstrumenterTest extends AnyFunSuite with Matchers { +class NotNullInstrumenterTest extends AnyFunSuite with Matchers with InstrumenterTestHarness { private val NotNullFqn = "org.jetbrains.annotations.NotNull" @@ -173,31 +168,10 @@ class NotNullInstrumenterTest extends AnyFunSuite with Matchers { invokeStatic(loadClass(dir, "KotlinLike"), "test", null) // no assertion is generated } - private def annotationsJar: Path = - Paths.get(classOf[org.jetbrains.annotations.NotNull].getProtectionDomain.getCodeSource.getLocation.toURI) - - private def compileFixture(sources: Map[String, String], debugInfo: Boolean = false): Path = { - val dir = Files.createTempDirectory("notnull-instrumentation-test") - val javaFiles = sources.map { case (relativePath, content) => - val file = dir.resolve(relativePath) - Files.createDirectories(file.getParent) - Files.write(file, content.getBytes(StandardCharsets.UTF_8)) - file.toString - }.toSeq - val args = Seq("-classpath", annotationsJar.toString, "-d", dir.toString) ++ - (if (debugInfo) Seq("-g") else Seq.empty) ++ - javaFiles - val exitCode = ToolProvider.getSystemJavaCompiler.run(null, null, null, args: _*) - assert(exitCode == 0, s"javac exited with code $exitCode") - dir - } - private def instrumentAll(dir: Path, annotations: Seq[String]): Unit = { val loader = new URLClassLoader(Array(dir.toUri.toURL, annotationsJar.toUri.toURL)) try { - val stream = Files.walk(dir) - val classFiles = try stream.iterator().asScala.filter(_.toString.endsWith(".class")).toList finally stream.close() - classFiles.foreach(NotNullInstrumenter.instrument(_, annotations, loader)) + classFiles(dir).foreach(NotNullInstrumenter.instrument(_, annotations, loader)) } finally { loader.close() } @@ -212,19 +186,4 @@ class NotNullInstrumenterTest extends AnyFunSuite with Matchers { instrumentAll(dir, annotations) dir } - - private def classLoader(dir: Path): ClassLoader = - new URLClassLoader(Array(dir.toUri.toURL), getClass.getClassLoader) - - private def loadClass(dir: Path, name: String): Class[_] = - classLoader(dir).loadClass(name) - - private def invokeStatic(clazz: Class[_], methodName: String, args: AnyRef*): AnyRef = { - val method = clazz.getMethods.find(_.getName == methodName) - .getOrElse(fail(s"Method $methodName not found in ${clazz.getName}")) - method.invoke(null, args: _*) - } - - private def interceptCause(clazz: Class[_], methodName: String, args: AnyRef*): Throwable = - intercept[InvocationTargetException](invokeStatic(clazz, methodName, args: _*)).getCause } diff --git a/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenterTest.scala b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenterTest.scala new file mode 100644 index 00000000..4338d325 --- /dev/null +++ b/ideaSupport/src/test/scala/org/jetbrains/sbtidea/instrumentation/ThreadingAnnotationInstrumenterTest.scala @@ -0,0 +1,207 @@ +package org.jetbrains.sbtidea.instrumentation + +import org.objectweb.asm.{ClassReader, ClassVisitor, Label, MethodVisitor, Opcodes} +import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.matchers.should.Matchers + +import java.nio.file.{Files, Path} +import scala.collection.JavaConverters.asScalaBufferConverter +import scala.collection.mutable + +/** + * Tests the threading annotation instrumentation against stub versions of the IntelliJ Platform + * annotation and assertion classes, compiled at test runtime. The stub `ThreadingAssertions` + * methods record their invocations, so the tests can assert exactly which assertion was + * generated for which annotation. + */ +class ThreadingAnnotationInstrumenterTest extends AnyFunSuite with Matchers with InstrumenterTestHarness { + + test("each threading annotation generates a call to its ThreadingAssertions method") { + val dir = compileAndInstrument( + "Annotated.java" -> + """import com.intellij.util.concurrency.annotations.*; + |public class Annotated { + | @RequiresEdt + | public static void edt() {} + | @RequiresBackgroundThread + | public static void background() {} + | @RequiresReadLock + | public static void readLock() {} + | @RequiresReadLockAbsence + | public static void readLockAbsence() {} + | @RequiresWriteLock + | public static void writeLock() {} + |}""".stripMargin + ) + val loader = classLoader(dir) + val clazz = loader.loadClass("Annotated") + Seq("edt", "background", "readLock", "readLockAbsence", "writeLock") + .foreach(clazz.getMethod(_).invoke(null)) + recordedCalls(loader) shouldBe Seq( + "assertEventDispatchThread", + "assertBackgroundThread", + "softAssertReadAccess", + "assertNoReadAccess", + "assertWriteAccess" + ) + } + + test("the assertion is generated before the method body") { + val dir = compileAndInstrument( + "BodyOrder.java" -> + """import com.intellij.util.concurrency.ThreadingAssertions; + |import com.intellij.util.concurrency.annotations.RequiresEdt; + |public class BodyOrder { + | @RequiresEdt + | public static void test() { ThreadingAssertions.calls.add("body"); } + |}""".stripMargin + ) + val loader = classLoader(dir) + loader.loadClass("BodyOrder").getMethod("test").invoke(null) + recordedCalls(loader) shouldBe Seq("assertEventDispatchThread", "body") + } + + test("generateAssertion = false disables the instrumentation") { + val dir = compileThreadingFixture( + "OptedOut.java" -> + """import com.intellij.util.concurrency.annotations.RequiresEdt; + |public class OptedOut { + | @RequiresEdt(generateAssertion = false) + | public static void test() {} + |}""".stripMargin + ) + val classFile = dir.resolve("OptedOut.class") + val before = Files.readAllBytes(classFile) + ThreadingAnnotationInstrumenter.instrument(classFile) + val after = Files.readAllBytes(classFile) + assert(java.util.Arrays.equals(before, after), "class file must not be modified") + val loader = classLoader(dir) + loader.loadClass("OptedOut").getMethod("test").invoke(null) + recordedCalls(loader) shouldBe empty + } + + test("classes without threading annotations are not modified") { + val dir = compileThreadingFixture( + "Unannotated.java" -> + """public class Unannotated { + | public static void test() {} + |}""".stripMargin + ) + val classFile = dir.resolve("Unannotated.class") + val before = Files.readAllBytes(classFile) + ThreadingAnnotationInstrumenter.instrument(classFile) + val after = Files.readAllBytes(classFile) + assert(java.util.Arrays.equals(before, after), "class file must not be modified") + } + + test("the generated assertion is covered by a line number entry for the first line of the method") { + val dir = compileThreadingFixture( + "LineNumbers.java" -> + """import com.intellij.util.concurrency.ThreadingAssertions; + |import com.intellij.util.concurrency.annotations.RequiresEdt; + |public class LineNumbers { + | @RequiresEdt + | public static void test() { + | ThreadingAssertions.calls.add("body"); + | } + |}""".stripMargin + ) + val classFile = dir.resolve("LineNumbers.class") + + val originalEvents = methodCodeEvents(Files.readAllBytes(classFile), "test") + val firstLineOfMethod = originalEvents.collectFirst { case Line(number) => number } + .getOrElse(fail("the fixture must be compiled with line numbers")) + + ThreadingAnnotationInstrumenter.instrument(classFile) + + // The injected call must be covered by a line number entry pointing at the first line of the method, + // like in JPS builds spawned by IntelliJ IDEA (which pass -Dtmh.generate.line.numbers=true). + // Without that entry, the assertion call would precede the first line number of the method. + val instrumentedEvents = methodCodeEvents(Files.readAllBytes(classFile), "test") + instrumentedEvents.take(2) shouldBe Seq( + Line(firstLineOfMethod), + Invoke("com/intellij/util/concurrency/ThreadingAssertions", "assertEventDispatchThread") + ) + + // The rewritten class must still pass the JVM bytecode verifier and run. + val loader = classLoader(dir) + loader.loadClass("LineNumbers").getMethod("test").invoke(null) + recordedCalls(loader) shouldBe Seq("assertEventDispatchThread", "body") + } + + test("a method with two threading annotations gets only one assertion (matching IntelliJ's instrumenter)") { + val dir = compileAndInstrument( + "DoubleAnnotated.java" -> + """import com.intellij.util.concurrency.annotations.*; + |public class DoubleAnnotated { + | @RequiresEdt + | @RequiresWriteLock + | public static void test() {} + |}""".stripMargin + ) + val loader = classLoader(dir) + loader.loadClass("DoubleAnnotated").getMethod("test").invoke(null) + recordedCalls(loader) should have size 1 + } + + private val ThreadingAssertionsSource: (String, String) = + "com/intellij/util/concurrency/ThreadingAssertions.java" -> + """package com.intellij.util.concurrency; + |import java.util.ArrayList; + |import java.util.List; + |public class ThreadingAssertions { + | public static final List calls = new ArrayList<>(); + | public static void assertEventDispatchThread() { calls.add("assertEventDispatchThread"); } + | public static void assertBackgroundThread() { calls.add("assertBackgroundThread"); } + | public static void softAssertReadAccess() { calls.add("softAssertReadAccess"); } + | public static void assertNoReadAccess() { calls.add("assertNoReadAccess"); } + | public static void assertWriteAccess() { calls.add("assertWriteAccess"); } + |}""".stripMargin + + private val AnnotationSources: Seq[(String, String)] = + Seq("RequiresEdt", "RequiresBackgroundThread", "RequiresReadLock", "RequiresReadLockAbsence", "RequiresWriteLock") + .map { name => + s"com/intellij/util/concurrency/annotations/$name.java" -> + s"""package com.intellij.util.concurrency.annotations; + |import java.lang.annotation.*; + |@Retention(RetentionPolicy.CLASS) + |@Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) + |public @interface $name { + | boolean generateAssertion() default true; + |}""".stripMargin + } + + private def compileThreadingFixture(fixtureSources: (String, String)*): Path = + compileFixture((AnnotationSources ++ Seq(ThreadingAssertionsSource) ++ fixtureSources).toMap) + + private def compileAndInstrument(fixtureSources: (String, String)*): Path = { + val dir = compileThreadingFixture(fixtureSources: _*) + classFiles(dir).foreach(ThreadingAnnotationInstrumenter.instrument) + dir + } + + /** Reads the invocations recorded by the stub `ThreadingAssertions` in the given fixture classloader. */ + private def recordedCalls(loader: ClassLoader): Seq[String] = + loader.loadClass("com.intellij.util.concurrency.ThreadingAssertions") + .getField("calls").get(null).asInstanceOf[java.util.List[String]].asScala.toList + + private sealed trait CodeEvent + private case class Line(number: Int) extends CodeEvent + private case class Invoke(owner: String, name: String) extends CodeEvent + + /** Extracts line number entries and method invocations of a method's code, in bytecode order. */ + private def methodCodeEvents(classBytes: Array[Byte], methodName: String): Seq[CodeEvent] = { + val events = mutable.Buffer.empty[CodeEvent] + new ClassReader(classBytes).accept(new ClassVisitor(Opcodes.ASM9) { + override def visitMethod(access: Int, name: String, descriptor: String, signature: String, exceptions: Array[String]): MethodVisitor = + if (name != methodName) null + else new MethodVisitor(Opcodes.ASM9) { + override def visitLineNumber(line: Int, start: Label): Unit = + events += Line(line) + override def visitMethodInsn(opcode: Int, owner: String, name: String, descriptor: String, isInterface: Boolean): Unit = + events += Invoke(owner, name) + } + }, 0) + events.toList + } +}