6863465: javac doesn't detect circular subclass dependencies via qualified names

Summary: class inheritance circularity check should look at trees, not just symbols
Reviewed-by: jjg

src/share/classes/com/sun/tools/javac/comp/Check.java

@@ -60,6 +60,7 @@ public class Check {
     private final Names names;
     private final Log log;
     private final Symtab syms;
+    private final Enter enter;
     private final Infer infer;
     private final Types types;
     private final JCDiagnostic.Factory diags;
@@ -86,6 +87,7 @@ public class Check {
         names = Names.instance(context);
         log = Log.instance(context);
         syms = Symtab.instance(context);
+        enter = Enter.instance(context);
         infer = Infer.instance(context);
         this.types = Types.instance(context);
         diags = JCDiagnostic.Factory.instance(context);
@@ -1727,6 +1729,113 @@ public class Check {
             return undef;
         }
 
+    void checkNonCyclicDecl(JCClassDecl tree) {
+        CycleChecker cc = new CycleChecker();
+        cc.scan(tree);
+        if (!cc.errorFound && !cc.partialCheck) {
+            tree.sym.flags_field |= ACYCLIC;
+        }
+    }
+
+    class CycleChecker extends TreeScanner {
+
+        List<Symbol> seenClasses = List.nil();
+        boolean errorFound = false;
+        boolean partialCheck = false;
+
+        private void checkSymbol(DiagnosticPosition pos, Symbol sym) {
+            if (sym != null && sym.kind == TYP) {
+                Env<AttrContext> classEnv = enter.getEnv((TypeSymbol)sym);
+                if (classEnv != null) {
+                    DiagnosticSource prevSource = log.currentSource();
+                    try {
+                        log.useSource(classEnv.toplevel.sourcefile);
+                        scan(classEnv.tree);
+                    }
+                    finally {
+                        log.useSource(prevSource.getFile());
+                    }
+                } else if (sym.kind == TYP) {
+                    checkClass(pos, sym, List.<JCTree>nil());
+                }
+            } else {
+                //not completed yet
+                partialCheck = true;
+            }
+        }
+
+        @Override
+        public void visitSelect(JCFieldAccess tree) {
+            super.visitSelect(tree);
+            checkSymbol(tree.pos(), tree.sym);
+        }
+
+        @Override
+        public void visitIdent(JCIdent tree) {
+            checkSymbol(tree.pos(), tree.sym);
+        }
+
+        @Override
+        public void visitTypeApply(JCTypeApply tree) {
+            scan(tree.clazz);
+        }
+
+        @Override
+        public void visitTypeArray(JCArrayTypeTree tree) {
+            scan(tree.elemtype);
+        }
+
+        @Override
+        public void visitClassDef(JCClassDecl tree) {
+            List<JCTree> supertypes = List.nil();
+            if (tree.getExtendsClause() != null) {
+                supertypes = supertypes.prepend(tree.getExtendsClause());
+            }
+            if (tree.getImplementsClause() != null) {
+                for (JCTree intf : tree.getImplementsClause()) {
+                    supertypes = supertypes.prepend(intf);
+                }
+            }
+            checkClass(tree.pos(), tree.sym, supertypes);
+        }
+
+        void checkClass(DiagnosticPosition pos, Symbol c, List<JCTree> supertypes) {
+            if ((c.flags_field & ACYCLIC) != 0)
+                return;
+            if (seenClasses.contains(c)) {
+                errorFound = true;
+                noteCyclic(pos, (ClassSymbol)c);
+            } else if (!c.type.isErroneous()) {
+                try {
+                    seenClasses = seenClasses.prepend(c);
+                    if (c.type.tag == CLASS) {
+                        if (supertypes.nonEmpty()) {
+                            scan(supertypes);
+                        }
+                        else {
+                            ClassType ct = (ClassType)c.type;
+                            if (ct.supertype_field == null ||
+                                    ct.interfaces_field == null) {
+                                //not completed yet
+                                partialCheck = true;
+                                return;
+                            }
+                            checkSymbol(pos, ct.supertype_field.tsym);
+                            for (Type intf : ct.interfaces_field) {
+                                checkSymbol(pos, intf.tsym);
+                            }
+                        }
+                        if (c.owner.kind == TYP) {
+                            checkSymbol(pos, c.owner);
+                        }
+                    }
+                } finally {
+                    seenClasses = seenClasses.tail;
+                }
+            }
+        }
+    }
+
     /** Check for cyclic references. Issue an error if the
      *  symbol of the type referred to has a LOCKED flag set.
      *

src/share/classes/com/sun/tools/javac/comp/MemberEnter.java

@@ -927,7 +927,7 @@ public class MemberEnter extends JCTree.Visitor implements Completer {
                 tp.accept(new TypeAnnotate(baseEnv));
             tree.accept(new TypeAnnotate(env));
 
-            chk.checkNonCyclic(tree.pos(), c.type);
+            chk.checkNonCyclicDecl(tree);
 
             attr.attribTypeVariables(tree.typarams, baseEnv);
 

test/tools/javac/6863465/T6863465a.java

+/**
+ * @test /nodynamiccopyright/
+ * @bug     6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author  Maurizio Cimadamore
+ * @compile/fail/ref=T6863465a.out -XDrawDiagnostics T6863465a.java
+ */
+
+class T6863465a {
+    static class a { static interface b {} }
+    static class c extends a implements z.y {}
+    static class x { static interface y {} }
+    static class z extends x implements c.b {}
+}

test/tools/javac/6863465/T6863465a.out

+T6863465a.java:11:12: compiler.err.cyclic.inheritance: T6863465a.c
+1 error

test/tools/javac/6863465/T6863465b.java

+/**
+ * @test /nodynamiccopyright/
+ * @bug     6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author  Maurizio Cimadamore
+ * @compile/fail/ref=T6863465b.out -XDrawDiagnostics T6863465b.java
+ */
+
+class T6863465b {
+    static class a { static interface b { static interface d {} } }
+    static class c extends a implements z.y, z.d {}
+    static class x { static interface y {} }
+    static class z extends x implements c.b {}
+}

test/tools/javac/6863465/T6863465b.out

+T6863465b.java:11:12: compiler.err.cyclic.inheritance: T6863465b.c
+1 error

test/tools/javac/6863465/T6863465c.java

+/**
+ * @test /nodynamiccopyright/
+ * @bug     6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author  Maurizio Cimadamore
+ * @compile/fail/ref=T6863465c.out -XDrawDiagnostics T6863465c.java
+ */
+
+class T6863465c {
+    static class x { static interface y {} }
+    static class z extends x implements c.b {}
+    static class a { static interface b { static interface d {} } }
+    static class c extends a implements z.y, z.d {}
+}

test/tools/javac/6863465/T6863465c.out

+T6863465c.java:13:47: compiler.err.cant.resolve.location: kindname.class, d, , , kindname.class, T6863465c.z
+T6863465c.java:11:12: compiler.err.cyclic.inheritance: T6863465c.z
+2 errors

test/tools/javac/6863465/T6863465d.java

+/**
+ * @test /nodynamiccopyright/
+ * @bug     6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author  Maurizio Cimadamore
+ * @compile/fail/ref=T6863465d.out -XDrawDiagnostics T6863465d.java
+ */
+
+class T6863465d {
+    static class a { static interface b { static interface d {} } }
+    static class c extends a implements z.y, z.d {}
+    static class x { static interface y { static interface w {} } }
+    static class z extends x implements c.b, c.w {}
+}

test/tools/javac/6863465/T6863465d.out

+T6863465d.java:13:47: compiler.err.cant.resolve.location: kindname.class, w, , , kindname.class, T6863465d.c
+T6863465d.java:11:12: compiler.err.cyclic.inheritance: T6863465d.c
+2 errors

test/tools/javac/6863465/TestCircularClassfile.java

+/*
+ * Copyright (c) 2010, 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.
+ *
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 6863465
+ * @summary  javac doesn't detect circular subclass dependencies via qualified names
+ * @run main TestCircularClassfile
+ */
+
+import java.io.*;
+import java.net.URI;
+import java.util.Arrays;
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticListener;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+
+import com.sun.source.util.JavacTask;
+import java.util.EnumSet;
+
+public class TestCircularClassfile {
+
+    enum ClassName {
+        A("A"),
+        B("B"),
+        C("C"),
+        OBJECT("Object");
+
+        String name;
+
+        ClassName(String name) {
+            this.name = name;
+        }
+    }
+
+    static class JavaSource extends SimpleJavaFileObject {
+
+        final static String sourceStub = "class #C extends #S {}";
+
+        String source;
+
+        public JavaSource(ClassName clazz, ClassName sup) {
+            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+            source = sourceStub.replace("#C", clazz.name).replace("#S", sup.name);
+        }
+
+        @Override
+        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+            return source;
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        int count = 0;
+        for (ClassName clazz : EnumSet.of(ClassName.A, ClassName.B, ClassName.C)) {
+            for (ClassName sup : EnumSet.of(ClassName.A, ClassName.B, ClassName.C)) {
+                if (sup.ordinal() < clazz.ordinal()) continue;
+                check("sub_"+count++, clazz, sup);
+            }
+        }
+    }
+
+    static JavaSource[] initialSources = new JavaSource[] {
+            new JavaSource(ClassName.A, ClassName.OBJECT),
+            new JavaSource(ClassName.B, ClassName.A),
+            new JavaSource(ClassName.C, ClassName.B)
+        };
+
+    static String workDir = System.getProperty("user.dir");
+
+    static void check(String destPath, ClassName clazz, ClassName sup) throws Exception {
+        File destDir = new File(workDir, destPath); destDir.mkdir();
+        final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+        JavacTask ct = (JavacTask)tool.getTask(null, null, null,
+                Arrays.asList("-d", destPath), null, Arrays.asList(initialSources));
+        ct.generate();
+        File fileToRemove = new File(destPath, clazz.name + ".class");
+        fileToRemove.delete();
+        JavaSource newSource = new JavaSource(clazz, sup);
+        DiagnosticChecker checker = new DiagnosticChecker();
+        ct = (JavacTask)tool.getTask(null, null, checker,
+                Arrays.asList("-cp", destPath), null, Arrays.asList(newSource));
+        ct.analyze();
+        if (!checker.errorFound) {
+            throw new AssertionError(newSource.source);
+        }
+    }
+
+    static class DiagnosticChecker implements DiagnosticListener<JavaFileObject> {
+
+        boolean errorFound = false;
+
+        public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+            if (diagnostic.getKind() == Diagnostic.Kind.ERROR &&
+                    diagnostic.getCode().equals("compiler.err.cyclic.inheritance")) {
+                errorFound = true;
+            }
+        }
+    }
+}

test/tools/javac/CyclicInheritance.out

@@ -4,6 +4,6 @@ CyclicInheritance.java:20:1: compiler.err.cyclic.inheritance: I1
 CyclicInheritance.java:22:1: compiler.err.cyclic.inheritance: I11
 CyclicInheritance.java:27:1: compiler.err.cyclic.inheritance: C211
 CyclicInheritance.java:31:1: compiler.err.cyclic.inheritance: C212
-CyclicInheritance.java:36:27: compiler.err.report.access: C221.I, private, C221
-CyclicInheritance.java:40:24: compiler.err.report.access: C222.C, private, C222
+CyclicInheritance.java:36:1: compiler.err.cyclic.inheritance: C221
+CyclicInheritance.java:40:1: compiler.err.cyclic.inheritance: C222
 8 errors

test/tools/javac/NameCollision.out

 NameCollision.java:13:31: compiler.err.intf.expected.here
-1 error
+NameCollision.java:13:5: compiler.err.cyclic.inheritance: NameCollision.Runnable
+2 errors