Add support for deletion of generated files when a source change indicates that a file should no longer be generated.

8 files changed, 86 insertions, 101 deletions

diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/AptPlugin.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/AptPlugin.java
index ce1e307548..2773e82e83 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/AptPlugin.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/AptPlugin.java
@@ -123,6 +123,18 @@ public class AptPlugin extends Plugin {
 	}
 	
 	/**
+	 * Convenience wrapper around log(IStatus), to log an exception
+	 * with severity of WARNING.
+	 */
+	public static void logWarning(Throwable e, String message) {
+		// TODO: before ship, remove this printing. Instead just log
+		// Note: we don't include the stack here, but it goes in the log
+		System.err.println(message);
+		
+		log(createWarningStatus(e, message));
+	}
+	
+	/**
 	 * Convenience wrapper for rethrowing exceptions as CoreExceptions,
 	 * with severity of ERROR.
 	 */
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/APTDispatchRunnable.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/APTDispatchRunnable.java
index 98ea132bf4..16f8c843e2 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/APTDispatchRunnable.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/APTDispatchRunnable.java
@@ -573,7 +573,7 @@ import com.sun.mirror.declaration.AnnotationTypeDeclaration;
 			}
 
 			final Map<IFile, Set<IFile>> allGeneratedFiles = processorEnv.getAllGeneratedFiles();
-			final Set<IFile> modifiedGeneratedFiles = processorEnv.getModifiedGeneratedFiles();
+			final Set<IFile> modifiedGeneratedSourceFiles = processorEnv.getModifiedGeneratedSourceFiles();
 			
 			// any files that were generated for this parent on the last
 			// run, but are no longer generated should be removed
@@ -595,7 +595,7 @@ import com.sun.mirror.declaration.AnnotationTypeDeclaration;
 					allDeletedFiles.addAll(deletedFiles);		
 			}		
 			
-			APTResult result = new APTResult( modifiedGeneratedFiles, 
+			APTResult result = new APTResult( modifiedGeneratedSourceFiles, 
 											  allDeletedFiles, 
 											  currentRoundDispatchedBatchFactories,
 											  processorEnv.getTypeDependencies(), 
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/BinaryFileOutputStream.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/BinaryFileOutputStream.java
index 28646dc80d..330ad919eb 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/BinaryFileOutputStream.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/BinaryFileOutputStream.java
@@ -30,58 +30,65 @@ import org.eclipse.jdt.apt.core.internal.util.FileSystemUtil;
 public class BinaryFileOutputStream extends ByteArrayOutputStream {
 
 	private final IFile _file;
+	private final ProcessorEnvImpl _env;
 	
-	public BinaryFileOutputStream(IFile file) {
+	public BinaryFileOutputStream(IFile file, ProcessorEnvImpl env) {
 		_file = file;
+		_env = env;
 	}
 	
 	@Override
 	public void close() throws IOException {
 		super.close();
-		InputStream contents = null;
+		
+		InputStream contents = new ByteArrayInputStream(toByteArray());
 		try {
-			contents = new ByteArrayInputStream(toByteArray());
+			
+			boolean contentsChanged = true;
 			if (!_file.exists()) {
 				saveToDisk(contents, true);
-				return;
 			}
-			boolean needToWriteData = true;
-			InputStream in = null;
-			InputStream oldData = null;
-			try {
-				// Only write the contents if the data is different
-				in = new ByteArrayInputStream(toByteArray());
-				oldData = new BufferedInputStream(_file.getContents());
-				if (FileSystemUtil.compareStreams(in, oldData)) {
-					needToWriteData = false;
-				}
-			}
-			catch (CoreException ce) {
-				// Ignore -- couldn't read the old data, so assume it's different
-			}
-			finally {
+			else {
+				InputStream in = null;
+				InputStream oldData = null;
 				try {
-					if (in != null) in.close(); 
-				} 
-				catch (IOException ioe) {
+					// Only write the contents if the data is different
+					in = new ByteArrayInputStream(toByteArray());
+					oldData = new BufferedInputStream(_file.getContents());
+					if (FileSystemUtil.compareStreams(in, oldData)) {
+						contentsChanged = false;
+					}
 				}
-				try {
-					if (oldData != null) oldData.close(); 
-				} 
-				catch (IOException ioe) {
+				catch (CoreException ce) {
+					// Ignore -- couldn't read the old data, so assume it's different
+					contentsChanged = true;
+				}
+				finally {
+					closeInputStream(in);
+					closeInputStream(oldData);
+				}
+				if (contentsChanged) {
+					contents.reset();
+					saveToDisk(contents, false);
 				}
-			}
-			if (needToWriteData) {
-				contents.reset();
-				saveToDisk(contents, false);
 			}
 		}
 		finally {
+			closeInputStream(contents);
+		}
+			
+		IFile parentFile = _env.getFile();
+		if (parentFile != null) {
+			_env.getAptProject().getGeneratedFileManager().addEntryToFileMaps(parentFile, _file);
+		}
+	}
+	
+	private void closeInputStream(InputStream stream) {
+		if (stream != null) {
 			try {
-				if (contents != null) contents.close(); 
-			} 
-			catch (IOException ioe) {
+				stream.close();
 			}
+			catch (IOException ioe) {}
 		}
 	}
 	
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FileRefreshJob.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FileRefreshJob.java
deleted file mode 100644
index c4949d9a19..0000000000
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FileRefreshJob.java
+++ /dev/null
@@ -1,42 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2005 BEA Systems, Inc. 
- * 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:
- *    jgarms@bea.com - initial API and implementation
- *    
- *******************************************************************************/
-package org.eclipse.jdt.apt.core.internal.env;
-
-import org.eclipse.core.resources.IFile;
-import org.eclipse.core.resources.IResource;
-import org.eclipse.core.runtime.CoreException;
-import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.Status;
-import org.eclipse.core.runtime.jobs.Job;
-
-public class FileRefreshJob extends Job {
-	
-	private final IFile _file;
-	
-	FileRefreshJob(final IFile file) {
-		super(file.toString());
-		_file = file;
-	}
-
-	@Override
-	protected IStatus run(IProgressMonitor monitor) {
-		try {
-			_file.getParent().refreshLocal(IResource.DEPTH_INFINITE, monitor);
-		}
-		catch (CoreException ce) {
-			return ce.getStatus();
-		}
-		return Status.OK_STATUS;
-	}
-
-}
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FilerImpl.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FilerImpl.java
index e34d15f246..2b1c217492 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FilerImpl.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/FilerImpl.java
@@ -91,7 +91,7 @@ public class FilerImpl implements Filer {
     	path = path.append(name.replace('.', File.separatorChar) + ".class"); //$NON-NLS-1$
     	
         IFile file = _env.getProject().getFile(path);
-        return new BinaryFileOutputStream(file);
+        return new BinaryFileOutputStream(file, _env);
     }
 	
 	public boolean hasGeneratedClassFile(){ return _generatedClassFiles; }
@@ -125,7 +125,7 @@ public class FilerImpl implements Filer {
     	
     	IPath path = getOutputFileForLocation( loc, pkg, relPath );
     	IFile file = _env.getProject().getFile(path);
-    	OutputStream binaryOut = new BinaryFileOutputStream(file);
+    	OutputStream binaryOut = new BinaryFileOutputStream(file, _env);
  
     	if (charsetName == null) {
     		return new PrintWriter(binaryOut);
@@ -161,7 +161,7 @@ public class FilerImpl implements Filer {
     	
     	IPath path = getOutputFileForLocation( loc, pkg, relPath );
     	IFile file = _env.getProject().getFile(path);
-    	return new BinaryFileOutputStream(file);
+    	return new BinaryFileOutputStream(file, _env);
     }
 	
     /**
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/JavaSourceFilePrintWriter.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/JavaSourceFilePrintWriter.java
index bf5950548d..4a40f2d165 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/JavaSourceFilePrintWriter.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/JavaSourceFilePrintWriter.java
@@ -24,6 +24,10 @@ import org.eclipse.jdt.core.ICompilationUnit;
 
 public class JavaSourceFilePrintWriter extends PrintWriter {
 
+	private final StringWriter _sw;
+    private final String _typeName;
+    private final ProcessorEnvImpl _env;
+	
     public JavaSourceFilePrintWriter( String typeName, StringWriter sw, ProcessorEnvImpl env )
     {
         super( sw );
@@ -58,16 +62,11 @@ public class JavaSourceFilePrintWriter extends PrintWriter {
 	            throw new IllegalStateException( "Unexpected phase value: " + phase ); //$NON-NLS-1$
 	        }
 	        if (result != null) {
-	        	_env.addGeneratedFile(result.getFile(), result.isModified());
+	        	_env.addGeneratedSourceFile(result.getFile(), result.isModified());
 	        }
     	}
     	catch (CoreException ce) {
     		AptPlugin.log(ce, "Unable to generate type when JavaSourceFilePrintWriter was closed"); //$NON-NLS-1$
     	}
     }
-			
-	
-    private StringWriter _sw;
-    private String _typeName;
-    private ProcessorEnvImpl _env;
 }
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/ProcessorEnvImpl.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/ProcessorEnvImpl.java
index ec3aa1b38a..1abfa3b8ef 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/ProcessorEnvImpl.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/env/ProcessorEnvImpl.java
@@ -91,8 +91,8 @@ public class ProcessorEnvImpl extends BaseProcessorEnv implements EclipseAnnotat
     private Map<IFile, List<IProblem>> _allProblems;
     
 	// Stores the generated java files from parent to child
-    private Map<IFile, Set<IFile>> _allGeneratedFiles = new HashMap<IFile, Set<IFile>>();
-    private Set<IFile> _modifiedGeneratedFiles = new HashSet<IFile>();
+    private Map<IFile, Set<IFile>> _allGeneratedSourceFiles = new HashMap<IFile, Set<IFile>>();
+    private Set<IFile> _modifiedGeneratedSourceFiles = new HashSet<IFile>();
 	private Set<AnnotationProcessorListener> _listeners = null;
 	private final FilerImpl _filer;
 	private boolean _isClosed = false;
@@ -404,31 +404,31 @@ public class ProcessorEnvImpl extends BaseProcessorEnv implements EclipseAnnotat
 		return Collections.unmodifiableSet(_listeners);
 	}
 
-	public void addGeneratedFile( IFile f, boolean contentsChanged ) {		
+	public void addGeneratedSourceFile( IFile f, boolean contentsChanged ) {		
 		// Add first to the map of parent -> child
 		IFile parent = getFile();
-		Set<IFile> children = _allGeneratedFiles.get(parent);
+		Set<IFile> children = _allGeneratedSourceFiles.get(parent);
 		if (children == null) {
 			children = new HashSet<IFile>();
-			_allGeneratedFiles.put(parent, children);
+			_allGeneratedSourceFiles.put(parent, children);
 		}
 		children.add(f);
 		
 		if (contentsChanged)
-			_modifiedGeneratedFiles.add(f);
+			_modifiedGeneratedSourceFiles.add(f);
 	}
 
     public ICompilationUnit getCompilationUnit(){ return _unit; }
     
-    public Map<IFile, Set<IFile>> getAllGeneratedFiles(){ return _allGeneratedFiles; }
+    public Map<IFile, Set<IFile>> getAllGeneratedFiles(){ return _allGeneratedSourceFiles; }
     
-    public Set<IFile> getModifiedGeneratedFiles() { return _modifiedGeneratedFiles; }
+    public Set<IFile> getModifiedGeneratedSourceFiles() { return _modifiedGeneratedSourceFiles; }
 
 	/**
 	 * @return true iff source files has been generated.
 	 *         Always return false when this environment is closed.
 	 */
-	public boolean hasGeneratedSourceFiles(){ return !_allGeneratedFiles.isEmpty();  }
+	public boolean hasGeneratedSourceFiles(){ return !_allGeneratedSourceFiles.isEmpty();  }
 
 	/**
 	 * @return true iff class files has been generated.
@@ -517,8 +517,8 @@ public class ProcessorEnvImpl extends BaseProcessorEnv implements EclipseAnnotat
     	_units = null;
     	_allProblems = null;
         _modelCompUnit2astCompUnit.clear();		
-		_allGeneratedFiles = null;
-		_modifiedGeneratedFiles = null;
+		_allGeneratedSourceFiles = null;
+		_modifiedGeneratedSourceFiles = null;
 		if(_listeners != null)
 			_listeners.clear();
 		_isClosed = true;
diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/generatedfile/GeneratedFileManager.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/generatedfile/GeneratedFileManager.java
index 15aea963de..740ef09c92 100644
--- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/generatedfile/GeneratedFileManager.java
+++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/generatedfile/GeneratedFileManager.java
@@ -197,6 +197,8 @@ public class GeneratedFileManager {
 	throws CoreException
 	{
 		if( _skipTypeGeneration ) return null;
+		// If the generated package fragment root wasn't set,
+		// then our classpath is incorrect. Add a marker and return
 		else if( _generatedPackageFragmentRoot == null ){			
 			String message = Messages.bind(
 					Messages.GeneratedFileManager_missing_classpath_entry, 
@@ -317,8 +319,9 @@ public class GeneratedFileManager {
 			// as refactorings then fail in the future, which is worse
 			// than allowing a user to modify a generated file.
 			
-			// during a batch build
-			if( parentFile != null ){
+			// during a batch build, parentFile will be null.
+			// Only keep track of ownership in iterative builds
+			if( parentFile != null ) {
 				addEntryToFileMaps( parentFile, file );
 			}
 			return new FileGenerationResult(file, contentsDiffer);
@@ -656,7 +659,13 @@ public class GeneratedFileManager {
 			final IFolder genFolder = getGeneratedSourceFolder();
 			assert genFolder != null : "Generated folder == null"; //$NON-NLS-1$
 			IContainer parent = generatedFile.getParent();
-			generatedFile.delete(true, true, progressMonitor);
+			try {
+				generatedFile.delete(true, true, progressMonitor);
+			}
+			catch (CoreException ce) {
+				// File was locked or read-only
+				AptPlugin.logWarning(ce, "Failed to delete file: " + generatedFile); //$NON-NLS-1$
+			}
 			// not deleting the generated source folder and only 
 			// delete generated folders containing the generated file.
 			while( !genFolder.equals(parent) && parent != null && parent.isDerived() ){				
@@ -1221,7 +1230,7 @@ public class GeneratedFileManager {
 		}
 	}
 	
-	private void addEntryToFileMaps( IFile parentFile, IFile generatedFile )
+	public void addEntryToFileMaps( IFile parentFile, IFile generatedFile )
 	{
 		synchronized ( this )
 		{