Merge pull request #520 from NativeScript/pete/fix-inner-classes

Change Java inner classes convention

Author: petekanev

runtime/src/main/java/com/tns/MethodResolver.java

@@ -211,15 +211,19 @@ static Constructor<?> resolveConstructor(Class<?> clazz, Object[] args) throws C
 	{
 		Constructor<?>[] constructors = clazz.getConstructors();
 
+		int argLen = (args != null) ? args.length : 0;
+
 		if (constructors.length == 1)
 		{
+			if(argLen != constructors[0].getParameterTypes().length) {
+				return null;
+			}
+
 			return constructors[0];
 		}
 
 		ArrayList<Tuple<Constructor<?>, Integer>> candidates = new ArrayList<Tuple<Constructor<?>, Integer>>();
 
-		int argLen = (args != null) ? args.length : 0;
-
 		for (Constructor<?> constructor : constructors)
 		{
 			int dist = 0;
@@ -229,11 +233,11 @@ static Constructor<?> resolveConstructor(Class<?> clazz, Object[] args) throws C
 
 			if (args == null)
 			{
-
 				if (paramTypes.length == 0)
 				{
 					return constructor;
 				}
+
 				success = false;
 			}
 			else
@@ -273,6 +277,7 @@ static Constructor<?> resolveConstructor(Class<?> clazz, Object[] args) throws C
 				{
 					return constructor;
 				}
+
 				candidates.add(new Tuple<Constructor<?>, Integer>(constructor, Integer.valueOf(dist)));
 			}
 		}

runtime/src/main/java/com/tns/Runtime.java

@@ -54,6 +54,10 @@ void passUncaughtExceptionToJs(Throwable ex, String stackTrace)
 
 	private final static HashSet<ClassLoader> classLoaderCache = new HashSet<ClassLoader>();
 
+	private final static String FAILED_CTOR_RESOLUTION_MSG = "Check the number and type of arguments.\n" +
+			"Primitive types need to be manually wrapped in their respective Object wrappers.\n" +
+			"If you are creating an instance of an inner class, make sure to always provide reference to the outer `this` as the first argument.";
+
 	private final SparseArray<Object> strongInstances = new SparseArray<Object>();
 
 	private final SparseArray<WeakReference<Object>> weakInstances = new SparseArray<WeakReference<Object>>();
@@ -791,7 +795,7 @@ private String resolveConstructorSignature(Class<?> clazz, Object[] args) throws
 
 		if (res == null)
 		{
-			throw new Exception("Failed resolving constructor on class " + clazz.getName());
+			throw new Exception("Failed resolving constructor for class \'" + clazz.getName() + "\' with " + (args != null ? args.length : 0) + " parameters. " + FAILED_CTOR_RESOLUTION_MSG);
 		}
 
 		return res;

runtime/src/main/jni/ArgsWrapper.h

@@ -21,14 +21,13 @@ namespace tns
 	struct ArgsWrapper
 	{
 		public:
-			ArgsWrapper(const v8::FunctionCallbackInfo<v8::Value>& a, ArgType t, v8::Local<v8::Object> _outerThis)
+			ArgsWrapper(const v8::FunctionCallbackInfo<v8::Value>& a, ArgType t)
 			:
-					args(a), type(t), outerThis(_outerThis)
+					args(a), type(t)
 			{
 			}
 			v8::FunctionCallbackInfo<v8::Value> args;
 			ArgType type;
-			v8::Local<v8::Object> outerThis;
 	};
 }
 

runtime/src/main/jni/CallbackHandlers.cpp

@@ -101,13 +101,15 @@ bool CallbackHandlers::RegisterInstance(Isolate *isolate, const Local<Object> &j
         JavaObjectIdScope objIdScope(env, CURRENT_OBJECTID_FIELD_ID, runtime->GetJavaRuntime(),
                                      javaObjectID);
 
-        if (argWrapper.type == ArgType::Interface) {
-            instance = env.NewObject(generatedJavaClass, mi.mid);
-        }
-        else {
-            // resolve arguments before passing them on to the constructor
-            JsArgConverter argConverter(argWrapper.args, mi.signature, argWrapper.outerThis);
-            auto ctorArgs = argConverter.ToArgs();
+		if(argWrapper.type == ArgType::Interface)
+		{
+			instance = env.NewObject(generatedJavaClass, mi.mid);
+		}
+		else
+		{
+			// resolve arguments before passing them on to the constructor
+			JsArgConverter argConverter(argWrapper.args, mi.signature);
+			auto ctorArgs = argConverter.ToArgs();
 
             instance = env.NewObjectA(generatedJavaClass, mi.mid, ctorArgs);
         }

runtime/src/main/jni/JsArgConverter.cpp

@@ -18,7 +18,7 @@ using namespace std;
 using namespace tns;
 
 JsArgConverter::JsArgConverter(const v8::FunctionCallbackInfo<Value>& args, bool hasImplementationObject, const string& methodSignature, MetadataEntry *entry)
-: m_isolate(args.GetIsolate()), m_env(JEnv()), m_methodSignature(methodSignature), m_isValid(true), m_error(Error()), m_tokens(nullptr)
+		: m_isolate(args.GetIsolate()), m_env(JEnv()), m_methodSignature(methodSignature), m_isValid(true), m_error(Error()), m_tokens(nullptr)
 {
 	m_argsLen = !hasImplementationObject ? args.Length() : args.Length() - 1;
 
@@ -52,50 +52,8 @@ JsArgConverter::JsArgConverter(const v8::FunctionCallbackInfo<Value>& args, bool
 	}
 }
 
-JsArgConverter::JsArgConverter(const v8::FunctionCallbackInfo<Value>& args, const string& methodSignature, const Local<Object>& outerThis)
-: m_isolate(args.GetIsolate()), m_env(JEnv()), m_methodSignature(methodSignature), m_isValid(true), m_error(Error()), m_tokens(nullptr)
-{
-	auto isInnerClass = !outerThis.IsEmpty();
-	if (isInnerClass)
-	{
-		m_argsLen = args.Length() + 1;
-	}
-	else
-	{
-		m_argsLen = args.Length();
-	}
-
-	JniSignatureParser parser(m_methodSignature);
-	m_tokens2 = parser.Parse();
-	m_tokens = &m_tokens2;
-
-	for (int i = 0; i < m_argsLen; i++)
-	{
-		if (isInnerClass)
-		{
-			if (i == 0)
-			{
-				m_isValid = ConvertArg(outerThis, i);
-			}
-			else
-			{
-				m_isValid = ConvertArg(args[i - 1], i);
-			}
-		}
-		else
-		{
-			m_isValid = ConvertArg(args[i], i);
-		}
-
-		if (!m_isValid)
-		{
-			break;
-		}
-	}
-}
-
 JsArgConverter::JsArgConverter(const v8::FunctionCallbackInfo<Value>& args, const string& methodSignature)
-: m_isolate(args.GetIsolate()), m_env(JEnv()), m_methodSignature(methodSignature), m_isValid(true), m_error(Error()), m_tokens(nullptr)
+		: m_isolate(args.GetIsolate()), m_env(JEnv()), m_methodSignature(methodSignature), m_isValid(true), m_error(Error()), m_tokens(nullptr)
 {
 	m_argsLen = args.Length();
 
@@ -107,6 +65,7 @@ JsArgConverter::JsArgConverter(const v8::FunctionCallbackInfo<Value>& args, cons
 	{
 		m_isValid = ConvertArg(args[i], i);
 
+
 		if (!m_isValid)
 		{
 			break;
@@ -324,8 +283,8 @@ bool JsArgConverter::ConvertJavaScriptNumber(const Local<Value>& jsValue, int in
 	bool success = true;
 
 	jvalue value =
-	{
-			0 };
+			{
+					0 };
 
 	const auto& typeSignature = m_tokens->at(index);
 

runtime/src/main/jni/JsArgConverter.h

@@ -16,8 +16,6 @@ namespace tns
 
 			JsArgConverter(const v8::FunctionCallbackInfo<v8::Value>& args, const std::string& methodSignature);
 
-			JsArgConverter(const v8::FunctionCallbackInfo<v8::Value>& args, const std::string& methodSignature, const v8::Local<v8::Object>& outerThis);
-
 			~JsArgConverter();
 
 			jvalue* ToArgs();

runtime/src/main/jni/JsArgToArrayConverter.cpp

@@ -23,7 +23,7 @@ using namespace tns;
  * Converts a single JavaScript (V8) object to its respective Java representation
  */
 JsArgToArrayConverter::JsArgToArrayConverter(Isolate *isolate, const v8::Local<Value>& arg, bool isImplementationObject, int classReturnType)
-	: m_isolate(isolate), m_arr(nullptr), m_argsAsObject(nullptr), m_argsLen(0), m_isValid(false), m_error(Error()), m_return_type(classReturnType)
+		: m_isolate(isolate), m_arr(nullptr), m_argsAsObject(nullptr), m_argsLen(0), m_isValid(false), m_error(Error()), m_return_type(classReturnType)
 {
 	if (!isImplementationObject)
 	{
@@ -38,18 +38,10 @@ JsArgToArrayConverter::JsArgToArrayConverter(Isolate *isolate, const v8::Local<V
 /*
  * Converts an array of JavaScript (V8) objects to a Java array of objects
  */
-JsArgToArrayConverter::JsArgToArrayConverter(const v8::FunctionCallbackInfo<Value>& args, bool hasImplementationObject, const Local<Object>& outerThis)
-	: m_isolate(args.GetIsolate()), m_arr(nullptr), m_argsAsObject(nullptr), m_argsLen(0), m_isValid(false), m_error(Error()), m_return_type(static_cast<int>(Type::Null))
+JsArgToArrayConverter::JsArgToArrayConverter(const v8::FunctionCallbackInfo<Value>& args, bool hasImplementationObject)
+		: m_isolate(args.GetIsolate()), m_arr(nullptr), m_argsAsObject(nullptr), m_argsLen(0), m_isValid(false), m_error(Error()), m_return_type(static_cast<int>(Type::Null))
 {
-	auto isInnerClass = !outerThis.IsEmpty();
-	if (isInnerClass)
-	{
-		m_argsLen = args.Length() + 1;
-	}
-	else
-	{
-		m_argsLen = !hasImplementationObject ? args.Length() : args.Length() - 2;
-	}
+	m_argsLen = !hasImplementationObject ? args.Length() : args.Length() - 2;
 
 	bool success = true;
 
@@ -60,21 +52,7 @@ JsArgToArrayConverter::JsArgToArrayConverter(const v8::FunctionCallbackInfo<Valu
 
 		for (int i = 0; i < m_argsLen; i++)
 		{
-			if (isInnerClass)
-			{
-				if (i == 0)
-				{
-					success = ConvertArg(outerThis, i);
-				}
-				else
-				{
-					success = ConvertArg(args[i - 1], i);
-				}
-			}
-			else
-			{
-				success = ConvertArg(args[i], i);
-			}
+			success = ConvertArg(args[i], i);
 
 			if (!success)
 			{
@@ -294,7 +272,7 @@ bool JsArgToArrayConverter::ConvertArg(const Local<Value>& arg, int index)
 
 			case CastType::None:
 				obj = objectManager->GetJavaObjectByJsObject(jsObj);
-                
+
 				castValue = jsObj->GetHiddenValue(V8StringConstants::GetNullNodeName());
 
 				if(!castValue.IsEmpty()) {

runtime/src/main/jni/JsArgToArrayConverter.h

@@ -11,7 +11,7 @@ namespace tns
 	class JsArgToArrayConverter
 	{
 		public:
-			JsArgToArrayConverter(const v8::FunctionCallbackInfo<v8::Value>& args, bool hasImplementationObject, const v8::Local<v8::Object>& outerThis = v8::Local<v8::Object>());
+			JsArgToArrayConverter(const v8::FunctionCallbackInfo<v8::Value>& args, bool hasImplementationObject);
 
 			JsArgToArrayConverter(v8::Isolate *isolate, const v8::Local<v8::Value>& arg, bool isImplementationObject, int classReturnType);