Skip to content

Commit d83432d

Browse files
jjohnstnjjohnstn
authored
Fix adding blocks to not duplicate comment in else (eclipse-jdt#2058)
* Fix adding blocks to not duplicate comment in else - refactor ControlStatementsCleanUp and ControlStatementsFix to jdt.core.manipulation - fix ControlStatementsFix to not look from the if statement start when looking for comments to add after block start - add new test to CleanUptest - fixes eclipse-jdt#2057 * Fix CleanUpStressTest
1 parent 4369701 commit d83432d

File tree

4 files changed

+49
-8
lines changed

4 files changed

+49
-8
lines changed

org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/ControlStatementsCleanUp.java renamed to org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/ControlStatementsCleanUp.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2011 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
1010
*
1111
* Contributors:
1212
* IBM Corporation - initial API and implementation
13+
* Red Hat Inc. - refactored to jdt.core.manipulation
1314
*******************************************************************************/
1415
package org.eclipse.jdt.internal.ui.fix;
1516

org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/ControlStatementsFix.java renamed to org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/ControlStatementsFix.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2022 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
1010
*
1111
* Contributors:
1212
* IBM Corporation - initial API and implementation
13+
* Red Hat Inc.- refactored to jdt.core.manipulation
1314
*******************************************************************************/
1415
package org.eclipse.jdt.internal.corext.fix;
1516

@@ -44,7 +45,7 @@
4445
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
4546

4647

47-
public class ControlStatementsFix extends CompilationUnitRewriteOperationsFix {
48+
public class ControlStatementsFix extends CompilationUnitRewriteOperationsFixCore {
4849

4950
private final static class ControlStatementFinder extends GenericVisitor {
5051

@@ -187,6 +188,7 @@ public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalM
187188
String label;
188189
ASTNode expression= null;
189190
int statementType= -1;
191+
int defaultStartPosition= fControlStatement.getStartPosition();
190192
if (fBodyProperty == IfStatement.THEN_STATEMENT_PROPERTY) {
191193
label = FixMessages.CodeStyleFix_ChangeIfToBlock_desription;
192194
expression= ((IfStatement)fControlStatement).getExpression();
@@ -195,7 +197,8 @@ public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalM
195197
}
196198
} else if (fBodyProperty == IfStatement.ELSE_STATEMENT_PROPERTY) {
197199
label = FixMessages.CodeStyleFix_ChangeElseToBlock_description;
198-
expression= ((IfStatement)fControlStatement).getExpression();
200+
Statement thenStatement= ((IfStatement)fControlStatement).getThenStatement();
201+
defaultStartPosition= thenStatement.getStartPosition() + thenStatement.getLength();
199202
} else {
200203
label = FixMessages.CodeStyleFix_ChangeControlToBlock_description;
201204
if (fBodyProperty == WhileStatement.BODY_PROPERTY) {
@@ -218,7 +221,7 @@ public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalM
218221
// If single body statement is on next line, we need to preserve any comments pertaining to the
219222
// control statement (e.g. NLS comment for if expression)
220223
if (controlStatementLine != bodyLine) {
221-
int startPosition= expression == null ? fControlStatement.getStartPosition() : (expression.getStartPosition() + cuRoot.getExtendedLength(expression));
224+
int startPosition= expression == null ? defaultStartPosition : (expression.getStartPosition() + cuRoot.getExtendedLength(expression));
222225
List<Comment> comments= cuRoot.getCommentList();
223226
for (Comment comment : comments) {
224227
int commentLine= cuRoot.getLineNumber(comment.getStartPosition());

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpStressTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2022 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -3109,7 +3109,7 @@ protected TestResult start(final String args[]) throws Exception {
31093109
} else if (args[i].equals("-v")) { //$NON-NLS-1$
31103110
System.err.println("JUnit " + Version.id() //$NON-NLS-1$
31113111
+ " by Kent Beck and Erich Gamma"); //$NON-NLS-1$
3112-
} else { // $NON-NLS-1$
3112+
} else {
31133113
testCase = args[i];
31143114
}
31153115
}

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2024 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -20579,6 +20579,43 @@ void foo(int i) {
2057920579
assertRefactoringResultAsExpected(new ICompilationUnit[] {cu1}, new String[] {expected1}, null);
2058020580
}
2058120581

20582+
@Test
20583+
public void testAddParenthesesIssue2057() throws Exception {
20584+
20585+
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
20586+
String sample= """
20587+
package test1;
20588+
public class E {
20589+
public void foo(String x) {
20590+
if (x.equals("abc")) //$NON-NLS-1$
20591+
System.out.println(x);
20592+
else
20593+
System.out.println("def"); //$NON-NLS-1$
20594+
}
20595+
}
20596+
""";
20597+
ICompilationUnit cu1= pack1.createCompilationUnit("E.java", sample, false, null);
20598+
20599+
enable(CleanUpConstants.CONTROL_STATEMENTS_USE_BLOCKS);
20600+
enable(CleanUpConstants.CONTROL_STATEMENTS_USE_BLOCKS_ALWAYS);
20601+
20602+
sample= """
20603+
package test1;
20604+
public class E {
20605+
public void foo(String x) {
20606+
if (x.equals("abc")) { //$NON-NLS-1$
20607+
System.out.println(x);
20608+
} else {
20609+
System.out.println("def"); //$NON-NLS-1$
20610+
}
20611+
}
20612+
}
20613+
""";
20614+
String expected1= sample;
20615+
20616+
assertRefactoringResultAsExpected(new ICompilationUnit[] {cu1}, new String[] {expected1}, null);
20617+
}
20618+
2058220619
@Test
2058320620
public void testRemoveParentheses01() throws Exception {
2058420621

0 commit comments

Comments
 (0)