Non-empty @name not allowed

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/FieldInjections.java

@@ -30,10 +30,10 @@
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.api.proc.CallableProcedure;
-import org.neo4j.kernel.impl.messages.Messages;
+import org.neo4j.messages.Messages;
 import org.neo4j.procedure.Context;
 
-import static org.neo4j.kernel.impl.messages.Messages.proc_static_field_annotated_as_context;
+import static org.neo4j.messages.Messages.proc_static_field_annotated_as_context;
 
 /**
  * Injects annotated fields with appropriate values.

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/MethodSignatureCompiler.java

@@ -28,8 +28,13 @@
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.api.proc.ProcedureSignature.FieldSignature;
+import org.neo4j.messages.Messages;
 import org.neo4j.procedure.Name;
 
+import static org.neo4j.messages.Messages.proc_argument_missing_name;
+import static org.neo4j.messages.Messages.proc_argument_name_empty;
+import static org.neo4j.messages.Messages.proc_unmappable_argument_type;
+
 /**
  * Given a java method, figures out a valid {@link org.neo4j.kernel.api.proc.ProcedureSignature} field signature.
  * Basically, it takes the java signature and spits out the same signature described as Neo4j types.
@@ -56,21 +61,27 @@ public List<FieldSignature> signatureFor( Method method ) throws ProcedureExcept
             if ( !param.isAnnotationPresent( Name.class ) )
             {
                 throw new ProcedureException( Status.Procedure.FailedRegistration,
-                        "Argument at position %d in method `%s` is missing an `@%s` annotation. " +
-                        "Please add the annotation, recompile the class and try again.", i, method.getName(),
-                        Name.class.getSimpleName() );
+                        Messages.get( proc_argument_missing_name, i, method.getName(),
+                        Name.class.getSimpleName()) );
             }
             String name = param.getAnnotation( Name.class ).value();
 
+            if( name.trim().length() == 0 )
+            {
+                throw new ProcedureException( Status.Procedure.FailedRegistration,
+                        Messages.get( proc_argument_name_empty, i, method.getName() ) );
+            }
+
             try
             {
                 signature.add(new FieldSignature( name, typeMappers.neoTypeFor( type ) ));
             }
             catch ( ProcedureException e )
             {
                 throw new ProcedureException( e.status(),
-                        "Argument `%s` at position %d in `%s` with type `%s` cannot be converted to a Neo4j type: %s",
-                        name, i, method.getName(), param.getType().getSimpleName(), e.getLocalizedMessage() );
+                        Messages.get( proc_unmappable_argument_type,
+                            name, i, method.getName(), param.getType().getSimpleName(),
+                            e.getMessage() ) );
             }
 
         }

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/OutputMappers.java

@@ -33,13 +33,13 @@
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.api.proc.ProcedureSignature;
 import org.neo4j.kernel.api.proc.ProcedureSignature.FieldSignature;
-import org.neo4j.kernel.impl.messages.Messages;
+import org.neo4j.messages.Messages;
 
 import static java.lang.reflect.Modifier.isPublic;
 import static java.lang.reflect.Modifier.isStatic;
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
-import static org.neo4j.kernel.impl.messages.Messages.proc_invalid_return_type_description;
+import static org.neo4j.messages.Messages.proc_invalid_return_type_description;
 
 /**
  * Takes user-defined record classes, and does two things: Describe the class as a {@link ProcedureSignature}, and provide a mechanism to convert
@@ -137,8 +137,7 @@ public OutputMapper mapper( Method method ) throws ProcedureException
         if ( cls != Stream.class )
         {
             throw new ProcedureException( Status.Procedure.TypeError,
-                    Messages.get( proc_invalid_return_type_description,
-                            cls.getSimpleName(), cls.getSimpleName() ));
+                    Messages.get( proc_invalid_return_type_description, cls.getSimpleName() ));
         }
 
         ParameterizedType genType = (ParameterizedType) method.getGenericReturnType();
@@ -195,15 +194,20 @@ private void assertIsValidRecordClass( Class<?> userClass ) throws ProcedureExce
             || userClass.getPackage() != null && userClass.getPackage().getName().startsWith( "java." ) )
         {
             throw new ProcedureException( Status.Procedure.TypeError,
-                    Messages.get( proc_invalid_return_type_description,
-                        userClass.getSimpleName(), userClass.getSimpleName() ));
+                    Messages.get( proc_invalid_return_type_description, userClass.getSimpleName() ));
         }
     }
 
     private List<Field> instanceFields( Class<?> userClass )
     {
         return asList( userClass.getDeclaredFields() ).stream()
-                .filter( ( f ) -> !isStatic( f.getModifiers() ) && !f.getName().contains( "$" ) )
+                .filter( ( f ) -> !isStatic( f.getModifiers() ) &&
+                                  !isSynthetic( f.getModifiers() ) )
                 .collect( toList() );
     }
+
+    private static boolean isSynthetic( int modifiers )
+    {
+        return (modifiers & 0x00001000) != 0;
+    }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/TypeMappers.java

@@ -29,6 +29,7 @@
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.api.proc.Neo4jTypes.AnyType;
+import org.neo4j.messages.Messages;
 
 import static org.neo4j.kernel.api.proc.Neo4jTypes.NTAny;
 import static org.neo4j.kernel.api.proc.Neo4jTypes.NTBoolean;
@@ -38,6 +39,7 @@
 import static org.neo4j.kernel.api.proc.Neo4jTypes.NTMap;
 import static org.neo4j.kernel.api.proc.Neo4jTypes.NTNumber;
 import static org.neo4j.kernel.api.proc.Neo4jTypes.NTString;
+import static org.neo4j.messages.Messages.proc_unmappable_type;
 
 public class TypeMappers
 {
@@ -140,11 +142,11 @@ private NeoValueConverter toList( NeoValueConverter inner )
 
     private ProcedureException javaToNeoMappingError( Type cls )
     {
-        return new ProcedureException( Status.Statement.InvalidType, "Don't know how to map `%s` to the Neo4j Type " +
-                                                                     "System. Please refer to " +
-                                                                     "to the documentation for full details. For " +
-                                                                     "your reference, known types are: %s",
-                cls, Iterables.toList(javaToNeo.keySet()) );
+        List<Type> types = Iterables.toList( javaToNeo.keySet() );
+        types.sort( (a,b)->a.toString().compareTo( b.toString() ) );
+
+        return new ProcedureException( Status.Statement.InvalidType,
+                Messages.get( proc_unmappable_type, cls, types ));
     }
 
     public static class SimpleConverter implements NeoValueConverter

community/kernel/src/main/java/org/neo4j/messages/Messages.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.messages;
+
+import java.text.MessageFormat;
+
+/**
+ * This is a beach head - if you are implementing code that prints human language
+ * messages in any way, through errors, warnings, explanations or any other
+ * mechanism - please help expand, refactor, move and use this this class to do
+ * so.
+ *
+ * The intention of this is to slowly introduce a proper i18n message API,
+ * backed by the standard i18n support in Java.
+ *
+ * The messages in this file are templates fed to the {@link MessageFormat} formatter.
+ * If you are including things like times, dates or other locale-sensitive things,
+ * please use the pattern utilities provided by {@link MessageFormat} to format them,
+ * since that will make it much easier to internationalize them in the future.
+ */
+public interface Messages
+{
+    // Note: Single-quotes, `'` and curly brackets have special meaning
+    // to MessageFormat and need to be escaped as "''" and "'{'", respectively.
+
+    // Note: This file is going to become very large. As it does, please use
+    // best judgement to split it into appropriate categories and sub-files.
+
+    Message proc_invalid_return_type_description =
+            msg("Procedures must return a Stream of records, where a record is a concrete class\n" +
+                "that you define, with public non-final fields defining the fields in the record.\n" +
+                "If you''d like your procedure to return `{0}`, you could define a record class like:\n" +
+                "public class Output '{'\n" +
+                "    public {0} out;\n" +
+                "'}'\n" +
+                "\n" +
+                "And then define your procedure as returning `Stream<Output>`.");
+
+    Message proc_static_field_annotated_as_context =
+            msg("The field `{0}` in the class named `{1}` is annotated as a @Context field,\n" +
+                "but it is static. @Context fields must be public, non-final and non-static,\n" +
+                "because they are reset each time a procedure is invoked.");
+
+    Message proc_unmappable_type =
+            msg("Don''t know how to map `{0}` to the Neo4j Type System.\n" +
+                "Please refer to to the documentation for full details.\n" +
+                "For your reference, known types are: {1}");
+
+    Message proc_unmappable_argument_type =
+            msg("Argument `{0}` at position {1} in `{2}` with\n" +
+                "type `{3}` cannot be converted to a Neo4j type: {4}" );
+
+    Message proc_argument_missing_name =
+            msg("Argument at position {0} in method `{1}` is missing an `@{2}` annotation.\n" +
+                "Please add the annotation, recompile the class and try again." );
+
+    Message proc_argument_name_empty =
+            msg("Argument at position {0} in method `{1}` is annotated with a name,\n" +
+                "but the name is empty, please provide a non-empty name for the argument." );
+
+    interface Message
+    {
+        String defaultMessage();
+    }
+
+    // Implementation note:
+    // We use this silly-looking indirection of calling Messages#get(..)
+    // instead of just calling message.defaultMessage() wherever we need
+    // these messages. The reason is the intended direction of this interface,
+    // which is that we'd be able to plug i18n lookups in underneath, so if
+    // you in the future are running Neo4j with a German locale, you'd get
+    // german messages out on the other end.
+
+    /**
+     * Get a message, optionally filling in positional arguments.
+     * @param msg the message to output
+     * @param args optional dynamic parts of the message
+     * @return the message with the dynamic parameters inserted
+     */
+    static String get( Message msg, Object ... args )
+    {
+        return MessageFormat.format( msg.defaultMessage(), args );
+    }
+
+    /** Create a new message */
+    static Message msg(String defaultMessage)
+    {
+        return () -> defaultMessage;
+    }
+}

community/kernel/src/test/java/org/neo4j/kernel/impl/proc/MethodSignatureCompilerTest.java

@@ -97,14 +97,12 @@ public void shouldGiveHelpfulErrorOnUnmappable() throws Throwable
 
         // Expect
         exception.expect( ProcedureException.class );
-        exception.expectMessage( "Argument `name` at position 0 in `echoWithInvalidType` with type " +
-                                 "`UnmappableRecord` cannot be converted to a Neo4j type: Don't know how to map " +
+        exception.expectMessage( "Argument `name` at position 0 in `echoWithInvalidType` with\n" +
+                                 "type `UnmappableRecord` cannot be converted to a Neo4j type: Don't know how to map " +
                                  "`class org.neo4j.kernel.impl.proc.MethodSignatureCompilerTest$UnmappableRecord` to " +
-                                 "the Neo4j Type System. Please refer to to the documentation for full details. For " +
-                                 "your reference, known types are: [long, double, class java.lang.Number, class java" +
-                                 ".lang.Object, class java.lang.String, class java.lang.Long, class java.lang.Double," +
-                                 " interface java.util.List, interface java.util.Map, boolean, class java.lang" +
-                                 ".Boolean]" );
+                                 "the Neo4j Type System.\n" +
+                                 "Please refer to to the documentation for full details.\n" +
+                                 "For your reference, known types are:" );
 
         // When
         new MethodSignatureCompiler(new TypeMappers()).signatureFor( echo );
@@ -118,9 +116,9 @@ public void shouldGiveHelpfulErrorOnMissingAnnotations() throws Throwable
 
         // Expect
         exception.expect( ProcedureException.class );
-        exception.expectMessage( "Argument at position 1 in method `echoWithoutAnnotations` is missing an " +
-                                 "`@Name` annotation. Please add the annotation, recompile the class and " +
-                                 "try again." );
+        exception.expectMessage( "Argument at position 1 in method `echoWithoutAnnotations` is missing an `@Name` " +
+                                 "annotation.\n" +
+                                 "Please add the annotation, recompile the class and try again." );
 
         // When
         new MethodSignatureCompiler(new TypeMappers()).signatureFor( echo );

community/kernel/src/test/java/org/neo4j/kernel/impl/proc/OutputMappersTest.java

@@ -157,14 +157,13 @@ public void shouldWarnAgainstStdLibraryClassesSinceTheseIndicateUserError() thro
 
         // Expect
         exception.expect( ProcedureException.class );
-        exception.expectMessage( "Procedures must return a Stream of records, where a record is a concrete class that you define, with public non-final " +
-                                 "fields defining the fields in the record. If you'd like your procedure to return `Long`, you could define a record class " +
+        exception.expectMessage( "Procedures must return a Stream of records, where a record is a concrete class\n" +
+                                 "that you define, with public non-final fields defining the fields in the record.\n" +
+                                 "If you'd like your procedure to return `Long`, you could define a record class " +
                                  "like:\n" +
                                  "public class Output {\n" +
                                  "    public Long out;\n" +
-                                 "}\n" +
-                                 "\n" +
-                                 "And then define your procedure as returning `Stream<Output>`." );
+                                 "}" );
 
         // When
         mapper(Long.class);

community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureJarLoaderTest.java

@@ -117,8 +117,13 @@ public void shouldGiveHelpfulErrorOnInvalidProcedure() throws Throwable
 
         // Expect
         exception.expect( ProcedureException.class );
-        exception.expectMessage( "A procedure must return a `java.util.stream.Stream`, " +
-                                 "`ClassWithInvalidProcedure.booleansAreNotAcceptableReturnTypes` returns `boolean`." );
+        exception.expectMessage( "Procedures must return a Stream of records, where a record is a concrete class\n" +
+                                 "that you define, with public non-final fields defining the fields in the record.\n" +
+                                 "If you'd like your procedure to return `boolean`, you could define a record class " +
+                                 "like:\n" +
+                                 "public class Output {\n" +
+                                 "    public boolean out;\n" +
+                                 "}\n" );
 
         // When
         jarloader.loadProcedures( jar );