8187359: JShell: Give comprehensible error when user method name matches Object method
authorrfield
Fri, 20 Oct 2017 19:08:25 -0700
changeset 47499 b3ea71b70f7b
parent 47498 aa0344e6f39b
child 47500 9b702b6608f9
8187359: JShell: Give comprehensible error when user method name matches Object method Reviewed-by: jlahoda
src/jdk.jshell/share/classes/jdk/jshell/Eval.java
src/jdk.jshell/share/classes/jdk/jshell/resources/l10n.properties
test/langtools/jdk/jshell/MethodsTest.java
--- a/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Oct 20 15:39:50 2017 -0700
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Oct 20 19:08:25 2017 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -48,6 +48,7 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.io.Writer;
+import java.util.Arrays;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import jdk.jshell.ExpressionToTypeInfo.ExpressionInfo;
@@ -100,6 +101,12 @@
 
     private final JShell state;
 
+    // The set of names of methods on Object
+    private final Set<String> objectMethods = Arrays
+            .stream(Object.class.getMethods())
+            .map(m -> m.getName())
+            .collect(toSet());
+
     Eval(JShell state) {
         this.state = state;
     }
@@ -528,10 +535,26 @@
     private List<Snippet> processMethod(String userSource, Tree unitTree, String compileSource, ParseTask pt) {
         TreeDependencyScanner tds = new TreeDependencyScanner();
         tds.scan(unitTree);
-        TreeDissector dis = TreeDissector.createByFirstClass(pt);
+        final TreeDissector dis = TreeDissector.createByFirstClass(pt);
+
+        final MethodTree mt = (MethodTree) unitTree;
+        final String name = mt.getName().toString();
+        if (objectMethods.contains(name)) {
+            // The name matches a method on Object, short of an overhaul, this
+            // fails, see 8187137.  Generate a descriptive error message
 
-        MethodTree mt = (MethodTree) unitTree;
-        String name = mt.getName().toString();
+            // The error position will be the position of the name, find it
+            long possibleStart = dis.getEndPosition(mt.getReturnType());
+            Range possibleRange = new Range((int) possibleStart,
+                    dis.getStartPosition(mt.getBody()));
+            String possibleNameSection = possibleRange.part(compileSource);
+            int offset = possibleNameSection.indexOf(name);
+            long start = offset < 0
+                    ? possibleStart // something wrong, punt
+                    : possibleStart + offset;
+
+            return compileFailResult(new DiagList(objectMethodNameDiag(name, start)), userSource, Kind.METHOD);
+        }
         String parameterTypes
                 = mt.getParameters()
                 .stream()
@@ -900,6 +923,47 @@
         return PREFIX_PATTERN.matcher(ste.getClassName()).find();
     }
 
+    /**
+     * Construct a diagnostic for a method name matching an Object method name
+     * @param name the method name
+     * @param nameStart the position within the source of the method name
+     * @return the generated diagnostic
+     */
+    private Diag objectMethodNameDiag(String name, long nameStart) {
+        return new Diag() {
+            @Override
+            public boolean isError() {
+                return true;
+            }
+
+            @Override
+            public long getPosition() {
+                return nameStart;
+            }
+
+            @Override
+            public long getStartPosition() {
+                return nameStart;
+            }
+
+            @Override
+            public long getEndPosition() {
+                return nameStart + name.length();
+            }
+
+            @Override
+            public String getCode() {
+                return "jdk.eval.error.object.method";
+            }
+
+            @Override
+            public String getMessage(Locale locale) {
+                return state.messageFormat("jshell.diag.object.method.fatal",
+                        String.join(" ", objectMethods));
+            }
+        };
+    }
+
     private DiagList modifierDiagnostics(ModifiersTree modtree,
             final TreeDissector dis, boolean isAbstractProhibited) {
 
--- a/src/jdk.jshell/share/classes/jdk/jshell/resources/l10n.properties	Fri Oct 20 15:39:50 2017 -0700
+++ b/src/jdk.jshell/share/classes/jdk/jshell/resources/l10n.properties	Fri Oct 20 19:08:25 2017 -0700
@@ -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
@@ -27,6 +27,7 @@
 jshell.diag.modifier.plural.ignore = Modifiers {0} not permitted in top-level declarations, ignored
 jshell.diag.modifier.single.fatal = Modifier {0} not permitted in top-level declarations
 jshell.diag.modifier.single.ignore = Modifier {0} not permitted in top-level declarations, ignored
+jshell.diag.object.method.fatal = JShell method names must not match Object methods: {0}
 
 jshell.exc.null = Snippet must not be null
 jshell.exc.alien = Snippet not from this JShell: {0}
--- a/test/langtools/jdk/jshell/MethodsTest.java	Fri Oct 20 15:39:50 2017 -0700
+++ b/test/langtools/jdk/jshell/MethodsTest.java	Fri Oct 20 19:08:25 2017 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8080357 8167643
+ * @bug 8080357 8167643 8187359
  * @summary Tests for EvaluationState.methods
  * @build KullaTesting TestingInputStream ExpectedDiagnostic
  * @run testng MethodsTest
@@ -230,6 +230,24 @@
         assertActiveKeys();
     }
 
+    public void objectMethodNamedMethodsErrors() {
+        assertDeclareFail("boolean equals(double d1, double d2) {  return d1 == d2; }",
+                new ExpectedDiagnostic("jdk.eval.error.object.method", 8, 14, 8, -1, -1, Diagnostic.Kind.ERROR));
+        assertNumberOfActiveMethods(0);
+        assertActiveKeys();
+
+        assertDeclareFail("void          wait() { }",
+                new ExpectedDiagnostic("jdk.eval.error.object.method", 14, 18, 14, -1, -1, Diagnostic.Kind.ERROR));
+        assertNumberOfActiveMethods(0);
+        assertActiveKeys();
+
+        assertDeclareFail("  String   toString() throws NullPointerException{ }",
+                new ExpectedDiagnostic("jdk.eval.error.object.method", 11, 19, 11, -1, -1, Diagnostic.Kind.ERROR));
+        assertNumberOfActiveMethods(0);
+        assertActiveKeys();
+
+    }
+
 
     public void methodsAccessModifierIgnored() {
         Snippet f = methodKey(assertEval("public String f() {return null;}",