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();
         }