Bugzilla – Full Text Bug Listing
| Summary: | [change method signature] leaves unwanted white space between empty parenthesis | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Rüdiger Herrmann <ruediger.herrmann> | ||||
| Component: | Core | Assignee: | Manoj N Palat <manoj.palat> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | jarthana, markus.kell.r, Olivier_Thomann | ||||
| Version: | 3.7 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | All | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
|
Description
Rüdiger Herrmann
This is a problem in the ASTRewrite. We just call ListRewrite#remove(..) to remove the method parameter. Created attachment 183844 [details]
example project
Project to reproduce.
Code formatter is default Eclipse profile with White Space > Declarations > Methods > 'after opening parenthesis' and 'before closing parenthesis' checked.
Note: If these options are not checked, then it's acceptable for the ASTRewrite to not remove all spaces (since the original code already was formatted correctly).
Released regression test: org.eclipse.jdt.core.tests.rewrite.describing.ASTRewritingMethodDeclTest#_testListRemoves3 Right now the formatting preferences are not used on remove. Ayushman, if you get some time, please take a look. Another case that formatter has to cover (from bug 331412)- * create a method with no arguments like so: void foo() * use the Change Method Signature refactoring to add an argument * the resulting method is: void foo(Object arg) -> the refactoring does not insert white space characters after opening and before closing parenthesis. Expected outcome would be: void foo( Object arg ) The formatter settings are set to have white space after opening parenthesis and before closing parenthesis. *** Bug 331412 has been marked as a duplicate of this bug. *** This bug is really annoying. Maybe someone with JDT insight could help me fix this? I managed to write a failing test case but now I'm not sure how to proceed. In ASTRewriteAnalyzer$ListRewriter#rewriteList line 692 the 'end' variable is assigned, but by one too short in this case. Would 'getStartOfNextNode()' be the right place to handle the special-case 'empty list'? Or should I leave the ASTRewrite as it is and re-format the affected text region when the rewrite was applied? In which case I would welcone hints where and how this should happen? (In reply to Rüdiger Herrmann from comment #7) > I managed to write a failing test case but now I'm not sure how to proceed. We already have a test, see comment 3. > In ASTRewriteAnalyzer$ListRewriter#rewriteList line 692 the 'end' variable > is assigned, but by one too short in this case. Would 'getStartOfNextNode()' > be the right place to handle the special-case 'empty list'? The fix needs to be somewhere in ASTRewriteAnalyzer#visit(MethodDeclaration) or a method called from there. We wouldn't accept a solution that uses the formatter after the rewrite is done. I can't give you a definitive answer without investigating and fixing the whole problem, but I think rewriteNodeList and rewriteList don't have enough context to know if the whitespace needs to be removed. The fix will probably go into visit(MethodDeclaration) itself. Since this is a very specific formatter option, you can maybe just read the DefaultCodeFormatterConstants option and then do what's necessary. Rüdiger, Manoj will help you with review and releasing. But I am not sure how much time he will have, though. (In reply to Jayaprakash Arthanareeswaran from comment #9) > Rüdiger, Manoj will help you with review and releasing. But I am not sure > how much time he will have, though. Thanks for your support. I report here as soon as I have a patch ready. However, it may take a while since I am working on this in my spare time. 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. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. 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. |