Code maintenance for refactoring, by Emanuel Graf, bug 226645.

9 files changed, 49 insertions, 206 deletions

diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/RefactoringBaseTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/RefactoringBaseTest.java
index f705bc44d3c..4ba392967e9 100644
--- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/RefactoringBaseTest.java
+++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/RefactoringBaseTest.java
@@ -62,29 +62,29 @@ public abstract class RefactoringBaseTest extends BaseTestFramework implements I
 	}
 	
 	protected void assertEquals(TestSourceFile file, IFile file2) throws Exception {
-		StringBuffer code = getCodeFromIFile(file2);
-		assertEquals(file.getExpectedSource(), TestHelper.unifyNewLines(code.toString()));
+		String code = getCodeFromIFile(file2);
+		assertEquals(file.getExpectedSource(), TestHelper.unifyNewLines(code));
 	}
 	
 	protected void compareFiles(Map<String,TestSourceFile> testResourceFiles) throws Exception {
 		for (String fileName : testResourceFiles.keySet()) {
-			TestSourceFile file = testResourceFiles.get(fileName);
+			String expectedSource = testResourceFiles.get(fileName).getExpectedSource();
 			IFile iFile = project.getFile(new Path(fileName));
-			StringBuffer code = getCodeFromIFile(iFile);
-			assertEquals(TestHelper.unifyNewLines(file.getExpectedSource()), TestHelper.unifyNewLines(code.toString()));
+			String code = getCodeFromIFile(iFile);
+			assertEquals(TestHelper.unifyNewLines(expectedSource), TestHelper.unifyNewLines(code));
 		}
 	}
 
-	protected StringBuffer getCodeFromIFile(IFile file) throws Exception {
+	protected String getCodeFromIFile(IFile file) throws Exception {
 		BufferedReader br = new BufferedReader(new InputStreamReader(file.getContents()));
-		StringBuffer code = new StringBuffer();
+		StringBuilder code = new StringBuilder();
 		String line;
 		while((line = br.readLine()) != null) {
 			code.append(line);
 			code.append('\n');
 		}
 		br.close();
-		return code;
+		return code.toString();
 	}
 
 	@Override
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java
index 382ce400bfb..a261cbe2602 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java
@@ -68,11 +68,10 @@ public abstract class CRefactoring extends Refactoring {
 
 	protected String name = Messages.HSRRefactoring_name; 
 	protected IFile file;
-	protected ISelection selection;
+	private ISelection selection;
 	protected RefactoringStatus initStatus;
 	protected IASTTranslationUnit unit;
 	private IIndex fIndex;
-	public static final String NEWLINE = System.getProperty("line.separator"); //$NON-NLS-1$
 
 	public CRefactoring(IFile file, ISelection selection) {
 		this.file = file;
@@ -240,23 +239,25 @@ public abstract class CRefactoring extends Refactoring {
 		return collector.createFinalChange();
 	}
 	
-	protected void collectModifications(IProgressMonitor pm, ModificationCollector collector)
-		throws CoreException, OperationCanceledException {
-	}
+	abstract protected void collectModifications(IProgressMonitor pm, ModificationCollector collector)
+		throws CoreException, OperationCanceledException;
 
 	@Override
 	public String getName() {
 		return name;
 	}
-	
-	protected boolean loadTranslationUnit(RefactoringStatus status, IProgressMonitor mon) {
+
+	protected ITextSelection getTextSelection() {
+		return (ITextSelection) selection;
+	}
+
+	private boolean loadTranslationUnit(RefactoringStatus status,
+			IProgressMonitor mon) {
 		SubMonitor subMonitor = SubMonitor.convert(mon, 10);
 		if (file != null) {
 			try {
-				subMonitor.subTask(Messages.HSRRefactoring_PM_ParseTU); 
-				ITranslationUnit tu = (ITranslationUnit) CCorePlugin
-						.getDefault().getCoreModel().create(file);
-				unit = tu.getAST(fIndex, AST_STYLE);
+				subMonitor.subTask(Messages.HSRRefactoring_PM_ParseTU);
+				unit = loadTranslationUnit(file);
 				subMonitor.worked(2);
 				if(isProgressMonitorCanceld(subMonitor, initStatus)) {
 					return true;
@@ -279,6 +280,11 @@ public abstract class CRefactoring extends Refactoring {
 		return true;
 	}
 
+	protected IASTTranslationUnit loadTranslationUnit(IFile file) throws CoreException {
+		ITranslationUnit tu = (ITranslationUnit) CCorePlugin.getDefault().getCoreModel().create(file);
+		return tu.getAST(fIndex, AST_STYLE);
+	}
+
 	private static class ExpressionPosition {
 		public int start;
 		public int end;
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/RefactoringRunner.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/RefactoringRunner.java
index 0c668e1d12b..f7ab9356367 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/RefactoringRunner.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/RefactoringRunner.java
@@ -13,7 +13,7 @@ package org.eclipse.cdt.internal.ui.refactoring;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.jface.viewers.ISelection;
-import org.eclipse.ui.IWorkbenchWindow;
+import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.PlatformUI;
 
 /**
@@ -26,17 +26,12 @@ public abstract class RefactoringRunner {
 
 	protected IFile file;
 	protected ISelection selection;
-	protected IWorkbenchWindow window;
+	protected Shell shell;
 
-	public RefactoringRunner(IFile file, ISelection selection, IWorkbenchWindow window) {
-		super();
+	public RefactoringRunner(IFile file, ISelection selection) {
 		this.file = file;
 		this.selection = selection;
-		if(window != null) {
-			this.window = window;
-		}else {
-			this.window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
-		}
+		shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell();
 	}
 	
 	public abstract void run();
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
index bc84ad1d728..0b7c13ad109 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2008 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik  
  * Rapperswil, University of applied sciences and others
  * All rights reserved. This program and the accompanying materials 
  * are made available under the terms of the Eclipse Public License v1.0 
@@ -99,13 +99,7 @@ public class ExtractConstantRefactoring extends CRefactoring {
 
 		if(isProgressMonitorCanceld(sm, initStatus)) return initStatus;
 		
-		ITextSelection textSelection = null;
-		if (selection instanceof ITextSelection) {
-			textSelection = (ITextSelection) selection;
-		} else {
-			initStatus.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected); 
-			return initStatus;
-		}
+		ITextSelection textSelection = getTextSelection();
 		sm.worked(1);
 
 		if(isProgressMonitorCanceld(sm, initStatus)) return initStatus;
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringRunner.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringRunner.java
index 0d78d602819..5933a61e7b0 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringRunner.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringRunner.java
@@ -15,7 +15,6 @@ import org.eclipse.core.resources.IFile;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation;
-import org.eclipse.ui.IWorkbenchWindow;
 
 import org.eclipse.cdt.ui.CUIPlugin;
 
@@ -29,8 +28,8 @@ import org.eclipse.cdt.internal.ui.refactoring.RefactoringRunner;
  */
 public class ExtractConstantRefactoringRunner extends RefactoringRunner  {
 
-	public ExtractConstantRefactoringRunner(IFile file, ISelection selection, IWorkbenchWindow window) {
-		super(file, selection, window);
+	public ExtractConstantRefactoringRunner(IFile file, ISelection selection) {
+		super(file, selection);
 	}
 
 	@Override
@@ -43,7 +42,7 @@ public class ExtractConstantRefactoringRunner extends RefactoringRunner  {
 		try {
 			refactoring.lockIndex();
 			try {
-				operator.run(window.getShell(), refactoring.getName());
+				operator.run(shell, refactoring.getName());
 			}
 			finally {
 				refactoring.unlockIndex();
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileContentHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileContentHelper.java
index 67aef3d8adb..b8a26ffad0e 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileContentHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileContentHelper.java
@@ -22,7 +22,7 @@ import org.eclipse.core.runtime.CoreException;
 import org.eclipse.cdt.ui.CUIPlugin;
 
 /**
- * Some helper methods to access part of the content of an ifile 
+ * Some helper methods to access part of the content of an iFile 
  * 
  * @author Emanuel Graf
  *
@@ -32,12 +32,9 @@ public class FileContentHelper {
 	private static final int bufferSize = 512;
 
 	public static String getContent(IFile file, int start) throws CoreException, IOException{
-		
 		InputStreamReader reader = getReaderForFile(file);	
 		skip(start, reader);
-		
 		return readRest(reader);
-		
 	}
 	
 	public static String getContent(IFile file, int start, int length) {
@@ -72,8 +69,6 @@ public class FileContentHelper {
 		while((bytesRead = reader.read(buffer)) >= 0){
 			content.append(buffer, 0, bytesRead);
 		}
-
-		
 		return content.toString();
 	}
 	
@@ -90,14 +85,13 @@ public class FileContentHelper {
 		}
 	}
 
-	private static void skip(int start, InputStreamReader r) throws IOException {
+	private static void skip(int count, InputStreamReader r) throws IOException {
 		long skipped = 0;
-		while(skipped >= 0 && start > 0 && r.ready()){
-			skipped = r.skip(start);
+		while(skipped >= 0 && count > 0 && r.ready()){
+			skipped = r.skip(count);
 			if(skipped > 0){
-				start -= skipped;
+				count -= skipped;
 			}
 		}
 	}
-
 }
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileHelper.java
index ace50f12194..f80c9d54e4f 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/FileHelper.java
@@ -11,67 +11,23 @@
  *******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring.utils;
 
-import java.io.IOException;
-import java.io.InputStream;
-
 import org.eclipse.core.resources.IFile;
-import org.eclipse.core.resources.IProject;
-import org.eclipse.core.resources.ProjectScope;
 import org.eclipse.core.resources.ResourcesPlugin;
-import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Path;
-import org.eclipse.core.runtime.Platform;
-import org.eclipse.core.runtime.preferences.IScopeContext;
-import org.eclipse.core.runtime.preferences.InstanceScope;
-import org.eclipse.jface.text.TextUtilities;
 
-import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 
+/**
+ * Helper class concerning files.
+ * 
+ * @author Lukas Felber
+ *
+ */
 public class FileHelper {
 
-	private static final String DEFAULT_LINE_DELIMITTER = "\n"; //$NON-NLS-1$
-
 	public static IFile getIFilefromIASTNode(IASTNode node) {
 		IPath implPath = new Path(node.getContainingFilename());
 		return ResourcesPlugin.getWorkspace().getRoot().getFileForLocation(implPath);
 	}
-	
-	public static boolean isFirstWithinSecondLocation(IASTFileLocation loc1, IASTFileLocation loc2){
-		
-		boolean isEquals = true;
-		
-		isEquals &= loc1.getFileName().equals(loc2.getFileName());
-		isEquals &= loc1.getNodeOffset() >= loc2.getNodeOffset();
-		isEquals &= loc1.getNodeOffset()+loc1.getNodeLength() <= loc2.getNodeOffset() + loc2.getNodeLength();
-		
-		return isEquals;
-	}
-
-	public static String determineLineDelimiter(IFile file) {
-		StringBuilder fileContent = new StringBuilder();
-		try {
-			InputStream fis = file.getContents();
-			byte[] buffer = new byte[1024];
-			int read;
-			while ((read = fis.read(buffer)) >= 0)
-				fileContent.append(new String(buffer, 0, read));			
-		} catch (CoreException e) {
-		} catch (IOException e) {
-		} catch (NullPointerException e){
-		}
-	
-		IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject();
-		IScopeContext[] scopeContext;
-		if(project != null){
-			scopeContext = new IScopeContext[] { new ProjectScope(project)};
-		}
-		else{
-			scopeContext = new IScopeContext[] { new InstanceScope()};
-		}
-		String platformDefaultLineDelimiter = System.getProperty("line.separator", DEFAULT_LINE_DELIMITTER); //$NON-NLS-1$
-		String defaultLineDelimiter = Platform.getPreferencesService().getString(Platform.PI_RUNTIME, Platform.PREF_LINE_SEPARATOR, platformDefaultLineDelimiter, scopeContext);
-		return TextUtilities.determineLineDelimiter(fileContent.toString(), defaultLineDelimiter);
-	}
 }
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/OffsetHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/OffsetHelper.java
deleted file mode 100644
index d564c3393d3..00000000000
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/OffsetHelper.java
+++ /dev/null
@@ -1,100 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik  
- * Rapperswil, University of applied sciences and others
- * All rights reserved. This program and the accompanying materials 
- * are made available under the terms of the Eclipse Public License v1.0 
- * which accompanies this distribution, and is available at 
- * http://www.eclipse.org/legal/epl-v10.html  
- * 
- * Contributors: 
- * Institute for Software - initial API and implementation 
- ******************************************************************************/
-package org.eclipse.cdt.internal.ui.refactoring.utils;
-
-import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
-import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation;
-import org.eclipse.cdt.core.dom.ast.IASTNode;
-import org.eclipse.cdt.core.dom.ast.IASTNodeLocation;
-
-import org.eclipse.cdt.internal.core.dom.parser.ASTNode;
-
-/**
- * Some helper methods that calculate offsets from nodes.
- * 
- * @author Emanuel Graf
- *
- */
-public class OffsetHelper {
-	
-	public static int getOffsetIncludingComment(IASTNode node) {
-		int nodeStart = Integer.MAX_VALUE;
-		IASTNodeLocation[] nodeLocations = node.getNodeLocations();
-		if (nodeLocations.length != 1) {
-			int offset;
-			for (IASTNodeLocation location : nodeLocations) {
-				if (location instanceof IASTMacroExpansionLocation) {
-					IASTMacroExpansionLocation macroLoc = (IASTMacroExpansionLocation) location;
-					offset = macroLoc.asFileLocation().getNodeOffset();
-				}else {
-					offset = location.asFileLocation().getNodeOffset();
-				}
-				if(offset < nodeStart) nodeStart = offset;
-			}
-		} else {
-			nodeStart = node.getFileLocation().getNodeOffset();
-		}
-		
-		return nodeStart;
-	}
-	
-	public static int getEndOffsetIncludingComments(IASTNode node) {
-		int fileOffset = 0;
-		int length = 0;
-		
-		IASTNodeLocation[] nodeLocations = node.getNodeLocations();
-		if (nodeLocations.length != 1) {
-			for (IASTNodeLocation location : nodeLocations) {
-				if (location instanceof IASTMacroExpansionLocation) {
-					IASTMacroExpansionLocation macroLoc = (IASTMacroExpansionLocation) location;
-					fileOffset = macroLoc.asFileLocation().getNodeOffset();
-					length = macroLoc.asFileLocation().getNodeLength();
-				}else {
-					fileOffset = location.asFileLocation().getNodeOffset();
-					length = location.asFileLocation().getNodeLength();
-				}
-			}
-		} else {
-			IASTFileLocation loc = node.getFileLocation();
-			
-			fileOffset = loc.getNodeOffset();
-			length = loc.getNodeLength();
-		}
-		return fileOffset + length;
-		
-	}
-
-	public static int getEndOffsetWithoutComments(IASTNode node) {
-		return node.getFileLocation().getNodeOffset() + node.getFileLocation().getNodeLength();
-	}
-	
-	public static int getLengthIncludingComment(IASTNode node) {
-		return OffsetHelper.getEndOffsetIncludingComments(node) - OffsetHelper.getOffsetIncludingComment(node);
-	}
-
-	public static int getNodeOffset(ASTNode node) {
-		return node.getOffset();
-	}
-
-	public static int getNodeEndPoint(ASTNode node) {
-		return node.getOffset() + node.getLength();
-	}
-
-	public static int getStartingLineNumber(IASTNode node) {
-		return node.getFileLocation().getStartingLineNumber();
-	}
-
-	public static int getEndingLineNumber(IASTNode node) {
-		return node.getFileLocation().getEndingLineNumber();
-	}	
-	
-}
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractConstantAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractConstantAction.java
index 2e6c0dfc77b..d2cbc1e5cba 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractConstantAction.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractConstantAction.java
@@ -11,6 +11,7 @@
 
 package org.eclipse.cdt.ui.refactoring.actions;
 
+
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.jface.text.ITextSelection;
@@ -22,7 +23,7 @@ import org.eclipse.cdt.core.model.IWorkingCopy;
 import org.eclipse.cdt.internal.ui.refactoring.extractconstant.ExtractConstantRefactoringRunner;
 
 /**
- * Launches a rename refactoring.
+ * Launches a extract constant refactoring.
  */          
 public class ExtractConstantAction extends RefactoringAction {
     
@@ -35,12 +36,10 @@ public class ExtractConstantAction extends RefactoringAction {
 	}
 
 	@Override
-	public void run(IShellProvider shellProvider, IWorkingCopy wc, ITextSelection s) {
+	public void run(IShellProvider shellProvider, IWorkingCopy wc, ITextSelection selection) {
 		IResource res= wc.getResource();
 		if (res instanceof IFile) {
-			new ExtractConstantRefactoringRunner((IFile) res, 
-					fEditor.getSelectionProvider().getSelection(), 
-					fEditor.getSite().getWorkbenchWindow()).run();
+			new ExtractConstantRefactoringRunner((IFile) res, selection).run();
 		}
 	}