[THRIFT-4727] javascript compiler generates struct code with duplicate `case 0` statements

• Type: Bug
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: 0.7, 0.8, 0.9, 0.10.0, 0.11.0
• Component/s: JavaScript - Compiler
• Assignee: Unassigned
• Reporter: Xiao Wang
• Created: 07/Jan/19 09:30
• Updated: 07/Jan/19 09:33

Description

Reproducible steps:

Javascript compiler would always generate duplicate `case 0` codes;

1. Below is a demo IDL file:

// test.thrift
struct Obj {
   1: string name;
}

service Test {
   Obj test()
}

2. Generate code using

thrift -r --gen js:node test.thrift

3. The related problematic code is like below:

// part of Test.js
......
Test_test_result.prototype.read = function(input) {
  input.readStructBegin();
  while (true)
  {
    var ret = input.readFieldBegin();
    var fname = ret.fname;
    var ftype = ret.ftype;
    var fid = ret.fid;
    if (ftype == Thrift.Type.STOP) {
      break;
    }
    switch (fid)
    {
      case 0:
      if (ftype == Thrift.Type.STRUCT) {
        this.success = new ttypes.Obj();
        this.success.read(input);
      } else {
        input.skip(ftype);
      }
      break;
      case 0:
        input.skip(ftype);
        break;
      default:
        input.skip(ftype);
    }
    input.readFieldEnd();
  }
  input.readStructEnd();
  return;
};
......

Root cause:

The code makes this bug was imported due to bugfix THRIFT-1089.

Commit address: https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4

Potential fix:

I think remove below code should fix this problem:

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

This code seems to satisfy jslint about 8 years ago, but now this also fails the static code scan both in jslint and sonarqube default profile.

I could propose a PR if it's OK to fix it this way.