8236715: JShell: Records with errors are not properly corraled

Correctly corralling record classes, and providing correct messages to the user. Reviewed-by: rfield, vromero
openjdk · Jan 13, 2020 · 6fc159f · 6fc159f
1 parent 2afe1c6
commit 6fc159f
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 17 deletions.
diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -596,6 +596,7 @@ public enum FormatCase implements Selector<FormatCase> {
         INTERFACE("interface declaration"),
         ENUM("enum declaration"),
         ANNOTATION("annotation interface declaration"),
+        RECORD("record declaration"),
         METHOD("method declaration -- note: {type}==parameter-types"),
         VARDECL("variable declaration without init"),
         VARINIT("variable declaration with init"),

diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, 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
@@ -3324,6 +3324,9 @@ private boolean cmdTypes(String arg) {
                 case ANNOTATION_TYPE_SUBKIND:
                     kind = "@interface";
                     break;
+                case RECORD_SUBKIND:
+                    kind = "record";
+                    break;
                 default:
                     assert false : "Wrong kind" + ck.subKind();
                     kind = "class";
@@ -3819,6 +3822,9 @@ private void displayDeclarationAndValue() {
                 case ANNOTATION_TYPE_SUBKIND:
                     custom(FormatCase.ANNOTATION, ((TypeDeclSnippet) sn).name());
                     break;
+                case RECORD_SUBKIND:
+                    custom(FormatCase.RECORD, ((TypeDeclSnippet) sn).name());
+                    break;
                 case METHOD_SUBKIND:
                     custom(FormatCase.METHOD, ((MethodSnippet) sn).name(), ((MethodSnippet) sn).parameterTypes());
                     break;

diff --git a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2020, 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
@@ -855,6 +855,7 @@ The case selector kind describes the kind of snippet.  The values are:\n\t\
    interface  -- interface declaration\n\t\
    enum       -- enum declaration\n\t\
    annotation -- annotation interface declaration\n\t\
+   record     -- record declaration\n\t\
    method     -- method declaration -- note: {type}==parameter-types\n\t\
    vardecl    -- variable declaration without init\n\t\
    varinit    -- variable declaration with init\n\t\
@@ -1158,12 +1159,12 @@ startup.feedback = \
 /set format verbose action '  update overwrote' overwrote-update    \n\
 /set format verbose action '  update dropped' dropped-update    \n\
 \n\
-/set format verbose until ', however, it cannot be instantiated or its methods invoked until'   defined-class-primary    \n\
+/set format verbose until ', however, it cannot be instantiated or its methods invoked until'   defined-class,record-primary    \n\
 /set format verbose until ', however, its methods cannot be invoked until'                      defined-interface-primary    \n\
 /set format verbose until ', however, it cannot be used until'                                  defined-enum,annotation-primary    \n\
 /set format verbose until ', however, it cannot be invoked until'                               defined-method-primary    \n\
 /set format verbose until ', however, it cannot be referenced until'                            notdefined-primary    \n\
-/set format verbose until ' which cannot be instantiated or its methods invoked until'          defined-class-update    \n\
+/set format verbose until ' which cannot be instantiated or its methods invoked until'          defined-class,record-update    \n\
 /set format verbose until ' whose methods cannot be invoked until'                              defined-interface-update    \n\
 /set format verbose until ' which cannot be invoked until'                                      defined-method-update    \n\
 /set format verbose until ' which cannot be referenced until'                                   notdefined-update    \n\
@@ -1183,6 +1184,7 @@ startup.feedback = \
 /set format verbose typeKind 'interface'              interface    \n\
 /set format verbose typeKind 'enum'                   enum    \n\
 /set format verbose typeKind 'annotation interface'   annotation    \n\
+/set format verbose typeKind 'record'                 record    \n\
 \n\
 /set format verbose result '{name} ==> {value}{post}'                                        added,modified,replaced-ok-primary    \n\
 \n\
@@ -1194,10 +1196,10 @@ startup.feedback = \
 /set format verbose display '{pre}{action} variable {name}{post}'                            dropped-vardecl,varinit,expression    \n\
 /set format verbose display '{pre}{action} variable {name}, reset to null{post}'             replaced-vardecl,varinit-ok-update    \n\
 \n\
-/set format verbose display '{pre}{action} {typeKind} {name}{resolve}{post}'                 class,interface,enum,annotation    \n\
+/set format verbose display '{pre}{action} {typeKind} {name}{resolve}{post}'                 class,interface,enum,annotation,record    \n\
 /set format verbose display '{pre}{action} method {name}({type}){resolve}{post}'             method    \n\
 \n\
-/set format verbose display '{pre}attempted to use {typeKind} {name}{resolve}{post}'         used-class,interface,enum,annotation    \n\
+/set format verbose display '{pre}attempted to use {typeKind} {name}{resolve}{post}'         used-class,interface,enum,annotation,record    \n\
 /set format verbose display '{pre}attempted to call method {name}({type}){resolve}{post}'    used-method    \n\
 \n\
 /set truncation verbose 80\n\
@@ -1211,7 +1213,7 @@ startup.feedback = \
 \n\
 /set prompt concise 'jshell> '   '   ...> '    \n\
 \n\
-/set format concise display ''                                                              class,interface,enum,annotation,method,assignment,varinit,vardecl-ok    \n\
+/set format concise display ''                                                              class,interface,enum,annotation,record,method,assignment,varinit,vardecl-ok    \n\
 \n\
 /set feedback normal    \n\
 \n\

diff --git a/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java b/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -42,6 +42,10 @@
 import static com.sun.tools.javac.code.Flags.STATIC;
 import static com.sun.tools.javac.code.Flags.INTERFACE;
 import static com.sun.tools.javac.code.Flags.ENUM;
+import static com.sun.tools.javac.code.Flags.RECORD;
+import static com.sun.tools.javac.code.Flags.SYNTHETIC;
+import com.sun.tools.javac.tree.JCTree.Tag;
+import com.sun.tools.javac.tree.TreeInfo;
 import jdk.jshell.Wrap.CompoundWrap;
 import jdk.jshell.Wrap.Range;
 import jdk.jshell.Wrap.RangeWrap;
@@ -114,6 +118,7 @@ private String defaultConstructor(JCClassDecl tree) {
     public void visitClassDef(JCClassDecl tree) {
         boolean isEnum = (tree.mods.flags & ENUM) != 0;
         boolean isInterface = (tree.mods.flags & INTERFACE ) != 0;
+        boolean isRecord = (tree.mods.flags & RECORD ) != 0;
         int classBegin = dis.getStartPosition(tree);
         int classEnd = dis.getEndPosition(tree);
         //debugWrap("visitClassDef: %d-%d = %s\n", classBegin, classEnd, source.substring(classBegin, classEnd));
@@ -151,8 +156,12 @@ public void visitClassDef(JCClassDecl tree) {
                 // non-enum
                 boolean constructorSeen = false;
                 for (List<? extends JCTree> l = tree.defs; l.nonEmpty(); l = l.tail) {
-                    wrappedDefs.append("\n   ");
                     JCTree t = l.head;
+                    if (isRecord && t.hasTag(Tag.VARDEF) && (TreeInfo.flags(t) & RECORD) != 0) {
+                        //record parameters are part of the record's header
+                        continue;
+                    }
+                    wrappedDefs.append("\n   ");
                     switch (t.getKind()) {
                         case METHOD:
                             constructorSeen = constructorSeen || ((MethodTree)t).getName() == tree.name.table.names.init;
@@ -166,7 +175,14 @@ public void visitClassDef(JCClassDecl tree) {
                     }
                     wrappedDefs.append(corral(t));
                 }
-                if (!constructorSeen && !isInterface && !isEnum) {
+                if (!constructorSeen && isRecord) {
+                    // Generate a default constructor, since
+                    // this is a regular record and there are no constructors
+                    if (wrappedDefs.length() > 0) {
+                        wrappedDefs.append("\n ");
+                    }
+                    wrappedDefs.append("  public " + tree.name.toString() + " " + resolutionExceptionBlock);
+                } else if (!constructorSeen && !isInterface && !isEnum) {
                     // Generate a default constructor, since
                     // this is a regular class and there are no constructors
                     if (wrappedDefs.length() > 0) {
@@ -175,7 +191,9 @@ public void visitClassDef(JCClassDecl tree) {
                     wrappedDefs.append(defaultConstructor(tree));
                 }
             }
-            bodyBegin = dis.getStartPosition(tree.defs.head);
+            if (!isRecord) {
+                bodyBegin = dis.getStartPosition(tree.defs.head);
+            }
         }
         Object defs = wrappedDefs.length() == 1
             ? wrappedDefs.first()

diff --git a/test/langtools/jdk/jshell/RecordsTest.java b/test/langtools/jdk/jshell/RecordsTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8235474
+ * @bug 8235474 8236715
  * @summary Tests for evalution of records
  * @modules jdk.jshell
  * @build KullaTesting TestingInputStream ExpectedDiagnostic
@@ -33,6 +33,8 @@
 import org.testng.annotations.Test;
 
 import javax.lang.model.SourceVersion;
+import jdk.jshell.Snippet.Status;
+import jdk.jshell.UnresolvedReferenceException;
 import static org.testng.Assert.assertEquals;
 import org.testng.annotations.BeforeMethod;
 
@@ -46,6 +48,24 @@ public void testRecordClass() {
         assertEval("r.i()", "42");
     }
 
+    public void testRecordCorralling() {
+        //simple record with a mistake that can be fixed by corralling:
+        assertEval("record R1(int i) { int g() { return j; } }", ste(MAIN_SNIPPET, Status.NONEXISTENT, Status.RECOVERABLE_DEFINED, true, null));
+        assertEval("R1 r1 = new R1(1);", null, UnresolvedReferenceException.class, DiagCheck.DIAG_OK, DiagCheck.DIAG_OK, added(Status.VALID));
+        //record with a concise constructor and a mistake take can be fixed by corralling:
+        assertEval("record R2(int i) { public R2 {} int g() { return j; } }", ste(MAIN_SNIPPET, Status.NONEXISTENT, Status.RECOVERABLE_DEFINED, true, null));
+        assertEval("R2 r2 = new R2(1);", null, UnresolvedReferenceException.class, DiagCheck.DIAG_OK, DiagCheck.DIAG_OK, added(Status.VALID));
+        //record with a full constructor and a mistake take can be fixed by corralling:
+        assertEval("record R3(int i) { public R3(int i) {this.i = i;} int g() { return j; } }", ste(MAIN_SNIPPET, Status.NONEXISTENT, Status.RECOVERABLE_DEFINED, true, null));
+        assertEval("R3 r3 = new R3(1);", null, UnresolvedReferenceException.class, DiagCheck.DIAG_OK, DiagCheck.DIAG_OK, added(Status.VALID));
+        //record with an accessor and a mistake take can be fixed by corralling:
+        assertEval("record R4(int i) { public int i() { return i; } int g() { return j; } }", ste(MAIN_SNIPPET, Status.NONEXISTENT, Status.RECOVERABLE_DEFINED, true, null));
+        assertEval("R4 r4 = new R4(1);", null, UnresolvedReferenceException.class, DiagCheck.DIAG_OK, DiagCheck.DIAG_OK, added(Status.VALID));
+        //record with an accessor with a mistake take can be fixed by corralling:
+        assertEval("record R5(int i) { public int i() { return j; } }", ste(MAIN_SNIPPET, Status.NONEXISTENT, Status.RECOVERABLE_DEFINED, true, null));
+        assertEval("R5 r5 = new R5(1);", null, UnresolvedReferenceException.class, DiagCheck.DIAG_OK, DiagCheck.DIAG_OK, added(Status.VALID));
+    }
+
     public void testRecordField() {
         assertEquals(varKey(assertEval("String record = \"\";")).name(), "record");
         assertEval("record.length()", "0");

diff --git a/test/langtools/jdk/jshell/ToolLocalSimpleTest.java b/test/langtools/jdk/jshell/ToolLocalSimpleTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -100,4 +100,8 @@ public void testSwitchExpressionCompletion() {
         // can't set --enable-preview for local, ignore
     }
 
+    @Override
+    public void testRecords() {
+        // can't set --enable-preview for local, ignore
+    }
 }
diff --git a/test/langtools/jdk/jshell/ToolSimpleTest.java b/test/langtools/jdk/jshell/ToolSimpleTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513 8170015 8170368 8172102 8172103  8165405 8173073 8173848 8174041 8173916 8174028 8174262 8174797 8177079 8180508 8177466 8172154 8192979 8191842 8198573 8198801 8210596 8210959 8215099 8199623
+ * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513 8170015 8170368 8172102 8172103  8165405 8173073 8173848 8174041 8173916 8174028 8174262 8174797 8177079 8180508 8177466 8172154 8192979 8191842 8198573 8198801 8210596 8210959 8215099 8199623 8236715
  * @summary Simple jshell tool tests
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -902,4 +902,14 @@ public void testUpdateFalsePositive() {
                 a -> assertCommandOutputContains(a, "a", "A@")
         );
     }
+
+    @Test
+    public void testRecords() {
+        test(new String[] {"--enable-preview"},
+                (a) -> assertCommandOutputContains(a, "record R(int i) { public int g() { return j; } }",
+                        "|  created record R, however, it cannot be instantiated or its methods invoked until variable j is declared"),
+                (a) -> assertCommandOutputContains(a, "new R(0)",
+                        "|  attempted to use record R which cannot be instantiated or its methods invoked until variable j is declared")
+        );
+    }
 }