Community
Participate
Working Groups
Build ID: I20090803-1800 Refer : bug 279715 Steps To Reproduce: Testcase : 1.public class Try { // public static final int B= 12; // public static final int C= B - 1; //inline C // public static final int K= 99; public static void main(String[] args) { int B= 12; int C= B - 1; //inline C int K= 99; int f1= K - 1 - C; int f2= K - C - C - C; int x1= K + C; int x2= K - C; int x3= K + 1 - C; int x4= K - 1 + C; int x5= K + 1 + C - C - C; } } Select C and invoke inline... 2. I see the ITE for both local variable and constant inlining : java.lang.reflect.InvocationTargetException at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:421) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2.run(RefactoringWizardDialog2.java:330) at org.eclipse.ltk.ui.refactoring.RefactoringWizard.createChange(RefactoringWizard.java:583) at org.eclipse.ltk.ui.refactoring.RefactoringWizard.computeUserInputSuccessorPage(RefactoringWizard.java:422) at org.eclipse.ltk.ui.refactoring.UserInputWizardPage.computeSuccessorPage(UserInputWizardPage.java:74) at org.eclipse.ltk.ui.refactoring.UserInputWizardPage.getNextPage(UserInputWizardPage.java:114) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2.nextOrPreviewPressed(RefactoringWizardDialog2.java:495) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2.access$2(RefactoringWizardDialog2.java:492) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2$1.widgetSelected(RefactoringWizardDialog2.java:691) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:228) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1002) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3880) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3473) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation$1.run(RefactoringWizardOpenOperation.java:143) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation.run(RefactoringWizardOpenOperation.java:155) at org.eclipse.jdt.internal.ui.refactoring.actions.RefactoringStarter.activate(RefactoringStarter.java:38) at org.eclipse.jdt.internal.corext.refactoring.RefactoringExecutionStarter.startInlineConstantRefactoring(RefactoringExecutionStarter.java:321) at org.eclipse.jdt.internal.ui.refactoring.actions.InlineConstantAction.tryInlineConstant(InlineConstantAction.java:149) at org.eclipse.jdt.ui.actions.InlineAction.run(InlineAction.java:118) at org.eclipse.jdt.ui.actions.SelectionDispatchAction.dispatchRun(SelectionDispatchAction.java:278) at org.eclipse.jdt.ui.actions.SelectionDispatchAction.run(SelectionDispatchAction.java:250) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584) at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501) at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1002) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3880) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3473) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2405) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2369) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2221) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:500) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:493) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:559) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:514) at org.eclipse.equinox.launcher.Main.run(Main.java:1311) Caused by: java.lang.IllegalArgumentException at org.eclipse.jdt.core.formatter.IndentManipulation.changeIndent(IndentManipulation.java:270) at org.eclipse.jdt.internal.corext.util.Strings.changeIndent(Strings.java:487) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.prepareInitializerForLocation(InlineConstantRefactoring.java:555) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.inlineReference(InlineConstantRefactoring.java:486) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.getChange(InlineConstantRefactoring.java:472) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring.checkFinalConditions(InlineConstantRefactoring.java:810) at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1800) at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121) Root exception: java.lang.IllegalArgumentException at org.eclipse.jdt.core.formatter.IndentManipulation.changeIndent(IndentManipulation.java:270) at org.eclipse.jdt.internal.corext.util.Strings.changeIndent(Strings.java:487) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.prepareInitializerForLocation(InlineConstantRefactoring.java:555) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.inlineReference(InlineConstantRefactoring.java:486) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring$InlineTargetCompilationUnit.getChange(InlineConstantRefactoring.java:472) at org.eclipse.jdt.internal.corext.refactoring.code.InlineConstantRefactoring.checkFinalConditions(InlineConstantRefactoring.java:810) at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1800) at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121) More information:
My bad.. I set the tab size to 0 while testing :(
Still a bug with tab size = 0. Reopening it . None of the refactoring works in this case. See log above for ITE.
Moving to Core. All methods of IndentManipulation should accept 0 for indentWidth, not only the one that was fixed with bug 283133. Another test case with the default formatter profile, tab width set to 0: public class Ind { public static void main(String[] args) { System.out.println("a"); } } Extracting "a" to a local variable gives: Root exception: java.lang.IllegalArgumentException at org.eclipse.jdt.core.formatter.IndentManipulation.extractIndentString(IndentManipulation.java:141) at org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteFormatter.getIndentString(ASTRewriteFormatter.java:206) at org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer.doTextInsert(ASTRewriteAnalyzer.java:1186) at org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer$ListRewriter.rewriteList(ASTRewriteAnalyzer.java:567) at org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer.rewriteParagraphList(ASTRewriteAnalyzer.java:995) ...
(In reply to comment #3) I dont think the method IndentManipulation.trimIndent should accept 0 for indentWidth. It doesnt really make sense to "trim" an indent which is already 0. Same is with IndentManipulation.changeIndent which also calls trimIndent.
Look at this API from a client perspective. Clients typically read the indentWidth from the preferences and then pass it to these methods. It doesn't make sense to put extra code for indentWidth==0 at every call site. That's too much code duplication and error prone (too easy to forget).
Srikanth, I'll take care of this one while you are working on the parser.
(In reply to comment #6) > Srikanth, I'll take care of this one while you are working on the parser. Olivier, I had assigned this to myself while waiting for Ayush to create a bugzilla account and subsequently, didn't get around to reassign it to him. He has spent some cycles on this and is about to propose a patch. Perhaps he can be the assignee and you can be the reviewer/committer for this ?
Created attachment 144009 [details] rg.eclipse.jdt.core.formatter.IndentManipulation now handles the cases of indentWidth and tabWidth set to zero. Inlining, extracting local variable and formatting now work with value 0. In the above patch, methods in org.eclipse.jdt.core.formatter.IndentManipulation have been modified to handle indentWidth = 0 and tabWidth = 0. A new method calculateSpaceEquivalents(int,int) has been created out of 3 lines of code that were repeated in 3 methods - all of which failed to handle the case where tabWidth = 0, leading to a divide by zero exception(Any suggestions for an alternative name for this helper method?). With this patch, inline, extract local variable and formatting all work when indent width and tab width are zero.
(In reply to comment #7) > Perhaps he can be the assignee and you can be the reviewer/committer > for this ? Sure. Ayushman, please request a review.
Created attachment 144223 [details] Modified version of previous attachment Fixed a few problems with the earlier patch. Added two new testcases- testBug285565d and testBug285565e. I'm not too sure if the behaviour of IndentManipulation.extractIndentString() is correct for indentWidth = 0. Olivier can you please check it out?
Do we really need to add this check ? * <li>the given <code>indentUnitsToRemove</code> is greater than zero when indentWidth equals zero</li> inside the getChangeIndentEdits method. Otherwise it looks good.
Created attachment 144259 [details] Same patch with some formatting
Created attachment 144357 [details] Patch with one exception throwing condition removed Thanks for pointing it out Olivier. I'd initially put that condition because it seemed logical that indentUnitsToRemove should not be greater than zero when the indentWidth is already zero. But the rest of the code is robust enough to handle such a case. So removing the condition. No other modification from the previous patch.
Markus, if the last patch is matching what you expect, I think we can release it.
Issues/comments? The thread's been dormant since quite some time now.
I'll go forward and release the last patch today.
Last patch looks good to me. I didn't check the implementation in detail, but the API is fine and it worked fine for all examples I tried.
Released for 3.6M2. Regression tests added in: org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#testBug285565a org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#testBug285565b org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#testBug285565c org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#testBug285565d org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#testBug285565e
Verified for 3.6M2 using I20090914-1800
Verified for 3.6M2 using I20090914-1800 build
(In reply to comment #20) Ooops.. Put the message on the wrong bug id.. Anyways this was already verified!
