From da6e6ae91894bc52fc04234fbd2610f8969399f4 Mon Sep 17 00:00:00 2001 From: Roger Meier Date: Tue, 15 Mar 2011 09:55:33 +0000 Subject: [PATCH] THRIFT-1089 JavaScript Quality Assurance with lint git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1081707 13f79535-47bb-0310-9956-ffa450edef68 --- compiler/cpp/src/generate/t_js_generator.cc | 87 +++++---- lib/js/test/build.xml | 34 +++- lib/js/test/ivy.xml | 3 +- lib/js/thrift.js | 189 +++++++++++--------- 4 files changed, 190 insertions(+), 123 deletions(-) diff --git a/compiler/cpp/src/generate/t_js_generator.cc b/compiler/cpp/src/generate/t_js_generator.cc index bd640db7..9def5da9 100644 --- a/compiler/cpp/src/generate/t_js_generator.cc +++ b/compiler/cpp/src/generate/t_js_generator.cc @@ -550,10 +550,10 @@ void t_js_generator::generate_js_struct_definition(ofstream& out, } } - out << indent() << "if (args != null) {" << endl; + out << indent() << "if (args) {" << endl; for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) { - out << indent() << indent() << "if (null != args." << (*m_iter)->get_name() << ") {" << endl + out << indent() << indent() << "if (!args." << (*m_iter)->get_name() << ") {" << endl << indent() << indent() << indent() << "this." << (*m_iter)->get_name() << " = args." << (*m_iter)->get_name() << ";" << endl << indent() << indent() << "}" << endl; } @@ -607,40 +607,50 @@ void t_js_generator::generate_js_struct_reader(ofstream& out, // Check for field STOP marker and break - indent(out) << "if (ftype == Thrift.Type.STOP)" << endl; + indent(out) << "if (ftype == Thrift.Type.STOP) {" << endl; indent_up(); indent(out) << "break;" << endl; indent_down(); + indent(out) << "}" << endl; + if (!fields.empty()) { + // Switch statement on the field we are reading + indent(out) << "switch (fid)" << endl; - // Switch statement on the field we are reading - indent(out) << "switch (fid)" << endl; + scope_up(out); - scope_up(out); + // Generate deserialization code for known cases + for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { - // Generate deserialization code for known cases - for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { + indent(out) << "case " << (*f_iter)->get_key() << ":" << endl; + indent(out) << "if (ftype == " << type_to_enum((*f_iter)->get_type()) << ") {" << endl; - indent(out) << "case " << (*f_iter)->get_key() << ":" << endl; - indent(out) << "if (ftype == " << type_to_enum((*f_iter)->get_type()) << ") {" << endl; + indent_up(); + generate_deserialize_field(out, *f_iter, "this."); + indent_down(); - indent_up(); - generate_deserialize_field(out, *f_iter, "this."); - indent_down(); + indent(out) << "} else {" << endl; - indent(out) << "} else {" << endl; + indent(out) << " input.skip(ftype);" << endl; - indent(out) << " input.skip(ftype);" << endl; + out << + indent() << "}" << endl << + indent() << "break;" << endl; - out << - indent() << "}" << endl << - indent() << "break;" << endl; + } + if (fields.size() == 1) { + // pseudo case to make jslint happy + indent(out) << "case 0:" << endl; + indent(out) << " input.skip(ftype);" << endl; + indent(out) << " break;" << endl; + } + // In the default case we skip the field + indent(out) << "default:" << endl; + indent(out) << " input.skip(ftype);" << endl; + scope_down(out); + } else { + indent(out) << "input.skip(ftype);" << endl; } - // In the default case we skip the field - indent(out) << "default:" << endl; - indent(out) << " input.skip(ftype);" << endl; - - scope_down(out); indent(out) << "input.readFieldEnd();" << endl; @@ -670,7 +680,7 @@ void t_js_generator::generate_js_struct_writer(ofstream& out, indent(out) << "output.writeStructBegin('" << name << "');" << endl; for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { - out << indent() << "if (null != this." << (*f_iter)->get_name() << ") {" << endl; + out << indent() << "if (this." << (*f_iter)->get_name() << ") {" << endl; indent_up(); indent(out) << @@ -951,7 +961,7 @@ void t_js_generator::generate_service_client(t_service* tservice) { } else { f_service_ << indent() << " this.input = input;" << endl << - indent() << " this.output = null == output ? input : output;" << endl << + indent() << " this.output = (!output) ? input : output;" << endl << indent() << " this.seqid = 0;" << endl; } @@ -1129,7 +1139,7 @@ void t_js_generator::generate_service_client(t_service* tservice) { vector::const_iterator x_iter; for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) { f_service_ << - indent() << "if (null != result." << (*x_iter)->get_name() << ") {" << endl << + indent() << "if (null !== result." << (*x_iter)->get_name() << ") {" << endl << indent() << " " << render_recv_throw("result." + (*x_iter)->get_name()) << endl << indent() << "}" << endl; } @@ -1137,7 +1147,7 @@ void t_js_generator::generate_service_client(t_service* tservice) { // Careful, only return result if not a void function if (!(*f_iter)->get_returntype()->is_void()) { f_service_ << - indent() << "if (null != result.success) {" << endl << + indent() << "if (null !== result.success) {" << endl << indent() << " " << render_recv_return("result.success") << endl << indent() << "}" << endl; f_service_ << @@ -1269,6 +1279,7 @@ void t_js_generator::generate_deserialize_container(ofstream &out, string ktype = tmp("_ktype"); string vtype = tmp("_vtype"); string etype = tmp("_etype"); + string rtmp3 = tmp("_rtmp3"); t_field fsize(g_type_i32, size); t_field fktype(g_type_byte, ktype); @@ -1276,7 +1287,7 @@ void t_js_generator::generate_deserialize_container(ofstream &out, t_field fetype(g_type_byte, etype); out << indent() << "var " << size << " = 0;" << endl; - out << indent() << "var rtmp3;" << endl; + out << indent() << "var " << rtmp3 << ";" << endl; // Declare variables, read header @@ -1286,10 +1297,10 @@ void t_js_generator::generate_deserialize_container(ofstream &out, indent() << "var " << ktype << " = 0;" << endl << indent() << "var " << vtype << " = 0;" << endl; - out << indent() << "rtmp3 = input.readMapBegin();" << endl; - out << indent() << ktype << " = rtmp3.ktype;" << endl; - out << indent() << vtype << " = rtmp3.vtype;" << endl; - out << indent() << size << " = rtmp3.size;" << endl; + out << indent() << rtmp3 << " = input.readMapBegin();" << endl; + out << indent() << ktype << " = " << rtmp3 << ".ktype;" << endl; + out << indent() << vtype << " = " << rtmp3 << ".vtype;" << endl; + out << indent() << size << " = " << rtmp3 << ".size;" << endl; } else if (ttype->is_set()) { @@ -1297,18 +1308,18 @@ void t_js_generator::generate_deserialize_container(ofstream &out, out << indent() << prefix << " = [];" << endl << indent() << "var " << etype << " = 0;" << endl << - indent() << "rtmp3 = input.readSetBegin();" << endl << - indent() << etype << " = rtmp3.etype;"<is_list()) { out << indent() << prefix << " = [];" << endl << indent() << "var " << etype << " = 0;" << endl << - indent() << "rtmp3 = input.readListBegin();" << endl << - indent() << etype << " = rtmp3.etype;"< + xmlns:artifact="antlib:org.apache.maven.artifact.ant" + xmlns:jsl="antlib:com.googlecode.jslint4java"> Java Script Test based on Thrift Java Library @@ -51,6 +52,8 @@ + + @@ -116,6 +119,35 @@ + + + + + + + + + + + + + check if gjslint is available: + + + + + + + + + + + + + diff --git a/lib/js/test/ivy.xml b/lib/js/test/ivy.xml index fdd2d902..a9016597 100644 --- a/lib/js/test/ivy.xml +++ b/lib/js/test/ivy.xml @@ -17,6 +17,7 @@ - + + diff --git a/lib/js/thrift.js b/lib/js/thrift.js index 5c1fdf19..b8ca2c02 100644 --- a/lib/js/thrift.js +++ b/lib/js/thrift.js @@ -54,13 +54,16 @@ var Thrift = { objectLength: function(obj) { var length = 0; - for (k in obj) - if (obj.hasOwnProperty(k)) + for (var k in obj) { + if (obj.hasOwnProperty(k)) { length++; + } + } + return length; }, - inherits: function(constructor, superConstructor) { + inherits: function(constructor, superConstructor) { //Prototypal Inheritance http://javascript.crockford.com/prototypal.html function F() {} F.prototype = superConstructor.prototype; @@ -73,7 +76,7 @@ Thrift.TException = {}; Thrift.TException.prototype = { initialize: function(message, code) { this.message = message; - this.code = (code == null) ? 0 : code; + this.code = (code === null) ? 0 : code; } }; @@ -89,17 +92,18 @@ Thrift.TApplicationExceptionType = { Thrift.TApplicationException = function(message, code) { this.message = message; - this.code = (code == null) ? 0 : code; + this.code = (code === null) ? 0 : code; }; Thrift.TApplicationException.prototype = { read: function(input) { while (1) { - ret = input.readFieldBegin(); + var ret = input.readFieldBegin(); - if (ret.ftype == Thrift.Type.STOP) + if (ret.ftype == Thrift.Type.STOP) { break; + } var fid = ret.fid; @@ -183,9 +187,9 @@ Thrift.Transport.prototype = { //Gets the browser specific XmlHttpRequest Object getXmlHttpRequestObject: function() { - try { return new XMLHttpRequest() } catch (e) {} - try { return new ActiveXObject('Msxml2.XMLHTTP') } catch (e) {} - try { return new ActiveXObject('Microsoft.XMLHTTP') } catch (e) {} + try { return new XMLHttpRequest(); } catch (e1) { } + try { return new ActiveXObject('Msxml2.XMLHTTP'); } catch (e2) { } + try { return new ActiveXObject('Microsoft.XMLHTTP'); } catch (e3) { } throw "Your browser doesn't support the XmlHttpRequest object."; @@ -194,22 +198,26 @@ Thrift.Transport.prototype = { flush: function() { //async mode - if (this.url == undefined || this.url == '') + if (this.url === undefined || this.url === '') { return this.send_buf; + } var xreq = this.getXmlHttpRequestObject(); - if (xreq.overrideMimeType) + if (xreq.overrideMimeType) { xreq.overrideMimeType('application/json'); + } xreq.open('POST', this.url, false); xreq.send(this.send_buf); - if (xreq.readyState != 4) + if (xreq.readyState != 4) { throw 'encountered an unknown ajax ready state: ' + xreq.readyState; + } - if (xreq.status != 200) + if (xreq.status != 200) { throw 'encountered a unknown request status: ' + xreq.status; + } this.recv_buf = xreq.responseText; this.recv_buf_sz = this.recv_buf.length; @@ -235,13 +243,15 @@ Thrift.Transport.prototype = { read: function(len) { var avail = this.wpos - this.rpos; - if (avail == 0) + if (avail === 0) { return ''; + } var give = len; - if (avail < len) + if (avail < len) { give = avail; + } var ret = this.read_buf.substr(this.rpos, give); this.rpos += give; @@ -251,7 +261,7 @@ Thrift.Transport.prototype = { }, readAll: function() { - return this.recv_buf; + return this.recv_buf; }, write: function(buf) { @@ -285,17 +295,17 @@ Thrift.Protocol.Type[Thrift.Type.SET] = '"set"'; Thrift.Protocol.RType = {}; -Thrift.Protocol.RType['tf'] = Thrift.Type.BOOL; -Thrift.Protocol.RType['i8'] = Thrift.Type.BYTE; -Thrift.Protocol.RType['i16'] = Thrift.Type.I16; -Thrift.Protocol.RType['i32'] = Thrift.Type.I32; -Thrift.Protocol.RType['i64'] = Thrift.Type.I64; -Thrift.Protocol.RType['dbl'] = Thrift.Type.DOUBLE; -Thrift.Protocol.RType['rec'] = Thrift.Type.STRUCT; -Thrift.Protocol.RType['str'] = Thrift.Type.STRING; -Thrift.Protocol.RType['map'] = Thrift.Type.MAP; -Thrift.Protocol.RType['lst'] = Thrift.Type.LIST; -Thrift.Protocol.RType['set'] = Thrift.Type.SET; +Thrift.Protocol.RType.tf = Thrift.Type.BOOL; +Thrift.Protocol.RType.i8 = Thrift.Type.BYTE; +Thrift.Protocol.RType.i16 = Thrift.Type.I16; +Thrift.Protocol.RType.i32 = Thrift.Type.I32; +Thrift.Protocol.RType.i64 = Thrift.Type.I64; +Thrift.Protocol.RType.dbl = Thrift.Type.DOUBLE; +Thrift.Protocol.RType.rec = Thrift.Type.STRUCT; +Thrift.Protocol.RType.str = Thrift.Type.STRING; +Thrift.Protocol.RType.map = Thrift.Type.MAP; +Thrift.Protocol.RType.lst = Thrift.Type.LIST; +Thrift.Protocol.RType.set = Thrift.Type.SET; Thrift.Protocol.Version = 1; @@ -307,10 +317,10 @@ Thrift.Protocol.prototype = { //Write functions writeMessageBegin: function(name, messageType, seqid) { - this.tstack = new Array(); - this.tpos = new Array(); - - this.tstack.push([Thrift.Protocol.Version, '"' + + this.tstack = []; + this.tpos = []; + + this.tstack.push([Thrift.Protocol.Version, '"' + name + '"', messageType, seqid]); }, @@ -338,10 +348,11 @@ Thrift.Protocol.prototype = { var str = '{'; var first = true; for (var key in struct) { - if (first) + if (first) { first = false; - else + } else { str += ','; + } str += key + ':' + struct[key]; } @@ -351,18 +362,18 @@ Thrift.Protocol.prototype = { }, writeFieldBegin: function(name, fieldType, fieldId) { - this.tpos.push(this.tstack.length); - this.tstack.push({ 'fieldId': '"' + - fieldId + '"', 'fieldType': Thrift.Protocol.Type[fieldType] + this.tpos.push(this.tstack.length); + this.tstack.push({ 'fieldId': '"' + + fieldId + '"', 'fieldType': Thrift.Protocol.Type[fieldType] }); }, writeFieldEnd: function() { var value = this.tstack.pop(); - var fieldInfo = this.tstack.pop(); - - this.tstack[this.tstack.length - 1][fieldInfo.fieldId] = '{' + + var fieldInfo = this.tstack.pop(); + + this.tstack[this.tstack.length - 1][fieldInfo.fieldId] = '{' + fieldInfo.fieldType + ':' + value + '}'; this.tpos.pop(); }, @@ -373,7 +384,7 @@ Thrift.Protocol.prototype = { writeMapBegin: function(keyType, valType, size) { //size is invalid, we'll set it on end. - this.tpos.push(this.tstack.length); + this.tpos.push(this.tstack.length); this.tstack.push([Thrift.Protocol.Type[keyType], Thrift.Protocol.Type[valType], 0]); }, @@ -381,11 +392,13 @@ Thrift.Protocol.prototype = { writeMapEnd: function() { var p = this.tpos.pop(); - if (p == this.tstack.length) + if (p == this.tstack.length) { return; + } - if ((this.tstack.length - p - 1) % 2 != 0) + if ((this.tstack.length - p - 1) % 2 !== 0) { this.tstack.push(''); + } var size = (this.tstack.length - p - 1) / 2; @@ -398,7 +411,7 @@ Thrift.Protocol.prototype = { var k = this.tstack.pop(); if (first) { first = false; - }else { + } else { map = ',' + map; } @@ -515,8 +528,8 @@ Thrift.Protocol.prototype = { // Reading functions readMessageBegin: function(name, messageType, seqid) { - this.rstack = new Array(); - this.rpos = new Array(); + this.rstack = []; + this.rpos = []; this.robj = eval(this.transport.readAll()); @@ -527,9 +540,9 @@ Thrift.Protocol.prototype = { throw 'Wrong thrift protocol version: ' + version; } - r['fname'] = this.robj.shift(); - r['mtype'] = this.robj.shift(); - r['rseqid'] = this.robj.shift(); + r.fname = this.robj.shift(); + r.mtype = this.robj.shift(); + r.rseqid = this.robj.shift(); //get to the main obj @@ -544,18 +557,20 @@ Thrift.Protocol.prototype = { readStructBegin: function(name) { var r = {}; - r['fname'] = ''; + r.fname = ''; //incase this is an array of structs - if (this.rstack[this.rstack.length - 1] instanceof Array) + if (this.rstack[this.rstack.length - 1] instanceof Array) { this.rstack.push(this.rstack[this.rstack.length - 1].shift()); + } return r; }, readStructEnd: function() { - if (this.rstack[this.rstack.length - 2] instanceof Array) + if (this.rstack[this.rstack.length - 2] instanceof Array) { this.rstack.pop(); + } }, readFieldBegin: function() { @@ -566,12 +581,14 @@ Thrift.Protocol.prototype = { //get a fieldId for (var f in (this.rstack[this.rstack.length - 1])) { - if (f == null) continue; + if (f === null) { + continue; + } - fid = parseInt(f); + fid = parseInt(f, 10); this.rpos.push(this.rstack.length); - var field = this.rstack[this.rstack.length - 1][fid] + var field = this.rstack[this.rstack.length - 1][fid]; //remove so we don't see it again delete this.rstack[this.rstack.length - 1][fid]; @@ -585,18 +602,20 @@ Thrift.Protocol.prototype = { //should only be 1 of these but this is the only //way to match a key - for (var f in (this.rstack[this.rstack.length - 1])) { - if (Thrift.Protocol.RType[f] == null) continue; + for (var i in (this.rstack[this.rstack.length - 1])) { + if (Thrift.Protocol.RType[i] === null) { + continue; + } - ftype = Thrift.Protocol.RType[f]; + ftype = Thrift.Protocol.RType[i]; this.rstack[this.rstack.length - 1] = - this.rstack[this.rstack.length - 1][f]; + this.rstack[this.rstack.length - 1][i]; } } - r['fname'] = ''; - r['ftype'] = ftype; - r['fid'] = fid; + r.fname = ''; + r.ftype = ftype; + r.fid = fid; return r; @@ -606,8 +625,9 @@ Thrift.Protocol.prototype = { var pos = this.rpos.pop(); //get back to the right place in the stack - while (this.rstack.length > pos) + while (this.rstack.length > pos) { this.rstack.pop(); + } }, @@ -616,9 +636,9 @@ Thrift.Protocol.prototype = { var map = this.rstack.pop(); var r = {}; - r['ktype'] = Thrift.Protocol.RType[map.shift()]; - r['vtype'] = Thrift.Protocol.RType[map.shift()]; - r['size'] = map.shift(); + r.ktype = Thrift.Protocol.RType[map.shift()]; + r.vtype = Thrift.Protocol.RType[map.shift()]; + r.size = map.shift(); this.rpos.push(this.rstack.length); @@ -636,8 +656,8 @@ Thrift.Protocol.prototype = { var list = this.rstack[this.rstack.length - 1]; var r = {}; - r['etype'] = Thrift.Protocol.RType[list.shift()]; - r['size'] = list.shift(); + r.etype = Thrift.Protocol.RType[list.shift()]; + r.size = list.shift(); this.rpos.push(this.rstack.length); @@ -661,10 +681,10 @@ Thrift.Protocol.prototype = { readBool: function() { var r = this.readI32(); - if (r != null && r['value'] == '1') { - r['value'] = true; - }else { - r['value'] = false; + if (r !== null && r.value == '1') { + r.value = true; + } else { + r.value = false; } return r; @@ -680,28 +700,31 @@ Thrift.Protocol.prototype = { readI32: function(f) { - if (f == undefined) + if (f === undefined) { f = this.rstack[this.rstack.length - 1]; + } var r = {}; if (f instanceof Array) { - if (f.length == 0) - r['value'] = undefined; - else - r['value'] = f.shift(); - - }else if (f instanceof Object) { + if (f.length === 0) { + r.value = undefined; + } else { + r.value = f.shift(); + } + } else if (f instanceof Object) { for (var i in f) { - if (i == null) continue; - this.rstack.push(f[i]) + if (i === null) { + continue; + } + this.rstack.push(f[i]); delete f[i]; - r['value'] = i; + r.value = i; break; } } else { - r['value'] = f; + r.value = f; this.rstack.pop(); } -- 2.17.1