Community
Participate
Working Groups
If been wondering for some years :) why the TypeConverters don't set enclosingType on member type declarations. Only today I found an additional inconsistency in this area, but I should say that I haven't yet found a test case where this actually matters. I started to look into this because I found the reconciler reporting bogus errors, which actually never show up at the UI. For Object Teams I would like to rely on the enclosingType field to always be set for member types. Does that seem to be a sound assumption?
Created attachment 175173 [details] proposed patch This patch adds a quite obvious line to both SourceTypeConverter and BinaryTypeConverter: typeDeclaration.memberTypes[i].enclosingType = typeDeclaration; If this was omitted on purpose, I'd love to learn why. The other inconsistency I was seeing was actually false alarm: For a while I was thinking that also SourceElementNotifier should be changed to pass the current type as declaringType into notifySourceElementRequestor(TypeDeclaration,boolean,TypeDeclaration,ImportReference) (Call in line 678). However, on debugging this I see the difference between the declaringType of a local type and the enclosingType of a member type. Here I simply suggest to rename the parameter to avoid confusion. No big deal.
Jay, please take it forward. TIA.
Patch looks good. I will release later today. About the declaringType parameter, I don't seem to run into a case where a non-null is passed for declaringType. I mean, if the declaringType is a declaring type of a local type, won't it be covered as field's declaring type? Besides for consistency case (we use enclosing type for outer/inner types) we might want to leave it as is but with a clarifying documentation, of course.
Jay, I saw you already committed the patch - not good, because - every source file that gets changes from someone that is not covered by the company mentioned in the copyright notice must be added like this: name <e-mail> - bugzilla summary - bugzilla URL e.g. Dani Megert <dani@eclipse.org> - My bug summary - https://bugs.eclipse.org... - the iplog+ flag must be set on the patch in the this bug
(In reply to comment #4) > Jay, I saw you already committed the patch - not good, because > > - every source file that gets changes from someone that is not covered by > the company mentioned in the copyright notice must be added like this: > name <e-mail> - bugzilla summary - bugzilla URL > e.g. > Dani Megert <dani@eclipse.org> - My bug summary - https://bugs.eclipse.org... > > - the iplog+ flag must be set on the patch in the this bug Sorry, I thought the 'Others' in the copyright should cover this. Is it too late to add to the copyright and set the iplog+ now?
>Sorry, I thought the 'Others' in the copyright should cover this. http://www.eclipse.org/legal/copyrightandlicensenotice.php >Is it too >late to add to the copyright and set the iplog+ now? No, just add the line and update the flag.
Released in HEAD for 3.7 M1. Verification to be done by code inspection.
Verified for 3.7M1 using build I20100802-1800 looking at the source code
