Skip to content

Initial nodejs string wrap #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 167 additions & 3 deletions javascript/lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@
*/
#include <stdio.h> // `printf` for debug builds
#include <stdlib.h> // `malloc` to export strings into UTF-8
#include <string.h> // strncmp

#include <node_api.h> // `napi_*` functions

#include <stringzilla/stringzilla.h> // `sz_*` functions

typedef struct {
napi_env env_;
napi_ref wrapper_;
sz_cptr_t start;
sz_size_t length;
} sz_wrapper_t;

napi_value indexOfAPI(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
Expand Down Expand Up @@ -107,18 +115,174 @@ napi_value countAPI(napi_env env, napi_callback_info info) {

return js_count;
}
napi_value str_startswith(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
napi_value jsthis;
sz_wrapper_t *this;
sz_wrapper_t *arg;

napi_get_cb_info(env, info, &argc, args, &jsthis, NULL);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling for napi_get_cb_info call. The function returns napi_status which should be checked. If it fails, the function could proceed with invalid args/jsthis values. Should check return value and handle error cases.

📚 Relevant Docs


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

napi_unwrap(env, jsthis, (void **)&this);
napi_unwrap(env, args[0], (void **)&arg);

napi_value ret;
if (this->length < arg->length) { napi_get_boolean(env, false, &ret); }
else if (strncmp(this->start, arg->start, arg->length) == 0) { napi_get_boolean(env, true, &ret); }
else { napi_get_boolean(env, false, &ret); }
return ret;
}
napi_value str_endswith(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
napi_value jsthis;
sz_wrapper_t *this;
sz_wrapper_t *arg;

napi_get_cb_info(env, info, &argc, args, &jsthis, NULL);
napi_unwrap(env, jsthis, (void **)&this);
napi_unwrap(env, args[0], (void **)&arg);

napi_value ret;
if (this->length < arg->length) { napi_get_boolean(env, false, &ret); }
else if (strncmp(this->start + (this->length - arg->length), arg->start, arg->length) == 0) {
napi_get_boolean(env, true, &ret);
}
else { napi_get_boolean(env, false, &ret); }
return ret;
}


napi_value str_count(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
napi_value jsthis;
sz_wrapper_t *this;
sz_wrapper_t *arg;

napi_get_cb_info(env, info, &argc, args, &jsthis, NULL);
napi_unwrap(env, jsthis, (void **)&this);
napi_unwrap(env, args[0], (void **)&arg);

sz_string_view_t haystack = {this->start, this->length};
sz_string_view_t needle = {arg->start, arg->length};

bool overlap = false;
if (argc > 1) { napi_get_value_bool(env, args[1], &overlap); }

size_t count = 0;
if (needle.length == 0 || haystack.length == 0 || haystack.length < needle.length) { count = 0; }
else if (overlap) {
while (haystack.length) {
sz_cptr_t ptr = sz_find(haystack.start, haystack.length, needle.start, needle.length);
sz_bool_t found = ptr != NULL;
sz_size_t offset = found ? (sz_size_t)(ptr - haystack.start) : haystack.length;
count += found;
haystack.start += offset + found;
haystack.length -= offset + found;
}
}
else {
while (haystack.length) {
sz_cptr_t ptr = sz_find(haystack.start, haystack.length, needle.start, needle.length);
sz_bool_t found = ptr != NULL;
sz_size_t offset = found ? (sz_size_t)(ptr - haystack.start) : haystack.length;
count += found;
haystack.start += offset + needle.length;
haystack.length -= offset + needle.length * found;
}
}

napi_value js_count;
napi_create_bigint_uint64(env, count, &js_count);
return js_count;
}
napi_value _str_find(sz_find_t finder, napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
napi_value jsthis;
sz_wrapper_t *this;
sz_wrapper_t *needle;

napi_get_cb_info(env, info, &argc, args, &jsthis, NULL);
napi_unwrap(env, jsthis, (void **)&this);
napi_unwrap(env, args[0], (void **)&needle);


napi_value js_result;
if (needle->length == 0) { napi_create_bigint_int64(env, 0, &js_result); }
else {
sz_cptr_t result = finder(this->start, this->length, needle->start, needle->length);

// In JavaScript, if `indexOf` is unable to indexOf the specified value, then it should return -1
if (result == NULL) { napi_create_bigint_int64(env, -1, &js_result); }
else { napi_create_bigint_uint64(env, result - this->start, &js_result); }
}

return js_result;
}

napi_value str_find(napi_env env, napi_callback_info info) {
return _str_find(sz_find, env, info);
}
napi_value str_rfind(napi_env env, napi_callback_info info) {
return _str_find(sz_rfind, env, info);
}

static void destroy(napi_env _unused_env, void *obj, void *_unused_hint) {
sz_wrapper_t *sz = (sz_wrapper_t *)obj;
napi_delete_reference(sz->env_, sz->wrapper_);
free((void *)sz->start);
free(sz);
}

static napi_value create_instance(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
napi_value jsthis;
sz_wrapper_t *obj = malloc(sizeof(sz_wrapper_t));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No NULL check after malloc. If allocation fails, the function continues with an invalid pointer which could cause a crash. Should check if obj is NULL and handle the error case appropriately.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

napi_get_cb_info(env, info, &argc, args, &jsthis, NULL);
// if (type_check_data_buffer(env, args, argc) != ADDON_OK) {
// return NULL;
//}
// napi_get_buffer_info(env, args[0], &data, &buf_len);
// TODO type check
napi_get_value_string_utf8(env, args[0], NULL, 0, (size_t *)&obj->length);
obj->start = malloc(obj->length + 1);
napi_get_value_string_utf8(env, args[0], (char *)obj->start, obj->length + 1, (size_t *)&obj->length);

obj->env_ = env;
napi_wrap(env, jsthis, (void *)obj, destroy, NULL, &obj->wrapper_);
return jsthis;
}

napi_value Init(napi_env env, napi_value exports) {

// Define an array of property descriptors
napi_property_descriptor findDesc = {"indexOf", 0, indexOfAPI, 0, 0, 0, napi_default, 0};
napi_property_descriptor countDesc = {"count", 0, countAPI, 0, 0, 0, napi_default, 0};
napi_property_descriptor properties[] = {findDesc, countDesc};
napi_property_descriptor properties[] = {
{"indexOf", NULL, indexOfAPI, NULL, NULL, NULL, napi_default, NULL},
{"find", NULL, indexOfAPI, NULL, NULL, NULL, napi_default, NULL},
{"count", NULL, countAPI, NULL, NULL, NULL, napi_default, NULL}
};

// Define the properties on the `exports` object
size_t propertyCount = sizeof(properties) / sizeof(properties[0]);
napi_define_properties(env, exports, propertyCount, properties);

napi_property_descriptor obj_properties[] = {
{"indexOf", NULL, str_find, NULL, NULL, NULL, napi_default, NULL},
{"find", NULL, str_find, NULL, NULL, NULL, napi_default, NULL},
{"rfind", NULL, str_rfind, NULL, NULL, NULL, napi_default, NULL},
{"startswith", NULL, str_startswith, NULL, NULL, NULL, napi_default, NULL},
{"endswith", NULL, str_endswith, NULL, NULL, NULL, napi_default, NULL},
{"count", NULL, str_count, NULL, NULL, NULL, napi_default, NULL}
};
napi_value cons;

napi_define_class(env, "Str", NAPI_AUTO_LENGTH, create_instance, NULL,
sizeof(obj_properties) / sizeof(*obj_properties), obj_properties, &cons);
napi_set_named_property(env, exports, "Str", cons);

return exports;
}

Expand Down
8 changes: 6 additions & 2 deletions javascript/stringzilla.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module.exports = {
* @param {string} needle
* @returns {bigint}
*/
find: compiled.find,
indexOf: compiled.indexOf,
indexOfB: compiled.indexOfB,

/**
* Searches for a substring in a larger string.
Expand All @@ -18,5 +19,8 @@ module.exports = {
* @param {boolean} overlap
* @returns {bigint}
*/
count: compiled.count
count: compiled.count,

Str: compiled.Str
};

106 changes: 105 additions & 1 deletion scripts/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import test from 'node:test';
import bindings from 'bindings';
import assert from 'node:assert';

const stringzilla = bindings('stringzilla');
const stringzilla = bindings('../../build/Release/stringzilla');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a relative path ('../../build/Release/stringzilla') with the 'bindings' module is incorrect and could cause module resolution failures. The 'bindings' module is designed to automatically locate native addons and should be used with just the module name. The original code using bindings('stringzilla') was correct. The change to a relative path breaks the standard module resolution mechanism and could fail when the package is installed in different locations.

📚 Relevant Docs


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)


test('Find Word in Text - Positive Case', () => {
const result = stringzilla.indexOf('hello world, hello john', 'hello');
Expand Down Expand Up @@ -68,3 +68,107 @@ test('Count Words - Empty String Inputs', () => {
const result_3 = stringzilla.count('', '');
assert.strictEqual(result_3, 0n);
});

test('Str Count - Empty String Inputs', () => {
const a = new stringzilla.Str('hello world');
const b = new stringzilla.Str('hi');
const empty = new stringzilla.Str('');

assert.strictEqual(a.count(empty), 0n);
assert.strictEqual(empty.count(b), 0n);
assert.strictEqual(empty.count(empty), 0n);
});
test('Str.count - No Occurence', () => {
const a = new stringzilla.Str('hello world');
const b = new stringzilla.Str('hi');

const result_1 = a.count(b);
assert.strictEqual(result_1, 0n);
});
test('Str.count - Multiple Occurrences with Overlap Test', () => {
const a = new stringzilla.Str('abababab');
const b = new stringzilla.Str('aba');

const result_2 = a.count(b, true);
assert.strictEqual(result_2, 3n);
const result_1 = a.count(b);
assert.strictEqual(result_1, 2n);

});

test('Str.count - Multiple Occurrences', () => {
const a = new stringzilla.Str('abigababzzzzzzzzzzzzzzzzzzzbigzzzzzzzzzzzzfdsafbig');
const b = new stringzilla.Str('big');

const res = a.count(b, true);
assert.strictEqual(res, 3n);

});
test('Str.count - Single Occurrence', () => {
const a = new stringzilla.Str('azigababzzzzzzzzzzzzzzzzzzzzigzzzzzzzzzzzzfdsafbig');
const b = new stringzilla.Str('big');

const res = a.count(b, true);
assert.strictEqual(res, 1n);

});


test('Str.find - Positive Case', () => {
const a = new stringzilla.Str('Can you ifnd me here with find');
const b = new stringzilla.Str('find');
assert.strictEqual(a.find(b), 26n);
});

test('Str.find - Negative Case (Word Not Found)', () => {
const a = new stringzilla.Str('Can you ifnd me here with find');
const b = new stringzilla.Str('z');
assert.strictEqual(a.find(b), -1n);
});

test('Str.find - Negative Case (Empty String Inputs)', () => {
const a = new stringzilla.Str('hello world');
const b = new stringzilla.Str('hi');
const empty = new stringzilla.Str('');

assert.strictEqual(a.find(empty), 0n);
assert.strictEqual(empty.find(b), -1n);
assert.strictEqual(empty.find(empty), 0n);
});
test('Str.rfind', () => {
const a = new stringzilla.Str('Can you ifnd me here with find');
const b = new stringzilla.Str('n');
const can = new stringzilla.Str('Can');
const z = new stringzilla.Str('z');
const empty = new stringzilla.Str('');
assert.strictEqual(a.rfind(b), 28n);
assert.strictEqual(a.rfind(z), -1n);
assert.strictEqual(a.rfind(can), 0n);
assert.strictEqual(a.rfind(empty), 0n);
assert.strictEqual(empty.rfind(z), -1n);
assert.strictEqual(empty.rfind(empty), 0n);
});
test('Str.startswith', () => {
const a = new stringzilla.Str('Can you ifnd me here with find');
const b = new stringzilla.Str('n');
const can = new stringzilla.Str('Can');
const empty = new stringzilla.Str('');
assert.strictEqual(a.startswith(b), false);
assert.strictEqual(a.startswith(can), true);
assert.strictEqual(a.startswith(empty), true);
assert.strictEqual(empty.startswith(a), false);
assert.strictEqual(empty.startswith(empty), true);
});
test('Str.endswith', () => {
const a = new stringzilla.Str('Can you ifnd me here with find');
const b = new stringzilla.Str('n');
const can = new stringzilla.Str('find');
const empty = new stringzilla.Str('');
assert.strictEqual(a.endswith(b), false);
assert.strictEqual(a.endswith(can), true);
assert.strictEqual(a.endswith(empty), true);
assert.strictEqual(empty.endswith(a), false);
assert.strictEqual(empty.endswith(empty), true);
});