Unify basic instance creation and related error reporting (#9704)

12 files changed, 101 insertions, 40 deletions

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index ae467be123..43f893e33f 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -2261,12 +2261,10 @@ public class Binder<BEAN> implements Serializable {
     private HasValue<?> makeFieldInstance(
             Class<? extends HasValue<?>> fieldClass) {
         try {
-            return fieldClass.newInstance();
-        } catch (InstantiationException | IllegalAccessException e) {
-            throw new IllegalStateException(
-                    String.format("Couldn't create an '%s' type instance",
-                            fieldClass.getName()),
-                    e);
+            return ReflectTools.createInstance(fieldClass);
+        } catch (IllegalArgumentException e) {
+            // Rethrow as the exception type declared for bindInstanceFields
+            throw new IllegalStateException(e);
         }
     }
 
diff --git a/server/src/main/java/com/vaadin/navigator/Navigator.java b/server/src/main/java/com/vaadin/navigator/Navigator.java
index 23d6cc5f91..6b18cd4000 100644
--- a/server/src/main/java/com/vaadin/navigator/Navigator.java
+++ b/server/src/main/java/com/vaadin/navigator/Navigator.java
@@ -34,6 +34,7 @@ import com.vaadin.ui.ComponentContainer;
 import com.vaadin.ui.CssLayout;
 import com.vaadin.ui.SingleComponentContainer;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 /**
  * A navigator utility that allows switching of views in a part of an
@@ -315,15 +316,7 @@ public class Navigator implements Serializable {
         @Override
         public View getView(String viewName) {
             if (this.viewName.equals(viewName)) {
-                try {
-                    View view = viewClass.newInstance();
-                    return view;
-                } catch (InstantiationException | IllegalAccessException e) {
-                    // TODO error handling
-                    throw new RuntimeException(e);
-                }
-                // TODO error handling
-
+                return ReflectTools.createInstance(viewClass);
             }
             return null;
         }
@@ -1061,11 +1054,7 @@ public class Navigator implements Serializable {
         setErrorProvider(new ViewProvider() {
             @Override
             public View getView(String viewName) {
-                try {
-                    return viewClass.newInstance();
-                } catch (Exception e) {
-                    throw new RuntimeException(e);
-                }
+                return ReflectTools.createInstance(viewClass);
             }
 
             @Override
diff --git a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java
index 3bce462522..fc05f67860 100644
--- a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java
+++ b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java
@@ -45,6 +45,7 @@ import com.vaadin.ui.Component.Event;
 import com.vaadin.ui.HasComponents;
 import com.vaadin.ui.LegacyComponent;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 import elemental.json.JsonObject;
 import elemental.json.JsonValue;
@@ -295,7 +296,7 @@ public abstract class AbstractClientConnector
      */
     protected SharedState createState() {
         try {
-            return getStateType().newInstance();
+            return ReflectTools.createInstance(getStateType());
         } catch (Exception e) {
             throw new RuntimeException("Error creating state of type "
                     + getStateType().getName() + " for " + getClass().getName(),
diff --git a/server/src/main/java/com/vaadin/server/BootstrapHandler.java b/server/src/main/java/com/vaadin/server/BootstrapHandler.java
index 01fd2bf880..13a4852164 100644
--- a/server/src/main/java/com/vaadin/server/BootstrapHandler.java
+++ b/server/src/main/java/com/vaadin/server/BootstrapHandler.java
@@ -53,6 +53,7 @@ import com.vaadin.shared.communication.PushMode;
 import com.vaadin.ui.Dependency;
 import com.vaadin.ui.Dependency.Type;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 import elemental.json.Json;
 import elemental.json.JsonException;
@@ -262,7 +263,7 @@ public abstract class BootstrapHandler extends SynchronizedRequestHandler {
 
     /**
      * The URI resolver used in the bootstrap process.
-     * 
+     *
      * @since 8.1
      */
     protected static class BootstrapUriResolver extends VaadinUriResolver {
@@ -541,7 +542,8 @@ public abstract class BootstrapHandler extends SynchronizedRequestHandler {
             Class<? extends ViewportGenerator> viewportGeneratorClass = viewportGeneratorClassAnnotation
                     .value();
             try {
-                viewportContent = viewportGeneratorClass.newInstance()
+                viewportContent = ReflectTools
+                        .createInstance(viewportGeneratorClass)
                         .getViewport(context.getRequest());
             } catch (Exception e) {
                 throw new RuntimeException(
diff --git a/server/src/main/java/com/vaadin/server/JsonCodec.java b/server/src/main/java/com/vaadin/server/JsonCodec.java
index 9c105d505e..a7f94de468 100644
--- a/server/src/main/java/com/vaadin/server/JsonCodec.java
+++ b/server/src/main/java/com/vaadin/server/JsonCodec.java
@@ -46,6 +46,7 @@ import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.communication.UidlValue;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.ConnectorTracker;
+import com.vaadin.util.ReflectTools;
 
 import elemental.json.Json;
 import elemental.json.JsonArray;
@@ -611,7 +612,7 @@ public class JsonCodec implements Serializable {
         Class<?> targetClass = getClassForType(targetType);
 
         try {
-            Object decodedObject = targetClass.newInstance();
+            Object decodedObject = ReflectTools.createInstance(targetClass);
             for (BeanProperty property : getProperties(targetClass)) {
 
                 String fieldName = property.getName();
diff --git a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
index f3a56a2c53..4524442531 100644
--- a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
+++ b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
@@ -39,6 +39,7 @@ import com.vaadin.ui.ConnectorTracker;
 import com.vaadin.ui.HasComponents;
 import com.vaadin.ui.SelectiveRenderer;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 import elemental.json.JsonObject;
 import elemental.json.JsonValue;
@@ -124,7 +125,7 @@ public class LegacyCommunicationManager implements Serializable {
         }
 
         try {
-            SharedState referenceState = stateType.newInstance();
+            SharedState referenceState = ReflectTools.createInstance(stateType);
             EncodeResult encodeResult = JsonCodec.encode(referenceState, null,
                     stateType, null);
             return encodeResult.getEncodedValue();
diff --git a/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java b/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java
index 9f22905664..831239096a 100644
--- a/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java
+++ b/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java
@@ -20,6 +20,8 @@ import javax.portlet.PortletConfig;
 import javax.portlet.PortletException;
 import javax.portlet.PortletRequest;
 
+import com.vaadin.util.ReflectTools;
+
 public class LegacyVaadinPortlet extends VaadinPortlet {
 
     private static final LegacyApplicationUIProvider provider = new LegacyApplicationUIProvider() {
@@ -70,7 +72,7 @@ public class LegacyVaadinPortlet extends VaadinPortlet {
             throws PortletException {
         try {
             Class<? extends LegacyApplication> applicationClass = getApplicationClass();
-            return applicationClass.newInstance();
+            return ReflectTools.createInstance(applicationClass);
         } catch (Exception e) {
             throw new PortletException(e);
         }
diff --git a/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java b/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java
index 40f76ebd1a..70174fdc32 100644
--- a/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java
+++ b/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java
@@ -20,6 +20,8 @@ import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
+import com.vaadin.util.ReflectTools;
+
 public class LegacyVaadinServlet extends VaadinServlet {
 
     private static final UIProvider provider = new LegacyApplicationUIProvider() {
@@ -69,7 +71,7 @@ public class LegacyVaadinServlet extends VaadinServlet {
             throws ServletException {
         try {
             Class<? extends LegacyApplication> applicationClass = getApplicationClass();
-            return applicationClass.newInstance();
+            return ReflectTools.createInstance(applicationClass);
         } catch (Exception e) {
             throw new ServletException(e);
         }
diff --git a/server/src/main/java/com/vaadin/server/ServletPortletHelper.java b/server/src/main/java/com/vaadin/server/ServletPortletHelper.java
index bf7b1c8452..a679e97d2e 100644
--- a/server/src/main/java/com/vaadin/server/ServletPortletHelper.java
+++ b/server/src/main/java/com/vaadin/server/ServletPortletHelper.java
@@ -22,6 +22,7 @@ import java.util.Properties;
 import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 /**
  * Contains helper methods shared by {@link VaadinServlet} and
@@ -176,17 +177,13 @@ public class ServletPortletHelper implements Serializable {
             Class<?> providerClass = classLoader.loadClass(uiProviderProperty);
             Class<? extends UIProvider> subclass = providerClass
                     .asSubclass(UIProvider.class);
-            return subclass.newInstance();
+            return ReflectTools.createInstance(subclass);
         } catch (ClassNotFoundException e) {
             throw new ServiceException(
                     "Could not load UIProvider class " + uiProviderProperty, e);
         } catch (ClassCastException e) {
             throw new ServiceException("UIProvider class " + uiProviderProperty
                     + " does not extend UIProvider", e);
-        } catch (InstantiationException | IllegalAccessException e) {
-            throw new ServiceException(
-                    "Could not instantiate UIProvider " + uiProviderProperty,
-                    e);
         }
     }
 
diff --git a/server/src/main/java/com/vaadin/server/UIProvider.java b/server/src/main/java/com/vaadin/server/UIProvider.java
index a652b423ff..11d5d43109 100644
--- a/server/src/main/java/com/vaadin/server/UIProvider.java
+++ b/server/src/main/java/com/vaadin/server/UIProvider.java
@@ -31,6 +31,7 @@ import com.vaadin.annotations.Widgetset;
 import com.vaadin.shared.communication.PushMode;
 import com.vaadin.shared.ui.ui.Transport;
 import com.vaadin.ui.UI;
+import com.vaadin.util.ReflectTools;
 
 public abstract class UIProvider implements Serializable {
 
@@ -40,13 +41,7 @@ public abstract class UIProvider implements Serializable {
     public abstract Class<? extends UI> getUIClass(UIClassSelectionEvent event);
 
     public UI createInstance(UICreateEvent event) {
-        try {
-            return event.getUIClass().newInstance();
-        } catch (InstantiationException e) {
-            throw new RuntimeException("Could not instantiate UI class", e);
-        } catch (IllegalAccessException e) {
-            throw new RuntimeException("Could not access UI class", e);
-        }
+        return ReflectTools.createInstance(event.getUIClass());
     }
 
     /**
diff --git a/server/src/main/java/com/vaadin/ui/declarative/Design.java b/server/src/main/java/com/vaadin/ui/declarative/Design.java
index b051c68a90..78288737a0 100644
--- a/server/src/main/java/com/vaadin/ui/declarative/Design.java
+++ b/server/src/main/java/com/vaadin/ui/declarative/Design.java
@@ -44,6 +44,7 @@ import com.vaadin.ui.Composite;
 import com.vaadin.ui.CustomComponent;
 import com.vaadin.ui.declarative.DesignContext.ComponentCreatedEvent;
 import com.vaadin.ui.declarative.DesignContext.ComponentCreationListener;
+import com.vaadin.util.ReflectTools;
 
 /**
  * Design is used for reading a component hierarchy from an html string or input
@@ -175,7 +176,7 @@ public class Design implements Serializable {
                             + " which is not a Vaadin Component class";
 
             try {
-                return componentClass.newInstance();
+                return ReflectTools.createInstance(componentClass);
             } catch (Exception e) {
                 throw new DesignException(
                         "Could not create component " + fullyQualifiedClassName,
diff --git a/server/src/main/java/com/vaadin/util/ReflectTools.java b/server/src/main/java/com/vaadin/util/ReflectTools.java
index 8ff8ddcbb0..4728c76c47 100644
--- a/server/src/main/java/com/vaadin/util/ReflectTools.java
+++ b/server/src/main/java/com/vaadin/util/ReflectTools.java
@@ -20,6 +20,7 @@ import java.beans.PropertyDescriptor;
 import java.io.Serializable;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 
 /**
  * An util class with helpers for reflection operations. Used internally by
@@ -29,6 +30,14 @@ import java.lang.reflect.Method;
  * @since 6.2
  */
 public class ReflectTools implements Serializable {
+
+    static final String CREATE_INSTANCE_FAILED = "Unable to create an instance of {0}. Make sure it has a no-arg constructor";
+    static final String CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS = "Unable to create an instance of {0}. Make sure the class is static if it is a nested class.";
+    static final String CREATE_INSTANCE_FAILED_ACCESS_EXCEPTION = "Unable to create an instance of {0}. Make sure the class is public and that is has a public no-arg constructor.";
+    static final String CREATE_INSTANCE_FAILED_NO_PUBLIC_NOARG_CONSTRUCTOR = "Unable to create an instance of {0}. Make sure the class has a public no-arg constructor.";
+    static final String CREATE_INSTANCE_FAILED_LOCAL_CLASS = "Cannot instantiate local class '%s'. Move class declaration outside the method.";
+    static final String CREATE_INSTANCE_FAILED_CONSTRUCTOR_THREW_EXCEPTION = "Unable to create an instance of {0}. The constructor threw an exception.";
+
     /**
      * Locates the method in the given class. Returns null if the method is not
      * found. Throws an ExceptionInInitializerError if there is a problem
@@ -249,4 +258,67 @@ public class ReflectTools implements Serializable {
 
         return currentClass;
     }
+
+    /**
+     * Creates a instance of the given class with a no-arg constructor.
+     * <p>
+     * Catches all exceptions which might occur and wraps them in a
+     * {@link IllegalArgumentException} with a descriptive error message hinting
+     * of what might be wrong with the class that could not be instantiated.
+     *
+     * @param cls
+     *            the class to instantiate
+     * @return an instance of the class
+     * @since 8.1.1
+     */
+    public static <T> T createInstance(Class<T> cls) {
+        checkClassAccessibility(cls);
+        try {
+            return cls.getConstructor().newInstance();
+        } catch (NoSuchMethodException e) {
+            throw new IllegalArgumentException(String.format(
+                    CREATE_INSTANCE_FAILED_NO_PUBLIC_NOARG_CONSTRUCTOR,
+                    cls.getName()), e);
+        } catch (InstantiationException e) {
+            if (cls.isMemberClass() && !Modifier.isStatic(cls.getModifiers())) {
+                throw new IllegalArgumentException(String.format(
+                        CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS,
+                        cls.getName()), e);
+            } else {
+                throw new IllegalArgumentException(
+                        String.format(CREATE_INSTANCE_FAILED, cls.getName()),
+                        e);
+            }
+        } catch (IllegalAccessException e) {
+            throw new IllegalArgumentException(String.format(
+                    CREATE_INSTANCE_FAILED_ACCESS_EXCEPTION, cls.getName()), e);
+        } catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException(
+                    String.format(CREATE_INSTANCE_FAILED, cls.getName()), e);
+        } catch (InvocationTargetException e) {
+            throw new IllegalArgumentException(String.format(
+                    CREATE_INSTANCE_FAILED_CONSTRUCTOR_THREW_EXCEPTION,
+                    cls.getName()), e);
+        }
+    }
+
+    /**
+     * Makes a check whether the provided class is externally accessible for
+     * instantiation (e.g. it's not inner class (nested and not static) and is
+     * not a local class).
+     *
+     * @param cls
+     *            type to check
+     */
+    private static void checkClassAccessibility(Class<?> cls) {
+        if (cls.isMemberClass() && !Modifier.isStatic(cls.getModifiers())) {
+            throw new IllegalArgumentException(String.format(
+                    CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS,
+                    cls.getName()));
+        } else if (cls.isLocalClass()) {
+            throw new IllegalArgumentException(String
+                    .format(CREATE_INSTANCE_FAILED_LOCAL_CLASS, cls.getName()));
+        }
+    }
+
 }