THRIFT-1089 JavaScript Quality Assurance with lint
git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1081707 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_js_generator.cc b/compiler/cpp/src/generate/t_js_generator.cc
index bd640db..9def5da 100644
--- a/compiler/cpp/src/generate/t_js_generator.cc
+++ b/compiler/cpp/src/generate/t_js_generator.cc
@@ -550,10 +550,10 @@
}
}
- 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 @@
// 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;
+ 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;
- out <<
- indent() << "}" << endl <<
- indent() << "break;" << 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 @@
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 @@
} 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 @@
vector<t_field*>::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 @@
// 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 @@
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 @@
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 @@
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 @@
out <<
indent() << prefix << " = [];" << endl <<
indent() << "var " << etype << " = 0;" << endl <<
- indent() << "rtmp3 = input.readSetBegin();" << endl <<
- indent() << etype << " = rtmp3.etype;"<<endl<<
- indent() << size << " = rtmp3.size;"<<endl;
+ indent() << rtmp3 << " = input.readSetBegin();" << endl <<
+ indent() << etype << " = " << rtmp3 << ".etype;"<<endl<<
+ indent() << size << " = " << rtmp3 << ".size;"<<endl;
} else if (ttype->is_list()) {
out <<
indent() << prefix << " = [];" << endl <<
indent() << "var " << etype << " = 0;" << endl <<
- indent() << "rtmp3 = input.readListBegin();" << endl <<
- indent() << etype << " = rtmp3.etype;"<<endl<<
- indent() << size << " = rtmp3.size;"<<endl;
+ indent() << rtmp3 << " = input.readListBegin();" << endl <<
+ indent() << etype << " = " << rtmp3 << ".etype;"<<endl<<
+ indent() << size << " = " << rtmp3 << ".size;"<<endl;
}
// For loop iterates over elements
diff --git a/lib/js/test/build.xml b/lib/js/test/build.xml
index ee38edb..be4e7d5 100644
--- a/lib/js/test/build.xml
+++ b/lib/js/test/build.xml
@@ -18,7 +18,8 @@
-->
<project name="Java Script Test" default="test" basedir="."
xmlns:ivy="antlib:org.apache.ivy.ant"
- xmlns:artifact="antlib:org.apache.maven.artifact.ant">
+ xmlns:artifact="antlib:org.apache.maven.artifact.ant"
+ xmlns:jsl="antlib:com.googlecode.jslint4java">
<description>Java Script Test based on Thrift Java Library</description>
@@ -51,6 +52,8 @@
<pathelement location="${jar.file}" />
</path>
+ <taskdef uri="antlib:com.googlecode.jslint4java" resource="com/googlecode/jslint4java/antlib.xml" classpathref="libs.classpath" />
+
<target name="dependencies">
<fail>
<condition>
@@ -116,6 +119,35 @@
</exec>
</target>
+ <target name="lint" description="code quality checks" depends="gjslint, jslint"/>
+
+ <target name="jslint">
+ <!--
+ the following options would probably make sense in the future:
+ browser,undef,eqeqeq,plusplus,bitwise,regexp,strict,newcap,immed
+ -->
+ <jsl:jslint options="evil,forin,browser,bitwise,regexp,newcap,immed">
+ <formatter type="plain" />
+ <fileset dir="${genjs}" includes="**/*.js" />
+ <fileset dir=".." includes="thrift.js" />
+ </jsl:jslint>
+ </target>
+
+ <target name="check-gjslint">
+ <echo>check if gjslint is available:</echo>
+ <exec executable="gjslint" failifexecutionfails="no" resultproperty="gjslint.present" failonerror="false">
+ <arg line="--helpshort"/>
+ </exec>
+ </target>
+
+ <target name="gjslint" depends="check-gjslint" if="gjslint.present">
+ <exec executable="gjslint" failifexecutionfails="no">
+ <arg line="--nojsdoc"/>
+ <arg line="${genjs}/*.js"/>
+ <arg line="../thrift.js"/>
+ </exec>
+ </target>
+
<target name="clean">
<delete dir="${build}" />
<delete dir="${genjava}" />
diff --git a/lib/js/test/ivy.xml b/lib/js/test/ivy.xml
index fdd2d90..a901659 100644
--- a/lib/js/test/ivy.xml
+++ b/lib/js/test/ivy.xml
@@ -17,6 +17,7 @@
<ivy-module version="2.0">
<info organisation="org.apache.thrift" module="jstest"/>
<dependencies>
- <dependency org="org.apache.httpcomponents" name="httpclient" rev="4.0.1" conf="* -> *,!sources,!javadoc"/>
+ <dependency org="org.apache.httpcomponents" name="httpclient" rev="4.0.1" conf="* -> *,!sources,!javadoc"/>
+ <dependency org="com.googlecode.jslint4java" name="jslint4java-ant" rev="1.4.2" conf="* -> *,!sources,!javadoc"/>
</dependencies>
</ivy-module>
diff --git a/lib/js/thrift.js b/lib/js/thrift.js
index 5c1fdf1..b8ca2c0 100644
--- a/lib/js/thrift.js
+++ b/lib/js/thrift.js
@@ -54,13 +54,16 @@
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.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.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 @@
//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 @@
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 @@
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 @@
},
readAll: function() {
- return this.recv_buf;
+ return this.recv_buf;
},
write: function(buf) {
@@ -285,17 +295,17 @@
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 @@
//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 @@
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 @@
},
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 @@
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 @@
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 @@
var k = this.tstack.pop();
if (first) {
first = false;
- }else {
+ } else {
map = ',' + map;
}
@@ -515,8 +528,8 @@
// 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 @@
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 @@
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 @@
//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 @@
//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 @@
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 @@
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 @@
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 @@
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 @@
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();
}