Code streamlining.

8 files changed, 98 insertions, 126 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<IASTName> references;
 		private List<IASTName> 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,36 +216,38 @@ 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) {
-			this.isOutput = isOutput;
+		public void setMustBeOutput(boolean mustBeOutput) {
+			this.mustBeOutput = mustBeOutput;
 		}
 
-		public boolean isReturnValue() {
-			return isReturnValue;
+		public boolean isOutput() {
+			return mustBeOutput || isOutput;
 		}
 
-		public void setReturnValue(boolean isReturnValue) {
-			this.isReturnValue = isReturnValue;
+		public void setIsOutput(boolean isOutput) {
+			Assert.isTrue(isOutput || !mustBeOutput);
+			this.isOutput = isOutput;
 		}
 
-		public boolean isUserSetIsReference() {
-			return userSetIsReference;
+		public boolean mustBeReturnValue() {
+			return mustBeReturnValue;
 		}
 
-		public void setUserSetIsReference(boolean userSetIsReference) {
-			this.userSetIsReference = userSetIsReference;
+		public void setMustBeReturnValue(boolean mustBeReturnValue) {
+			this.mustBeReturnValue = mustBeReturnValue;
 		}
 
-		public boolean isUserSetIsReturnValue() {
-			return userSetIsReturnValue;
+		public boolean isReturnValue() {
+			return mustBeReturnValue || isReturnValue;
 		}
 
-		public void setUserSetIsReturnValue(boolean userSetIsReturnValue) {
-			this.userSetIsReturnValue = userSetIsReturnValue;
+		public void setReturnValue(boolean isReturnValue) {
+			Assert.isTrue(isReturnValue || !mustBeReturnValue);
+			this.isReturnValue = isReturnValue;
 		}
 
 		public String getUserSetName() {
@@ -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<NameInformation> selectedNames = null;
 
 		for (NameInformation nameInfo : getInterfaceNames()) {
-			if (nameInfo.isReturnValue() == isReturnValue) {
+			if (nameInfo.mustBeReturnValue() == isReturnValue) {
 				if (selectedNames == null) {
 					selectedNames = new ArrayList<NameInformation>();
 				}
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,117 +80,116 @@ 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);
+					widgetSelected(e);
+				}
 	
+				@Override
+				public void widgetSelected(SelectionEvent e) {
+					info.setReturnVariable(null);
+					
 					for (Button button : returnButtons) {
-						if (voidReturn.getSelection()) {
+						if (checkboxVoidReturn.getSelection()) {
 							button.setSelection(false);
 							button.notifyListeners(SWT.Selection, new Event());
 						}
-						button.setEnabled(!voidReturn.getSelection());
+						button.setEnabled(!checkboxVoidReturn.getSelection());
 					}
 				}
-	
-				@Override
-				public void widgetSelected(SelectionEvent e) {
-					widgetDefaultSelected(e);
-				}
 			});
 		}
 		
@@ -207,16 +201,4 @@ public class ChooserComposite extends Composite {
 		column.setText(string);
 		column.setWidth(100);
 	}
-	
-	void onVisibilityOrReturnChange(List<NameInformation> 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<NameInformation> parameterNames, INodeFactory nodeFactory) {
 		List<IASTParameterDeclaration> result = new ArrayList<IASTParameterDeclaration>(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;
 									}