Community
Participate
Working Groups
In the method ImportRewriteAnalyzer.getNewImportString, there is a ';' added to the import string. This is the correct behavior for Java, but we are using this class for Organizing imports in a Groovy script. In Groovy, it is not natural to have a ';' after imports. I would like a way to configure ImportRewriteAnalyzer so that ';' will not be added after imports. This fix would have to be propagated back to ImportRewrite, since that clients can set ';' preferences there. Alternatively, it would be nice to remove the final from ImportRewriteAnalyzer, and make it possible to create a sub-class that does not add the ';'. Then ImportRewrite would have to be made configurable to accept this custom ImportRewriteAnalyzer. I understand that neither of these classes are API, but we require their use from the Groovy-Eclipse plugin. The alternative from our standpoint is to make copies of both of those classes, but this is not something that we would like to do.
The ImportRewriteAnalyzer shouldn't bee hard to fix. The problem is that ImportRewrite is API, and we shouldn't add APIs that wouldn't make sense for Java. Can you call a package-visible method on ImportRewrite, e.g. setAppendSemicolon(boolean)? If not, I guess we'd have to make ImportRewrite non-final and add @noextend and a protected method.
Well, I could, but only through reflection. From my point of view, this would be sufficient. An alternative, and maybe cleaner option would be to add something to the ImportRewriteContext. Could there be some way that clients could subclass ImportRewriteContext with a way to optionally override a method in it called: public boolean doAppendSemiColon() { return true; } Currently, the ImportRewriteContext is set in the ImportRewrite's constructor, but could there be a way of overriding that?
ImportRewriteContext is not a good hook for this. The context is used in the add*Import(..) methods and it carries the logic that determines whether a given name is visible at a certain location in the code. But if I understood your use case correctly, you want to override this behavior for all Groovy compilation units. If that's the case, then I think it would be better to leave ImportRewrite as it is but change ImportRewriteAnalyzer.getNewImportString(..) so that the decision is based on the concrete kind of ICompilationUnit you use. E.g. using a global (internal) JDTCoreTweaks class that defaults to Java behavior but where language extenders can register different behavior for their own kind of ICompilationUnit.
The JDTCoreTweaks class would be something worth considering, but it would have to operate on content type/file extension. Would this be more appropriate for the e4 branch? Or perhaps I'm not understanding exactly what you're describing. I am wondering if there's something that can be done in the 3.7 timeframe.
Personally I don't like it too much for 3.7 unless you provide a patch that implements Markus's suggestion in comment 3.
That sounds reasonable to me. I'll do what I can to submit a patch. I'm guessing that in the JDTCoreTweaks (not sure that I like that name) class that Markus mentions, it would be best to avoid instanceof tests. To sketch out an idea: In this particular case then, perhaps it could make sense to delegate to the ICompilationUnit instance and create a method call "requiresSemicolons()". This returns true for jdt.internal.core.CompilationUnit, but would return false for Groovy's implementation. (So, in this particular scenario, a JDTCoreTweaks is not necessary.)
(In reply to comment #6) > ... delegate to the > ICompilationUnit instance and create a method call "requiresSemicolons()". > This returns true for jdt.internal.core.CompilationUnit, but would return false > for Groovy's implementation. Nope, that would again pollute a JDT API with something that's not relevant for the Java language.
(In reply to comment #7) > Nope, that would again pollute a JDT API with something that's not relevant for the Java language. I concur with this. And I also believe the JDTCoreTweaks (not sure I like that name either) class is needed to future extensions.
JDTCoreTweaks is just a placeholder name I came up with. In Platform/UI, there's a org.eclipse.ui.internalTweaklets extension point, and org.eclipse.ui.internal.tweaklets.Tweaklets is used to allow specific tweaklets to modify the default behavior (everything internal and subject to change).
I'll work with Boris to provision the current HEAD contents of JDT/Core in an e4 incubation project so that this kind of issues can be implemented there through experiments.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
