From 5ec22ca4bc475c0c7532e85ff9aac64c1246c196 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 1 Jul 2019 23:57:11 +0200 Subject: [PATCH 1/4] v2: implement Node.load() Signed-off-by: Alexander Bezzubov --- .travis.yml | 11 - src/main/native/jni_utils.cc | 78 +++- src/main/native/jni_utils.h | 21 + .../native/org_bblfsh_client_v2_Context__.h | 21 + .../org_bblfsh_client_v2_libuast_Libuast.cc | 400 +++++++++++++++++- .../scala/org/bblfsh/client/v2/Context.scala | 10 +- .../scala/org/bblfsh/client/v2/JNode.scala | 61 ++- src/test/resources/Tiny.java | 1 + .../client/v2/BblfshClientBaseTest.scala | 25 ++ .../client/v2/BblfshClientCloseTest.scala | 2 +- .../client/v2/BblfshClientLoadTest.scala | 65 +++ .../client/v2/BblfshClientParseTest.scala | 24 +- .../BblfshClientSupportedLanguagesTest.scala | 2 +- .../client/v2/BblfshClientVersionTest.scala | 2 +- .../org/bblfsh/client/v2/JNodeTest.scala | 74 ++++ 15 files changed, 730 insertions(+), 67 deletions(-) create mode 100644 src/main/native/org_bblfsh_client_v2_Context__.h create mode 100644 src/test/resources/Tiny.java create mode 100644 src/test/scala/org/bblfsh/client/v2/BblfshClientBaseTest.scala create mode 100644 src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala create mode 100644 src/test/scala/org/bblfsh/client/v2/JNodeTest.scala diff --git a/.travis.yml b/.travis.yml index ab7eb09..fc2d9c5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,12 +18,10 @@ stages: - name: release if: tag IS present - jobs: include: - name: 'All tests' stage: test - if: tag IS present # TODO(bzz): enable on PRs as soon as migrated to V2 install: &test_setup_anchor - docker run --privileged -d -p 9432:9432 --name bblfsh bblfsh/bblfshd - docker exec -it bblfsh bblfshctl driver install --recommended @@ -34,15 +32,6 @@ jobs: after_failure: &failure_logs_anchor - docker logs bblfsh - - name: 'V2: passing tests' # TODO(#83): remove, after both tests sets converge - install: *test_setup_anchor - script: - - sudo apt-get install -y binutils - - ./sbt assembly - - ./sbt "testOnly *Close* *ClientVersion* *SupportedLanguages* *BblfshClientParseTest" - - after_failure: *failure_logs_anchor - - name: 'Cross-compile, release & publish to Sonatype' stage: release before_install: diff --git a/src/main/native/jni_utils.cc b/src/main/native/jni_utils.cc index 28662f2..84caf06 100644 --- a/src/main/native/jni_utils.cc +++ b/src/main/native/jni_utils.cc @@ -1,9 +1,5 @@ #include "jni_utils.h" -// Class fully qualified names -const char *CLS_NODE = "org/bblfsh/client/v2/Node"; -const char *CLS_CTX = "org/bblfsh/client/v2/Context"; - extern JavaVM *jvm; // FIXME(bzz): double-check and document JNIEnv *getJNIEnv() { @@ -21,16 +17,38 @@ JNIEnv *getJNIEnv() { return pEnv; } +// Class fully qualified names +const char *CLS_NODE = "org/bblfsh/client/v2/Node"; +const char *CLS_CTX = "org/bblfsh/client/v2/Context"; + +const char *CLS_JNODE = "org/bblfsh/client/v2/JNode"; +const char *CLS_JNULL = "org/bblfsh/client/v2/JNull"; +const char *CLS_JSTR = "org/bblfsh/client/v2/JString"; +const char *CLS_JINT = "org/bblfsh/client/v2/JInt"; +const char *CLS_JFLT = "org/bblfsh/client/v2/JFloat"; +const char *CLS_JBOL = "org/bblfsh/client/v2/JBool"; +const char *CLS_JUINT = "org/bblfsh/client/v2/JUint"; +const char *CLS_JARR = "org/bblfsh/client/v2/JArray"; +const char *CLS_JOBJ = "org/bblfsh/client/v2/JObject"; + +const char *METHOD_JNODE_KEY_AT = "(I)Ljava/lang/String;"; +const char *METHOD_JNODE_VALUE_AT = "(I)Lorg/bblfsh/client/v2/JNode;"; +const char *METHOD_JOBJ_ADD = + "(Ljava/lang/String;Lorg/bblfsh/client/v2/JNode;)Lscala/collection/" + "mutable/Buffer;"; +const char *METHOD_JARR_ADD = + "(Lorg/bblfsh/client/v2/JNode;)Lscala/collection/mutable/Buffer;"; + jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, ...) { jclass cls = env->FindClass(className); if (env->ExceptionOccurred() || !cls) { - return NULL; + return nullptr; } jmethodID initId = env->GetMethodID(cls, "", initSign); if (env->ExceptionOccurred() || !initId) { - return NULL; + return nullptr; } va_list varargs; @@ -38,7 +56,7 @@ jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, jobject instance = env->NewObjectV(cls, initId, varargs); va_end(varargs); if (env->ExceptionOccurred() || !instance) { - return NULL; + return nullptr; } return instance; @@ -56,3 +74,49 @@ jfieldID getField(JNIEnv *env, jobject obj, const char *name) { } return jfid; } + +static jmethodID MethodID(JNIEnv *env, const char *method, + const char *signature, const char *className) { + jclass cls = env->FindClass(className); + if (env->ExceptionOccurred() || !cls) { + return nullptr; + } + + jmethodID mId = env->GetMethodID(cls, method, signature); + if (env->ExceptionOccurred()) { + return nullptr; + } + + return mId; +} + +jint IntMethod(JNIEnv *env, const char *method, const char *signature, + const char *className, const jobject *object) { + jmethodID mId = MethodID(env, method, signature, className); + if (env->ExceptionOccurred() || !mId) return 0; + // TODO(bzz): add better error handling + // ExceptionOccurred() + // ExceptionDescribe() + // ExceptionClear() + // ExceptionCheck() + // ThrowNew("failed to call $className.$method") to JVM + + jint res = env->CallIntMethod(*object, mId); + if (env->ExceptionOccurred()) return 0; + + return res; +} + +jobject ObjectMethod(JNIEnv *env, const char *method, const char *signature, + const char *className, const jobject *object, ...) { + jmethodID mId = MethodID(env, method, signature, className); + if (env->ExceptionOccurred() || !mId) return nullptr; + + va_list varargs; + va_start(varargs, object); + jobject res = env->CallObjectMethodV(*object, mId, varargs); + va_end(varargs); + if (env->ExceptionOccurred() || !res) return nullptr; + + return res; +} diff --git a/src/main/native/jni_utils.h b/src/main/native/jni_utils.h index ce00fd5..f3ab76f 100644 --- a/src/main/native/jni_utils.h +++ b/src/main/native/jni_utils.h @@ -6,8 +6,29 @@ extern const char *CLS_NODE; extern const char *CLS_CTX; +extern const char *CLS_JNODE; +extern const char *CLS_JNULL; +extern const char *CLS_JSTR; +extern const char *CLS_JINT; +extern const char *CLS_JFLT; +extern const char *CLS_JBOL; +extern const char *CLS_JUINT; +extern const char *CLS_JARR; +extern const char *CLS_JOBJ; + +extern const char *METHOD_JNODE_KEY_AT; +extern const char *METHOD_JNODE_VALUE_AT; +extern const char *METHOD_JOBJ_ADD; +extern const char *METHOD_JARR_ADD; + JNIEnv *getJNIEnv(); jobject NewJavaObject(JNIEnv *, const char *, const char *, ...); jfieldID getField(JNIEnv *env, jobject obj, const char *name); +jint IntMethod(JNIEnv *, const char *, const char *, const char *, + const jobject *); + +jobject ObjectMethod(JNIEnv *, const char *, const char *, const char *, + const jobject *, ...); + #endif \ No newline at end of file diff --git a/src/main/native/org_bblfsh_client_v2_Context__.h b/src/main/native/org_bblfsh_client_v2_Context__.h new file mode 100644 index 0000000..aa77e87 --- /dev/null +++ b/src/main/native/org_bblfsh_client_v2_Context__.h @@ -0,0 +1,21 @@ +/* DO NOT EDIT THIS FILE - it is machine generated */ +#include +/* Header for class org_bblfsh_client_v2_Context__ */ + +#ifndef _Included_org_bblfsh_client_v2_Context__ +#define _Included_org_bblfsh_client_v2_Context__ +#ifdef __cplusplus +extern "C" { +#endif +/* + * Class: org_bblfsh_client_v2_Context__ + * Method: create + * Signature: ()J + */ +JNIEXPORT jlong JNICALL Java_org_bblfsh_client_v2_Context_00024_create + (JNIEnv *, jobject); + +#ifdef __cplusplus +} +#endif +#endif diff --git a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc index f0d4b1a..050acfb 100644 --- a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc +++ b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc @@ -1,6 +1,10 @@ -#include "org_bblfsh_client_v2_libuast_Libuast.h" +#include + #include "jni_utils.h" #include "org_bblfsh_client_v2_Context.h" +#include "org_bblfsh_client_v2_Context__.h" +#include "org_bblfsh_client_v2_Node.h" +#include "org_bblfsh_client_v2_libuast_Libuast.h" #include "libuast.h" #include "libuast.hpp" @@ -10,12 +14,7 @@ JavaVM *jvm; namespace { -jobject asJvmBuffer(uast::Buffer buf) { - JNIEnv *env = getJNIEnv(); - return env->NewDirectByteBuffer(buf.ptr, buf.size); -} - -const char *nativeContext = "nativeContext"; +constexpr char nativeContext[] = "nativeContext"; jfieldID getHandleField(JNIEnv *env, jobject obj, const char *name) { jclass cls = env->GetObjectClass(obj); @@ -45,6 +44,15 @@ void setHandle(JNIEnv *env, jobject obj, T *t, const char *name) { env->SetLongField(obj, getHandleField(env, obj, name), handle); } +jobject asJvmBuffer(uast::Buffer buf) { + JNIEnv *env = getJNIEnv(); + return env->NewDirectByteBuffer(buf.ptr, buf.size); +} + +// ========================================== +// External UAST Context (managed by libuast) +// ========================================== + class ContextExt { private: uast::Context *ctx; @@ -103,9 +111,354 @@ class ContextExt { return asJvmBuffer(data); } }; + +// ================================================ +// UAST Node interface (called from libuast) +// ================================================ +class Interface; + +class Node : public uast::Node { + private: + Interface *iface; + jobject obj; // Node owns a (global) reference + NodeKind kind; + + jobject keys; + std::string *str; + + // kindOf returns a kind of a JVM object. + // Borrows the reference. + static NodeKind kindOf(jobject obj) { + JNIEnv *env = getJNIEnv(); + // TODO(bzz): expose JNode.kind & replace type comparison \w a string test + if (!obj) { + return NODE_NULL; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JSTR))) { + return NODE_STRING; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JINT))) { + return NODE_INT; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JFLT))) { + return NODE_FLOAT; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JBOL))) { + return NODE_BOOL; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JUINT))) { + return NODE_BOOL; + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JARR))) { + return NODE_ARRAY; + } + return NODE_OBJECT; + } + + Node *lookupOrCreate(jobject obj); + + public: + friend class Interface; + friend class Context; + + // Node creates a new node associated with a given JVM object and sets the + // kind. Creates a new global reference. + Node(Interface *i, NodeKind k, jobject v) : keys(nullptr), str(nullptr) { + iface = i; + obj = getJNIEnv()->NewGlobalRef(v); + kind = k; + } + + // Node creates a new node associated with a given JVM object and + // automatically determines the kind. Creates a new global reference. + Node(Interface *i, jobject v) : keys(nullptr), str(nullptr) { + iface = i; + obj = getJNIEnv()->NewGlobalRef(v); + kind = kindOf(v); + } + + ~Node() { + JNIEnv *env = getJNIEnv(); + if (keys) { + env->DeleteGlobalRef(keys); + keys = nullptr; + } + if (obj) { + env->DeleteGlobalRef(obj); + } + if (str) { + delete str; + } + } + + jobject toJ(); + + NodeKind Kind() { return kind; } + + // TODO(#90) all 'As*' are unused stubs, make them buletprof and test + std::string *AsString() { + if (!str) { + JNIEnv *env = getJNIEnv(); + const char *utf = env->GetStringUTFChars((jstring)obj, 0); + str = new std::string(utf); + env->ReleaseStringUTFChars((jstring)obj, utf); + } + + std::string *s = new std::string(*str); + return s; + } + int64_t AsInt() { + JNIEnv *env = getJNIEnv(); + jclass cls = env->FindClass("java/lang/Integer"); + jmethodID valueId = env->GetMethodID(cls, "longValue", "()J"); + long long value = (long long)env->CallLongMethod(obj, valueId); + return (int64_t)(value); + } + uint64_t AsUint() { + JNIEnv *env = getJNIEnv(); + jclass cls = env->FindClass("java/lang/Integer"); + jmethodID valueId = env->GetMethodID(cls, "intValue", "()I"); + jlong value = env->CallIntMethod(obj, valueId); + + jmethodID mId = env->GetMethodID(cls, "toUnsignedLong", "(I)J"); + jlong v = env->CallLongMethod(obj, mId, value); + + return (uint64_t)(v); + } + double AsFloat() { + JNIEnv *env = getJNIEnv(); + jclass cls = env->FindClass("java/lang/Double"); + jmethodID valueId = env->GetMethodID(cls, "floatValue", "()F"); + float value = (float)env->CallFloatMethod(obj, valueId); + return value; + } + bool AsBool() { + JNIEnv *env = getJNIEnv(); + // TODO(bzz) check failures, cache classes, read 'value' filed + jclass cls = env->FindClass("java/lang/Boolean"); + jmethodID valueId = env->GetMethodID(cls, "booleanValue", "()Z"); + bool value = (bool)env->CallBooleanMethod(obj, valueId); + return value; + } + size_t Size() { + jint size = IntMethod(getJNIEnv(), "size", "()I", CLS_JNODE, &obj); + assert(int32_t(size) >= 0); + + return size; + } + std::string *KeyAt(size_t i) { + if (!obj || i >= Size()) return nullptr; + + JNIEnv *env = getJNIEnv(); + jstring key = (jstring)ObjectMethod(env, "keyAt", METHOD_JNODE_KEY_AT, + CLS_JNODE, &obj); + + const char *k = env->GetStringUTFChars(key, 0); + std::string *s = new std::string(k); + env->ReleaseStringUTFChars(key, k); + + return s; + } + Node *ValueAt(size_t i) { + if (!obj || i >= Size()) return nullptr; + + JNIEnv *env = getJNIEnv(); + jobject val = + ObjectMethod(env, "valueAt", METHOD_JNODE_VALUE_AT, CLS_JNODE, &obj); + return lookupOrCreate(env->NewGlobalRef(val)); // new ref + } + + void SetValue(size_t i, Node *val) { + jobject v = nullptr; + if (val && val->obj) { + v = val->obj; + } else { // FIXME(bzz) + // v = "new JNull instance"; // Py_None; + } + + jobject res = + ObjectMethod(getJNIEnv(), "add", METHOD_JARR_ADD, CLS_JARR, &obj, v); + if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM + throw std::runtime_error(std::string("failed to call ") + .append(CLS_JARR) + .append(".add() from Node::SetValue()")); + } + } + void SetKeyValue(std::string key, Node *val) { + JNIEnv *env = getJNIEnv(); + jobject v = nullptr; + if (val && val->obj) { + v = val->obj; + } else { + v = NewJavaObject(env, CLS_JNULL, "()V"); + } + + jstring k = env->NewStringUTF(key.data()); + + jobject res = + ObjectMethod(env, "add", METHOD_JOBJ_ADD, CLS_JOBJ, &obj, k, v); + if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM + throw std::runtime_error( + std::string("failed to call JObject.add() from Node::SetKeyValue(") + .append(key) + .append(")")); + } + } +}; + +class Context; + +class Interface : public uast::NodeCreator { + private: + std::map obj2node; + + // lookupOrCreate either creates a new object or returns existing one. + // In the second case it creates a new reference. + Node *lookupOrCreate(jobject obj) { + if (!obj) return nullptr; + + Node *node = obj2node[obj]; + if (node) return node; + + node = new Node(this, obj); + obj2node[obj] = node; + return node; + } + + // create makes a new object with a specified kind. + // Steals the reference. + Node *create(NodeKind kind, jobject obj) { + Node *node = new Node(this, kind, obj); + obj2node[obj] = node; + return node; + } + + public: + friend class Node; + friend class Context; + + Interface() {} + ~Interface() { + // Only needs to deallocate Nodes, since they own + // the same object as used in the map key. + for (auto it : obj2node) { + delete (it.second); + } + } + + // toJ returns a JVM object associated with a node. + // Returns a new reference. + jobject toJ(Node *node) { + if (node == nullptr) return nullptr; + jobject obj = getJNIEnv()->NewGlobalRef(node->obj); + return obj; + } + + // abstract methods from NodeCreator + Node *NewObject(size_t size) { + jobject m = NewJavaObject(getJNIEnv(), CLS_JOBJ, "()V"); + checkJvmException("failed to create new " + std::string(CLS_JOBJ)); + return create(NODE_OBJECT, m); + } + Node *NewArray(size_t size) { + jobject arr = NewJavaObject(getJNIEnv(), CLS_JARR, "(I)V", size); + checkJvmException("failed to create new " + std::string(CLS_JARR)); + return create(NODE_ARRAY, arr); + } + Node *NewString(std::string v) { + JNIEnv *env = getJNIEnv(); + jobject str = env->NewStringUTF(v.data()); + jobject arr = NewJavaObject(env, CLS_JSTR, "(Ljava/lang/String;)V", str); + checkJvmException("failed to create new " + std::string(CLS_JSTR)); + return create(NODE_STRING, arr); + } + Node *NewInt(int64_t v) { + jobject i = NewJavaObject(getJNIEnv(), CLS_JINT, "(J)V", v); + checkJvmException("failed to create new " + std::string(CLS_JINT)); + return create(NODE_INT, i); + } + Node *NewUint(uint64_t v) { + jobject i = NewJavaObject(getJNIEnv(), CLS_JUINT, "(J)V", v); + checkJvmException("failed to create new " + std::string(CLS_JUINT)); + return create(NODE_UINT, i); + } + Node *NewFloat(double v) { + jobject i = NewJavaObject(getJNIEnv(), CLS_JFLT, "(D)V", v); + checkJvmException("failed to create new " + std::string(CLS_JFLT)); + return create(NODE_FLOAT, i); + } + Node *NewBool(bool v) { + jobject i = NewJavaObject(getJNIEnv(), CLS_JBOL, "(Z)V", v); + checkJvmException("failed to create new " + std::string(CLS_JBOL)); + return create(NODE_BOOL, i); + } +}; + +// toJ returns a JVM object associated with a node. +// Returns a new reference. +jobject Node::toJ() { return iface->toJ(this); } + +// lookupOrCreate either creates a new object or returns existing one. +// In the second case it creates a new reference. +Node *Node::lookupOrCreate(jobject obj) { return iface->lookupOrCreate(obj); } + +class Context { + private: + Interface *iface; + uast::PtrInterface *impl; + uast::Context *ctx; + + // toJ returns a JVM object associated with a node. + // Returns a new reference. + jobject toJ(Node *node) { + if (node == nullptr) return nullptr; + return iface->toJ(node); + } + + public: + Context() { + // create a class that makes and tracks UAST nodes + iface = new Interface(); + // create an implementation that will handle libuast calls + impl = new uast::PtrInterface(iface); + // create a new UAST context based on this implementation + ctx = impl->NewContext(); + } + ~Context() { + delete (ctx); + delete (impl); + delete (iface); + } + + // RootNode returns a root UAST node, if set. + // Returns a new reference. + jobject RootNode() { + Node *root = ctx->RootNode(); + return toJ(root); // new ref + } + + jobject LoadFrom(jobject src) { // JNode + JNIEnv *env = getJNIEnv(); + + ContextExt *nodeExtCtx = getHandle(env, src, "ctx"); + if (!nodeExtCtx) { + throw std::runtime_error("Cannot get Node.ctx"); + } + auto sctx = nodeExtCtx->ctx; + NodeHandle snode = + reinterpret_cast(getHandle(env, src, "handle")); + if (!snode) { + throw std::runtime_error("Cannot get Node.handle"); + } + + Node *node = uast::Load(sctx, snode, ctx); + if (!node) { + throw std::runtime_error("Failed to uast::Load()"); + } + return toJ(node); // new ref + } +}; + } // namespace -// v2.libuast.Libuast() +// ========================================== +// v2.libuast.Libuast() +// ========================================== + JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( JNIEnv *env, jobject self, jobject directBuf) { UastFormat format = UAST_BINARY; // TODO: make it arg @@ -132,6 +485,7 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( if (env->ExceptionCheck() == JNI_TRUE || !jCtxExt) { jCtxExt = nullptr; delete (ctx); + delete (p); env->ExceptionDescribe(); throw std::runtime_error("failed to instantiate Context class"); } @@ -139,12 +493,24 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( return jCtxExt; } +// TODO(#86): implement JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_filter( JNIEnv *, jobject, jobject, jstring) { - return nullptr; // TODO(bzz): implement + return nullptr; +} + +// ========================================== +// v2.Context() +// ========================================== + +JNIEXPORT jlong JNICALL +Java_org_bblfsh_client_v2_Context_00024_create(JNIEnv *env, jobject self) { + auto c = new Context(); + uast::Context *ctx; // TODO(#90): init from c on encode() impl + auto p = new ContextExt(ctx); + return (long)p; } -// v2.Context() JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_Context_root(JNIEnv *env, jobject self) { ContextExt *p = getHandle(env, self, nativeContext); @@ -166,10 +532,16 @@ JNIEXPORT void JNICALL Java_org_bblfsh_client_v2_Context_dispose(JNIEnv *env, delete p; } -// v2.Node() -JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_Node_load(JNIEnv *, - jobject) { - return nullptr; // TODO(bzz): implement +// ========================================== +// v2.Node() +// ========================================== + +JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_Node_load(JNIEnv *env, + jobject self) { + auto ctx = new Context(); + jobject node = ctx->LoadFrom(self); + delete (ctx); + return node; } JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) { diff --git a/src/main/scala/org/bblfsh/client/v2/Context.scala b/src/main/scala/org/bblfsh/client/v2/Context.scala index 9d8cb82..5e3dfe5 100644 --- a/src/main/scala/org/bblfsh/client/v2/Context.scala +++ b/src/main/scala/org/bblfsh/client/v2/Context.scala @@ -1,13 +1,15 @@ package org.bblfsh.client.v2 import java.nio.ByteBuffer - + case class Context(nativeContext: Long) { + def this() = this(Context.create()) + @native def root(): Node @native def encode(n: Node): ByteBuffer @native def dispose() - @native def filter() + @native def filter() // TODO(bzz) // TODO(bzz): add loading of the root node, after clarifying when it's needed // https://github.com/bblfsh/client-python/blob/master/bblfsh/pyuast.cc#L364 @@ -17,3 +19,7 @@ case class Context(nativeContext: Long) { this.dispose() } } + +object Context { + @native def create(): Long +} \ No newline at end of file diff --git a/src/main/scala/org/bblfsh/client/v2/JNode.scala b/src/main/scala/org/bblfsh/client/v2/JNode.scala index b3c3f01..7cceeb2 100644 --- a/src/main/scala/org/bblfsh/client/v2/JNode.scala +++ b/src/main/scala/org/bblfsh/client/v2/JNode.scala @@ -1,15 +1,60 @@ package org.bblfsh.client.v2 +import scala.collection.mutable + /** UAST nodes representation on the JVM side. * * Mirrors https://godoc.org/github.com/bblfsh/sdk/uast/nodes */ -sealed abstract class JNode -case class JString(s: String) extends JNode -case class JDouble(num: Double) extends JNode -case class JLong(num: Long) extends JNode -case class JInt(num: BigInt) extends JNode -case class JBool(value: Boolean) extends JNode -case class JObject(obj: List[JField]) extends JNode -case class JArray(arr: List[JNode]) extends JNode +sealed abstract class JNode { + def children: Seq[JNode] = this match { + case JObject(l) => l map (_._2) + case JArray(l) => l + case _ => Seq() + } + + def size: Int = this match { + case JObject(l) => l.size + case JArray(l) => l.size + case JString(l) => l.length + case JNothing => 0 + case _ => 1 + } + + def keyAt(i: Int): String = this match { + case o: JObject => o.obj(i)._1 + case _ => "" + } + def valueAt(i: Int): JNode = this match { + case o: JObject => o.obj(i)._2 + case c: JArray => c.arr(i) + case _ => JNothing + } + + def apply(k: String): JNode = this match { + case o: JObject => o.obj.filter( _._1 == k ).head._2 + case _ => JNothing + } + +} + +case object JNothing extends JNode // 'zero' value for JNode +case class JNull() extends JNode +case class JString(str: String) extends JNode +case class JFloat(num: Double) extends JNode +case class JUint(num: Long) extends JNode +case class JInt(num: Long) extends JNode +case class JBool(value: Boolean) extends JNode +case class JObject(obj: mutable.Buffer[JField]) extends JNode { + def this() = this(mutable.Buffer[JField]()) + def add(k: String, v: JNode) = { + obj += ((k, v)) + } +} +case class JArray(arr: mutable.Buffer[JNode]) extends JNode { + def this(size: Int) = this(new mutable.ArrayBuffer[JNode](size)) + def add(v: JNode) = { + arr += v + } +} diff --git a/src/test/resources/Tiny.java b/src/test/resources/Tiny.java new file mode 100644 index 0000000..e1368fc --- /dev/null +++ b/src/test/resources/Tiny.java @@ -0,0 +1 @@ +public class Tiny {}; \ No newline at end of file diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientBaseTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientBaseTest.scala new file mode 100644 index 0000000..1d2eb76 --- /dev/null +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientBaseTest.scala @@ -0,0 +1,25 @@ +package org.bblfsh.client.v2 + +import gopkg.in.bblfsh.sdk.v2.protocol.driver.ParseResponse +import org.scalatest.{BeforeAndAfter, BeforeAndAfterAll, FlatSpec, Matchers} + +import scala.io.Source + +class BblfshClientBaseTest extends FlatSpec + with BeforeAndAfter + with BeforeAndAfterAll + with Matchers { + + val client = BblfshClient("localhost", 9432) + val fileName = "src/test/resources/SampleJavaFile.java" + var resp: ParseResponse = _ + + before { + val fileContent = Source.fromFile(fileName).getLines.mkString("\n") + resp = client.parse(fileName, fileContent) + } + + override def afterAll { + client.close() + } +} diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientCloseTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientCloseTest.scala index a8a4d8d..c97bd53 100644 --- a/src/test/scala/org/bblfsh/client/v2/BblfshClientCloseTest.scala +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientCloseTest.scala @@ -4,7 +4,7 @@ import org.scalatest.{BeforeAndAfter, FunSuite} class BblfshClientClose extends FunSuite with BeforeAndAfter { - val client = BblfshClient("0.0.0.0", 9432) + val client = BblfshClient("localhost", 9432) test("Check close") { // call client method to check connection works diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala new file mode 100644 index 0000000..e143fa4 --- /dev/null +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala @@ -0,0 +1,65 @@ +package org.bblfsh.client.v2 + + +class BblfshClientLoadTest extends BblfshClientBaseTest { + + import BblfshClient._ // enables uast.* methods + + override val fileName = "src/test/resources/Tiny.java" + + "Loading Go -> JVM of a real tree" should "bring JNode tree to memory" in { + val uast = resp.uast.decode() + val rootNode: Node = uast.root() + + println(s"Loading $rootNode") + val root = rootNode.load() + + root should not be Nil + root.getClass shouldBe classOf[JObject] + root.children should not be empty + root.children.size shouldBe 6 + + val arr = root.children(1) + arr should not be (null) + arr.getClass shouldBe classOf[JArray] + arr.children.size shouldBe 1 + + val str = arr.children(0) + str.getClass shouldBe classOf[JString] + + val nil = root("imports") + nil should be (JNull()) + + println(s"Result size: ${root.children.size}\n") + root.children.foreach(println) + } + + // TODO(#90) a stub for testing JNode encoding impl + /*"Loading Go -> JVM for a simple encoded tree" should "bring JNode tree to memory" in { + val rootTree: JNode = JArray(Buffer( + JObject(Buffer( + "k1" -> JString("v1") + )), + JString("test") + )) + + val ctx = Context() + + ctx.encode(rootTree) + }*/ + + // TODO(#90) a stub for more extensive testing of JNode encoding impl + /*"Decoding, loading & encoding to different context" should "produce the same results" in { + val uast = resp.uast.decode() + val rootNode: Node = uast.root() + + println(s"Loading $rootNode") + val root = rootNode.load() + + val ctx = Context() + val data = ctx.encode(root) + + data shouldBe equal resp.uast + }*/ + +} diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientParseTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientParseTest.scala index ba8ceba..bb0ad6e 100644 --- a/src/test/scala/org/bblfsh/client/v2/BblfshClientParseTest.scala +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientParseTest.scala @@ -2,31 +2,11 @@ package org.bblfsh.client.v2 import java.nio.ByteBuffer -import gopkg.in.bblfsh.sdk.v2.protocol.driver.ParseResponse -import org.scalatest.{BeforeAndAfter, BeforeAndAfterAll, FlatSpec, Matchers} -import scala.io.Source - -class BblfshClientParseTest extends FlatSpec - with BeforeAndAfter - with BeforeAndAfterAll - with Matchers { - - val client = BblfshClient("0.0.0.0", 9432) - val fileName = "src/test/resources/SampleJavaFile.java" - val fileContent = Source.fromFile(fileName).getLines.mkString("\n") - var resp: ParseResponse = _ +class BblfshClientParseTest extends BblfshClientBaseTest { import BblfshClient._ // enables uast.* methods - before { - resp = client.parse(fileName, fileContent) - } - - override def afterAll { - client.close() - } - "Parsed UAST for .java file" should "not be empty" in { assert(resp != null) assert(resp.errors.isEmpty) @@ -61,7 +41,7 @@ class BblfshClientParseTest extends FlatSpec val encodedBytes: ByteBuffer = uastCtx.encode(rootNode) - encodedBytes.capacity should be (resp.uast.asReadOnlyByteBuffer.capacity) + encodedBytes.capacity should be(resp.uast.asReadOnlyByteBuffer.capacity) encodedBytes shouldEqual resp.uast.asReadOnlyByteBuffer println(resp.uast.asReadOnlyByteBuffer) diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientSupportedLanguagesTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientSupportedLanguagesTest.scala index 0d55f04..bb1b9d3 100644 --- a/src/test/scala/org/bblfsh/client/v2/BblfshClientSupportedLanguagesTest.scala +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientSupportedLanguagesTest.scala @@ -4,7 +4,7 @@ import gopkg.in.bblfsh.sdk.v2.protocol.driver.SupportedLanguagesResponse import org.scalatest.{BeforeAndAfter, FunSuite} class BblfshClientSupportedLanguagesTest extends FunSuite with BeforeAndAfter { - val client = BblfshClient("0.0.0.0", 9432) + val client = BblfshClient("localhost", 9432) var resp: SupportedLanguagesResponse = _ before { diff --git a/src/test/scala/org/bblfsh/client/v2/BblfshClientVersionTest.scala b/src/test/scala/org/bblfsh/client/v2/BblfshClientVersionTest.scala index 1a9914f..ecfce8a 100644 --- a/src/test/scala/org/bblfsh/client/v2/BblfshClientVersionTest.scala +++ b/src/test/scala/org/bblfsh/client/v2/BblfshClientVersionTest.scala @@ -6,7 +6,7 @@ import org.scalatest.{BeforeAndAfter, FunSuite} import scala.io.Source class BblfshClientVersionTest extends FunSuite with BeforeAndAfter { - val client = BblfshClient("0.0.0.0", 9432) + val client = BblfshClient("localhost", 9432) val fileName = "src/test/resources/SampleJavaFile.java" val fileContent = Source.fromFile(fileName).getLines.mkString("\n") var resp: VersionResponse = _ diff --git a/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala b/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala new file mode 100644 index 0000000..24b2b6b --- /dev/null +++ b/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala @@ -0,0 +1,74 @@ +package org.bblfsh.client.v2 + +import org.scalatest.{BeforeAndAfter, FlatSpec, Matchers} + +import scala.collection.mutable.Buffer + + +class JNodeTest extends FlatSpec + with BeforeAndAfter + with Matchers { + + val rootTree = JArray(Buffer( + JObject(Buffer( + "k1" -> JString("v1"), + "k2" -> JBool(false) + )), + JString("test") + )) + + "JNode" should "expose children" in { + rootTree.children.size shouldEqual 2 + rootTree.children(1).children shouldBe empty + } + + "JNode" should "expose size" in { + rootTree.size shouldEqual rootTree.children.size + + val obj = rootTree.children(0) + obj.size shouldEqual obj.children.size + + val str = rootTree.children(1) + str.size shouldEqual str.asInstanceOf[JString].str.length + } + + "JNode" should "expose valueAt" in { + val valueAtEqualsChildren = (node: JNode) => { + for (i <- 0 to node.size - 1) { + node.valueAt(i) shouldEqual node.children(i) + } + } + + val arr = rootTree + valueAtEqualsChildren(arr) + + val obj = rootTree.children(0) + valueAtEqualsChildren(obj) + } + + "JNode" should "expose get by key" in { + val obj = rootTree.children(0) + obj("k1").getClass shouldBe classOf[JString] + obj("k1") shouldBe JString("v1") // TODO(bzz) add implicit conversions + + obj("k2").getClass shouldBe classOf[JBool] + obj("k2") shouldBe JBool(false) + } + + "JNode object and array" should "expose add" in { + // object + val obj = rootTree.children(0).asInstanceOf[JObject] + var sizeBeforeAdd = obj.size + obj.add("b", JNull()) + + obj.size shouldEqual sizeBeforeAdd + 1 + + // array + val arr = rootTree + sizeBeforeAdd = arr.size + arr.add(JNull()) + + arr.size shouldEqual sizeBeforeAdd + 1 + } + +} From 0c294f33e22b6d93aece18bd29243f96a9177d0d Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Tue, 2 Jul 2019 00:00:38 +0200 Subject: [PATCH 2/4] v2: better error handling Replace std::runtime_error and in-band error codes with JVM exceptions. Signed-off-by: Alexander Bezzubov --- src/main/native/jni_utils.cc | 91 ++++++++++++------- src/main/native/jni_utils.h | 7 ++ .../org_bblfsh_client_v2_libuast_Libuast.cc | 59 +++++------- .../scala/org/bblfsh/client/v2/Context.scala | 2 +- 4 files changed, 90 insertions(+), 69 deletions(-) diff --git a/src/main/native/jni_utils.cc b/src/main/native/jni_utils.cc index 84caf06..94867d0 100644 --- a/src/main/native/jni_utils.cc +++ b/src/main/native/jni_utils.cc @@ -1,6 +1,9 @@ #include "jni_utils.h" +#include -extern JavaVM *jvm; // FIXME(bzz): double-check and document +// TODO(bzz): double-check and document. Suggestion and more context at +// https://github.com/bblfsh/client-scala/pull/84#discussion_r288347756 +extern JavaVM *jvm; JNIEnv *getJNIEnv() { JNIEnv *pEnv = NULL; @@ -20,6 +23,8 @@ JNIEnv *getJNIEnv() { // Class fully qualified names const char *CLS_NODE = "org/bblfsh/client/v2/Node"; const char *CLS_CTX = "org/bblfsh/client/v2/Context"; +const char *CLS_OBJ = "java/lang/Object"; +const char *CLS_RE = "java/lang/RuntimeException"; const char *CLS_JNODE = "org/bblfsh/client/v2/JNode"; const char *CLS_JNULL = "org/bblfsh/client/v2/JNull"; @@ -31,6 +36,7 @@ const char *CLS_JUINT = "org/bblfsh/client/v2/JUint"; const char *CLS_JARR = "org/bblfsh/client/v2/JArray"; const char *CLS_JOBJ = "org/bblfsh/client/v2/JObject"; +// Method signatures const char *METHOD_JNODE_KEY_AT = "(I)Ljava/lang/String;"; const char *METHOD_JNODE_VALUE_AT = "(I)Lorg/bblfsh/client/v2/JNode;"; const char *METHOD_JOBJ_ADD = @@ -39,53 +45,66 @@ const char *METHOD_JOBJ_ADD = const char *METHOD_JARR_ADD = "(Lorg/bblfsh/client/v2/JNode;)Lscala/collection/mutable/Buffer;"; +const char *METHOD_OBJ_TO_STR = "()Ljava/lang/String;"; + +void checkJvmException(std::string msg) { + JNIEnv *env = getJNIEnv(); + if (env->ExceptionCheck()) { + // env->ExceptionDescribe(); // prints to stdout, un-comment for debuging + auto err = env->ExceptionOccurred(); + + jmethodID toString = env->GetMethodID(env->FindClass(CLS_OBJ), "toString", + METHOD_OBJ_TO_STR); + jstring s = (jstring)env->CallObjectMethod(err, toString); + const char *utf = env->GetStringUTFChars(s, 0); + env->ReleaseStringUTFChars(s, utf); + + env->ExceptionClear(); + + auto exception = env->FindClass(CLS_RE); + env->ThrowNew(exception, msg.append(": ").append(utf).data()); + } +} + jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, ...) { jclass cls = env->FindClass(className); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException(std::string("failed to find a class ").append(className)); jmethodID initId = env->GetMethodID(cls, "", initSign); - if (env->ExceptionOccurred() || !initId) { - return nullptr; - } + checkJvmException( + std::string("failed to call constructor for ").append(className)); va_list varargs; va_start(varargs, initSign); jobject instance = env->NewObjectV(cls, initId, varargs); va_end(varargs); - if (env->ExceptionOccurred() || !instance) { - return nullptr; - } + checkJvmException( + std::string("failed get varargs for constructor of ").append(className)); return instance; } jfieldID getField(JNIEnv *env, jobject obj, const char *name) { jclass cls = env->GetObjectClass(obj); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException("failed get an object class"); jfieldID jfid = env->GetFieldID(cls, name, "J"); - if (env->ExceptionOccurred() || !jfid) { - return nullptr; - } + checkJvmException(std::string("failed get a field ").append(name)); + return jfid; } static jmethodID MethodID(JNIEnv *env, const char *method, const char *signature, const char *className) { jclass cls = env->FindClass(className); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException(std::string("failed to find a class ").append(className)); jmethodID mId = env->GetMethodID(cls, method, signature); - if (env->ExceptionOccurred()) { - return nullptr; - } + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); return mId; } @@ -93,16 +112,18 @@ static jmethodID MethodID(JNIEnv *env, const char *method, jint IntMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object) { jmethodID mId = MethodID(env, method, signature, className); - if (env->ExceptionOccurred() || !mId) return 0; - // TODO(bzz): add better error handling - // ExceptionOccurred() - // ExceptionDescribe() - // ExceptionClear() - // ExceptionCheck() - // ThrowNew("failed to call $className.$method") to JVM + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); jint res = env->CallIntMethod(*object, mId); - if (env->ExceptionOccurred()) return 0; + checkJvmException(std::string("failed call a method ") + .append(className) + .append(".") + .append(method) + .append(" using signature ") + .append(signature)); return res; } @@ -110,13 +131,19 @@ jint IntMethod(JNIEnv *env, const char *method, const char *signature, jobject ObjectMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object, ...) { jmethodID mId = MethodID(env, method, signature, className); - if (env->ExceptionOccurred() || !mId) return nullptr; + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); va_list varargs; va_start(varargs, object); jobject res = env->CallObjectMethodV(*object, mId, varargs); va_end(varargs); - if (env->ExceptionOccurred() || !res) return nullptr; + checkJvmException(std::string("failed get varargs for ") + .append(className) + .append(".") + .append(method)); return res; } diff --git a/src/main/native/jni_utils.h b/src/main/native/jni_utils.h index f3ab76f..a04818c 100644 --- a/src/main/native/jni_utils.h +++ b/src/main/native/jni_utils.h @@ -2,9 +2,12 @@ #define _Included_org_bblfsh_client_libuast_Libuast_jni_utils #include +#include extern const char *CLS_NODE; extern const char *CLS_CTX; +extern const char *CLS_OBJ; +extern const char *CLS_RE; extern const char *CLS_JNODE; extern const char *CLS_JNULL; @@ -21,6 +24,10 @@ extern const char *METHOD_JNODE_VALUE_AT; extern const char *METHOD_JOBJ_ADD; extern const char *METHOD_JARR_ADD; +extern const char *METHOD_OBJ_TO_STR; + +void checkJvmException(std::string); + JNIEnv *getJNIEnv(); jobject NewJavaObject(JNIEnv *, const char *, const char *, ...); jfieldID getField(JNIEnv *env, jobject obj, const char *name); diff --git a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc index 050acfb..9b8768c 100644 --- a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc +++ b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc @@ -189,7 +189,7 @@ class Node : public uast::Node { NodeKind Kind() { return kind; } - // TODO(#90) all 'As*' are unused stubs, make them buletprof and test + // TODO(#90): implement and test (all 'As*' are unused stubs for now) std::string *AsString() { if (!str) { JNIEnv *env = getJNIEnv(); @@ -263,20 +263,18 @@ class Node : public uast::Node { } void SetValue(size_t i, Node *val) { + JNIEnv *env = getJNIEnv(); jobject v = nullptr; if (val && val->obj) { v = val->obj; - } else { // FIXME(bzz) - // v = "new JNull instance"; // Py_None; + } else { + v = NewJavaObject(env, CLS_JNULL, "()V"); } - jobject res = - ObjectMethod(getJNIEnv(), "add", METHOD_JARR_ADD, CLS_JARR, &obj, v); - if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM - throw std::runtime_error(std::string("failed to call ") - .append(CLS_JARR) - .append(".add() from Node::SetValue()")); - } + ObjectMethod(getJNIEnv(), "add", METHOD_JARR_ADD, CLS_JARR, &obj, v); + checkJvmException(std::string("failed to call ") + .append(CLS_JARR) + .append(".add() from Node::SetValue()")); } void SetKeyValue(std::string key, Node *val) { JNIEnv *env = getJNIEnv(); @@ -291,12 +289,10 @@ class Node : public uast::Node { jobject res = ObjectMethod(env, "add", METHOD_JOBJ_ADD, CLS_JOBJ, &obj, k, v); - if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM - throw std::runtime_error( - std::string("failed to call JObject.add() from Node::SetKeyValue(") - .append(key) - .append(")")); - } + checkJvmException( + std::string("failed to call JObject.add() from Node::SetKeyValue(") + .append(key) + .append(")")); } }; @@ -435,21 +431,16 @@ class Context { JNIEnv *env = getJNIEnv(); ContextExt *nodeExtCtx = getHandle(env, src, "ctx"); - if (!nodeExtCtx) { - throw std::runtime_error("Cannot get Node.ctx"); - } + checkJvmException("failed to get Node.ctx"); + auto sctx = nodeExtCtx->ctx; NodeHandle snode = reinterpret_cast(getHandle(env, src, "handle")); - if (!snode) { - throw std::runtime_error("Cannot get Node.handle"); - } + checkJvmException("failed to get Node.handle"); Node *node = uast::Load(sctx, snode, ctx); - if (!node) { - throw std::runtime_error("Failed to uast::Load()"); - } - return toJ(node); // new ref + checkJvmException("failed to uast::Load()"); + return toJ(node); } }; @@ -465,13 +456,10 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( // works only with ByteBuffer.allocateDirect() void *buf = env->GetDirectBufferAddress(directBuf); - if (env->ExceptionCheck() == JNI_TRUE) { - return nullptr; - } + checkJvmException("failed to use buffer for direct access"); + jlong len = env->GetDirectBufferCapacity(directBuf); - if (env->ExceptionCheck() == JNI_TRUE) { - return nullptr; - } + checkJvmException("failed to get buffer capacity"); // another option (instead of XXX) is to use // GetPrimitiveArrayCritical @@ -482,12 +470,11 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( auto p = new ContextExt(ctx); jobject jCtxExt = NewJavaObject(env, CLS_CTX, "(J)V", p); - if (env->ExceptionCheck() == JNI_TRUE || !jCtxExt) { + if (env->ExceptionCheck() || !jCtxExt) { jCtxExt = nullptr; delete (ctx); delete (p); - env->ExceptionDescribe(); - throw std::runtime_error("failed to instantiate Context class"); + checkJvmException("failed to instantiate Context class"); } return jCtxExt; @@ -547,7 +534,7 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_Node_load(JNIEnv *env, JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) { JNIEnv *env; if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_8) != JNI_OK) { - return -1; + return JNI_ERR; } jvm = vm; diff --git a/src/main/scala/org/bblfsh/client/v2/Context.scala b/src/main/scala/org/bblfsh/client/v2/Context.scala index 5e3dfe5..9460237 100644 --- a/src/main/scala/org/bblfsh/client/v2/Context.scala +++ b/src/main/scala/org/bblfsh/client/v2/Context.scala @@ -22,4 +22,4 @@ case class Context(nativeContext: Long) { object Context { @native def create(): Long -} \ No newline at end of file +} From 52e3e65d50a73984c3f39fe5e6708948cb235273 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Wed, 3 Jul 2019 17:50:57 +0200 Subject: [PATCH 3/4] v2: convenience for JNode literal creation Signed-off-by: Alexander Bezzubov --- .../scala/org/bblfsh/client/v2/JNode.scala | 22 +++++++++++++++++-- .../org/bblfsh/client/v2/JNodeTest.scala | 10 ++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/main/scala/org/bblfsh/client/v2/JNode.scala b/src/main/scala/org/bblfsh/client/v2/JNode.scala index 7cceeb2..8c44e0e 100644 --- a/src/main/scala/org/bblfsh/client/v2/JNode.scala +++ b/src/main/scala/org/bblfsh/client/v2/JNode.scala @@ -1,5 +1,7 @@ package org.bblfsh.client.v2 +import java.io.Serializable + import scala.collection.mutable /** UAST nodes representation on the JVM side. @@ -46,15 +48,31 @@ case class JFloat(num: Double) extends JNode case class JUint(num: Long) extends JNode case class JInt(num: Long) extends JNode case class JBool(value: Boolean) extends JNode + case class JObject(obj: mutable.Buffer[JField]) extends JNode { def this() = this(mutable.Buffer[JField]()) def add(k: String, v: JNode) = { obj += ((k, v)) } } +case object JObject { + def apply[T <: (Product with Serializable with JNode)](ns: (String, T)*) = { + val jo = new JObject() + jo.obj ++= ns + jo + } +} + case class JArray(arr: mutable.Buffer[JNode]) extends JNode { def this(size: Int) = this(new mutable.ArrayBuffer[JNode](size)) - def add(v: JNode) = { - arr += v + def add(n: JNode) = { + arr += n } } +case object JArray { + def apply[T <: (Product with Serializable with JNode)](ns: T *) = { + val ja = new JArray(ns.length) + ja.arr ++= ns + ja + } +} \ No newline at end of file diff --git a/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala b/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala index 24b2b6b..d5ab7a1 100644 --- a/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala +++ b/src/test/scala/org/bblfsh/client/v2/JNodeTest.scala @@ -2,20 +2,18 @@ package org.bblfsh.client.v2 import org.scalatest.{BeforeAndAfter, FlatSpec, Matchers} -import scala.collection.mutable.Buffer - class JNodeTest extends FlatSpec with BeforeAndAfter with Matchers { - val rootTree = JArray(Buffer( - JObject(Buffer( + val rootTree = JArray( + JObject( "k1" -> JString("v1"), "k2" -> JBool(false) - )), + ), JString("test") - )) + ) "JNode" should "expose children" in { rootTree.children.size shouldEqual 2 From c8091e6f1d0da9e246214f5ed3610fcf623886f7 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Thu, 4 Jul 2019 12:59:43 +0200 Subject: [PATCH 4/4] address review feedback - better error handling - better string literals - doc added Signed-off-by: Alexander Bezzubov --- src/main/native/jni_utils.cc | 105 ++++++++++++------ src/main/native/jni_utils.h | 52 +++++---- .../org_bblfsh_client_v2_libuast_Libuast.cc | 6 +- .../scala/org/bblfsh/client/v2/Context.scala | 2 +- .../scala/org/bblfsh/client/v2/JNode.scala | 16 +-- 5 files changed, 110 insertions(+), 71 deletions(-) diff --git a/src/main/native/jni_utils.cc b/src/main/native/jni_utils.cc index 94867d0..602e9aa 100644 --- a/src/main/native/jni_utils.cc +++ b/src/main/native/jni_utils.cc @@ -20,49 +20,78 @@ JNIEnv *getJNIEnv() { return pEnv; } -// Class fully qualified names -const char *CLS_NODE = "org/bblfsh/client/v2/Node"; -const char *CLS_CTX = "org/bblfsh/client/v2/Context"; -const char *CLS_OBJ = "java/lang/Object"; -const char *CLS_RE = "java/lang/RuntimeException"; - -const char *CLS_JNODE = "org/bblfsh/client/v2/JNode"; -const char *CLS_JNULL = "org/bblfsh/client/v2/JNull"; -const char *CLS_JSTR = "org/bblfsh/client/v2/JString"; -const char *CLS_JINT = "org/bblfsh/client/v2/JInt"; -const char *CLS_JFLT = "org/bblfsh/client/v2/JFloat"; -const char *CLS_JBOL = "org/bblfsh/client/v2/JBool"; -const char *CLS_JUINT = "org/bblfsh/client/v2/JUint"; -const char *CLS_JARR = "org/bblfsh/client/v2/JArray"; -const char *CLS_JOBJ = "org/bblfsh/client/v2/JObject"; - -// Method signatures -const char *METHOD_JNODE_KEY_AT = "(I)Ljava/lang/String;"; -const char *METHOD_JNODE_VALUE_AT = "(I)Lorg/bblfsh/client/v2/JNode;"; -const char *METHOD_JOBJ_ADD = +const char CLS_NODE[] = "org/bblfsh/client/v2/Node"; +const char CLS_CTX[] = "org/bblfsh/client/v2/Context"; +const char CLS_OBJ[] = "java/lang/Object"; +const char CLS_RE[] = "java/lang/RuntimeException"; +const char CLS_JNODE[] = "org/bblfsh/client/v2/JNode"; +const char CLS_JNULL[] = "org/bblfsh/client/v2/JNull"; +const char CLS_JSTR[] = "org/bblfsh/client/v2/JString"; +const char CLS_JINT[] = "org/bblfsh/client/v2/JInt"; +const char CLS_JFLT[] = "org/bblfsh/client/v2/JFloat"; +const char CLS_JBOOL[] = "org/bblfsh/client/v2/JBool"; +const char CLS_JUINT[] = "org/bblfsh/client/v2/JUint"; +const char CLS_JARR[] = "org/bblfsh/client/v2/JArray"; +const char CLS_JOBJ[] = "org/bblfsh/client/v2/JObject"; + +const char METHOD_JNODE_KEY_AT[] = "(I)Ljava/lang/String;"; +const char METHOD_JNODE_VALUE_AT[] = "(I)Lorg/bblfsh/client/v2/JNode;"; +const char METHOD_JOBJ_ADD[] = "(Ljava/lang/String;Lorg/bblfsh/client/v2/JNode;)Lscala/collection/" "mutable/Buffer;"; -const char *METHOD_JARR_ADD = +const char METHOD_JARR_ADD[] = "(Lorg/bblfsh/client/v2/JNode;)Lscala/collection/mutable/Buffer;"; -const char *METHOD_OBJ_TO_STR = "()Ljava/lang/String;"; +const char METHOD_OBJ_TO_STR[] = "()Ljava/lang/String;"; +// TODO(bzz): cache classes&methods in JNI_OnLoad should speed this up void checkJvmException(std::string msg) { JNIEnv *env = getJNIEnv(); - if (env->ExceptionCheck()) { - // env->ExceptionDescribe(); // prints to stdout, un-comment for debuging - auto err = env->ExceptionOccurred(); + auto err = env->ExceptionOccurred(); + if (err) { + env->ExceptionClear(); + + auto exceptionCls = env->FindClass(CLS_RE); + if (env->ExceptionCheck()) { + env->ExceptionClear(); + env->Throw(env->ExceptionOccurred()); + return; + } + + jclass cls = env->FindClass(CLS_OBJ); + if (env->ExceptionCheck()) { + env->ExceptionClear(); + env->ThrowNew( + exceptionCls, + msg.append(" - failed to find class ").append(CLS_OBJ).data()); + return; + } + + jmethodID toString = env->GetMethodID(cls, "toString", METHOD_OBJ_TO_STR); + if (env->ExceptionCheck()) { + env->ExceptionClear(); + env->ThrowNew(exceptionCls, + msg.append(" - failed to find method toString").data()); + return; + } - jmethodID toString = env->GetMethodID(env->FindClass(CLS_OBJ), "toString", - METHOD_OBJ_TO_STR); jstring s = (jstring)env->CallObjectMethod(err, toString); + if (env->ExceptionCheck() || !s) { + env->ThrowNew(exceptionCls, + msg.append(" - failed co call method toString").data()); + return; + } + const char *utf = env->GetStringUTFChars(s, 0); env->ReleaseStringUTFChars(s, utf); - env->ExceptionClear(); + // new RuntimeException(msg.data(), err) + jmethodID initId = env->GetMethodID( + cls, "", "(Ljava/lang/String;Ljava/lang/Throwable;)V"); + jthrowable exception = (jthrowable)env->NewObject( + exceptionCls, initId, msg.append(": ").append(utf).data(), err); - auto exception = env->FindClass(CLS_RE); - env->ThrowNew(exception, msg.append(": ").append(utf).data()); + env->Throw(exception); } } @@ -72,8 +101,10 @@ jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, checkJvmException(std::string("failed to find a class ").append(className)); jmethodID initId = env->GetMethodID(cls, "", initSign); - checkJvmException( - std::string("failed to call constructor for ").append(className)); + checkJvmException(std::string("failed to call a constructor with signature ") + .append(initSign) + .append(" for the class name ") + .append(className)); va_list varargs; va_start(varargs, initSign); @@ -87,7 +118,7 @@ jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, jfieldID getField(JNIEnv *env, jobject obj, const char *name) { jclass cls = env->GetObjectClass(obj); - checkJvmException("failed get an object class"); + checkJvmException("failed get the class of an object"); jfieldID jfid = env->GetFieldID(cls, name, "J"); checkJvmException(std::string("failed get a field ").append(name)); @@ -101,7 +132,7 @@ static jmethodID MethodID(JNIEnv *env, const char *method, checkJvmException(std::string("failed to find a class ").append(className)); jmethodID mId = env->GetMethodID(cls, method, signature); - checkJvmException(std::string("failed get a method ") + checkJvmException(std::string("failed to get method ") .append(className) .append(".") .append(method)); @@ -112,13 +143,13 @@ static jmethodID MethodID(JNIEnv *env, const char *method, jint IntMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object) { jmethodID mId = MethodID(env, method, signature, className); - checkJvmException(std::string("failed get a method ") + checkJvmException(std::string("failed to get method ") .append(className) .append(".") .append(method)); jint res = env->CallIntMethod(*object, mId); - checkJvmException(std::string("failed call a method ") + checkJvmException(std::string("failed to call method ") .append(className) .append(".") .append(method) @@ -131,7 +162,7 @@ jint IntMethod(JNIEnv *env, const char *method, const char *signature, jobject ObjectMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object, ...) { jmethodID mId = MethodID(env, method, signature, className); - checkJvmException(std::string("failed get a method ") + checkJvmException(std::string("failed to get method ") .append(className) .append(".") .append(method)); diff --git a/src/main/native/jni_utils.h b/src/main/native/jni_utils.h index a04818c..d1801ae 100644 --- a/src/main/native/jni_utils.h +++ b/src/main/native/jni_utils.h @@ -4,28 +4,34 @@ #include #include -extern const char *CLS_NODE; -extern const char *CLS_CTX; -extern const char *CLS_OBJ; -extern const char *CLS_RE; - -extern const char *CLS_JNODE; -extern const char *CLS_JNULL; -extern const char *CLS_JSTR; -extern const char *CLS_JINT; -extern const char *CLS_JFLT; -extern const char *CLS_JBOL; -extern const char *CLS_JUINT; -extern const char *CLS_JARR; -extern const char *CLS_JOBJ; - -extern const char *METHOD_JNODE_KEY_AT; -extern const char *METHOD_JNODE_VALUE_AT; -extern const char *METHOD_JOBJ_ADD; -extern const char *METHOD_JARR_ADD; - -extern const char *METHOD_OBJ_TO_STR; - +// Fully qualified Java class names +extern const char CLS_NODE[]; +extern const char CLS_CTX[]; +extern const char CLS_OBJ[]; +extern const char CLS_RE[]; + +// Fully qualified class names for Bablefish UAST types +extern const char CLS_JNODE[]; +extern const char CLS_JNULL[]; +extern const char CLS_JSTR[]; +extern const char CLS_JINT[]; +extern const char CLS_JFLT[]; +extern const char CLS_JBOOL[]; +extern const char CLS_JUINT[]; +extern const char CLS_JARR[]; +extern const char CLS_JOBJ[]; + +// Method signatures +extern const char METHOD_JNODE_KEY_AT[]; +extern const char METHOD_JNODE_VALUE_AT[]; +extern const char METHOD_JOBJ_ADD[]; +extern const char METHOD_JARR_ADD[]; +extern const char METHOD_OBJ_TO_STR[]; + +// Checks though JNI, if there is a pending excption on JVM side. +// +// Throws new RuntimeExpection to JVM in case there is, +// uses the origial one as a cause and the given string as a message. void checkJvmException(std::string); JNIEnv *getJNIEnv(); @@ -38,4 +44,4 @@ jint IntMethod(JNIEnv *, const char *, const char *, const char *, jobject ObjectMethod(JNIEnv *, const char *, const char *, const char *, const jobject *, ...); -#endif \ No newline at end of file +#endif diff --git a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc index 9b8768c..5c8d182 100644 --- a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc +++ b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc @@ -139,7 +139,7 @@ class Node : public uast::Node { return NODE_INT; } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JFLT))) { return NODE_FLOAT; - } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JBOL))) { + } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JBOOL))) { return NODE_BOOL; } else if (env->IsInstanceOf(obj, env->FindClass(CLS_JUINT))) { return NODE_BOOL; @@ -378,8 +378,8 @@ class Interface : public uast::NodeCreator { return create(NODE_FLOAT, i); } Node *NewBool(bool v) { - jobject i = NewJavaObject(getJNIEnv(), CLS_JBOL, "(Z)V", v); - checkJvmException("failed to create new " + std::string(CLS_JBOL)); + jobject i = NewJavaObject(getJNIEnv(), CLS_JBOOL, "(Z)V", v); + checkJvmException("failed to create new " + std::string(CLS_JBOOL)); return create(NODE_BOOL, i); } }; diff --git a/src/main/scala/org/bblfsh/client/v2/Context.scala b/src/main/scala/org/bblfsh/client/v2/Context.scala index 9460237..fe84523 100644 --- a/src/main/scala/org/bblfsh/client/v2/Context.scala +++ b/src/main/scala/org/bblfsh/client/v2/Context.scala @@ -9,7 +9,7 @@ case class Context(nativeContext: Long) { @native def encode(n: Node): ByteBuffer @native def dispose() - @native def filter() // TODO(bzz) + @native def filter() // TODO(bzz): add loading of the root node, after clarifying when it's needed // https://github.com/bblfsh/client-python/blob/master/bblfsh/pyuast.cc#L364 diff --git a/src/main/scala/org/bblfsh/client/v2/JNode.scala b/src/main/scala/org/bblfsh/client/v2/JNode.scala index 8c44e0e..98825d1 100644 --- a/src/main/scala/org/bblfsh/client/v2/JNode.scala +++ b/src/main/scala/org/bblfsh/client/v2/JNode.scala @@ -5,10 +5,11 @@ import java.io.Serializable import scala.collection.mutable /** UAST nodes representation on the JVM side. - * - * Mirrors https://godoc.org/github.com/bblfsh/sdk/uast/nodes - */ + * + * Mirrors https://godoc.org/github.com/bblfsh/sdk/uast/nodes + */ sealed abstract class JNode { + /* Dynamic dispatch is a convenience to be called from JNI */ def children: Seq[JNode] = this match { case JObject(l) => l map (_._2) case JArray(l) => l @@ -19,8 +20,7 @@ sealed abstract class JNode { case JObject(l) => l.size case JArray(l) => l.size case JString(l) => l.length - case JNothing => 0 - case _ => 1 + case _ => 0 } def keyAt(i: Int): String = this match { @@ -45,7 +45,9 @@ case object JNothing extends JNode // 'zero' value for JNode case class JNull() extends JNode case class JString(str: String) extends JNode case class JFloat(num: Double) extends JNode -case class JUint(num: Long) extends JNode +case class JUint(num: Long) extends JNode { + def get(): Long = java.lang.Integer.toUnsignedLong(num.toInt) +} case class JInt(num: Long) extends JNode case class JBool(value: Boolean) extends JNode @@ -75,4 +77,4 @@ case object JArray { ja.arr ++= ns ja } -} \ No newline at end of file +}