changeset 57634:1539b1e71dfa

8236715: JShell: Records with errors are not properly corraled Summary: Correctly corralling record classes, and providing correct messages to the user. Reviewed-by: rfield, vromero
author jlahoda
date Mon, 13 Jan 2020 10:49:58 +0100
parents e70d8459c2ba
children 231089800e42
files src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties src/jdk.jshell/share/classes/jdk/jshell/Corraller.java test/langtools/jdk/jshell/RecordsTest.java test/langtools/jdk/jshell/ToolLocalSimpleTest.java test/langtools/jdk/jshell/ToolSimpleTest.java
diffstat 7 files changed, 78 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
         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"),
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
                 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 @@
                 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;
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Mon Jan 13 15:31:23 2020 +0800
+++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
    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 @@
 /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 @@
 /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 @@
 /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 @@
 \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\
--- a/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
     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 @@
                 // non-enum
                 boolean constructorSeen = false;
                 for (List<? extends JCTree> l = tree.defs; l.nonEmpty(); l = l.tail) {
+                    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   ");
-                    JCTree t = l.head;
                     switch (t.getKind()) {
                         case METHOD:
                             constructorSeen = constructorSeen || ((MethodTree)t).getName() == tree.name.table.names.init;
@@ -166,7 +175,14 @@
                     }
                     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 @@
                     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()
--- a/test/langtools/jdk/jshell/RecordsTest.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/test/langtools/jdk/jshell/RecordsTest.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
         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");
--- a/test/langtools/jdk/jshell/ToolLocalSimpleTest.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/test/langtools/jdk/jshell/ToolLocalSimpleTest.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
         // can't set --enable-preview for local, ignore
     }
 
+    @Override
+    public void testRecords() {
+        // can't set --enable-preview for local, ignore
+    }
 }
--- a/test/langtools/jdk/jshell/ToolSimpleTest.java	Mon Jan 13 15:31:23 2020 +0800
+++ b/test/langtools/jdk/jshell/ToolSimpleTest.java	Mon Jan 13 10:49:58 2020 +0100
@@ -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 @@
                 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")
+        );
+    }
 }