[jnigen] Fix a bug where the package would either be fully resolved b…

…y sources or classes but not by both (#973)

.github/workflows/jnigen.yaml

@@ -408,7 +408,7 @@ jobs:
         run: |
           dart pub get
           dart run jni:setup
-          wget 'https://dart.dev/guides/language/specifications/DartLangSpec-v2.2.pdf'
+          wget 'https://raw.githubusercontent.com/dart-lang/site-www/a7f170389e210adc2aef810cc9a6fdbfa78059a5/src/guides/language/specifications/DartLangSpec-v2.2.pdf'
           dart run bin/pdf_info.dart DartLangSpec-v2.2.pdf
         working-directory: ./pkgs/jnigen/example/pdfbox_plugin/dart_example
       - name: Build flutter example for pdfbox_plugin

pkgs/jnigen/CHANGELOG.md

@@ -5,6 +5,8 @@
 - **Breaking Change** ([#792](https://github.com/dart-lang/native/issues/792)]):
   `static final String` fields get converted to `JString` getters instead of
   `static const String` fields in Dart.
+- Fixed a bug where a package would either be searched from sources or classes
+  but not both.
 
 ## 0.7.0
 

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/Main.java

@@ -15,10 +15,7 @@
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.LinkedHashMap;
-import java.util.List;
+import java.util.*;
 import javax.tools.DocumentationTool;
 import javax.tools.JavaFileObject;
 import javax.tools.ToolProvider;
@@ -36,7 +33,7 @@ public enum Backend {
 
   static SummarizerOptions options;
 
-  public static List<ClassDecl> runDocletWithClass(
+  public static Map<String, ClassDecl> runDocletWithClass(
       DocumentationTool javaDoc,
       Class<? extends Doclet> docletClass,
       List<JavaFileObject> fileObjects,
@@ -58,7 +55,7 @@ public static List<ClassDecl> runDocletWithClass(
     return SummarizerDoclet.getClasses();
   }
 
-  public static List<ClassDecl> runDoclet(
+  public static Map<String, ClassDecl> runDoclet(
       DocumentationTool javaDoc, List<JavaFileObject> javaFileObjects, SummarizerOptions options) {
     return runDocletWithClass(javaDoc, SummarizerDoclet.class, javaFileObjects, options);
   }
@@ -96,26 +93,14 @@ public static void main(String[] args) throws FileNotFoundException {
           sourceClasses, sourcePaths, javaDoc.getStandardFileManager(null, null, null));
     }
 
-    // remove found classes from binaryClasses, so that they don't need to be searched again.
-    // TODO: Tidy up this logic, move to ClassFinder class
-    for (var qualifiedName : options.args) {
-      if (sourceClasses.get(qualifiedName) != null) {
-        binaryClasses.remove(qualifiedName);
-      }
-    }
-
     if (options.backend != Backend.DOCLET) {
       ClassFinder.findJavaClasses(binaryClasses, classPaths);
     }
 
-    // remove duplicates (found as both source & binary), and determine if any class is not found.
     var notFound = new ArrayList<String>();
     for (var qualifiedName : options.args) {
       var foundSource = sourceClasses.get(qualifiedName) != null;
       var foundBinary = binaryClasses.get(qualifiedName) != null;
-      if (foundSource) {
-        binaryClasses.remove(qualifiedName);
-      }
       if (!foundBinary && !foundSource) {
         notFound.add(qualifiedName);
       }
@@ -131,20 +116,22 @@ public static void main(String[] args) throws FileNotFoundException {
 
     switch (options.backend) {
       case DOCLET:
-        JsonWriter.writeJSON(runDoclet(javaDoc, sourceFiles, options), output);
+        JsonWriter.writeJSON(runDoclet(javaDoc, sourceFiles, options).values(), output);
         break;
       case ASM:
-        JsonWriter.writeJSON(AsmSummarizer.run(classStreamProviders), output);
+        JsonWriter.writeJSON(AsmSummarizer.run(classStreamProviders).values(), output);
         break;
       case AUTO:
-        List<ClassDecl> decls = new ArrayList<>();
-        if (!sourceFiles.isEmpty()) {
-          decls.addAll(runDoclet(javaDoc, sourceFiles, options));
-        }
+        Map<String, ClassDecl> classes = new LinkedHashMap<>();
+        // Preferring DOCLET as the source of summary a class exists in
+        // both ASM and DOCLET.
         if (!classStreamProviders.isEmpty()) {
-          decls.addAll(AsmSummarizer.run(classStreamProviders));
+          classes.putAll(AsmSummarizer.run(classStreamProviders));
+        }
+        if (!sourceFiles.isEmpty()) {
+          classes.putAll(runDoclet(javaDoc, sourceFiles, options));
         }
-        JsonWriter.writeJSON(decls, output);
+        JsonWriter.writeJSON(classes.values(), output);
         break;
     }
   }

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/disasm/AsmClassVisitor.java

@@ -10,7 +10,6 @@
 import com.github.dart_lang.jnigen.apisummarizer.util.SkipException;
 import com.github.dart_lang.jnigen.apisummarizer.util.StreamUtil;
 import java.util.*;
-import java.util.stream.Collectors;
 import org.objectweb.asm.*;
 import org.objectweb.asm.signature.SignatureReader;
 
@@ -23,11 +22,11 @@ private static Param param(
     return param;
   }
 
-  public List<ClassDecl> getVisited() {
+  public Map<String, ClassDecl> getVisited() {
     return visited;
   }
 
-  List<ClassDecl> visited = new ArrayList<>();
+  Map<String, ClassDecl> visited = new LinkedHashMap<>();
   Stack<ClassDecl> visiting = new Stack<>();
 
   /// Actual access for the inner classes as originally defined.
@@ -64,14 +63,11 @@ public void visit(
   public void visitInnerClass(String name, String outerName, String innerName, int access) {
     var binaryName = name.replace('/', '.');
     actualAccess.put(binaryName, access);
-    var alreadyVisitedInnerClass =
-        visited.stream()
-            .filter(decl -> decl.binaryName.equals(binaryName))
-            .collect(Collectors.toList());
-    // If the order of visit is outer first inner second.
-    // We still want to correct the modifiers.
-    if (!alreadyVisitedInnerClass.isEmpty()) {
-      alreadyVisitedInnerClass.get(0).modifiers = TypeUtils.access(access);
+
+    if (visited.containsKey(binaryName)) {
+      // If the order of visit is outerClass-first, innerClass-second then only
+      // correct the modifiers.
+      visited.get(binaryName).modifiers = TypeUtils.access(access);
     }
   }
 
@@ -159,7 +155,8 @@ public AnnotationVisitor visitTypeAnnotation(
 
   @Override
   public void visitEnd() {
-    visited.add(popVisiting());
+    var classToAdd = popVisiting();
+    visited.put(classToAdd.binaryName, classToAdd);
   }
 
   private ClassDecl peekVisiting() {

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/disasm/AsmSummarizer.java

@@ -9,11 +9,12 @@
 import com.github.dart_lang.jnigen.apisummarizer.elements.ClassDecl;
 import com.github.dart_lang.jnigen.apisummarizer.util.InputStreamProvider;
 import java.util.List;
+import java.util.Map;
 import org.objectweb.asm.ClassReader;
 
 public class AsmSummarizer {
 
-  public static List<ClassDecl> run(List<InputStreamProvider> inputProviders) {
+  public static Map<String, ClassDecl> run(List<InputStreamProvider> inputProviders) {
     var visitor = new AsmClassVisitor();
     for (var provider : inputProviders) {
       var inputStream = provider.getInputStream();

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/doclet/SummarizerDoclet.java

@@ -38,11 +38,11 @@ public SourceVersion getSupportedSourceVersion() {
     return SourceVersion.RELEASE_11;
   }
 
-  public static List<ClassDecl> getClasses() {
+  public static Map<String, ClassDecl> getClasses() {
     return classes;
   }
 
-  public static List<ClassDecl> classes;
+  public static Map<String, ClassDecl> classes;
 
   @Override
   public boolean run(DocletEnvironment docletEnvironment) {
@@ -62,7 +62,7 @@ public static class SummaryCollector {
 
   public class SummarizingScanner extends ElementScanner9<Void, SummaryCollector> {
     List<Package> packages = new ArrayList<>();
-    List<ClassDecl> types = new ArrayList<>();
+    Map<String, ClassDecl> types = new LinkedHashMap<>();
     ElementBuilders builders = new ElementBuilders(utils);
 
     // Each element in collector is a stack
@@ -100,7 +100,8 @@ public Void visitType(TypeElement e, SummaryCollector collector) {
             var cls = builders.classDecl(e);
             collector.types.push(cls);
             super.visitType(e, collector);
-            types.add(collector.types.pop());
+            var puttingClass = collector.types.pop();
+            types.put(puttingClass.binaryName, puttingClass);
           } catch (SkipException skip) {
             Log.info("Skip type: %s", e.getQualifiedName());
           }

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/util/ClassFinder.java

@@ -63,7 +63,6 @@ public static Optional<List<String>> findClassAndChildren(
       classes.add(fqnWithSlashesSuffix);
       return Optional.of(classes);
     }
-
     // consider fqnWithSlashes as a directory
     List<String> children =
         entries.tailSet(fqnWithSlashesSlash).stream()
@@ -77,9 +76,9 @@ public static Optional<List<String>> findClassAndChildren(
   }
 
   public static <E> void findFilesInPath(
+      Map<String, List<E>> classes,
       String searchLocation,
       String suffix,
-      Map<String, List<E>> classes,
       Function<List<Path>, List<E>> mapper) {
     Path searchPath = Path.of(searchLocation);
 
@@ -142,7 +141,7 @@ public static <T> void find(
     for (var searchPath : searchPaths) {
       File searchFile = new File(searchPath);
       if (searchFile.isDirectory()) {
-        findFilesInPath(searchPath, suffix, classes, fileMapper);
+        findFilesInPath(classes, searchPath, suffix, fileMapper);
       } else if (searchFile.isFile() && searchPath.endsWith(".jar")) {
         var jarFile = wrapCheckedException(JarFile::new, searchPath);
         var useful = findFilesInJar(classes, jarFile, suffix, entryMapper);

pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/util/JsonWriter.java

@@ -6,10 +6,10 @@
 import com.github.dart_lang.jnigen.apisummarizer.elements.ClassDecl;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.List;
+import java.util.Collection;
 
 public class JsonWriter {
-  public static void writeJSON(List<ClassDecl> classes, OutputStream output) {
+  public static void writeJSON(Collection<ClassDecl> classes, OutputStream output) {
     var mapper = new ObjectMapper();
     Log.info("Writing JSON for %d classes", classes.size());
     mapper.enable(SerializationFeature.INDENT_OUTPUT);

pkgs/jnigen/lib/src/summary/summary.dart

@@ -189,7 +189,6 @@ Future<Classes> getSummary(Config config) async {
   if (json == null) {
     throw SummaryParseException('Expected JSON element from summarizer.');
   }
-  final list = json as List;
-  final classes = Classes.fromJson(list);
+  final classes = Classes.fromJson(json);
   return classes;
 }