Skip to content
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

Better output from impl Debug for JsValue. #1161

Merged
merged 14 commits into from
Jan 18, 2019
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ INSTALL_NODE_VIA_NVM: &INSTALL_NODE_VIA_NVM
rustup target add wasm32-unknown-unknown
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.8/install.sh | bash
source ~/.nvm/nvm.sh
nvm install v10.5
nvm install v10.9

INSTALL_GECKODRIVER: &INSTALL_GECKODRIVER
|
Expand Down
94 changes: 94 additions & 0 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,18 @@ impl<'a> Context<'a> {
))
})?;

self.bind("__wbindgen_is_array", &|me| {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: remove this

me.expose_get_object();
Ok(String::from(
"
function(i) {
const val = getObject(i);
return Array.isArray(val) ? 1 : 0;
}
",
))
})?;

self.bind("__wbindgen_is_function", &|me| {
me.expose_get_object();
Ok(String::from(
Expand Down Expand Up @@ -338,6 +350,88 @@ impl<'a> Context<'a> {
))
})?;

self.bind("__wbindgen_debug_string", &|me| {
me.expose_pass_string_to_wasm()?;
me.expose_get_object();
me.expose_uint32_memory();
Ok(String::from(
"
function(i, len_ptr) {
const toString = Object.prototype.toString;
const debug_str = val => {
Copy link
Member

Choose a reason for hiding this comment

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

I tried to come up with something that supported as rich of debug string output as this but was a little more terse. Not sure I really ended up on an improvement over what you have here, but for posterity:

function debug(val) {
  switch(typeof val) {
    case "undefined":
    case "number":
    case "boolean":
    case "symbol":
      return String(val);
    case "function":
      return `Function(${val.name || ""})`;
    case "object":
      if (val === null) {
        return "null";
      }
      const s = Object.prototype.toString.call(val);
      if (s === "[object Object]" || s === "[object Array]") {
        try {
          return JSON.stringify(val);
        } catch (_) {}
      }
      return s;
    default:
      return Object.prototype.toString.call(val);
  }
}

Copy link
Contributor Author

@richard-uk1 richard-uk1 Jan 11, 2019

Choose a reason for hiding this comment

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

This is cool. I did think about doing a big switch. If JSON.stringify can throw, I should catch it and just fall through like you do. If we can capture the function name that would be cool also. I'll try to merge the 2 impls.

Also since I'm assuming there's no stability guarantees on the output here we can always refine later.

  • get name of the function if possible
  • if JSON.stringify can throw, catch and ignore any exceptions.
  • get toString from Object.prototype

Copy link
Member

Choose a reason for hiding this comment

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

If JSON.stringify can throw

json-lol

This is also why I did Object.prototype.toString.call(val) rather than val.toString() or String(val) etc.

// primitive types
const type = typeof val;
if (type == 'number' || type == 'boolean' || val == null) {
return `${val}`;
}
if (type == 'string') {
return `\"${val}\"`;
}
if (type == 'symbol') {
const description = val.description;
if (description == null) {
return 'Symbol';
} else {
return `Symbol(${description})`;
}
}
if (type == 'function') {
const name = val.name;
if (typeof name == 'string' && name.length > 0) {
return `Function(${name})`;
} else {
return 'Function';
}
}
// objects
if (Array.isArray(val)) {
const length = val.length;
let debug = '[';
if (length > 0) {
debug += debug_str(val[0]);
}
for(let i = 1; i < length; i++) {
debug += ', ' + debug_str(val[i]);
}
debug += ']';
return debug;
}
// Test for built-in
const builtInMatches = /\\[object ([^\\]]+)\\]/.exec(toString.call(val));
let className;
if (builtInMatches.length > 1) {
className = builtInMatches[1];
} else {
// Failed to match the standard '[object ClassName]'
return toString.call(val);
}
if (className == 'Object') {
// we're a user defined class or Object
// JSON.stringify avoids problems with cycles, and is generally much
// easier than looping through ownProperties of `val`.
try {
return 'Object(' + JSON.stringify(val) + ')';
} catch (_) {
return 'Object';
}
}
// errors
if (val instanceof Error) {
return `${val.name}: ${val.message}\n${val.stack}`;
}
// TODO we could test for more things here, like `Set`s and `Map`s.
return className;
};
const val = getObject(i);
const debug = debug_str(val);
const ptr = passStringToWasm(debug);
getUint32Memory()[len_ptr / 4] = WASM_VECTOR_LEN;
return ptr;
}
",
))
})?;

self.bind("__wbindgen_cb_drop", &|me| {
me.expose_drop_ref();
Ok(String::from(
Expand Down
48 changes: 26 additions & 22 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,21 @@ impl JsValue {
pub fn is_function(&self) -> bool {
unsafe { __wbindgen_is_function(self.idx) == 1 }
}

/// Get a string representation of the JavaScript object for debugging
#[cfg(feature = "std")]
fn as_debug_string(&self) -> String {
unsafe {
let mut len = 0;
let ptr = __wbindgen_debug_string(self.idx, &mut len);
if ptr.is_null() {
unreachable!("`__wbindgen_debug_string` must return a valid string")
} else {
let data = Vec::from_raw_parts(ptr, len, len);
String::from_utf8_unchecked(data)
}
}
}
}

impl PartialEq for JsValue {
Expand Down Expand Up @@ -474,9 +489,11 @@ externs! {
fn __wbindgen_symbol_new(ptr: *const u8, len: usize) -> u32;
fn __wbindgen_is_symbol(idx: u32) -> u32;
fn __wbindgen_is_object(idx: u32) -> u32;
fn __wbindgen_is_array(idx: u32) -> u32;
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep sorry missed this last time.

fn __wbindgen_is_function(idx: u32) -> u32;
fn __wbindgen_is_string(idx: u32) -> u32;
fn __wbindgen_string_get(idx: u32, len: *mut usize) -> *mut u8;
fn __wbindgen_debug_string(idx: u32, len: *mut usize) -> *mut u8;
fn __wbindgen_throw(a: *const u8, b: usize) -> !;
fn __wbindgen_rethrow(a: u32) -> !;

Expand All @@ -503,30 +520,17 @@ impl Clone for JsValue {
}
}

#[cfg(feature = "std")]
impl fmt::Debug for JsValue {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
if let Some(n) = self.as_f64() {
return n.fmt(f);
}
#[cfg(feature = "std")]
{
if let Some(n) = self.as_string() {
return n.fmt(f);
}
}
if let Some(n) = self.as_bool() {
return n.fmt(f);
}
if self.is_null() {
return fmt::Display::fmt("null", f);
}
if self.is_undefined() {
return fmt::Display::fmt("undefined", f);
}
if self.is_symbol() {
return fmt::Display::fmt("Symbol(..)", f);
}
fmt::Display::fmt("[object]", f)
write!(f, "JsValue({})", self.as_debug_string())
}
}

#[cfg(not(feature = "std"))]
impl fmt::Debug for JsValue {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("JsValue")
}
}

Expand Down
14 changes: 14 additions & 0 deletions tests/wasm/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,17 @@ exports.js_eq_works = () => {
assert.strictEqual(wasm.eq_test(x, x), true);
assert.strictEqual(wasm.eq_test1(x), true);
};

exports.debug_values = () => ([
null,
undefined,
0,
1.0,
true,
[1,2,3],
"string",
{test: "object"},
[1.0, [2.0, 3.0]],
() => (null),
new Set(),
]);
28 changes: 25 additions & 3 deletions tests/wasm/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extern "C" {
fn js_works();
fn js_eq_works();
fn assert_null(v: JsValue);
fn debug_values() -> JsValue;
}

#[wasm_bindgen_test]
Expand Down Expand Up @@ -71,7 +72,7 @@ pub fn api_get_false() -> JsValue {
#[wasm_bindgen]
pub fn api_test_bool(a: &JsValue, b: &JsValue, c: &JsValue) {
assert_eq!(a.as_bool(), Some(true));
assert_eq!(format!("{:?}", a), "true");
assert_eq!(format!("{:?}", a), "JsValue(true)");
assert_eq!(b.as_bool(), Some(false));
assert_eq!(c.as_bool(), None);
}
Expand All @@ -80,7 +81,7 @@ pub fn api_test_bool(a: &JsValue, b: &JsValue, c: &JsValue) {
pub fn api_mk_symbol() -> JsValue {
let a = JsValue::symbol(None);
assert!(a.is_symbol());
assert_eq!(format!("{:?}", a), "Symbol(..)");
assert_eq!(format!("{:?}", a), "JsValue(Symbol)");
return a;
}

Expand All @@ -100,7 +101,7 @@ pub fn api_assert_symbols(a: &JsValue, b: &JsValue) {
#[wasm_bindgen]
pub fn api_acquire_string(a: &JsValue, b: &JsValue) {
assert_eq!(a.as_string().unwrap(), "foo");
assert_eq!(format!("{:?}", a), "\"foo\"");
assert_eq!(format!("{:?}", a), "JsValue(\"foo\")");
assert_eq!(b.as_string(), None);
}

Expand Down Expand Up @@ -145,3 +146,24 @@ fn memory_accessor_appears_to_work() {
.for_each(&mut |val, _, _| v.push(val));
assert_eq!(v, [3, 0, 0, 0]);
}

#[wasm_bindgen_test]
fn debug_output() {
let test_iter = debug_values().dyn_into::<js_sys::Array>().unwrap().values().into_iter();
let expecteds = vec![
"JsValue(null)",
"JsValue(undefined)",
"JsValue(0)",
"JsValue(1)",
"JsValue(true)",
"JsValue([1, 2, 3])",
"JsValue(\"string\")",
"JsValue(Object({\"test\":\"object\"}))",
"JsValue([1, [2, 3]])",
"JsValue(Function)",
"JsValue(Set)",
];
for (test, expected) in test_iter.zip(expecteds) {
assert_eq!(format!("{:?}", test.unwrap()), expected);
}
}