Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Commit b149524

Browse files
committed
address review feedback
Signed-off-by: Alexander Bezzubov <[email protected]>
1 parent 9fd1529 commit b149524

File tree

3 files changed

+40
-13
lines changed

3 files changed

+40
-13
lines changed

src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc

+31-5
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,30 @@ jobject asJvmBuffer(uast::Buffer buf) {
5050
return env->NewDirectByteBuffer(buf.ptr, buf.size);
5151
}
5252

53+
bool assertNotContext(jobject obj) {
54+
JNIEnv *env = getJNIEnv();
55+
if (isContext(obj, env)) {
56+
auto reCls = env->FindClass(CLS_RE);
57+
checkJvmException("failed to find class " + std::string(CLS_RE));
58+
59+
env->ThrowNew(reCls, "cannot use UAST Context as a Node");
60+
return false;
61+
}
62+
return true;
63+
}
64+
65+
bool isContext(jobject obj, JNIEnv *env) {
66+
if (!obj) return false;
67+
68+
jclass nodeCls = env->FindClass(CLS_NODE);
69+
checkJvmException("failed to find class " + std::string(CLS_NODE));
70+
71+
jclass jnodeCls = env->FindClass(CLS_JNODE);
72+
checkJvmException("failed to find class " + std::string(CLS_JNODE));
73+
74+
return env->IsInstanceOf(obj, nodeCls) || env->IsInstanceOf(obj, jnodeCls);
75+
}
76+
5377
// ==========================================
5478
// External UAST Context (managed by libuast)
5579
// ==========================================
@@ -76,7 +100,9 @@ class ContextExt {
76100
checkJvmException("failed to find class " + std::string(CLS_NODE));
77101

78102
if (!env->IsInstanceOf(obj, cls)) {
79-
const char *err = "ContextExt.toHandle() called not on Node type";
103+
auto err = std::string("ContextExt.toHandle() called not on")
104+
.append(CLS_NODE)
105+
.append(" type");
80106
ctx->SetError(err);
81107
return 0;
82108
}
@@ -103,7 +129,7 @@ class ContextExt {
103129
// Encode serializes the external UAST.
104130
// Borrows the reference.
105131
jobject Encode(jobject node, UastFormat format) {
106-
// if (!assertNotContext(node)) return nullptr;
132+
if (!assertNotContext(node)) return nullptr;
107133

108134
uast::Buffer data = ctx->Encode(toHandle(node), format);
109135
return asJvmBuffer(data);
@@ -358,7 +384,7 @@ class Interface : public uast::NodeCreator<Node *> {
358384
// toJ returns a JVM object associated with a node.
359385
// Returns a new reference.
360386
jobject toJ(Node *node) {
361-
if (node == nullptr) return nullptr;
387+
if (!node) return nullptr;
362388
jobject obj = getJNIEnv()->NewGlobalRef(node->obj);
363389
return obj;
364390
}
@@ -420,7 +446,7 @@ class Context {
420446
// toJ returns a JVM object associated with a node.
421447
// Returns a new reference.
422448
jobject toJ(Node *node) {
423-
if (node == nullptr) return nullptr;
449+
if (!node) return nullptr;
424450
return iface->toJ(node);
425451
}
426452
// toNode returns a node associated with a JVM object.
@@ -452,7 +478,7 @@ class Context {
452478
// Encode serializes UAST.
453479
// Creates a new reference.
454480
jobject Encode(jobject node, UastFormat format) {
455-
// if (!assertNotContext(node)) return nullptr;
481+
if (!assertNotContext(node)) return nullptr;
456482

457483
Node *n = toNode(node);
458484
uast::Buffer data = ctx->Encode(n, format);

src/main/scala/org/bblfsh/client/v2/ContextExt.scala

+4-4
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package org.bblfsh.client.v2
33
import java.nio.ByteBuffer
44

55
/**
6-
* pyuast.ContextExt
7-
*
86
* Represents Go-side results of Libuast.decode()
7+
*
8+
* This is equivalent of pyuast.ContextExt API
99
*/
1010
case class ContextExt(nativeContext: Long) {
1111
@native def root(): Node
@@ -21,9 +21,9 @@ case class ContextExt(nativeContext: Long) {
2121

2222

2323
/**
24-
* pyuast.Context
24+
* Represents JVM-side constructed tree
2525
*
26-
* Represents JVM-side constructed tree.
26+
* This is equivalent of pyuast.Context API
2727
*/
2828
case class Context(nativeContext: Long) {
2929
@native def root(): JNode

src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala

+5-4
Original file line numberDiff line numberDiff line change
@@ -53,23 +53,24 @@ class BblfshClientLoadTest extends BblfshClientBaseTest {
5353
}
5454

5555
"Decoding, loading & encoding to different context" should "produce the same results" in {
56+
// decode -> load -> encode, and compare bytes
5657
val uast = resp.uast.decode()
5758
val rootNode: Node = uast.root()
5859

5960
println(s"Loading $rootNode")
6061
val root = rootNode.load()
61-
6262
val ctx = Context()
6363
val data = ctx.encode(root)
6464

65+
data should equal (resp.uast.asReadOnlyByteBuffer())
66+
67+
// decode -> load -> encoded -> decode -> load, and compare trees
6568
val uast2 = BblfshClient.decode(data)
6669
val rootNode2: Node = uast2.root()
6770
println(s"Loading $rootNode2")
6871
val root2 = rootNode2.load()
69-
root2 should equal (root)
7072

71-
data should equal (resp.uast.asReadOnlyByteBuffer())
72-
// data.compareTo(resp.uast.asReadOnlyByteBuffer()) shouldBe 0
73+
root2 should equal (root)
7374
}
7475

7576
}

0 commit comments

Comments
 (0)