8129559: JShell: compilation fails if class, method or field is annotated and has modifiers
8080354: JShell: Runtime visible annotations cannot be retrieved
Reviewed-by: jlahoda
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/MaskCommentsAndModifiers.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/MaskCommentsAndModifiers.java Fri Nov 04 14:47:25 2016 -0700
@@ -36,10 +36,14 @@
*/
class MaskCommentsAndModifiers {
- private final static Set<String> IGNORED_MODIFERS =
+ private final static Set<String> IGNORED_MODIFIERS =
Stream.of( "public", "protected", "private", "static", "final" )
.collect( Collectors.toSet() );
+ private final static Set<String> OTHER_MODIFIERS =
+ Stream.of( "abstract", "strictfp", "transient", "volatile", "synchronized", "native", "default" )
+ .collect( Collectors.toSet() );
+
// Builder to accumulate non-masked characters
private final StringBuilder sbCleared = new StringBuilder();
@@ -52,24 +56,28 @@
// Entire input string length
private final int length;
- // Should leading modifiers be masked away
- private final boolean maskModifiers;
-
- // The next character
+ // The next character position
private int next = 0;
- // We have past any point where a top-level modifier could be
- private boolean inside = false;
+ // The current character
+ private int c;
+
+ // Do we mask-off ignored modifiers? Set by parameter and turned off after
+ // initial modifier section
+ private boolean maskModifiers;
// Does the string end with an unclosed '/*' style comment?
private boolean openComment = false;
- @SuppressWarnings("empty-statement")
MaskCommentsAndModifiers(String s, boolean maskModifiers) {
this.str = s;
this.length = s.length();
this.maskModifiers = maskModifiers;
- do { } while (next());
+ read();
+ while (c >= 0) {
+ next();
+ read();
+ }
}
String cleared() {
@@ -90,24 +98,33 @@
* Read the next character
*/
private int read() {
- if (next >= length) {
- return -1;
- }
- return str.charAt(next++);
+ return c = (next >= length)
+ ? -1
+ : str.charAt(next++);
}
- private void write(StringBuilder sb, int ch) {
+ private void unread() {
+ if (c >= 0) {
+ --next;
+ }
+ }
+
+ private void writeTo(StringBuilder sb, int ch) {
sb.append((char)ch);
}
private void write(int ch) {
- write(sbCleared, ch);
- write(sbMask, Character.isWhitespace(ch) ? ch : ' ');
+ if (ch != -1) {
+ writeTo(sbCleared, ch);
+ writeTo(sbMask, Character.isWhitespace(ch) ? ch : ' ');
+ }
}
private void writeMask(int ch) {
- write(sbMask, ch);
- write(sbCleared, Character.isWhitespace(ch) ? ch : ' ');
+ if (ch != -1) {
+ writeTo(sbMask, ch);
+ writeTo(sbCleared, Character.isWhitespace(ch) ? ch : ' ');
+ }
}
private void write(CharSequence s) {
@@ -122,99 +139,105 @@
}
}
- private boolean next() {
- return next(read());
- }
-
- private boolean next(int c) {
- if (c < 0) {
- return false;
- }
-
- if (c == '\'' || c == '"') {
- inside = true;
- write(c);
- int match = c;
- c = read();
- while (c != match) {
- if (c < 0) {
- return false;
- }
- if (c == '\n' || c == '\r') {
- write(c);
- return true;
- }
- if (c == '\\') {
- write(c);
- c = read();
- }
+ private void next() {
+ switch (c) {
+ case '\'':
+ case '"':
+ maskModifiers = false;
write(c);
- c = read();
- }
- write(c);
- return true;
- }
-
- if (c == '/') {
- c = read();
- if (c == '*') {
- writeMask('/');
- writeMask(c);
- int prevc = 0;
- while ((c = read()) != '/' || prevc != '*') {
- if (c < 0) {
- openComment = true;
- return false;
- }
- writeMask(c);
- prevc = c;
- }
- writeMask(c);
- return true;
- } else if (c == '/') {
- writeMask('/');
- writeMask(c);
- while ((c = read()) != '\n' && c != '\r') {
- if (c < 0) {
- return false;
- }
- writeMask(c);
- }
- writeMask(c);
- return true;
- } else {
- inside = true;
- write('/');
- // read character falls through
- }
- }
-
- if (Character.isJavaIdentifierStart(c)) {
- if (maskModifiers && !inside) {
- StringBuilder sb = new StringBuilder();
- do {
- write(sb, c);
- c = read();
- } while (Character.isJavaIdentifierPart(c));
- String id = sb.toString();
- if (IGNORED_MODIFERS.contains(id)) {
- writeMask(sb);
- } else {
- write(sb);
- if (id.equals("import")) {
- inside = true;
+ int match = c;
+ while (read() >= 0 && c != match && c != '\n' && c != '\r') {
+ write(c);
+ if (c == '\\') {
+ write(read());
}
}
- return next(c); // recurse to handle left-over character
- }
- } else if (!Character.isWhitespace(c)) {
- inside = true;
+ write(c); // write match // line-end
+ break;
+ case '/':
+ read();
+ switch (c) {
+ case '*':
+ writeMask('/');
+ writeMask(c);
+ int prevc = 0;
+ while (read() >= 0 && (c != '/' || prevc != '*')) {
+ writeMask(c);
+ prevc = c;
+ }
+ writeMask(c);
+ openComment = c < 0;
+ break;
+ case '/':
+ writeMask('/');
+ writeMask(c);
+ while (read() >= 0 && c != '\n' && c != '\r') {
+ writeMask(c);
+ }
+ writeMask(c);
+ break;
+ default:
+ maskModifiers = false;
+ write('/');
+ unread();
+ break;
+ }
+ break;
+ case '@':
+ do {
+ write(c);
+ read();
+ } while (Character.isJavaIdentifierPart(c));
+ while (Character.isWhitespace(c)) {
+ write(c);
+ read();
+ }
+ // if this is an annotation with arguments, process those recursively
+ if (c == '(') {
+ write(c);
+ boolean prevMaskModifiers = maskModifiers;
+ int parenCnt = 1;
+ while (read() >= 0) {
+ if (c == ')') {
+ if (--parenCnt == 0) {
+ break;
+ }
+ } else if (c == '(') {
+ ++parenCnt;
+ }
+ next(); // recurse to handle quotes and comments
+ }
+ write(c);
+ // stuff in annotation arguments doesn't effect inside determination
+ maskModifiers = prevMaskModifiers;
+ } else {
+ unread();
+ }
+ break;
+ default:
+ if (Character.isJavaIdentifierStart(c)) {
+ StringBuilder sb = new StringBuilder();
+ do {
+ writeTo(sb, c);
+ read();
+ } while (Character.isJavaIdentifierPart(c));
+ unread();
+ String id = sb.toString();
+ if (maskModifiers && IGNORED_MODIFIERS.contains(id)) {
+ writeMask(sb);
+ } else {
+ write(sb);
+ if (maskModifiers && !OTHER_MODIFIERS.contains(id)) {
+ maskModifiers = false;
+ }
+ }
+ } else {
+ if (!Character.isWhitespace(c)) {
+ maskModifiers = false;
+ }
+ write(c);
+ }
+ break;
}
-
- if (c < 0) {
- return false;
- }
- write(c);
- return true;
}
}
--- a/langtools/test/jdk/jshell/ClassMembersTest.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/test/jdk/jshell/ClassMembersTest.java Fri Nov 04 14:47:25 2016 -0700
@@ -38,6 +38,9 @@
import jdk.jshell.SourceCodeAnalysis;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import jdk.jshell.TypeDeclSnippet;
+import static jdk.jshell.Snippet.Status.OVERWRITTEN;
+import static jdk.jshell.Snippet.Status.VALID;
public class ClassMembersTest extends KullaTesting {
@@ -141,29 +144,36 @@
new ExpectedDiagnostic("compiler.err.non-static.cant.be.ref", 0, 8, 1, -1, -1, Diagnostic.Kind.ERROR));
}
- @Test(enabled = false) // TODO 8080354
- public void annotationTest() {
+ @Test(dataProvider = "retentionPolicyTestCase")
+ public void annotationTest(RetentionPolicy policy) {
assertEval("import java.lang.annotation.*;");
+ String annotationSource =
+ "@Retention(RetentionPolicy." + policy.toString() + ")\n" +
+ "@interface A {}";
+ assertEval(annotationSource);
+ String classSource =
+ "@A class C {\n" +
+ " @A C() {}\n" +
+ " @A void f() {}\n" +
+ " @A int f;\n" +
+ " @A class Inner {}\n" +
+ "}";
+ assertEval(classSource);
+ String isRuntimeVisible = policy == RetentionPolicy.RUNTIME ? "true" : "false";
+ assertEval("C.class.getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ assertEval("C.class.getDeclaredConstructor().getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ assertEval("C.class.getDeclaredMethod(\"f\").getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ assertEval("C.class.getDeclaredField(\"f\").getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ assertEval("C.Inner.class.getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ }
+
+ @DataProvider(name = "retentionPolicyTestCase")
+ public Object[][] retentionPolicyTestCaseGenerator() {
+ List<Object[]> list = new ArrayList<>();
for (RetentionPolicy policy : RetentionPolicy.values()) {
- String annotationSource =
- "@Retention(RetentionPolicy." + policy.toString() + ")\n" +
- "@interface A {}";
- assertEval(annotationSource);
- String classSource =
- "@A class C {\n" +
- " @A C() {}\n" +
- " @A void f() {}\n" +
- " @A int f;\n" +
- " @A class Inner {}\n" +
- "}";
- assertEval(classSource);
- String isRuntimeVisible = policy == RetentionPolicy.RUNTIME ? "true" : "false";
- assertEval("C.class.getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
- assertEval("C.class.getDeclaredConstructor().getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
- assertEval("C.class.getDeclaredMethod(\"f\").getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
- assertEval("C.class.getDeclaredField(\"f\").getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
- assertEval("C.Inner.class.getAnnotationsByType(A.class).length > 0;", isRuntimeVisible);
+ list.add(new Object[]{policy});
}
+ return list.toArray(new Object[list.size()][]);
}
@DataProvider(name = "memberTestCase")
--- a/langtools/test/jdk/jshell/ClassesTest.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/test/jdk/jshell/ClassesTest.java Fri Nov 04 14:47:25 2016 -0700
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8145239
+ * @bug 8145239 8129559 8080354
* @summary Tests for EvaluationState.classes
* @build KullaTesting TestingInputStream ExpectedDiagnostic
* @run testng ClassesTest
@@ -255,6 +255,25 @@
assertActiveKeys();
}
+ public void classesIgnoredModifiersAnnotation() {
+ assertEval("public @interface X { }");
+ assertEval("@X public interface A { }");
+ assertDeclareWarn1("@X static class B implements A { }",
+ new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 9, 0, -1, -1, Diagnostic.Kind.WARNING));
+ assertDeclareWarn1("@X final interface C extends A { }",
+ new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 8, 0, -1, -1, Diagnostic.Kind.WARNING));
+ assertActiveKeys();
+ }
+
+ public void classesIgnoredModifiersOtherModifiers() {
+ assertEval("strictfp public interface A { }");
+ assertDeclareWarn1("strictfp static class B implements A { }",
+ new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 15, 0, -1, -1, Diagnostic.Kind.WARNING));
+ assertDeclareWarn1("strictfp final interface C extends A { }",
+ new ExpectedDiagnostic("jdk.eval.warn.illegal.modifiers", 0, 14, 0, -1, -1, Diagnostic.Kind.WARNING));
+ assertActiveKeys();
+ }
+
public void ignoreModifierSpaceIssue() {
assertEval("interface I { void f(); } ");
// there should not be a space between 'I' and '{' to reproduce the failure
--- a/langtools/test/jdk/jshell/CompletenessTest.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/test/jdk/jshell/CompletenessTest.java Fri Nov 04 14:47:25 2016 -0700
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8149524 8131024 8165211 8080071 8130454 8167343
+ * @bug 8149524 8131024 8165211 8080071 8130454 8167343 8129559
* @summary Test SourceCodeAnalysis
* @build KullaTesting TestingInputStream
* @run testng CompletenessTest
@@ -176,6 +176,7 @@
"@interface Anno",
"void f()",
"void f() throws E",
+ "@A(",
};
static final String[] unknown = new String[] {
--- a/langtools/test/jdk/jshell/IgnoreTest.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/test/jdk/jshell/IgnoreTest.java Fri Nov 04 14:47:25 2016 -0700
@@ -22,7 +22,7 @@
*/
/*
- * @test
+ * @test 8129559
* @summary Test the ignoring of comments and certain modifiers
* @build KullaTesting TestingInputStream
* @run testng IgnoreTest
@@ -70,6 +70,39 @@
assertVariableDeclSnippet(x5, "x5", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
}
+ public void testVarModifierAnnotation() {
+ assertEval("@interface A { int value() default 0; }");
+ VarSnippet x1 = varKey(assertEval("@A public int x1;"));
+ assertVariableDeclSnippet(x1, "x1", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x2 = varKey(assertEval("@A(14) protected int x2;"));
+ assertVariableDeclSnippet(x2, "x2", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x3 = varKey(assertEval("@A(value=111)private int x3;"));
+ assertVariableDeclSnippet(x3, "x3", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x4 = (VarSnippet) assertDeclareWarn1("@A static int x4;", "jdk.eval.warn.illegal.modifiers");
+ assertVariableDeclSnippet(x4, "x4", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
+ VarSnippet x5 = (VarSnippet) assertDeclareWarn1("@A(1111) final int x5;", "jdk.eval.warn.illegal.modifiers");
+ assertVariableDeclSnippet(x5, "x5", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
+ }
+
+ public void testVarModifierOtherModifier() {
+ VarSnippet x1 = varKey(assertEval("volatile public int x1;"));
+ assertVariableDeclSnippet(x1, "x1", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x2 = varKey(assertEval("transient protected int x2;"));
+ assertVariableDeclSnippet(x2, "x2", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x3 = varKey(assertEval("transient private int x3;"));
+ assertVariableDeclSnippet(x3, "x3", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 0);
+ VarSnippet x4 = (VarSnippet) assertDeclareWarn1("volatile static int x4;", "jdk.eval.warn.illegal.modifiers");
+ assertVariableDeclSnippet(x4, "x4", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
+ VarSnippet x5 = (VarSnippet) assertDeclareWarn1("transient final int x5;", "jdk.eval.warn.illegal.modifiers");
+ assertVariableDeclSnippet(x5, "x5", "int", VALID, VAR_DECLARATION_SUBKIND, 0, 1);
+ }
+
+ public void testMisplacedIgnoredModifier() {
+ assertEvalFail("int public y;");
+ assertEvalFail("String private x;");
+ assertEvalFail("(protected 34);");
+ }
+
public void testMethodModifier() {
MethodSnippet m4 = (MethodSnippet) assertDeclareWarn1("static void m4() {}", "jdk.eval.warn.illegal.modifiers");
assertMethodDeclSnippet(m4, "m4", "()void", VALID, 0, 1);
@@ -77,6 +110,14 @@
assertMethodDeclSnippet(m5, "m5", "()void", VALID, 0, 1);
}
+ public void testMethodModifierAnnotation() {
+ assertEval("@interface A { int value() default 0; }");
+ MethodSnippet m4 = (MethodSnippet) assertDeclareWarn1("@A static void m4() {}", "jdk.eval.warn.illegal.modifiers");
+ assertMethodDeclSnippet(m4, "m4", "()void", VALID, 0, 1);
+ MethodSnippet m5 = (MethodSnippet) assertDeclareWarn1("@A(value=66)final void m5() {}", "jdk.eval.warn.illegal.modifiers");
+ assertMethodDeclSnippet(m5, "m5", "()void", VALID, 0, 1);
+ }
+
public void testClassModifier() {
TypeDeclSnippet c4 = (TypeDeclSnippet) assertDeclareWarn1("static class C4 {}", "jdk.eval.warn.illegal.modifiers");
assertTypeDeclSnippet(c4, "C4", VALID, CLASS_SUBKIND, 0, 1);
--- a/langtools/test/jdk/jshell/ModifiersTest.java Fri Nov 04 17:52:11 2016 +0000
+++ b/langtools/test/jdk/jshell/ModifiersTest.java Fri Nov 04 14:47:25 2016 -0700
@@ -22,7 +22,7 @@
*/
/*
- * @test 8167643
+ * @test 8167643 8129559
* @summary Tests for modifiers
* @build KullaTesting TestingInputStream ExpectedDiagnostic
* @run testng ModifiersTest
@@ -31,6 +31,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.function.Consumer;
import javax.tools.Diagnostic;
import org.testng.annotations.DataProvider;
@@ -45,42 +46,53 @@
String[] ignoredModifiers = new String[] {
"static", "final"
};
+ String[] silentlyIgnoredModifiers = new String[] {
+ "public", "protected", "private"
+ };
+ String[] before = new String[] {
+ "strictfp", "abstract", "@X", "@X(value=9)"
+ };
+ String context = "@interface X { int value() default 0; }";
+ Consumer<String> eval = this::assertEval;
+ Consumer<String> evalWarn = s -> assertDeclareWarn1(s, "jdk.eval.warn.illegal.modifiers");
for (String ignoredModifier : ignoredModifiers) {
for (ClassType classType : ClassType.values()) {
- testCases.add(new Object[] { ignoredModifier, classType });
+ testCases.add(new Object[] { ignoredModifier, classType, evalWarn, "", null });
+ }
+ }
+ for (String ignoredModifier : ignoredModifiers) {
+ for (String preface : before) {
+ testCases.add(new Object[] { ignoredModifier, ClassType.CLASS, evalWarn, preface, context});
+ }
+ }
+ for (String ignoredModifier : silentlyIgnoredModifiers) {
+ for (ClassType classType : ClassType.values()) {
+ testCases.add(new Object[] { ignoredModifier, classType, eval, "", null });
+ }
+ }
+ for (String ignoredModifier : silentlyIgnoredModifiers) {
+ for (String preface : before) {
+ testCases.add(new Object[] { ignoredModifier, ClassType.CLASS, eval, preface, context});
}
}
return testCases.toArray(new Object[testCases.size()][]);
}
@Test(dataProvider = "ignoredModifiers")
- public void ignoredModifiers(String modifier, ClassType classType) {
- assertDeclareWarn1(
- String.format("%s %s A {}", modifier, classType), "jdk.eval.warn.illegal.modifiers");
- assertNumberOfActiveClasses(1);
- assertClasses(clazz(classType, "A"));
- 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 });
- }
+ public void ignoredModifiers(String modifier, ClassType classType,
+ Consumer<String> eval, String preface, String context) {
+ if (context != null) {
+ assertEval(context);
}
- 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"));
+ String decl = String.format("%s %s %s A {}", preface, modifier, classType);
+ eval.accept(decl);
+ if (context != null) {
+ assertNumberOfActiveClasses(2);
+ assertClasses(clazz(ClassType.ANNOTATION, "X"), clazz(classType, "A"));
+ } else {
+ assertNumberOfActiveClasses(1);
+ assertClasses(clazz(classType, "A"));
+ }
assertActiveKeys();
}