changeset 48172:5e86806f57f9

8190939: JShell: gives a compiler error evaluating an expression of inaccessible type Reviewed-by: jlahoda
author rfield
date Fri, 24 Nov 2017 16:55:18 -0800
parents 9ec74010cadf
children 7bcdb571ae31
files src/jdk.jshell/share/classes/jdk/jshell/Eval.java src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java src/jdk.jshell/share/classes/jdk/jshell/Util.java test/langtools/jdk/jshell/InaccessibleExpressionTest.java
diffstat 4 files changed, 178 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Nov 24 13:18:36 2017 +0530
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Nov 24 16:55:18 2017 -0800
@@ -59,7 +59,6 @@
 import jdk.jshell.Snippet.SubKind;
 import jdk.jshell.TaskFactory.AnalyzeTask;
 import jdk.jshell.TaskFactory.BaseTask;
-import jdk.jshell.TaskFactory.CompileTask;
 import jdk.jshell.TaskFactory.ParseTask;
 import jdk.jshell.Wrap.CompoundWrap;
 import jdk.jshell.Wrap.Range;
@@ -454,7 +453,7 @@
                         name = "$" + ++varNumber;
                     }
                 }
-                guts = Wrap.tempVarWrap(compileSource, typeName, name);
+                guts = Wrap.tempVarWrap(compileSource, ei.accessibleTypeName, name);
                 Collection<String> declareReferences = null; //TODO
                 snip = new VarSnippet(state.keyMap.keyForVariable(name), userSource, guts,
                         name, SubKind.TEMP_VAR_EXPRESSION_SUBKIND, typeName, declareReferences, null);
--- a/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java	Fri Nov 24 13:18:36 2017 +0530
+++ b/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java	Fri Nov 24 16:55:18 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, 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
@@ -39,6 +39,8 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreePathScanner;
+import com.sun.tools.javac.code.Flags;
+import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
@@ -69,6 +71,7 @@
     public static class ExpressionInfo {
         ExpressionTree tree;
         String typeName;
+        String accessibleTypeName;
         String fullTypeName;
         List<String> parameterTypes;
         String enclosingInstanceType;
@@ -215,6 +218,63 @@
         return null;
     }
 
+    /**
+     * A type is accessible if it is public or if it is package-private and is a
+     * type defined in JShell.  Additionally, all its type arguments must be
+     * accessible
+     *
+     * @param type the type to check for accessibility
+     * @return true if the type name can be referenced
+     */
+    private boolean isAccessible(Type type) {
+        Symbol.TypeSymbol tsym = type.asElement();
+        return ((tsym.flags() & Flags.PUBLIC) != 0 ||
+                ((tsym.flags() & Flags.PRIVATE) == 0 &&
+                Util.isInJShellClass(tsym.flatName().toString()))) &&
+                 type.getTypeArguments().stream()
+                        .allMatch(this::isAccessible);
+    }
+
+    /**
+     * Return the superclass.
+     *
+     * @param type the type
+     * @return the superclass, or Object on error
+     */
+    private Type supertype(Type type) {
+        Type sup = types.supertype(type);
+        if (sup == Type.noType || sup == null) {
+            return syms.objectType;
+        }
+        return sup;
+    }
+
+    /**
+     * Find an accessible supertype.
+     *
+     * @param type the type
+     * @return the type, if it is accessible, otherwise a superclass or
+     * interface which is
+     */
+    private Type findAccessibleSupertype(Type type) {
+        // Iterate up the superclasses, see if any are accessible
+        for (Type sup = type; !types.isSameType(sup, syms.objectType); sup = supertype(sup)) {
+            if (isAccessible(sup)) {
+                return sup;
+            }
+        }
+        // Failing superclasses, look through superclasses for accessible interfaces
+        for (Type sup = type; !types.isSameType(sup, syms.objectType); sup = supertype(sup)) {
+            for (Type itf : types.interfaces(sup)) {
+                if (isAccessible(itf)) {
+                    return itf;
+                }
+            }
+        }
+        // Punt, return Object which is the supertype of everything
+        return syms.objectType;
+    }
+
     private ExpressionInfo treeToInfo(TreePath tp) {
         if (tp != null) {
             Tree tree = tp.getLeaf();
@@ -234,10 +294,12 @@
                         case NULL:
                             ei.isNonVoid = true;
                             ei.typeName = OBJECT_TYPE_NAME;
+                            ei.accessibleTypeName = OBJECT_TYPE_NAME;
                             break;
                         default: {
                             ei.isNonVoid = true;
                             ei.typeName = varTypeName(type, false);
+                            ei.accessibleTypeName = varTypeName(findAccessibleSupertype(type), false);
                             ei.fullTypeName = varTypeName(type, true);
                             break;
                         }
--- a/src/jdk.jshell/share/classes/jdk/jshell/Util.java	Fri Nov 24 13:18:36 2017 +0530
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Util.java	Fri Nov 24 16:55:18 2017 -0800
@@ -29,6 +29,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -82,6 +83,17 @@
         return sb.toString();
     }
 
+    /**
+     * Check if this is the name of something in JShell.
+     *
+     * @param s the name of the class, method, variable, ...
+     * @return true if it is, or is within a JShell defined wrapper class
+     */
+    static boolean isInJShellClass(String s) {
+        Matcher m = PREFIX_PATTERN.matcher(s);
+        return m.find() && m.start() == 0;
+    }
+
     static String asLetters(int i) {
         if (i == 0) {
             return "";
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/jdk/jshell/InaccessibleExpressionTest.java	Fri Nov 24 16:55:18 2017 -0800
@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2017, 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 8190939
+ * @summary test expressions whose type is inaccessible
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.jdeps/com.sun.tools.javap
+ * @library /tools/lib
+ * @build KullaTesting Compiler
+ * @run testng InaccessibleExpressionTest
+ */
+
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.testng.annotations.BeforeMethod;
+import jdk.jshell.VarSnippet;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+@Test
+public class InaccessibleExpressionTest extends KullaTesting {
+
+    @BeforeMethod
+    @Override
+    public void setUp() {
+        Path path = Paths.get("eit");
+        Compiler compiler = new Compiler();
+        compiler.compile(path,
+                "package priv;\n" +
+                "\n" +
+                "import java.util.function.Supplier;\n" +
+                "import java.util.ArrayList;\n" +
+                "\n" +
+                "public class GetPriv {\n" +
+                "   private enum Count { One };\n" +
+                "   public static Packp down() { return new Packp(); }\n" +
+                "   public static MyList list() { return new MyList(); }\n" +
+                "   public static Count priv() { return Count.One; }\n" +
+                "}\n" +
+                "\n" +
+                "class Packp extends Packp2 {\n" +
+                        "public String toString() { return \"Packp\"; } }\n" +
+                "\n" +
+                "class Packp2 implements Supplier<Integer>  {" +
+                        "public Integer get() { return 5; }}\n" +
+                "\n" +
+                "class MyList extends ArrayList<Integer> {}");
+        String tpath = compiler.getPath(path).toString();
+        setUp(b -> b
+                .remoteVMOptions("--class-path", tpath)
+                .compilerOptions("--class-path", tpath));
+    }
+
+    public void testExternal() {
+        assertEval("import static priv.GetPriv.*;");
+        VarSnippet down = varKey(assertEval("down()", "Packp"));
+        assertEquals(down.typeName(), "priv.Packp");
+        assertEval(down.name() + ".get()", "5");
+        VarSnippet list = varKey(assertEval("list()", "[]"));
+        assertEquals(list.typeName(), "priv.MyList");
+        assertEval(list.name() + ".size()", "0");
+        VarSnippet one = varKey(assertEval("priv()", "One"));
+        assertEquals(one.typeName(), "priv.GetPriv.Count");
+    }
+
+    public void testInternal() {
+        assertEval(
+                "class Top {" +
+                "    private class Inner {" +
+                "        public String toString() { return \"Inner\"; }" +
+                "    }" +
+                "    Inner n = new Inner(); }");
+        VarSnippet n = varKey(assertEval("new Top().n", "Inner"));
+        assertEquals(n.typeName(), "Top.Inner");
+    }
+
+}