From 60214efa55841af5e3dad58d76e7e9bb4156eac6 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Fri, 20 Jan 2012 19:17:27 -0800 Subject: [PATCH] Code streamlining. --- .../ExtractFunctionRefactoringTest.java | 1 - .../ui/refactoring/NodeContainer.java | 65 +++++---- .../extractfunction/ChooserComposite.java | 138 ++++++++---------- .../ExtractFunctionInformation.java | 11 +- .../ExtractFunctionInputPage.java | 9 -- .../ExtractFunctionRefactoring.java | 8 +- .../ExtractedFunctionConstructionHelper.java | 2 +- .../extractfunction/SimilarFinderVisitor.java | 2 +- 8 files changed, 104 insertions(+), 132 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java index d1aeb10eaa7..9e3e46baaa9 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java @@ -62,7 +62,6 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { for (NameInformation nameInfo : refactoringInfo.getParameterCandidates()) { if (returnValue.equals(String.valueOf(nameInfo.getName().getSimpleID()))) { refactoringInfo.setReturnVariable(nameInfo); - nameInfo.setUserSetIsReference(false); break; } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java index feff003ae47..353000961ef 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java @@ -18,6 +18,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import org.eclipse.core.runtime.Assert; import org.eclipse.core.runtime.ILog; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; @@ -66,13 +67,13 @@ public class NodeContainer { private final List references; private List referencesAfterCached; private int lastCachedReferencesHash; - private boolean isOutput; - private boolean isReturnValue; + private boolean mustBeOutput; + private boolean mustBeReturnValue; private boolean isConst; private boolean isWriteAccess; - private boolean userSetIsReference; - private boolean userSetIsReturnValue; + private boolean isOutput; + private boolean isReturnValue; private String userSetName; private int userOrder; @@ -128,8 +129,7 @@ public class NodeContainer { return !getReferencesAfterSelection().isEmpty(); } - public IASTParameterDeclaration getParameterDeclaration(boolean isReference, - INodeFactory nodeFactory) { + public IASTParameterDeclaration getParameterDeclaration(INodeFactory nodeFactory) { IASTDeclarator sourceDeclarator = (IASTDeclarator) getDeclaration().getParent(); IASTDeclSpecifier declSpec= null; @@ -159,7 +159,8 @@ public class NodeContainer { declarator.addPointerOperator(pointerOp.copy(CopyStyle.withLocations)); } - if (isReference && !hasReferenceOperartor(declarator)) { + boolean output = isOutput() && !isReturnValue(); + if (output && !hasReferenceOperator(declarator)) { if (nodeFactory instanceof ICPPNodeFactory) { declarator.addPointerOperator(((ICPPNodeFactory) nodeFactory).newReferenceOperator(false)); } else { @@ -172,7 +173,7 @@ public class NodeContainer { return nodeFactory.newParameterDeclaration(declSpec, declarator); } - public boolean hasReferenceOperartor(IASTDeclarator declarator) { + public boolean hasReferenceOperator(IASTDeclarator declarator) { for (IASTPointerOperator pOp : declarator.getPointerOperators()) { if (pOp instanceof ICPPASTReferenceOperator) { return true; @@ -215,38 +216,40 @@ public class NodeContainer { return name.toString() + (isDeclaredInSelection() ? " (declared inside)" : ""); //$NON-NLS-1$//$NON-NLS-2$ } - public boolean isOutput() { - return isOutput; + public boolean mustBeOutput() { + return mustBeOutput; } - public void setOutput(boolean isOutput) { + public void setMustBeOutput(boolean mustBeOutput) { + this.mustBeOutput = mustBeOutput; + } + + public boolean isOutput() { + return mustBeOutput || isOutput; + } + + public void setIsOutput(boolean isOutput) { + Assert.isTrue(isOutput || !mustBeOutput); this.isOutput = isOutput; } + public boolean mustBeReturnValue() { + return mustBeReturnValue; + } + + public void setMustBeReturnValue(boolean mustBeReturnValue) { + this.mustBeReturnValue = mustBeReturnValue; + } + public boolean isReturnValue() { - return isReturnValue; + return mustBeReturnValue || isReturnValue; } public void setReturnValue(boolean isReturnValue) { + Assert.isTrue(isReturnValue || !mustBeReturnValue); this.isReturnValue = isReturnValue; } - public boolean isUserSetIsReference() { - return userSetIsReference; - } - - public void setUserSetIsReference(boolean userSetIsReference) { - this.userSetIsReference = userSetIsReference; - } - - public boolean isUserSetIsReturnValue() { - return userSetIsReturnValue; - } - - public void setUserSetIsReturnValue(boolean userSetIsReturnValue) { - this.userSetIsReturnValue = userSetIsReturnValue; - } - public String getUserSetName() { return userSetName; } @@ -360,7 +363,7 @@ public class NodeContainer { if (declarations.add(nameInfo.getDeclaration())) { if (nameInfo.isDeclaredInSelection()) { if (nameInfo.isReferencedAfterSelection()) { - nameInfo.setReturnValue(true); + nameInfo.setMustBeReturnValue(true); interfaceNames.add(nameInfo); } } else { @@ -374,7 +377,7 @@ public class NodeContainer { } } if (nameInfo.isWriteAccess() && nameInfo.isReferencedAfterSelection()) { - nameInfo.setOutput(true); + nameInfo.setMustBeOutput(true); } interfaceNames.add(nameInfo); } @@ -389,7 +392,7 @@ public class NodeContainer { List selectedNames = null; for (NameInformation nameInfo : getInterfaceNames()) { - if (nameInfo.isReturnValue() == isReturnValue) { + if (nameInfo.mustBeReturnValue() == isReturnValue) { if (selectedNames == null) { selectedNames = new ArrayList(); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java index 81cfb2db0d3..69d9d694a38 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java @@ -12,7 +12,6 @@ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; import java.util.ArrayList; -import java.util.List; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.TableEditor; @@ -37,16 +36,12 @@ public class ChooserComposite extends Composite { private static final String COLUMN_NAME = Messages.ChooserComposite_Name; private static final String COLUMN_TYPE = Messages.ChooserComposite_Type; - private Button voidReturn; - - private final ExtractFunctionInputPage page; + private Button checkboxVoidReturn; public ChooserComposite(Composite parent, final ExtractFunctionInformation info, ExtractFunctionInputPage page) { super(parent, SWT.NONE); - this.page = page; - GridLayout layout = new GridLayout(); setLayout(layout); @@ -85,116 +80,115 @@ public class ChooserComposite extends Composite { // Button editor = new TableEditor(table); - final Button referenceButton = new Button(table, SWT.CHECK); - if (name.hasReferenceOperartor((IASTDeclarator) name.getDeclaration().getParent())) { - referenceButton.setSelection(true); - referenceButton.setEnabled(false); + final Button buttonOutput = new Button(table, SWT.CHECK); + if (name.hasReferenceOperator((IASTDeclarator) name.getDeclaration().getParent())) { + buttonOutput.setSelection(true); + buttonOutput.setEnabled(false); } else { - referenceButton.setSelection(name.isOutput()); + buttonOutput.setSelection(name.isOutput() && !name.isReturnValue()); + buttonOutput.setEnabled(!name.mustBeOutput() && !name.isReturnValue()); } - referenceButton.setBackground(table.getBackground()); - referenceButton.addSelectionListener(new SelectionListener() { + buttonOutput.setBackground(table.getBackground()); + buttonOutput.addSelectionListener(new SelectionListener() { @Override public void widgetDefaultSelected(SelectionEvent e) { - name.setUserSetIsReference(referenceButton.getSelection()); - onVisibilityOrReturnChange(info.getParameterCandidates()); + widgetSelected(e); } @Override public void widgetSelected(SelectionEvent e) { - widgetDefaultSelected(e); + if (buttonOutput.isEnabled()) { + name.setIsOutput(buttonOutput.getSelection()); + } } }); - referenceButton.pack(); - editor.minimumWidth = referenceButton.getSize().x; + buttonOutput.pack(); + editor.minimumWidth = buttonOutput.getSize().x; editor.horizontalAlignment = SWT.CENTER; - referenceButtons.add(referenceButton); - editor.setEditor(referenceButton, item, columnIndex++); + referenceButtons.add(buttonOutput); + editor.setEditor(buttonOutput, item, columnIndex++); - // Cosnt Button + // Const button editor = new TableEditor(table); - final Button constButton = new Button(table, SWT.CHECK); + final Button buttonConst = new Button(table, SWT.CHECK); - constButton.setSelection(name.isConst()); - constButton.setEnabled(!name.isWriteAccess()); + buttonConst.setSelection(name.isConst()); + buttonConst.setEnabled(!name.isWriteAccess()); - constButton.setBackground(table.getBackground()); - constButton.addSelectionListener(new SelectionListener() { + buttonConst.setBackground(table.getBackground()); + buttonConst.addSelectionListener(new SelectionListener() { @Override public void widgetDefaultSelected(SelectionEvent e) { - name.setConst(constButton.getSelection()); - onVisibilityOrReturnChange(info.getParameterCandidates()); + widgetSelected(e); } @Override public void widgetSelected(SelectionEvent e) { - widgetDefaultSelected(e); + name.setConst(buttonConst.getSelection()); } }); - constButton.pack(); - editor.minimumWidth = constButton.getSize().x; + buttonConst.pack(); + editor.minimumWidth = buttonConst.getSize().x; editor.horizontalAlignment = SWT.CENTER; // referenceButtons.add(referenceButton); - editor.setEditor(constButton, item, columnIndex++); + editor.setEditor(buttonConst, item, columnIndex++); if (info.isExtractExpression()) continue; // Skip the return radiobutton // Button editor = new TableEditor(table); - final Button returnButton = new Button(table, SWT.RADIO); - returnButton.setSelection(name.isReturnValue()); - name.setUserSetIsReference(name.isOutput()); - returnButton.setEnabled(info.getMandatoryReturnVariable() == null); - returnButton.setBackground(table.getBackground()); - returnButton.addSelectionListener(new SelectionListener() { + final Button buttonReturn = new Button(table, SWT.RADIO); + buttonReturn.setSelection(name.mustBeReturnValue()); + buttonReturn.setEnabled(info.getMandatoryReturnVariable() == null); + buttonReturn.setBackground(table.getBackground()); + buttonReturn.addSelectionListener(new SelectionListener() { @Override public void widgetDefaultSelected(SelectionEvent e) { - name.setUserSetIsReturnValue(returnButton.getSelection()); - if (returnButton.getSelection()) { - referenceButton.setSelection(false); - referenceButton.notifyListeners(SWT.Selection, new Event()); - } else if (name.isOutput()) { - referenceButton.setSelection(true); - referenceButton.notifyListeners(SWT.Selection, new Event()); - } - onVisibilityOrReturnChange(info.getParameterCandidates()); + widgetSelected(e); } @Override public void widgetSelected(SelectionEvent e) { - widgetDefaultSelected(e); + name.setReturnValue(buttonReturn.getSelection()); + if (buttonReturn.getSelection()) { + buttonOutput.setSelection(false); + buttonOutput.notifyListeners(SWT.Selection, new Event()); + } else if (name.mustBeOutput()) { + buttonOutput.setSelection(true); + buttonOutput.notifyListeners(SWT.Selection, new Event()); + } } }); - returnButton.pack(); - editor.minimumWidth = returnButton.getSize().x; + buttonReturn.pack(); + editor.minimumWidth = buttonReturn.getSize().x; editor.horizontalAlignment = SWT.CENTER; - returnButtons.add(returnButton); - editor.setEditor(returnButton, item, columnIndex++); + returnButtons.add(buttonReturn); + editor.setEditor(buttonReturn, item, columnIndex++); } } if (!info.isExtractExpression()) { - voidReturn = new Button(parent, SWT.CHECK | SWT.LEFT); - voidReturn.setText(Messages.ChooserComposite_NoReturnValue); - voidReturn.setEnabled(info.getMandatoryReturnVariable() == null); - voidReturn.addSelectionListener(new SelectionListener() { + checkboxVoidReturn = new Button(parent, SWT.CHECK | SWT.LEFT); + checkboxVoidReturn.setText(Messages.ChooserComposite_NoReturnValue); + checkboxVoidReturn.setEnabled(info.getMandatoryReturnVariable() == null); + checkboxVoidReturn.addSelectionListener(new SelectionListener() { @Override public void widgetDefaultSelected(SelectionEvent e) { - info.setReturnVariable(null); - - for (Button button : returnButtons) { - if (voidReturn.getSelection()) { - button.setSelection(false); - button.notifyListeners(SWT.Selection, new Event()); - } - button.setEnabled(!voidReturn.getSelection()); - } + widgetSelected(e); } @Override public void widgetSelected(SelectionEvent e) { - widgetDefaultSelected(e); + info.setReturnVariable(null); + + for (Button button : returnButtons) { + if (checkboxVoidReturn.getSelection()) { + button.setSelection(false); + button.notifyListeners(SWT.Selection, new Event()); + } + button.setEnabled(!checkboxVoidReturn.getSelection()); + } } }); } @@ -207,16 +201,4 @@ public class ChooserComposite extends Composite { column.setText(string); column.setWidth(100); } - - void onVisibilityOrReturnChange(List names) { - String variableUsedAfterBlock = null; - for (NameInformation information : names) { - if (information.isReferencedAfterSelection() && - !(information.isUserSetIsReference() || information.isUserSetIsReturnValue())) { - variableUsedAfterBlock = information.getName().toString(); - } - } - - page.errorWithAfterUsedVariable(variableUsedAfterBlock); - } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInformation.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInformation.java index 31cee127406..bd2cdd7ce5f 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInformation.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInformation.java @@ -64,11 +64,11 @@ public class ExtractFunctionInformation { return returnVariable; } - public void setReturnVariable(NameInformation returnVariable) { - if (returnVariable != null) { - returnVariable.setUserSetIsReturnValue(true); + public void setReturnVariable(NameInformation variable) { + if (variable != null) { + variable.setReturnValue(true); } - this.returnVariable = returnVariable; + this.returnVariable = variable; } public NameInformation getMandatoryReturnVariable() { @@ -76,6 +76,9 @@ public class ExtractFunctionInformation { } public void setMandatoryReturnVariable(NameInformation variable) { + if (variable != null) { + variable.setMustBeReturnValue(true); + } this.mandatoryReturnVariable = variable; this.returnVariable = variable; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInputPage.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInputPage.java index e43ea1e28fe..a2ea4836690 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInputPage.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInputPage.java @@ -91,13 +91,4 @@ public class ExtractFunctionInputPage extends UserInputWizardPage { setPageComplete(false); } } - - public void errorWithAfterUsedVariable(String variableUsedAfterBlock ) { - if (variableUsedAfterBlock == null) { - setErrorMessage(null); - checkName(); - } else { - setErrorMessage("The parameter '" + variableUsedAfterBlock + "' " + Messages.ExtractFunctionInputPage_1); //$NON-NLS-1$ //$NON-NLS-2$ - } - } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java index cb394bc9e83..0395770a62b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java @@ -194,12 +194,6 @@ public class ExtractFunctionRefactoring extends CRefactoring { NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); info.setMethodContext(context); - if (info.getMandatoryReturnVariable() != null) { - info.getMandatoryReturnVariable().setUserSetIsReturnValue(true); - } - for (NameInformation name : info.getParameterCandidates()) { - name.setUserSetIsReference(name.isOutput()); - } sm.done(); } finally { unlockIndex(); @@ -270,7 +264,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } } for (NameInformation name : info.getParameterCandidates()) { - if (name.isUserSetIsReturnValue()) { + if (name.isReturnValue()) { info.setReturnVariable(name); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java index cb9d3e2a099..b39f76cdda3 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java @@ -95,7 +95,7 @@ public abstract class ExtractedFunctionConstructionHelper { Collection parameterNames, INodeFactory nodeFactory) { List result = new ArrayList(parameterNames.size()); for (NameInformation name : parameterNames) { - result.add(name.getParameterDeclaration(name.isUserSetIsReference(), nodeFactory)); + result.add(name.getParameterDeclaration(nodeFactory)); } return result; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java index 100ec096d48..7edab6b0817 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java @@ -79,7 +79,7 @@ final class SimilarFinderVisitor extends ASTVisitor { if (orgName != null) { for (NameInformation orgNameInfo : refactoring.container.getParameterCandidates()) { if (orgName.equals(orgNameInfo.getDeclaration().getRawSignature()) && - (orgNameInfo.isOutput() || !nameInfo.isOutput())) { + (orgNameInfo.mustBeOutput() || !nameInfo.mustBeOutput())) { found = true; break; }