Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4727

javascript compiler generates struct code with duplicate `case 0` statements

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 0.7, 0.8, 0.9, 0.10.0, 0.11.0
    • None
    • JavaScript - Compiler
    • None

    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.

       

      Attachments

        Activity

          People

            Unassigned Unassigned
            seganw Xiao Wang
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated: