[ 
https://issues.apache.org/jira/browse/THRIFT-1835?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13560998#comment-13560998
 ] 

Nicolas Trésegnie commented on THRIFT-1835:
-------------------------------------------

Thanks for the patch and the test case.

I have some questions:

1) Is it appropriate to put these (5) different possibilities of failure in the 
same test case? The compiler will throw an error when the struct will be parsed 
so the other cases will never be tested. I don't know how the unit tests are 
managed in thrift so it could be a stupid question.

2) I tried to test separately the different problematic cases. (In the 
following piece of code, only the fifth case is active).

{code}
// testcase for THRIFT-1835 Compiler should fail when duplicate field names are 
present 

struct duplicate1 { 
  1: i32 field, 
  2: i32 field2 
} 
 
union duplicate2 { 
  1: i32 field, 
  2: i32 field2
} 
 
exception duplicate3 { 
  1: i32 field, 
  2: i32 field3 
} 
 
service thrift1835_1 {
  duplicate1 testcase( 1: i32 field, 2: i32 field2)
}

service thrift1835_2 {
  duplicate1 testcase() throws ( 1: duplicate3 dx, 2: duplicate3 dx)
} 

// eof
{code}
Here is the output i got with each case:
{code}
// Struct
[FAILURE:.../Test Thrift-1835 duplicate fields.thrift:8] Duplicate field field 
in duplicate1
// Union
[FAILURE:.../Test Thrift-1835 duplicate fields.thrift:13] Duplicate field field 
in duplicate2
// Exception
[FAILURE:.../Test Thrift-1835 duplicate fields.thrift:18] Duplicate field field 
in duplicate2
// Method arguments
[FAILURE:.../Test Thrift-1835 duplicate fields.thrift:20] Duplicate field field 
in testcase_args
// Exception arguments
Error: Duplicate field dx in testcase_result
{code}
* The last error message is in a different format, is it normal?
* The line referenced in the error message is not the line where the duplicated 
field appears. Would it be possible to use the right line number?
* Would it be possible to include the type of data structure containing the 
duplicated field in the error message?
                
> Compiler should fail when duplicate field names are present
> -----------------------------------------------------------
>
>                 Key: THRIFT-1835
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1835
>             Project: Thrift
>          Issue Type: Bug
>          Components: Compiler (General)
>    Affects Versions: 0.9
>            Reporter: Nicolas Trésegnie
>         Attachments: Test Thrift-1835 duplicate fields.thrift, 
> THRIFT-1835_Compiler_should_fail_when_duplicate_field_names_are_present.patch
>
>
> Thrift accept structs with a duplicate field name if the IDs are different. 
> However, the generated java code does not compile.
> {code}
> struct duplicate {
>     1: i32 field,
>     2: i32 field
> }
> {code}
> {code}
> private static final org.apache.thrift.protocol.TField FIELD_FIELD_DESC = ...
> private static final org.apache.thrift.protocol.TField FIELD_FIELD_DESC = ...
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to