8006225: tools/jdeps/Basic.java failes with AssertionError

Reviewed-by: alanb

langtools/src/share/classes/com/sun/tools/jdeps/Analyzer.java

@@ -0,0 +1,239 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.sun.tools.jdeps;
+
+import com.sun.tools.classfile.Dependency.Location;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.SortedSet;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+/**
+ * Dependency Analyzer.
+ */
+public class Analyzer {
+    /**
+     * Type of the dependency analysis.  Appropriate level of data
+     * will be stored.
+     */
+    public enum Type {
+        SUMMARY,
+        PACKAGE,
+        CLASS,
+        VERBOSE
+    };
+
+    private final Type type;
+    private final List<ArchiveDeps> results = new ArrayList<ArchiveDeps>();
+    private final Map<String, Archive> map = new HashMap<String, Archive>();
+    private final Archive NOT_FOUND
+        = new Archive(JdepsTask.getMessage("artifact.not.found"));
+
+    /**
+     * Constructs an Analyzer instance.
+     *
+     * @param type Type of the dependency analysis
+     */
+    public Analyzer(Type type) {
+        this.type = type;
+    }
+
+    /**
+     * Performs the dependency analysis on the given archives.
+     */
+    public void run(List<Archive> archives) {
+        for (Archive archive : archives) {
+            ArchiveDeps deps;
+            if (type == Type.CLASS || type == Type.VERBOSE) {
+                deps = new ClassVisitor(archive);
+            } else {
+                deps = new PackageVisitor(archive);
+            }
+            archive.visit(deps);
+            results.add(deps);
+        }
+
+        // set the required dependencies
+        for (ArchiveDeps result: results) {
+            for (Set<String> set : result.deps.values()) {
+                for (String target : set) {
+                    Archive source = getArchive(target);
+                    if (result.archive != source) {
+                        if (!result.requiredArchives.contains(source)) {
+                            result.requiredArchives.add(source);
+                        }
+                        // either a profile name or the archive name
+                        String tname = getProfile(target);
+                        if (tname.isEmpty()){
+                            tname = source.toString();
+                        }
+                        if (!result.targetNames.contains(tname)) {
+                            result.targetNames.add(tname);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    public interface Visitor {
+        /**
+         * Visits a recorded dependency from origin to target which can be
+         * a fully-qualified classname, a package name, a profile or
+         * archive name depending on the Analyzer's type.
+         */
+        void visit(String origin, String target);
+        /**
+         * Visits the source archive to its destination archive of
+         * a recorded dependency.
+         */
+        void visit(Archive source, Archive dest);
+    }
+
+    public void visitSummary(Visitor v) {
+        for (ArchiveDeps r : results) {
+            for (Archive a : r.requiredArchives) {
+                v.visit(r.archive, a);
+            }
+            for (String name : r.targetNames) {
+                v.visit(r.archive.getFileName(), name);
+            }
+        }
+    }
+
+    public void visit(Visitor v) {
+        for (ArchiveDeps r: results) {
+            for (Archive a : r.requiredArchives) {
+                v.visit(r.archive, a);
+            }
+            for (String origin : r.deps.keySet()) {
+                for (String target : r.deps.get(origin)) {
+                    // filter intra-dependency unless in verbose mode
+                    if (type == Type.VERBOSE || getArchive(origin) != getArchive(target)) {
+                        v.visit(origin, target);
+                    }
+                }
+            }
+        }
+    }
+
+    public Archive getArchive(String name) {
+        return map.containsKey(name) ? map.get(name) : NOT_FOUND;
+    }
+
+    public String getArchiveName(String name) {
+        return getArchive(name).getFileName();
+    }
+
+    public String getProfile(String name) {
+        String pn = type == Type.CLASS ? packageOf(name) : name;
+        Archive source = map.get(name);
+        if (source != null && PlatformClassPath.contains(source)) {
+            String profile = PlatformClassPath.getProfileName(pn);
+            if (profile.isEmpty()) {
+                return "JDK internal API (" + source.getFileName() + ")";
+            }
+            return profile;
+        }
+        return "";
+    }
+
+    private abstract class ArchiveDeps implements Archive.Visitor {
+        final Archive archive;
+        final Set<Archive> requiredArchives;
+        final SortedSet<String> targetNames;
+        final SortedMap<String, SortedSet<String>> deps;
+
+        ArchiveDeps(Archive archive) {
+            this.archive = archive;
+            this.requiredArchives = new HashSet<Archive>();
+            this.targetNames = new TreeSet<String>();
+            this.deps = new TreeMap<String, SortedSet<String>>();
+        }
+
+        void add(String loc) {
+            Archive a = map.get(loc);
+            if (a == null) {
+                map.put(loc, archive);
+            } else if (a != archive) {
+                // duplicated class warning?
+            }
+        }
+
+        void add(String origin, String target) {
+            SortedSet<String> set = deps.get(origin);
+            if (set == null) {
+                set = new TreeSet<String>();
+                deps.put(origin, set);
+            }
+            if (!set.contains(target)) {
+                set.add(target);
+            }
+        }
+
+        public abstract void visit(Location o, Location t);
+    }
+
+    private class ClassVisitor extends ArchiveDeps {
+        ClassVisitor(Archive archive) {
+            super(archive);
+        }
+        public void visit(Location l) {
+            add(l.getClassName());
+        }
+        public void visit(Location o, Location t) {
+            add(o.getClassName(), t.getClassName());
+        }
+    }
+
+    private class PackageVisitor extends ArchiveDeps {
+        PackageVisitor(Archive archive) {
+            super(archive);
+        }
+        public void visit(Location o, Location t) {
+            add(packageOf(o), packageOf(t));
+        }
+
+        public void visit(Location l) {
+            add(packageOf(l));
+        }
+
+        private String packageOf(Location loc) {
+            String pkg = loc.getPackageName();
+            return pkg.isEmpty() ? "<unnamed>" : pkg;
+        }
+    }
+
+    private static String packageOf(String cn) {
+        int i = cn.lastIndexOf('.');
+        return (i > 0) ? cn.substring(0, i) : "<unnamed>";
+    }
+}

langtools/src/share/classes/com/sun/tools/jdeps/Archive.java

@@ -24,43 +24,32 @@
  */
 package com.sun.tools.jdeps;
 
-import com.sun.tools.classfile.Dependency;
 import com.sun.tools.classfile.Dependency.Location;
 import java.io.File;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedMap;
-import java.util.SortedSet;
-import java.util.TreeMap;
-import java.util.TreeSet;
 
 /**
  * Represents the source of the class files.
  */
 public class Archive {
-    private static Map<String,Archive> archiveForClass = new HashMap<String,Archive>();
-    public static Archive find(Location loc) {
-        return archiveForClass.get(loc.getName());
-    }
-
     private final File file;
     private final String filename;
-    private final DependencyRecorder recorder;
     private final ClassFileReader reader;
+    private final Map<Location, Set<Location>> deps
+        = new HashMap<Location, Set<Location>>();
+
     public Archive(String name) {
         this.file = null;
         this.filename = name;
-        this.recorder = new DependencyRecorder();
         this.reader = null;
     }
 
     public Archive(File f, ClassFileReader reader) {
         this.file = f;
         this.filename = f.getName();
-        this.recorder = new DependencyRecorder();
         this.reader = reader;
     }
 
@@ -72,102 +61,37 @@ public class Archive {
         return filename;
     }
 
-    public void addClass(String classFileName) {
+    public void addClass(Location origin) {
-        Archive a = archiveForClass.get(classFileName);
+        Set<Location> set = deps.get(origin);
-        assert(a == null || a == this); // ## issue warning?
+        if (set == null) {
-        if (!archiveForClass.containsKey(classFileName)) {
+            set = new HashSet<Location>();
-            archiveForClass.put(classFileName, this);
+            deps.put(origin, set);
         }
     }
+    public void addClass(Location origin, Location target) {
+        Set<Location> set = deps.get(origin);
+        if (set == null) {
+            set = new HashSet<Location>();
+            deps.put(origin, set);
+        }
+        set.add(target);
+    }
 
-    public void addDependency(Dependency d) {
+    public void visit(Visitor v) {
-        recorder.addDependency(d);
+        for (Map.Entry<Location,Set<Location>> e: deps.entrySet()) {
-    }
+            v.visit(e.getKey());
-
+            for (Location target : e.getValue()) {
-    /**
+                v.visit(e.getKey(), target);
-     * Returns a sorted map of a class to its dependencies.
-     */
-    public SortedMap<Location, SortedSet<Location>> getDependencies() {
-        DependencyRecorder.Filter filter = new DependencyRecorder.Filter() {
-            public boolean accept(Location origin, Location target) {
-                 return (archiveForClass.get(origin.getName()) !=
-                            archiveForClass.get(target.getName()));
-        }};
-
-        SortedMap<Location, SortedSet<Location>> result =
-            new TreeMap<Location, SortedSet<Location>>(locationComparator);
-        for (Map.Entry<Location, Set<Location>> e : recorder.dependencies().entrySet()) {
-            Location o = e.getKey();
-            for (Location t : e.getValue()) {
-                if (filter.accept(o, t)) {
-                    SortedSet<Location> odeps = result.get(o);
-                    if (odeps == null) {
-                        odeps = new TreeSet<Location>(locationComparator);
-                        result.put(o, odeps);
-                    }
-                    odeps.add(t);
             }
         }
     }
-        return result;
-    }
-
-    /**
-     * Returns the set of archives this archive requires.
-     */
-    public Set<Archive> getRequiredArchives() {
-        SortedSet<Archive> deps = new TreeSet<Archive>(new Comparator<Archive>() {
-            public int compare(Archive a1, Archive a2) {
-                return a1.toString().compareTo(a2.toString());
-            }
-        });
-
-        for (Map.Entry<Location, Set<Location>> e : recorder.dependencies().entrySet()) {
-            Location o = e.getKey();
-            Archive origin = Archive.find(o);
-            for (Location t : e.getValue()) {
-                Archive target = Archive.find(t);
-                assert(origin != null && target != null);
-                if (origin != target) {
-                    if (!deps.contains(target)) {
-                        deps.add(target);
-                    }
-                }
-            }
-        }
-        return deps;
-    }
 
     public String toString() {
         return file != null ? file.getPath() : filename;
     }
 
-    private static class DependencyRecorder {
+    interface Visitor {
-        static interface Filter {
+        void visit(Location loc);
-            boolean accept(Location origin, Location target);
+        void visit(Location origin, Location target);
     }
-
-        public void addDependency(Dependency d) {
-            Set<Location> odeps = map.get(d.getOrigin());
-            if (odeps == null) {
-                odeps = new HashSet<Location>();
-                map.put(d.getOrigin(), odeps);
-            }
-            odeps.add(d.getTarget());
-        }
-
-        public Map<Location, Set<Location>> dependencies() {
-            return map;
-        }
-
-        private final Map<Location, Set<Location>> map =
-            new HashMap<Location, Set<Location>>();
-    }
-
-    private static Comparator<Location> locationComparator =
-        new Comparator<Location>() {
-            public int compare(Location o1, Location o2) {
-                return o1.toString().compareTo(o2.toString());
-            }
-        };
 }

langtools/src/share/classes/com/sun/tools/jdeps/JdepsTask.java

@@ -29,7 +29,6 @@ import com.sun.tools.classfile.ConstantPoolException;
 import com.sun.tools.classfile.Dependencies;
 import com.sun.tools.classfile.Dependencies.ClassFileError;
 import com.sun.tools.classfile.Dependency;
-import com.sun.tools.classfile.Dependency.Location;
 import java.io.*;
 import java.text.MessageFormat;
 import java.util.*;
@@ -42,7 +41,7 @@ class JdepsTask {
     class BadArgs extends Exception {
         static final long serialVersionUID = 8765093759964640721L;
         BadArgs(String key, Object... args) {
-            super(JdepsTask.this.getMessage(key, args));
+            super(JdepsTask.getMessage(key, args));
             this.key = key;
             this.args = args;
         }
@@ -105,24 +104,21 @@ class JdepsTask {
         new Option(false, "-s", "--summary") {
             void process(JdepsTask task, String opt, String arg) {
                 task.options.showSummary = true;
-                task.options.verbose = Options.Verbose.SUMMARY;
+                task.options.verbose = Analyzer.Type.SUMMARY;
             }
         },
         new Option(false, "-v", "--verbose") {
             void process(JdepsTask task, String opt, String arg) {
-                task.options.verbose = Options.Verbose.VERBOSE;
+                task.options.verbose = Analyzer.Type.VERBOSE;
             }
         },
         new Option(true, "-V", "--verbose-level") {
             void process(JdepsTask task, String opt, String arg) throws BadArgs {
-                switch (arg) {
+                if ("package".equals(arg)) {
-                    case "package":
+                    task.options.verbose = Analyzer.Type.PACKAGE;
-                        task.options.verbose = Options.Verbose.PACKAGE;
+                } else if ("class".equals(arg)) {
-                        break;
+                    task.options.verbose = Analyzer.Type.CLASS;
-                    case "class":
+                } else {
-                        task.options.verbose = Options.Verbose.CLASS;
-                        break;
-                    default:
                     throw task.new BadArgs("err.invalid.arg.for.option", opt);
                 }
             }
@@ -171,7 +167,6 @@ class JdepsTask {
                 task.options.fullVersion = true;
             }
         },
-
     };
 
     private static final String PROGNAME = "jdeps";
@@ -216,7 +211,7 @@ class JdepsTask {
                 showHelp();
                 return EXIT_CMDERR;
             }
-            if (options.showSummary && options.verbose != Options.Verbose.SUMMARY) {
+            if (options.showSummary && options.verbose != Analyzer.Type.SUMMARY) {
                 showHelp();
                 return EXIT_CMDERR;
             }
@@ -236,26 +231,14 @@ class JdepsTask {
     }
 
     private final List<Archive> sourceLocations = new ArrayList<Archive>();
-    private final Archive NOT_FOUND = new Archive(getMessage("artifact.not.found"));
     private boolean run() throws IOException {
         findDependencies();
-        switch (options.verbose) {
+        Analyzer analyzer = new Analyzer(options.verbose);
-            case VERBOSE:
+        analyzer.run(sourceLocations);
-            case CLASS:
+        if (options.verbose == Analyzer.Type.SUMMARY) {
-                printClassDeps(log);
+            printSummary(log, analyzer);
-                break;
+        } else {
-            case PACKAGE:
+            printDependencies(log, analyzer);
-                printPackageDeps(log);
-                break;
-            case SUMMARY:
-                for (Archive origin : sourceLocations) {
-                    for (Archive target : origin.getRequiredArchives()) {
-                        log.format("%-30s -> %s%n", origin, target);
-                    }
-                }
-                break;
-            default:
-                throw new InternalError("Should not reach here");
         }
         return true;
     }
@@ -331,7 +314,7 @@ class JdepsTask {
                 } catch (ConstantPoolException e) {
                     throw new ClassFileError(e);
                 }
-                a.addClass(classFileName);
+
                 if (!doneClasses.contains(classFileName)) {
                     doneClasses.add(classFileName);
                 }
@@ -341,7 +324,7 @@ class JdepsTask {
                         if (!doneClasses.contains(cn) && !deque.contains(cn)) {
                             deque.add(cn);
                         }
-                        a.addDependency(d);
+                        a.addClass(d.getOrigin(), d.getTarget());
                     }
                 }
             }
@@ -367,20 +350,21 @@ class JdepsTask {
                         } catch (ConstantPoolException e) {
                             throw new ClassFileError(e);
                         }
-                        a.addClass(classFileName);
                         if (!doneClasses.contains(classFileName)) {
                             // if name is a fully-qualified class name specified
                             // from command-line, this class might already be parsed
                             doneClasses.add(classFileName);
-                            if (depth > 0) {
                             for (Dependency d : finder.findDependencies(cf)) {
-                                    if (filter.accepts(d)) {
+                                if (depth == 0) {
+                                    // ignore the dependency
+                                    a.addClass(d.getOrigin());
+                                    break;
+                                } else if (filter.accepts(d)) {
+                                    a.addClass(d.getOrigin(), d.getTarget());
                                     String cn = d.getTarget().getName();
                                     if (!doneClasses.contains(cn) && !deque.contains(cn)) {
                                         deque.add(cn);
                                     }
-                                        a.addDependency(d);
-                                    }
                                 }
                             }
                         }
@@ -388,7 +372,7 @@ class JdepsTask {
                     }
                 }
                 if (cf == null) {
-                    NOT_FOUND.addClass(name);
+                    doneClasses.add(name);
                 }
             }
             unresolved = deque;
@@ -396,96 +380,44 @@ class JdepsTask {
         } while (!unresolved.isEmpty() && depth-- > 0);
     }
 
-    private void printPackageDeps(PrintWriter out) {
+    private void printSummary(final PrintWriter out, final Analyzer analyzer) {
-        for (Archive source : sourceLocations) {
+        Analyzer.Visitor visitor = new Analyzer.Visitor() {
-            SortedMap<Location, SortedSet<Location>> deps = source.getDependencies();
+            public void visit(String origin, String profile) {
-            if (deps.isEmpty())
+                if (options.showProfile) {
-                continue;
+                    out.format("%-30s -> %s%n", origin, profile);
-
+                }
-            for (Archive target : source.getRequiredArchives()) {
+            }
-                out.format("%s -> %s%n", source, target);
+            public void visit(Archive origin, Archive target) {
+                if (!options.showProfile) {
+                    out.format("%-30s -> %s%n", origin, target);
+                }
+            }
+        };
+        analyzer.visitSummary(visitor);
     }
 
-            Map<String, Archive> pkgs = new TreeMap<String, Archive>();
+    private void printDependencies(final PrintWriter out, final Analyzer analyzer) {
-            SortedMap<String, Archive> targets = new TreeMap<String, Archive>();
+        Analyzer.Visitor visitor = new Analyzer.Visitor() {
-            String pkg = "";
+            private String pkg = "";
-            for (Map.Entry<Location, SortedSet<Location>> e : deps.entrySet()) {
+            public void visit(String origin, String target) {
-                String cn = e.getKey().getClassName();
+                if (!origin.equals(pkg)) {
-                String p = packageOf(e.getKey());
+                    pkg = origin;
-                Archive origin = Archive.find(e.getKey());
+                    out.format("   %s (%s)%n", origin, analyzer.getArchiveName(origin));
-                assert origin != null;
+                }
-                if (!pkgs.containsKey(p)) {
+                Archive source = analyzer.getArchive(target);
-                    pkgs.put(p, origin);
+                String profile = options.showProfile ? analyzer.getProfile(target) : "";
-                } else if (pkgs.get(p) != origin) {
+                out.format("      -> %-50s %s%n", target,
-                    warning("warn.split.package", p, origin, pkgs.get(p));
+                           PlatformClassPath.contains(source)
+                               ? profile
+                               : analyzer.getArchiveName(target));
+            }
+            public void visit(Archive origin, Archive target) {
+                out.format("%s -> %s%n", origin, target);
+            }
+        };
+        analyzer.visit(visitor);
     }
 
-                if (!p.equals(pkg)) {
-                    printTargets(out, targets);
-                    pkg = p;
-                    targets.clear();
-                    out.format("   %s (%s)%n", p, origin.getFileName());
-                }
-
-                for (Location t : e.getValue()) {
-                    p = packageOf(t);
-                    Archive target = Archive.find(t);
-                    if (!targets.containsKey(p)) {
-                        targets.put(p, target);
-                    }
-                }
-            }
-            printTargets(out, targets);
-            out.println();
-        }
-    }
-
-    private void printTargets(PrintWriter out, Map<String, Archive> targets) {
-        for (Map.Entry<String, Archive> t : targets.entrySet()) {
-            String pn = t.getKey();
-            out.format("      -> %-40s %s%n", pn, getPackageInfo(pn, t.getValue()));
-        }
-    }
-
-    private String getPackageInfo(String pn, Archive source) {
-        if (PlatformClassPath.contains(source)) {
-            String name = PlatformClassPath.getProfileName(pn);
-            if (name.isEmpty()) {
-                return "JDK internal API (" + source.getFileName() + ")";
-            }
-            return options.showProfile ? name : "";
-        }
-        return source.getFileName();
-    }
-
-    private static String packageOf(Location loc) {
-        String pkg = loc.getPackageName();
-        return pkg.isEmpty() ? "<unnamed>" : pkg;
-    }
-
-    private void printClassDeps(PrintWriter out) {
-        for (Archive source : sourceLocations) {
-            SortedMap<Location, SortedSet<Location>> deps = source.getDependencies();
-            if (deps.isEmpty())
-                continue;
-
-            for (Archive target : source.getRequiredArchives()) {
-                out.format("%s -> %s%n", source, target);
-            }
-            out.format("%s%n", source);
-            for (Map.Entry<Location, SortedSet<Location>> e : deps.entrySet()) {
-                String cn = e.getKey().getClassName();
-                Archive origin = Archive.find(e.getKey());
-                out.format("   %s (%s)%n", cn, origin.getFileName());
-                for (Location t : e.getValue()) {
-                    cn = t.getClassName();
-                    Archive target = Archive.find(t);
-                    out.format("      -> %-60s %s%n", cn, getPackageInfo(t.getPackageName(), target));
-                }
-            }
-            out.println();
-        }
-    }
     public void handleOptions(String[] args) throws BadArgs {
         // process options
         for (int i=0; i < args.length; i++) {
@@ -570,7 +502,7 @@ class JdepsTask {
         }
     }
 
-    public String getMessage(String key, Object... args) {
+    static String getMessage(String key, Object... args) {
         try {
             return MessageFormat.format(ResourceBundleHelper.bundle.getString(key), args);
         } catch (MissingResourceException e) {
@@ -579,13 +511,6 @@ class JdepsTask {
     }
 
     private static class Options {
-        enum Verbose {
-            CLASS,
-            PACKAGE,
-            SUMMARY,
-            VERBOSE
-        };
-
         boolean help;
         boolean version;
         boolean fullVersion;
@@ -596,7 +521,7 @@ class JdepsTask {
         String regex;
         String classpath = "";
         int depth = 1;
-        Verbose verbose = Verbose.PACKAGE;
+        Analyzer.Type verbose = Analyzer.Type.PACKAGE;
         Set<String> packageNames = new HashSet<String>();
     }
 

langtools/test/tools/jdeps/Basic.java

@@ -68,11 +68,15 @@ public class Basic {
         test(new File(testDir, "Test.class"),
              new String[] {"java.lang"},
              new String[] {"-V", "package", "-e", "java\\.lang\\..*"});
-        // test -classpath and -all options
+        // test -classpath and wildcard options
         test(null,
              new String[] {"com.sun.tools.jdeps", "java.lang", "java.util",
-                           "java.util.regex", "java.io", "p"},
+                           "java.util.regex", "java.io"},
              new String[] {"--classpath", testDir.getPath(), "*"});
+        // -v shows intra-dependency
+        test(new File(testDir, "Test.class"),
+             new String[] {"java.lang.Object", "p.Foo"},
+             new String[] {"-v", "--classpath", testDir.getPath(), "Test.class"});
         return errors;
     }