[ https://issues.apache.org/jira/browse/THRIFT-4727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Xiao Wang updated THRIFT-4727: ------------------------------ Description: *Reproducible steps:* Javascript compiler would always generate duplicate `case 0` codes; 1. Below is a demo IDL file: {code:java} // test.thrift struct Obj { 1: string name; } service Test { Obj test() } {code} 2. Generate code using {code:java} thrift -r --gen js:node test.thrift{code} 3. The related problematic code is like below: {code:java} // 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; }; ......{code} *_Root cause:_* The code makes this bug was imported due to bugfix THRIFT-1089. Commit address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4|https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.] *_Potential fix:_* I think remove below code should fix this problem: {code:java} 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; } {code} 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. was: *Reproducible steps:* Javascript compiler would always generate duplicate `case 0` codes; 1. Below is a demo IDL file: {code:java} // test.thrift struct Obj { 1: string name; } service Test { Obj test() } {code} 2. Generate code using {code:java} thrift -r --gen js:node test.thrift{code} 3. The related problematic code is like below: {code:java} // 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; }; ......{code} *_Root cause:_* The code makes this bug was imported due to bugfix [THRIFT-1089|https://issues.apache.org/jira/browse/THRIFT-1089]. Commit address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.] *_Potential fix:_* I think remove below code should fix this problem: {code:java} 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; } {code} 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. > javascript compiler generates struct code with duplicate `case 0` statements > ----------------------------------------------------------------------------- > > Key: THRIFT-4727 > URL: https://issues.apache.org/jira/browse/THRIFT-4727 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Compiler > Affects Versions: 0.7, 0.8, 0.9, 0.10.0, 0.11.0 > Reporter: Xiao Wang > Priority: Major > > *Reproducible steps:* > Javascript compiler would always generate duplicate `case 0` codes; > 1. Below is a demo IDL file: > > {code:java} > // test.thrift > struct Obj { > 1: string name; > } > service Test { > Obj test() > } > {code} > 2. Generate code using > {code:java} > thrift -r --gen js:node test.thrift{code} > 3. The related problematic code is like below: > > > {code:java} > // 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; > }; > ......{code} > *_Root cause:_* > The code makes this bug was imported due to bugfix THRIFT-1089. > Commit > address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4|https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.] > *_Potential fix:_* > I think remove below code should fix this problem: > {code:java} > 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; > } > {code} > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)