This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
There has been some changes in jmi interfaces. Revert them. http://www.netbeans.org/servlets/ReadMsg?msgId=870009&listName=api-changes
The changes were not accidental. We are working on stabilization of the API, which involes some changes (the JMI API is marked "under development" - I thought that this status means that it is legal to change it). This particular change we made to make static imports better supported by the API (in case of static import there can be several imported elements), and fixed a bug when JavaClass did not extent Statement (it should as it can be declared as part of a statement block - local named class). With the change I also changed all clients. I would be happy if we could get an exception for this change (since the diff is quite complex) and do the fast track review (if necessary) without having to revert the changes now and having to put them back again later.
Our goal is to ensure that the change is the right one and that it is well justified, documented and sustainable. It is legal to do changes, however we want to ensure that the above conditions are satisfied and that is why you should follow at least the fast track review. We agreed to grant the right to not rollback until the review is over. Just turn this issue into review request soon. Make sure that you have updated the documentation - apichanges and usecases. Make sure that you have boost version number of the module and changed everyone's dependecy on it. Right now it seems to me that for example apichanges are completely missing for the module.
OK, thanks.
Note: making EnumClass (indirectly) extend Statement is odd, since this does not compile: public class M { public static void main(String[] args) { class C {} enum E {red, green, blue} // ERROR E e = E.red; } } error: "enum types must not be local"
I completed the tasks suggested by Jarda (created apichanges.xml, increased specification version, etc.). So, let me change this to a TASK requesting a fasttrack review of the API change. More detailed explanation of the change can be found in the apichanges.xml.
Similarly, it is incorrect for AnnotationType to extend Statement: public class M { public static void main(String[] args) { @interface A {} // ERROR @A int x = 1; } } error: "modifier interface not allowed here" and same for interfaces (~ JavaClass[@interface=true]): public class M { public static void main(String[] args) { interface A {} // ERROR A a = null; } } error: "modifier interface not allowed here" You might consider redesigning the javamodel hierarchy relating to JavaClass, AnnotationType, and JavaEnum, and add Interface separate from JavaClass. The hierarchy as of promo-D already looks strange, and this new change seems to make it worse.
My understanding of task is that it may or may not be finished. That is not appropriate here. This either has to pass the review or be rolled back. I believe defect with highest priority is much more appropriate. Please change it back.
Changing back to defect. Assigning to apireviews@netbeans.org. Please reassign back as soon as you come to a conclusion.
Jesse is right that the metamodel currently does not fully enforce syntax correctness. Do you consider this being a huge problem - i.e. are you suggesting rolling back the change? Note that this particular change (making JavaClass subclass of Statement) was a metamodel bugfix that legalized something that the implementation allowed already (to support local classes). AFAIK, Jesse's comment does not say that making JavaClass extend Statement is a wrong thing. However, the consequences of this change and the fact that JavaEnum and AnnotationType extend JavaClass are weird. A solution would be to refactor the metamodel to make JavaClass, JavaEnum and AnnotationType independent (in terms of inheritance). However that is a potentially huge change introducing some other problems (should we have UnresolvedAnnotationType and UnresolvedEnum besides UnresolvedClass? etc.) Anyway, I want to give such a refactoring a try in the future (perhaps during Christmass vacation if I will have time) and see whether it would be possible to make the model more clean that way. Then the issues raised by Jesse can be fixed. But I would not connect these issues to the changes this Issuezilla report describes.
I agree with Jesse. The fix is not correct. In the best case it's a hot fix. Class certainly is *not* a Statement. "Jesse is right that the metamodel currently does not fully enforce syntax correctness." then this is a huge problem. If the model doesn't enforce syntax correctness then what does it try to do? "Do you consider this being a huge problem - i.e. are you suggesting rolling back the change?" I do and kindly request the team to come back with the (to your best knowledgte) complete list of issues you know about the javamodel API problems and suggested fixes. Until we're highly confident that the model is right, there is no point to migrate modules from org.openide.src to the new API. I don't think we have that level of confidence now
"Until we're highly confident that the model is right, there is no point to migrate modules from org.openide.src to the new API." -- It is possible when we will be able to invalidate our announcement about support of JDK 1.5. Depends what is more important for us at this moment.
Well I would like to see an analysis of the areas in which the metamodel deviates from the actual grammar permitted of Java source - not obvious how much this matters in practice. I don't suppose anyone is going around trying to actually insert enum declarations into the middle of methods, so it is pretty theoretical. To Trung: making JavaClass extend Statement seems natural enough, since it is allowed where statements are; the problem is only that enums/interfaces/annotations are not permitted here. Or perhaps local classes deserve a separate type (using ClassDefinition) - after all, they cannot have access modifiers or be static. Anyway, clearly the real and serious bugfix mentioned (proper support for local classes e.g. in Fix Imports) takes immediate priority over any stylistic considerations about the model, so I would not recommend rolling the change back without a much better reason. (IMHO)
Comments to Trung: "then this is a huge problem. If the model doesn't enforce syntax correctness then what does it try to do?" It tries to model Java source code to allow better access to it for clients. Provides a way of inspecting/generating/modifying source code in a way that is more convenient (for some use-cases, such as refactorings or customizers, etc.) than the pure text manipulation. Could you please explain why a failure to strictly enforce syntactical correctness by the model is a *huge* problem? "Class certainly is *not* a Statement." Model is an aproximation of the reality. In our model class is a special kind of statement, since it can stand as a standalone entity in a statement block. We can have better granularity and model Java using 100 more model classes than we do currently to have it super pure and exact. But currently we found this aproximation being more practical. Martin: "Do you consider this being a huge problem - i.e. are you suggesting rolling back the change?" Trung: "I do and kindly request the team to come back..." Do you mean you do suggest doing a rollback? If so, could you explain what would be the advantages of doing that at this point?
I'd like to remind that there are two separate issues being discussed here which are connected only by the fact that they were integrated at the same day. I think that the static import fix is well accepted and nobody expressed any doubts that it is the proper fix. I just want to add that this is an incompatible change and imho the major version of the module should be increased (once per release). Either increase it immediatelly or create IZ bug to not forget to do it. Thanks. [imho this can close this part of review]. The "Class extends Statement" issue seems to get much more attention. I am able to accept the fact that the model just approximates the reality. If this change makes something possible which was impossible before, it imho may be justified enough. However I see other people not sharing this view and if the change was properly reviewed before integration, the integration would not happen due to strong objection by others. That is imho sufficient reason for rollbacking it. Btw. it does not make sence to measure "advantages at this point", we have to measure "advantages at the point before integration" - e.g. rollback == status quo. Jesse, Trungu or anyone other, if you want the "Class extends Statement" change to be rollbacked, say so in one of your comments and change the keyword to API_REVIEW. The team should then revert the change and come with a proper description of its future plan addressing your requests for "complete list of issues you know about the javamodel API problems and suggested fixes" and "analysis of the areas in which the metamodel deviates from the actual grammar permitted of Java source", etc.
"analysis of the areas in which the metamodel deviates from the actual grammar permitted of Java source" We will need to know the priority of this task and which other 5-day task we can remove from our current plan for 4.1 to do this. Also, when you are talking about the actual grammar, what grammar do you mean exactly? I am not aware of a declaratively described grammar of Java that would detect all syntax errors (e.g. the fact that an annonymous class cannot have a constructor, or that a method with "native" modifier cannot have a body is most likely - even by javac - detected later rather than during the syntactical analysis, although these are clearly syntactical rules).
Can I close this issue?
No response -> closing.
No response after closed -> verified
Reorganization of java component