Community
Participate
Working Groups
Build Identifier: 0.7.0.v201111012101 1) Create an RUI handler. 2) In anywhere of the handler, input below statements - do not use content assistant. ff FormField; 3) Press the ctrl + shift + O to organize import, you'll find that the FormField in the system package is imported. The expected result is to open a dialog and let user to select which FormField to import. Reproducible: Always
I ran a test in Java to see what happens if I define a class named String and then tried to declare a field of type String in another package within the same project. JP1 - packA String.java - packB TestClass.java private String field1; JDT does not provide a dialog when organize imports is invoked in TestClass, and String automatically defaults to the one in the Java Runtime, without adding any import declarations. If we use JDT as our guide, then we should not be prompting the user for which part to use, and was should also not be adding an import declaration for FormField in the MOF model.
Hi Rocky, Organize import will import those type which have error(Because those type are not import into current egl part). But system part can be reached by every customer part, so organize import will ignore system part. And If we check every type in customer part to check if each part is customer's want, it may cause some performance issue(We need to search it every time for every imported type). So I think this defect won't be fixed. It works as design. Xiaobin
Agree, close the defect.
In comment 2 it states that organize imports will ignore system parts, but in comment one it states that organize imports will add imports for system parts. Is content assist adding imports for the FormField in the system package, or is it not adding any imports in this situation?
(In reply to comment #4) > In comment 2 it states that organize imports will ignore system parts, but in > comment one it states that organize imports will add imports for system parts. > > Is content assist adding imports for the FormField in the system package, or is > it not adding any imports in this situation? I think I have mislead you, for "ignore" system parts. The "ignore" means, if you used a part, which defined in customer's projects. But there is a system part with the same name. And you did not use "import client.xxx.partname" in the head of the main handler. In this case, the compiler will regard the part as the system's part, and so the TypeBinding will be valid, so it will ignore it. For the content assist side. It's seems like we did not put the formfield into system package, So content assist does not know anything about the org.eclipse.edt.mof.egl.FormField; I have retrieved the org.eclipse.edt.compiler/eglsource package, and I did not see the formfield there. PAUL, Does EDT put all of system part into edtCompiler.eglar? And I remember we have a workitem to decide which system part could not be imported, will we support this in 0.7? Tony, added you to this discussion, seems like you owned which system part could not be imported.
I am reopening this defect since it is still under discussion. In my opinion, we should not be adding imports for 'system parts' when organize imports is invoked. In the case where a user defined part collides with a 'system part' (i.e. FormField), organize imports should not prompt the user, and should not add any imports. This means that the user will need to either add their own import statement or fully qualify the part name. I believe I have seen organize imports add import statements for system parts in various situations, including FormField, so I would say that this defect should be used to track the general solution of not adding imports for system parts. It is possible that we do not have the necessary information in the model for this in .7, in which case we can defer this until 1.0. We should not hard code anything in the tools regarding the definition of a system part, and should let the definition of the compiler/generator indicate what namespaces are considered system parts. Paul, should we consider renaming FormField in the core model to avoid this collision with the MVC framework? I realize that this could happen with any other name as well, but maybe there is a better name for the core model part in this particular case.
I have added code to SystemEnvironment so that the mof model parts (part in the the package org.eclipse.edt.mof.*) are no longer added to the map of parts that can be used without package qualification. This will eliminate the problem of organize imports adding the import for the mof FormField. This should suffice for 0.7. In the future we can determine the exact list of system packages that are implicitly imported. I am leaving this open, in case we want to make the change that Brian suggests (do not add import for system parts).
Change this "defect" to enhancement and change the target milestone to future. To track the enhancement of the exact list of system packages that are implicitly imported.
Change the title to describe the problem clearly.
Created attachment 211385 [details] Patch for this enhancement Two files was change for this enhancement. OrganizeImportsOperation.java OrganizeImportsVisitor.java In OrganizeImportsVisitor, update the method to judge one method is system part. In OrganizeImport file, comment one statement which try to add system part for unresolved type list. After the patch, Organize import will not try to import system part if one type was not resolved.
Changes seem reasonable. If we ever need to find out if a part is a system part when in SDK mode, we will need to come up with a different mechanism, since there is only 1 IR environment in SDK. One way to do this would be to add a method in SystemEnvironment: public boolean isSystemPart(String[] pkg, String partName) { ... }
If we are no longer using code, please remove it from the .java file and do not just comment it out.
Paul, Since we already have a function in IRUtils , so I did not add isSystemPart in SystemEnviroment. I have delete the code line which commented before. Thanks.
Verified in 0.8.0.v201202272101.
