Collect parsing errors to report them at the end

Bug: 403564102
Flag: EXEMPT code processing tool changes
Test: atest com.android.protolog.tool
Change-Id: Idd3a0623d41b2ebd529ab1dc5e9781ec1165ca5b

7 files changed, 148 insertions, 66 deletions

diff --git a/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessor.kt b/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessor.kt
index 47724b7a9e1d..03c3a15562a9 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessor.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessor.kt
@@ -24,5 +24,5 @@ interface ProtoLogCallProcessor {
         logCallVisitor: ProtoLogCallVisitor?,
         otherCallVisitor: MethodCallVisitor?,
         fileName: String
-    ): CompilationUnit
+    ): Collection<CodeProcessingException>
 }
diff --git a/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessorImpl.kt b/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessorImpl.kt
index 1f6db5fe2d37..44db2ba45845 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessorImpl.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessorImpl.kt
@@ -74,7 +74,7 @@ class ProtoLogCallProcessorImpl(
     }
 
     fun process(code: CompilationUnit, logCallVisitor: ProtoLogCallVisitor?, fileName: String):
-            CompilationUnit {
+            Collection<CodeProcessingException> {
         return process(code, logCallVisitor, null, fileName)
     }
 
@@ -83,7 +83,7 @@ class ProtoLogCallProcessorImpl(
         logCallVisitor: ProtoLogCallVisitor?,
         otherCallVisitor: MethodCallVisitor?,
         fileName: String
-    ): CompilationUnit {
+    ): Collection<CodeProcessingException> {
         CodeUtils.checkWildcardStaticImported(code, protoLogClassName, fileName)
         CodeUtils.checkWildcardStaticImported(code, protoLogGroupClassName, fileName)
 
@@ -93,50 +93,63 @@ class ProtoLogCallProcessorImpl(
                 protoLogGroupClassName)
         val staticGroupImports = CodeUtils.staticallyImportedMethods(code, protoLogGroupClassName)
 
+        val errors = mutableListOf<CodeProcessingException>()
         code.findAll(MethodCallExpr::class.java)
                 .filter { call ->
                     isProtoCall(call, isLogClassImported, staticLogImports)
                 }.forEach { call ->
                     val context = ParsingContext(fileName, call)
 
-                    val logMethods = LogLevel.entries.map { it.shortCode }
-                    if (logMethods.contains(call.name.id)) {
-                        // Process a log call
-                        if (call.arguments.size < 2) {
-                            throw InvalidProtoLogCallException("Method signature does not match " +
-                                    "any ProtoLog method: $call", context)
-                        }
+                    try {
+                        val logMethods = LogLevel.entries.map { it.shortCode }
+                        if (logMethods.contains(call.name.id)) {
+                            // Process a log call
+                            if (call.arguments.size < 2) {
+                                errors.add(InvalidProtoLogCallException(
+                                    "Method signature does not match " +
+                                            "any ProtoLog method: $call", context
+                                ))
+                                return@forEach
+                            }
 
-                        val messageString = CodeUtils.concatMultilineString(call.getArgument(1),
-                            context)
-                        val groupNameArg = call.getArgument(0)
-                        val groupName =
-                            getLogGroupName(groupNameArg, isGroupClassImported,
-                                staticGroupImports, fileName)
-                        if (groupName !in groupMap) {
-                            throw InvalidProtoLogCallException("Unknown group argument " +
-                                    "- not a ProtoLogGroup enum member: $call", context)
-                        }
+                            val messageString = CodeUtils.concatMultilineString(
+                                call.getArgument(1),
+                                context
+                            )
+                            val groupNameArg = call.getArgument(0)
+                            val groupName =
+                                getLogGroupName(
+                                    groupNameArg, isGroupClassImported,
+                                    staticGroupImports, fileName
+                                )
+                            if (groupName !in groupMap) {
+                                errors.add(InvalidProtoLogCallException(
+                                    "Unknown group argument " +
+                                            "- not a ProtoLogGroup enum member: $call", context
+                                ))
+                                return@forEach
+                            }
 
-                        try {
                             logCallVisitor?.processCall(
                                 call, messageString, getLevelForMethodName(
                                     call.name.toString(), call, context
                                 ), groupMap.getValue(groupName),
                                 context.lineNumber
                             )
-                        } catch (e: Throwable) {
-                            throw InvalidProtoLogCallException("Error processing log call: $call",
-                                context, e)
+                        } else if (call.name.id == "init") {
+                            // No processing
+                        } else {
+                            // Process non-log message calls
+                            otherCallVisitor?.processCall(call)
                         }
-                    } else if (call.name.id == "init") {
-                        // No processing
-                    } else {
-                        // Process non-log message calls
-                        otherCallVisitor?.processCall(call)
+                    } catch (e: Throwable) {
+                        errors.add(InvalidProtoLogCallException(
+                            "Error processing log call: $call",
+                            context, e
+                        ))
                     }
                 }
-        return code
+        return errors
     }
 
     private fun getLevelForMethodName(
diff --git a/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt b/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
index d8a2954545bb..e9f2e81b69db 100644
--- a/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/ProtoLogTool.kt
@@ -130,14 +130,16 @@ object ProtoLogTool {
                     val outSrc = try {
                         val code = tryParse(text, path)
                         if (containsProtoLogText(text, PROTOLOG_CLASS_NAME)) {
-                            transformer.processClass(text, path, packagePath(file, code), code)
+                            val (processedText, errors) = transformer.processClass(text, path, packagePath(file, code), code)
+                            errors.forEach { injector.reportProcessingError(it) }
+                            processedText
                         } else {
                             text
                         }
                     } catch (ex: ParsingException) {
                         // If we cannot parse this file, skip it (and log why). Compilation will
                         // fail in a subsequent build step.
-                        injector.reportParseError(ex)
+                        injector.reportProcessingError(ex)
                         text
                     }
                     path to outSrc
@@ -405,7 +407,7 @@ object ProtoLogTool {
                         } catch (ex: ParsingException) {
                             // If we cannot parse this file, skip it (and log why). Compilation will
                             // fail in a subsequent build step.
-                            injector.reportParseError(ex)
+                            injector.reportProcessingError(ex)
                             null
                         }
                     } else {
@@ -466,6 +468,14 @@ object ProtoLogTool {
         try {
             val command = CommandOptions(args)
             invoke(command)
+
+            if (injector.processingErrors.isNotEmpty()) {
+                injector.processingErrors.forEachIndexed { index, it ->
+                    println("CodeProcessingException " +
+                            "(${index + 1}/${injector.processingErrors.size}): \n${it.message}\n")
+                }
+                exitProcess(1)
+            }
         } catch (ex: InvalidCommandException) {
             println("InvalidCommandException: \n${ex.message}\n")
             showHelpAndExit()
@@ -489,12 +499,14 @@ object ProtoLogTool {
     }
 
     var injector = object : Injector {
+        override val processingErrors: MutableList<CodeProcessingException>
+            get() = mutableListOf()
         override fun fileOutputStream(file: String) = FileOutputStream(file)
         override fun readText(file: File) = file.readText()
         override fun readLogGroups(jarPath: String, className: String) =
                 ProtoLogGroupReader().loadFromJar(jarPath, className)
-        override fun reportParseError(ex: ParsingException) {
-            println("\n${ex.message}\n")
+        override fun reportProcessingError(ex: CodeProcessingException) {
+            processingErrors.add(ex)
         }
     }
 
@@ -502,7 +514,8 @@ object ProtoLogTool {
         fun fileOutputStream(file: String): OutputStream
         fun readText(file: File): String
         fun readLogGroups(jarPath: String, className: String): Map<String, LogGroup>
-        fun reportParseError(ex: ParsingException)
+        fun reportProcessingError(ex: CodeProcessingException)
+        val processingErrors: Collection<CodeProcessingException>
     }
 }
 
diff --git a/tools/protologtool/src/com/android/protolog/tool/SourceTransformer.kt b/tools/protologtool/src/com/android/protolog/tool/SourceTransformer.kt
index 76df0674df65..e97b0dd9a04b 100644
--- a/tools/protologtool/src/com/android/protolog/tool/SourceTransformer.kt
+++ b/tools/protologtool/src/com/android/protolog/tool/SourceTransformer.kt
@@ -66,13 +66,13 @@ class SourceTransformer(
         packagePath: String,
         compilationUnit: CompilationUnit =
             StaticJavaParser.parse(code)
-    ): String {
+    ): Pair<String, Collection<CodeProcessingException>> {
         this.path = path
         this.packagePath = packagePath
         processedCode = code.split('\n').toMutableList()
         offsets = IntArray(processedCode.size)
-        protoLogCallProcessor.process(compilationUnit, protoLogCallVisitor, otherCallVisitor, path)
-        return processedCode.joinToString("\n")
+        val processingErrors = protoLogCallProcessor.process(compilationUnit, protoLogCallVisitor, otherCallVisitor, path)
+        return Pair(processedCode.joinToString("\n"), processingErrors)
     }
 
     private val protoLogImplClassNode =
diff --git a/tools/protologtool/tests/com/android/protolog/tool/EndToEndTest.kt b/tools/protologtool/tests/com/android/protolog/tool/EndToEndTest.kt
index 0cbbd483fe59..71bab3ebf2db 100644
--- a/tools/protologtool/tests/com/android/protolog/tool/EndToEndTest.kt
+++ b/tools/protologtool/tests/com/android/protolog/tool/EndToEndTest.kt
@@ -17,6 +17,7 @@
 package com.android.protolog.tool
 
 import com.android.protolog.tool.ProtoLogTool.PROTOLOG_IMPL_SRC_PATH
+import com.android.protolog.tool.ProtoLogTool.injector
 import com.google.common.truth.Truth
 import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
@@ -102,6 +103,42 @@ class EndToEndTest {
         """.trimIndent())
     }
 
+    @Test
+    fun e2e_transform_withErrors() {
+        val srcs = mapOf(
+            "frameworks/base/org/example/Example.java" to """
+                    package org.example;
+                    import com.android.internal.protolog.ProtoLog;
+                    import static com.android.internal.protolog.ProtoLogGroup.GROUP;
+
+                    class Example {
+                        void method() {
+                            String argString = "hello";
+                            int argInt = 123;
+                            ProtoLog.d(GROUP, "Invalid format: %s %d %9 %", argString, argInt);
+                        }
+                    }
+                """.trimIndent())
+        val output = run(
+            srcs = srcs,
+            logGroup = LogGroup("GROUP", true, false, "TAG_GROUP"),
+            commandOptions = CommandOptions(arrayOf("transform-protolog-calls",
+                "--protolog-class", "com.android.internal.protolog.ProtoLog",
+                "--loggroups-class", "com.android.internal.protolog.ProtoLogGroup",
+                "--loggroups-jar", "not_required.jar",
+                "--viewer-config-file-path", "not_required.pb",
+                "--output-srcjar", "out.srcjar",
+                "frameworks/base/org/example/Example.java"))
+        )
+        val outSrcJar = assertLoadSrcJar(output, "out.srcjar")
+        // No change to source code on failure to process
+        Truth.assertThat(outSrcJar["frameworks/base/org/example/Example.java"])
+            .contains(srcs["frameworks/base/org/example/Example.java"])
+
+        Truth.assertThat(injector.processingErrors).hasSize(1)
+        Truth.assertThat(injector.processingErrors.first().message).contains("Invalid format")
+    }
+
     private fun assertLoadSrcJar(
         outputs: Map<String, ByteArray>,
         path: String
@@ -172,7 +209,11 @@ class EndToEndTest {
             override fun readLogGroups(jarPath: String, className: String) = mapOf(
                     logGroup.name to logGroup)
 
-            override fun reportParseError(ex: ParsingException) = throw AssertionError(ex)
+            override fun reportProcessingError(ex: CodeProcessingException) {
+                processingErrors.add(ex)
+            }
+
+            override val processingErrors: MutableList<CodeProcessingException> = mutableListOf()
         }
 
         ProtoLogTool.invoke(commandOptions)
diff --git a/tools/protologtool/tests/com/android/protolog/tool/ProtoLogCallProcessorImplTest.kt b/tools/protologtool/tests/com/android/protolog/tool/ProtoLogCallProcessorImplTest.kt
index 732824ae841c..8f765aecb431 100644
--- a/tools/protologtool/tests/com/android/protolog/tool/ProtoLogCallProcessorImplTest.kt
+++ b/tools/protologtool/tests/com/android/protolog/tool/ProtoLogCallProcessorImplTest.kt
@@ -21,7 +21,6 @@ import com.android.internal.protolog.common.LogLevel
 import com.github.javaparser.StaticJavaParser
 import com.github.javaparser.ast.expr.MethodCallExpr
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertThrows
 import org.junit.Test
 import com.google.common.truth.Truth
 
@@ -124,7 +123,7 @@ class ProtoLogCallProcessorImplTest {
         checkCalls()
     }
 
-    @Test(expected = InvalidProtoLogCallException::class)
+    @Test
     fun process_groupNotImported() {
         val code = """
             package org.example2;
@@ -138,7 +137,10 @@ class ProtoLogCallProcessorImplTest {
             }
         """
         groupMap["TEST"] = LogGroup("TEST", true, true, "WindowManager")
-        visitor.process(StaticJavaParser.parse(code), processor, "")
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "")
+
+        Truth.assertThat(errors).hasSize(1)
+        Truth.assertThat(errors.first()).isInstanceOf(InvalidProtoLogCallException::class.java)
     }
 
     @Test
@@ -159,7 +161,7 @@ class ProtoLogCallProcessorImplTest {
         assertEquals(0, calls.size)
     }
 
-    @Test(expected = InvalidProtoLogCallException::class)
+    @Test
     fun process_unknownGroup() {
         val code = """
             package org.example;
@@ -170,10 +172,13 @@ class ProtoLogCallProcessorImplTest {
                 }
             }
         """
-        visitor.process(StaticJavaParser.parse(code), processor, "")
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "")
+
+        Truth.assertThat(errors).hasSize(1)
+        Truth.assertThat(errors.first()).isInstanceOf(InvalidProtoLogCallException::class.java)
     }
 
-    @Test(expected = InvalidProtoLogCallException::class)
+    @Test
     fun process_staticGroup() {
         val code = """
             package org.example;
@@ -184,10 +189,13 @@ class ProtoLogCallProcessorImplTest {
                 }
             }
         """
-        visitor.process(StaticJavaParser.parse(code), processor, "")
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "")
+
+        Truth.assertThat(errors).hasSize(1)
+        Truth.assertThat(errors.first()).isInstanceOf(InvalidProtoLogCallException::class.java)
     }
 
-    @Test(expected = InvalidProtoLogCallException::class)
+    @Test
     fun process_badGroup() {
         val code = """
             package org.example;
@@ -198,10 +206,13 @@ class ProtoLogCallProcessorImplTest {
                 }
             }
         """
-        visitor.process(StaticJavaParser.parse(code), processor, "")
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "")
+
+        Truth.assertThat(errors).hasSize(1)
+        Truth.assertThat(errors.first()).isInstanceOf(InvalidProtoLogCallException::class.java)
     }
 
-    @Test(expected = InvalidProtoLogCallException::class)
+    @Test
     fun process_invalidSignature() {
         val code = """
             package org.example;
@@ -212,7 +223,10 @@ class ProtoLogCallProcessorImplTest {
                 }
             }
         """
-        visitor.process(StaticJavaParser.parse(code), processor, "")
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "")
+
+        Truth.assertThat(errors).hasSize(1)
+        Truth.assertThat(errors.first()).isInstanceOf(InvalidProtoLogCallException::class.java)
     }
 
     @Test
@@ -257,9 +271,10 @@ class ProtoLogCallProcessorImplTest {
             }
         }
 
-        val exception = assertThrows(InvalidProtoLogCallException::class.java) {
-            visitor.process(StaticJavaParser.parse(code), processor, "MyTestFile.java")
-        }
+        val errors = visitor.process(StaticJavaParser.parse(code), processor, "MyTestFile.java")
+        Truth.assertThat(errors).hasSize(1)
+
+        val exception = errors.first()
         Truth.assertThat(exception).hasMessageThat()
             .contains("Code processing error in MyTestFile.java:6")
         Truth.assertThat(exception.cause).hasMessageThat()
diff --git a/tools/protologtool/tests/com/android/protolog/tool/SourceTransformerTest.kt b/tools/protologtool/tests/com/android/protolog/tool/SourceTransformerTest.kt
index 674a907d68d9..271fc6064678 100644
--- a/tools/protologtool/tests/com/android/protolog/tool/SourceTransformerTest.kt
+++ b/tools/protologtool/tests/com/android/protolog/tool/SourceTransformerTest.kt
@@ -160,10 +160,10 @@ class SourceTransformerTest {
             visitor.processCall(code.findAll(MethodCallExpr::class.java)[0], "test %d %f",
                     LogLevel.WARN, LogGroup("TEST_GROUP", true, true, "WM_TEST"), 123)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {
@@ -201,10 +201,10 @@ class SourceTransformerTest {
             visitor.processCall(calls[2], "test %d %f",
                     LogLevel.WARN, LogGroup("TEST_GROUP", true, true, "WM_TEST"), 123)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE_MULTICALLS, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE_MULTICALLS, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {
@@ -238,10 +238,10 @@ class SourceTransformerTest {
                     "test %d %f abc %s\n test", LogLevel.WARN, LogGroup("TEST_GROUP",
                     true, true, "WM_TEST"), 123)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE_MULTILINE, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE_MULTILINE, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {
@@ -275,10 +275,10 @@ class SourceTransformerTest {
             visitor.processCall(code.findAll(MethodCallExpr::class.java)[0], "test",
                     LogLevel.WARN, LogGroup("TEST_GROUP", true, true, "WM_TEST"), 456)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE_NO_PARAMS, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE_NO_PARAMS, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {
@@ -309,10 +309,10 @@ class SourceTransformerTest {
             visitor.processCall(code.findAll(MethodCallExpr::class.java)[0], "test %d %f",
                     LogLevel.WARN, LogGroup("TEST_GROUP", true, false, "WM_TEST"), 789)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {
@@ -346,10 +346,10 @@ class SourceTransformerTest {
                     "test %d %f abc %s\n test", LogLevel.WARN, LogGroup("TEST_GROUP",
                     true, false, "WM_TEST"), 123)
 
-            invocation.arguments[0] as CompilationUnit
+            listOf<CodeProcessingException>()
         }
 
-        val out = sourceJarWriter.processClass(TEST_CODE_MULTILINE, PATH, PATH, code)
+        val (out, _) = sourceJarWriter.processClass(TEST_CODE_MULTILINE, PATH, PATH, code)
         code = StaticJavaParser.parse(out)
 
         val protoLogCalls = code.findAll(MethodCallExpr::class.java).filter {