# HG changeset patch
# User rfield
# Date 1477613476 25200
# Node ID 5843b57ce3a66a5a04798de5f23d9f522e970aa4
# Parent  44d30e3656f5470b00519062da0e8f2d61eb130a
8167643: JShell: silently ignore access modifiers (as semantically irrelevant)
Reviewed-by: jlahoda

diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Thu Oct 27 17:11:16 2016 -0700
@@ -794,7 +794,7 @@
 /set format verbose unrerr '{unresolved} is declared and these errors are corrected: {errors}'  unresolved1-error2    \n\
 /set format verbose unrerr '{unresolved} are declared and these errors are corrected: {errors}' unresolved2-error2    \n\
 \n\
-/set format verbose resolve '{until}{unrerr}'                                                   added,modified,replaced,used    \n\
+/set format verbose resolve '{until}{unrerr}'                                                   defined,notdefined-added,modified,replaced,used    \n\
 \n\
 /set format verbose typeKind 'class'                  class    \n\
 /set format verbose typeKind 'interface'              interface    \n\
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/src/jdk.jshell/share/classes/jdk/jshell/Eval.java
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Thu Oct 27 17:11:16 2016 -0700
@@ -855,6 +855,8 @@
                 case PUBLIC:
                 case PROTECTED:
                 case PRIVATE:
+                    // quietly ignore, user cannot see effects one way or the other
+                    break;
                 case STATIC:
                 case FINAL:
                     list.add(mod);
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/ClassesTest.java
--- a/langtools/test/jdk/jshell/ClassesTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/ClassesTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -247,14 +247,11 @@
     }
 
     public void classesIgnoredModifiers() {
-        assertDeclareWarn1("public interface A { }",
-                new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 6, 0, -1, -1, Diagnostic.Kind.WARNING));
+        assertEval("public interface A { }");
         assertDeclareWarn1("static class B implements A { }",
                 new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 6, 0, -1, -1, Diagnostic.Kind.WARNING));
         assertDeclareWarn1("final interface C extends A { }",
                 new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 5, 0, -1, -1, Diagnostic.Kind.WARNING));
-        assertDeclareWarn1("protected enum D implements C { }",
-                new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 9, 0, -1, -1, Diagnostic.Kind.WARNING));
         assertActiveKeys();
     }
 
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/ErrorTranslationTest.java
--- a/langtools/test/jdk/jshell/ErrorTranslationTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/ErrorTranslationTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -63,11 +63,8 @@
         List<ReplTest> list = new ArrayList<>();
         ExpectedDiagnostic[] diagnostics = new ExpectedDiagnostic[]{
                 newExpectedDiagnostic(0, 6, 0, -1, -1, Diagnostic.Kind.WARNING),
-                newExpectedDiagnostic(0, 9, 0, -1, -1, Diagnostic.Kind.WARNING),
-                newExpectedDiagnostic(0, 7, 0, -1, -1, Diagnostic.Kind.WARNING),
-                newExpectedDiagnostic(0, 6, 0, -1, -1, Diagnostic.Kind.WARNING),
                 newExpectedDiagnostic(0, 5, 0, -1, -1, Diagnostic.Kind.WARNING)};
-        String[] mods = {"public", "protected", "private", "static", "final"};
+        String[] mods = {"static", "final"};
         for (int i = 0; i < mods.length; ++i) {
             for (String code : new String[] {"class A {}", "void f() {}", "int a;"}) {
                 final int finalI = i;
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/IgnoreTest.java
--- a/langtools/test/jdk/jshell/IgnoreTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/IgnoreTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -58,12 +58,12 @@
     }
 
     public void testVarModifier() {
-        VarSnippet x1 = (VarSnippet) assertDeclareWarn1("public int x1;", "jdk.eval.warn.illegal.modifiers");
-        assertVariableDeclSnippet(x1, "x1", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
-        VarSnippet x2 = (VarSnippet) assertDeclareWarn1("protected int x2;", "jdk.eval.warn.illegal.modifiers");
-        assertVariableDeclSnippet(x2, "x2", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
-        VarSnippet x3 = (VarSnippet) assertDeclareWarn1("private int x3;", "jdk.eval.warn.illegal.modifiers");
-        assertVariableDeclSnippet(x3, "x3", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
+        VarSnippet x1 = varKey(assertEval("public int x1;"));
+        assertVariableDeclSnippet(x1, "x1", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+        VarSnippet x2 = varKey(assertEval("protected int x2;"));
+        assertVariableDeclSnippet(x2, "x2", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+        VarSnippet x3 = varKey(assertEval("private int x3;"));
+        assertVariableDeclSnippet(x3, "x3", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
         VarSnippet x4 = (VarSnippet) assertDeclareWarn1("static int x4;", "jdk.eval.warn.illegal.modifiers");
         assertVariableDeclSnippet(x4, "x4", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
         VarSnippet x5 = (VarSnippet) assertDeclareWarn1("final int x5;", "jdk.eval.warn.illegal.modifiers");
@@ -71,12 +71,6 @@
     }
 
     public void testMethodModifier() {
-        MethodSnippet m1 = (MethodSnippet) assertDeclareWarn1("public void m1() {}", "jdk.eval.warn.illegal.modifiers");
-        assertMethodDeclSnippet(m1, "m1", "()void", VALID, 0, 1);
-        MethodSnippet m2 = (MethodSnippet) assertDeclareWarn1("protected void m2() {}", "jdk.eval.warn.illegal.modifiers");
-        assertMethodDeclSnippet(m2, "m2", "()void", VALID, 0, 1);
-        MethodSnippet m3 = (MethodSnippet) assertDeclareWarn1("private void m3() {}", "jdk.eval.warn.illegal.modifiers");
-        assertMethodDeclSnippet(m3, "m3", "()void", VALID, 0, 1);
         MethodSnippet m4 = (MethodSnippet) assertDeclareWarn1("static void m4() {}", "jdk.eval.warn.illegal.modifiers");
         assertMethodDeclSnippet(m4, "m4", "()void", VALID, 0, 1);
         MethodSnippet m5 = (MethodSnippet) assertDeclareWarn1("final void m5() {}", "jdk.eval.warn.illegal.modifiers");
@@ -84,12 +78,6 @@
     }
 
     public void testClassModifier() {
-        TypeDeclSnippet c1 = (TypeDeclSnippet) assertDeclareWarn1("public class C1 {}", "jdk.eval.warn.illegal.modifiers");
-        assertTypeDeclSnippet(c1, "C1", VALID, CLASS_SUBKIND, 0, 1);
-        TypeDeclSnippet c2 = (TypeDeclSnippet) assertDeclareWarn1("protected class C2 {}", "jdk.eval.warn.illegal.modifiers");
-        assertTypeDeclSnippet(c2, "C2", VALID, CLASS_SUBKIND, 0, 1);
-        TypeDeclSnippet c3 = (TypeDeclSnippet) assertDeclareWarn1("private class C3 {}", "jdk.eval.warn.illegal.modifiers");
-        assertTypeDeclSnippet(c3, "C3", VALID, CLASS_SUBKIND, 0, 1);
         TypeDeclSnippet c4 = (TypeDeclSnippet) assertDeclareWarn1("static class C4 {}", "jdk.eval.warn.illegal.modifiers");
         assertTypeDeclSnippet(c4, "C4", VALID, CLASS_SUBKIND, 0, 1);
         TypeDeclSnippet c5 = (TypeDeclSnippet) assertDeclareWarn1("final class C5 {}", "jdk.eval.warn.illegal.modifiers");
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/KullaTesting.java
--- a/langtools/test/jdk/jshell/KullaTesting.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/KullaTesting.java	Thu Oct 27 17:11:16 2016 -0700
@@ -957,43 +957,33 @@
     }
 
     public enum ClassType {
-        CLASS("CLASS_SUBKIND") {
-            @Override
-            public String toString() {
-                return "class";
-            }
-        },
-        ENUM("ENUM_SUBKIND") {
-            @Override
-            public String toString() {
-                return "enum";
-            }
-        },
-        INTERFACE("INTERFACE_SUBKIND") {
-            @Override
-            public String toString() {
-                return "interface";
-            }
-        },
-        ANNOTATION("ANNOTATION_TYPE_SUBKIND") {
-            @Override
-            public String toString() {
-                return "@interface";
-            }
-        };
+        CLASS("CLASS_SUBKIND", "class", "class"),
+        ENUM("ENUM_SUBKIND", "enum", "enum"),
+        INTERFACE("INTERFACE_SUBKIND", "interface", "interface"),
+        ANNOTATION("ANNOTATION_TYPE_SUBKIND", "@interface", "annotation interface");
 
         private final String classType;
+        private final String name;
+        private final String displayed;
 
-        ClassType(String classType) {
+        ClassType(String classType, String name, String displayed) {
             this.classType = classType;
+            this.name = name;
+            this.displayed = displayed;
         }
 
         public String getClassType() {
             return classType;
         }
 
+        public String getDisplayed() {
+            return displayed;
+        }
+
         @Override
-        public abstract String toString();
+        public String toString() {
+            return name;
+        }
     }
 
     public static MemberInfo variable(String type, String name) {
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/MethodsTest.java
--- a/langtools/test/jdk/jshell/MethodsTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/MethodsTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8080357
+ * @bug 8080357 8167643
  * @summary Tests for EvaluationState.methods
  * @build KullaTesting TestingInputStream ExpectedDiagnostic
  * @run testng MethodsTest
@@ -230,31 +230,30 @@
         assertActiveKeys();
     }
 
-    public void methodsWarn() {
-        Snippet f = assertDeclareWarn1("public String f() {return null;}",
-                new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 6, 0, -1, -1, Diagnostic.Kind.WARNING),
-                added(VALID));
+
+    public void methodsAccessModifierIgnored() {
+        Snippet f = methodKey(assertEval("public String f() {return null;}",
+                added(VALID)));
         assertNumberOfActiveMethods(1);
         assertActiveKeys();
 
-        f = assertDeclareWarn1("protected String f() {return null;}",
-                new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 9, 0, -1, -1, Diagnostic.Kind.WARNING),
+        f = methodKey(assertEval("protected String f() {return null;}",
+                ste(MAIN_SNIPPET, VALID, VALID, false, null),
+                ste(f, VALID, OVERWRITTEN, false, MAIN_SNIPPET)));
+        assertNumberOfActiveMethods(1);
+        assertActiveKeys();
+
+        assertEval("private String f() {return null;}",
                 ste(MAIN_SNIPPET, VALID, VALID, false, null),
                 ste(f, VALID, OVERWRITTEN, false, MAIN_SNIPPET));
         assertNumberOfActiveMethods(1);
         assertActiveKeys();
+    }
 
-        f = assertDeclareWarn1("private String f() {return null;}",
-                new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 7, 0, -1, -1, Diagnostic.Kind.WARNING),
-                ste(MAIN_SNIPPET, VALID, VALID, false, null),
-                ste(f, VALID, OVERWRITTEN, false, MAIN_SNIPPET));
-        assertNumberOfActiveMethods(1);
-        assertActiveKeys();
-
-        f = assertDeclareWarn1("static String f() {return null;}",
+    public void methodsWarn() {
+        Snippet f = assertDeclareWarn1("static String f() {return null;}",
                 new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 6, 0, -1, -1, Diagnostic.Kind.WARNING),
-                ste(MAIN_SNIPPET, VALID, VALID, false, null),
-                ste(f, VALID, OVERWRITTEN, false, MAIN_SNIPPET));
+                added(VALID));
         assertNumberOfActiveMethods(1);
         assertActiveKeys();
 
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/ModifiersTest.java
--- a/langtools/test/jdk/jshell/ModifiersTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/ModifiersTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -22,7 +22,7 @@
  */
 
 /*
- * @test
+ * @test 8167643
  * @summary Tests for modifiers
  * @build KullaTesting TestingInputStream ExpectedDiagnostic
  * @run testng ModifiersTest
@@ -43,7 +43,7 @@
     public Object[][] getTestCases() {
         List<Object[]> testCases = new ArrayList<>();
         String[] ignoredModifiers = new String[] {
-            "public", "protected", "private", "static", "final"
+            "static", "final"
         };
         for (String ignoredModifier : ignoredModifiers) {
             for (ClassType classType : ClassType.values()) {
@@ -62,6 +62,28 @@
         assertActiveKeys();
     }
 
+    @DataProvider(name = "silentlyIgnoredModifiers")
+    public Object[][] getSilentTestCases() {
+        List<Object[]> testCases = new ArrayList<>();
+        String[] ignoredModifiers = new String[] {
+            "public", "protected", "private"
+        };
+        for (String ignoredModifier : ignoredModifiers) {
+            for (ClassType classType : ClassType.values()) {
+                testCases.add(new Object[] { ignoredModifier, classType });
+            }
+        }
+        return testCases.toArray(new Object[testCases.size()][]);
+    }
+
+    @Test(dataProvider = "silentlyIgnoredModifiers")
+    public void silentlyIgnoredModifiers(String modifier, ClassType classType) {
+        assertEval(String.format("%s %s A {}", modifier, classType));
+        assertNumberOfActiveClasses(1);
+        assertClasses(clazz(classType, "A"));
+        assertActiveKeys();
+    }
+
     public void accessToStaticFieldsOfClass() {
         assertEval("class A {" +
                 "int x = 14;" +
diff -r 44d30e3656f5 -r 5843b57ce3a6 langtools/test/jdk/jshell/ToolBasicTest.java
--- a/langtools/test/jdk/jshell/ToolBasicTest.java	Thu Oct 27 21:22:12 2016 +0000
+++ b/langtools/test/jdk/jshell/ToolBasicTest.java	Thu Oct 27 17:11:16 2016 -0700
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8143037 8142447 8144095 8140265 8144906 8146138 8147887 8147886 8148316 8148317 8143955 8157953 8080347 8154714 8166649
+ * @bug 8143037 8142447 8144095 8140265 8144906 8146138 8147887 8147886 8148316 8148317 8143955 8157953 8080347 8154714 8166649 8167643
  * @summary Tests for Basic tests for REPL tool
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -520,7 +520,7 @@
                     a -> assertCommand(a, "int a", ""),
                     a -> assertCommand(a, "void f() {}", ""),
                     a -> assertCommandCheckOutput(a, "aaaa", assertStartsWith("|  Error:")),
-                    a -> assertCommandCheckOutput(a, "public void f() {}", assertStartsWith("|  Warning:"))
+                    a -> assertCommandCheckOutput(a, "static void f() {}", assertStartsWith("|  Warning:"))
             );
         }
     }