Bug 269023 ErrorParserManager& ErrorPattern Tidy:

- JavaDoc improvements
  - EPM#findFilePath deprecated; #findFileName is the blessed method for mapping Build Output Strings -> IResources
  - Scratch Buffer methods deprecated in favour of IErrorParser2

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ErrorParserManager.java

@@ -34,13 +34,17 @@ import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Path;
 
 /**
+ * The purpose of ErrorParserManager is to delegate the work of error parsing 
+ * build output to {@link IErrorParser}s, assist in finding {@link IResource}s, and
+ * help create appropriate error/warning/info markers to be displayed
+ * by the Problems view.
+ * 
  * @noextend This class is not intended to be subclassed by clients.
  */
 public class ErrorParserManager extends OutputStream {
 
 	private int nOpens;
 
-	private final static String OLD_PREF_ERROR_PARSER = "errorOutputParser"; //$NON-NLS-1$
 	public final static String PREF_ERROR_PARSER = CCorePlugin.PLUGIN_ID + ".errorOutputParser"; //$NON-NLS-1$
 
 	private final IProject fProject;
@@ -60,44 +64,80 @@ public class ErrorParserManager extends OutputStream {
 
 	private boolean hasErrors = false;
 
+	private String cachedFileName = null;
+	private IFile cachedFile = null;
+
+	private static boolean isCygwin = true;
+
+	/**
+	 * Constructor.
+	 * 
+	 * @param builder - project builder.
+	 */
 	public ErrorParserManager(ACBuilder builder) {
 		this(builder.getProject(), builder);
 	}
 
+	/**
+	 * Constructor.
+	 * 
+	 * @param project - project being built.
+	 * @param markerGenerator - marker generator able to create markers.
+	 */
 	public ErrorParserManager(IProject project, IMarkerGenerator markerGenerator) {
 		this(project, markerGenerator, null);
 	}
 
+	/**
+	 * Constructor.
+	 * 
+	 * @param project - project being built.
+	 * @param markerGenerator - marker generator able to create markers.
+	 * @param parsersIDs - array of error parsers' IDs.
+	 */
 	public ErrorParserManager(IProject project, IMarkerGenerator markerGenerator, String[] parsersIDs) {
 		this(project, project.getLocation(), markerGenerator, parsersIDs);
 	}
 
+	/**
+	 * Constructor.
+	 * 
+	 * @param project - project being built.
+	 * @param workingDirectory - working directory of where the build is performed. 
+	 * @param markerGenerator - marker generator able to create markers.
+	 * @param parsersIDs - array of error parsers' IDs.
+	 */
 	public ErrorParserManager(IProject project, IPath workingDirectory, IMarkerGenerator markerGenerator, String[] parsersIDs) {
 		fProject = project;
-		if (parsersIDs == null) {
-			enableAllParsers();
-		} else {
-			fErrorParsers = new LinkedHashMap<String, IErrorParser[]>(parsersIDs.length);
-			for (String parsersID : parsersIDs) {
-				IErrorParser[] parsers = CCorePlugin.getDefault().getErrorParser(parsersID);
-				fErrorParsers.put(parsersID, parsers);
-			}
-		}
 		fMarkerGenerator = markerGenerator;
-		initErrorParserManager(workingDirectory);
-	}
-
-	private void initErrorParserManager(IPath workingDirectory) {
 		fDirectoryStack = new Vector<IPath>();
 		fErrors = new ArrayList<ProblemMarkerInfo>();
+		enableErrorParsers(parsersIDs);
 
 		fBaseDirectory = (workingDirectory == null || workingDirectory.isEmpty()) ? fProject.getLocation() : workingDirectory;
 	}
 
+	private void enableErrorParsers(String[] parsersIDs) {
+		if (parsersIDs == null) {
+			parsersIDs = CCorePlugin.getDefault().getAllErrorParsersIDs();
+		}
+		fErrorParsers = new LinkedHashMap<String, IErrorParser[]>(parsersIDs.length);
+		for (String parsersID : parsersIDs) {
+			IErrorParser[] parsers = CCorePlugin.getDefault().getErrorParser(parsersID);
+			fErrorParsers.put(parsersID, parsers);
+		}
+	}
+
+	/**
+	 * @return current project.
+	 */
 	public IProject getProject() {
 		return fProject;
 	}
 
+	/**
+	 * @return current working directory where build is being performed.
+	 */
 	public IPath getWorkingDirectory() {
 		if (fDirectoryStack.size() != 0) {
 			return fDirectoryStack.lastElement();
@@ -106,6 +146,13 @@ public class ErrorParserManager extends OutputStream {
 		return fBaseDirectory;
 	}
 
+	/**
+	 * {@link #pushDirectory} and {@link #popDirectory} are used to change working directory
+	 * from where file name is searched (see {@link #findFileInWorkspace}).
+	 * The intention is to handle make output of commands "cd dir" and "cd ..".
+	 * 
+	 * @param dir - another directory level to keep in stack.
+	 */
 	public void pushDirectory(IPath dir) {
 		if (dir != null) {
 			IPath pwd = null;
@@ -119,6 +166,13 @@ public class ErrorParserManager extends OutputStream {
 		}
 	}
 
+	/**
+	 * {@link #pushDirectory} and {@link #popDirectory} are used to change working directory
+	 * from where file name is searched (see {@link #findFileInWorkspace}).
+	 * The intention is to handle make output of commands "cd dir" and "cd ..".
+	 * 
+	 * @return previous build directory corresponding "cd .." command.
+	 */
 	public IPath popDirectory() {
 		int i = fDirectoryStack.size();
 		if (i != 0) {
@@ -129,29 +183,21 @@ public class ErrorParserManager extends OutputStream {
 		return new Path(""); //$NON-NLS-1$
 	}
 
+	/**
+	 * @return number of directories in the stack.
+	 */
 	public int getDirectoryLevel() {
 		return fDirectoryStack.size();
 	}
 
-	private void enableAllParsers() {
-		fErrorParsers = new LinkedHashMap<String, IErrorParser[]>();
-		String[] parserIDs = CCorePlugin.getDefault().getAllErrorParsersIDs();
-		for (String parserID : parserIDs) {
-			IErrorParser[] parsers = CCorePlugin.getDefault().getErrorParser(parserID);
-			fErrorParsers.put(parserID, parsers);
-		}
-		if (fErrorParsers.size() == 0) {
-			CCorePlugin.getDefault().getPluginPreferences().setValue(OLD_PREF_ERROR_PARSER, ""); // remove old prefs //$NON-NLS-1$
-		}
-	}
-
 	/**
 	 * This function used to populate member fFilesInProject which is not necessary
 	 * anymore. Now {@link ResourceLookup} is used for search and not collection of files
 	 * kept by {@code ErrorParserManager}.
+	 * @param parent - project.
+	 * @param result - resulting collection of files.
 	 * 
-	 * Use {@link #findFileName} and {@link #findFilePath} for searches.
+	 * @deprecated Use {@link #findFileName} for searches.
-	 * @deprecated
 	 */
 	@Deprecated
 	protected void collectFiles(IProject parent, final List<IResource> result) {
@@ -213,20 +259,47 @@ public class ErrorParserManager extends OutputStream {
 	}
 
 	/**
-	 * Returns the project file with the given name if that file can be uniquely identified.
+	 * Returns the file with the given name if that file can be uniquely identified.
-	 * Otherwise returns <code>null</code>.
+	 * Otherwise returns {@code null}.
+	 *
+	 * @param fileName - file name could be plain file name, absolute path or partial path
+	 * @return - file in the workspace or {@code null}.
 	 */
-    public IFile findFileName(String fileName) {
+	public IFile findFileName(String fileName) {
-    	IProject[] prjs = new IProject[] {fProject};
+		if (fileName.equals(cachedFileName))
-    	IPath path = new Path(fileName);
+			return cachedFile;
-    	IFile[] files = ResourceLookup.findFilesByName(path, prjs, false);
+
-    	if (files.length == 0)
+		IPath path = new Path(fileName);
-			files = ResourceLookup.findFilesByName(path, prjs, true);
+
-    	if (files.length == 1)
+		// Try to find exact match. If path is not absolute - searching in working directory.
-			return files[0];
+		IFile file = findFileInWorkspace(path);
-    	return null;
+
+		// Try to find best match considering known partial path
+		if (file==null && !path.isAbsolute()) {
+			IProject[] prjs = new IProject[] { fProject };
+			IFile[] files = ResourceLookup.findFilesByName(path, prjs, false);
+			if (files.length == 0)
+				files = ResourceLookup.findFilesByName(path, prjs, /* ignoreCase */ true);
+			if (files.length == 1)
+				file = files[0];
+		}
+
+		// Could be cygwin path
+		if (file==null && isCygwin && path.isAbsolute()) {
+			file = findCygwinFile(fileName);
+		}
+
+		cachedFileName = fileName;
+		cachedFile = file;
+		return file;
 	}
 
+	/**
+	 * Find exact match in the workspace. If path is not absolute search is done in working directory.
+	 * 
+	 * @param path - file path.
+	 * @return - file in the workspace or {@code null}.
+	 */
 	protected IFile findFileInWorkspace(IPath path) {
 		if (!path.isAbsolute()) {
 			path = getWorkingDirectory().append(path);
@@ -235,10 +308,10 @@ public class ErrorParserManager extends OutputStream {
 	}
 
 	/**
-	 * Use {@link #findFileName} and {@link #findFilePath} for searches.
+	 * @param fileName - file name.
+	 * @return {@code true} if the project contains more than one file with the given name.
 	 * 
-	 * Returns <code>true</code> if the project contains more than one file with the given name.
+	 * @deprecated Use {@link #findFileName} for searches.
-	 * @deprecated
 	 */
 	@Deprecated
 	public boolean isConflictingName(String fileName) {
@@ -249,44 +322,76 @@ public class ErrorParserManager extends OutputStream {
 	 * Called by the error parsers to find an IFile for a given
 	 * external filesystem 'location'
 	 * 
+	 * @param filePath - file path.
 	 * @return IFile representing the external location, or null if one 
-	 *               couldn't be found.
+	 *         couldn't be found.
+	 * 
+	 * @deprecated Use {@link #findFileName} for searches.
 	 */
+	@Deprecated
 	public IFile findFilePath(String filePath) {
 		IPath path = new Path(filePath);
 		IFile file = findFileInWorkspace(path);
 
 		// That didn't work, see if it is a cygpath
-		if (file == null) {
+		if (file == null && isCygwin) {
-			CygPath cygpath = null;
+			file = findCygwinFile(filePath);
-			try {
-				cygpath = new CygPath();
-				path = new Path(cygpath.getFileName(filePath));
-				if (fBaseDirectory.isPrefixOf(path)) {
-					int segments = fBaseDirectory.matchingFirstSegments(path);
-					path = path.removeFirstSegments(segments).setDevice(null);
-				}
-				file = findFileInWorkspace(path);
-			} catch (Exception e) {
-			}
-			finally {
-				if (cygpath != null)
-					cygpath.dispose();
-			}
 		}
 
 		return (file != null && file.exists()) ? file : null;
 	}
 
+	private IFile findCygwinFile(String filePath) {
+		IFile file=null;
+		IPath path;
+		CygPath cygpath = null;
+		try {
+			cygpath = new CygPath();
+			path = new Path(cygpath.getFileName(filePath));
+			if (fBaseDirectory.isPrefixOf(path)) {
+				int segments = fBaseDirectory.matchingFirstSegments(path);
+				path = path.removeFirstSegments(segments).setDevice(null);
+			}
+			file = findFileInWorkspace(path);
+		} catch (UnsupportedOperationException e) {
+			isCygwin = false;
+		} catch (Exception e) {
+		}
+		finally {
+			if (cygpath != null)
+				cygpath.dispose();
+		}
+		return file;
+	}
+
 	/**
-	 * Called by the error parsers.
+	 * Add marker to the list of error markers.
+	 * Markers are actually added in the end of processing in {@link #reportProblems()}.
+	 * 
+	 * @param file - resource to add the new marker.
+	 * @param lineNumber - line number of the error.
+	 * @param desc - description of the error.
+	 * @param severity - severity of the error.
+	 * @param varName - variable name.
 	 */
 	public void generateMarker(IResource file, int lineNumber, String desc, int severity, String varName) {
 		generateExternalMarker(file, lineNumber, desc, severity, varName, null);
 	}
 
 	/**
-	 * Called by the error parsers for external problem markers
+	 * Add marker to the list of error markers.
+	 * Markers are actually added in the end of processing in {@link #reportProblems()}.
+	 * 
+	 * @param file - resource to add the new marker.
+	 * @param lineNumber - line number of the error.
+	 * @param desc - description of the error.
+	 * @param severity - severity of the error, one of
+	 *        <br>{@link IMarkerGenerator#SEVERITY_INFO},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_WARNING},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_RESOURCE},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_BUILD}
+	 * @param varName - variable name.
+	 * @param externalPath - external path pointing to a file outside the workspace.
 	 */
 	public void generateExternalMarker(IResource file, int lineNumber, String desc, int severity, String varName, IPath externalPath) {
 		ProblemMarkerInfo problemMarkerInfo = new ProblemMarkerInfo(file, lineNumber, desc, severity, varName, externalPath);
@@ -296,7 +401,8 @@ public class ErrorParserManager extends OutputStream {
 	}
 
 	/**
-	 * Called by the error parsers.  Return the previous line, save in the working buffer.
+	 * Called by the error parsers.
+	 * @return the previous line, save in the working buffer.
 	 */
 	public String getPreviousLine() {
 		return new String((previousLine) == null ? "" : previousLine); //$NON-NLS-1$
@@ -304,7 +410,7 @@ public class ErrorParserManager extends OutputStream {
 
 	/**
 	 * Method setOutputStream.
-	 * @param os
+	 * @param os - output stream
 	 */
 	public void setOutputStream(OutputStream os) {
 		outputStream = os;
@@ -392,6 +498,15 @@ public class ErrorParserManager extends OutputStream {
 		}
 	}
 
+	/**
+	 * Create actual markers from the list of collected problems.
+	 * 
+	 * @return {@code true} if detected a problem indicating that build failed.
+	 *         The semantics of the return code is inconsistent. As far as build is concerned
+	 *         there is no difference between errors
+	 *         {@link IMarkerGenerator#SEVERITY_ERROR_RESOURCE} and
+	 *         {@link IMarkerGenerator#SEVERITY_ERROR_BUILD}
+	 */
 	public boolean reportProblems() {
 		boolean reset = false;
 		if (nOpens == 0) {
@@ -407,26 +522,40 @@ public class ErrorParserManager extends OutputStream {
 	}
 
 	/**
-	 * 
+	 * @return scratch buffer.
+	 * @deprecated Use IErrorParser2 interface to handle multiline messages rather than scratch buffer.
 	 */
+	@Deprecated
 	public String getScratchBuffer() {
 		return scratchBuffer.toString();
 	}
 
 	/**
-	 * @param line
+	 * @param line - input line.
+	 * @deprecated Use IErrorParser2 interface to handle multiline messages rather than scratch buffer.
 	 */
+	@Deprecated
 	public void appendToScratchBuffer(String line) {
 		scratchBuffer.append(line);
 	}
 
 	/**
-	 * 
+	 * @deprecated Use IErrorParser2 interface to handle multiline messages rather than scratch buffer.
 	 */
+	@Deprecated
 	public void clearScratchBuffer() {
 		scratchBuffer.setLength(0);
 	}
 
+	/**
+	 * @return {@code true} if errors attributed to resources detected
+	 * 
+	 * @deprecated The semantics of this function is inconsistent. As far as build is concerned
+	 *         there is no difference between errors
+	 *         {@link IMarkerGenerator#SEVERITY_ERROR_RESOURCE} and
+	 *         {@link IMarkerGenerator#SEVERITY_ERROR_BUILD}
+	 */
+	@Deprecated
 	public boolean hasErrors() {
 		return hasErrors;
 	}

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/errorparsers/ErrorPattern.java

@@ -17,6 +17,7 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.eclipse.cdt.core.ErrorParserManager;
+import org.eclipse.cdt.core.IMarkerGenerator;
 import org.eclipse.cdt.utils.CygPath;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.IPath;
@@ -36,15 +37,22 @@ public class ErrorPattern {
 	private final int groupVarName;
 	private final int severity;
 
+	private static boolean isCygwin = true;
+
 	/**
-	 * Full Pattern Constructor.
+	 * Full Pattern Constructor. Note that a group equal -1 means that
+	 * the parameter is missing in the error message.
 	 *
-	 * @param pattern
+	 * @param pattern - regular expression describing the message
-	 * @param groupFileName
+	 * @param groupFileName - matcher group of file name
-	 * @param groupLineNum
+	 * @param groupLineNum - matcher group of line number
-	 * @param groupDesc
+	 * @param groupDesc - matcher group of description
-	 * @param groupVarName
+	 * @param groupVarName - matcher group of variable name
-	 * @param severity
+	 * @param severity - severity, one of
+	 *        <br>{@link IMarkerGenerator#SEVERITY_INFO},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_WARNING},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_RESOURCE},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_BUILD}
 	 */
 	public ErrorPattern(String pattern,
 						int groupFileName,
@@ -64,9 +72,13 @@ public class ErrorPattern {
 	 * Pattern for errors not associated file a file
 	 * (e.g. make and linker errors).
 	 *
-	 * @param pattern
+	 * @param pattern - regular expression describing the message
-	 * @param groupDesc
+	 * @param groupDesc - matcher group of description
-	 * @param severity
+	 * @param severity - severity, one of
+	 *        <br>{@link IMarkerGenerator#SEVERITY_INFO},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_WARNING},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_RESOURCE},
+	 *        <br>{@link IMarkerGenerator#SEVERITY_ERROR_BUILD}
 	 */
 	public ErrorPattern(String pattern, int groupDesc, int severity) {
 		this(pattern, 0, 0, groupDesc, 0, severity);
@@ -75,19 +87,32 @@ public class ErrorPattern {
 	/**
 	 * Pattern for errors that should be skipped.
 	 * 
-	 * @param pattern
+	 * @param pattern - error pattern.
 	 */
 	public ErrorPattern(String pattern) {
 		this(pattern, 0, 0, 0, 0, -1);
 	}
+
+	/**
+	 * @param input - input line.
+	 * @return matcher to interpret the input line.
+	 */
 	public Matcher getMatcher(CharSequence input) {
 		return pattern.matcher(input);
 	}
 	
+	/**
+	 * @param matcher - matcher to parse the input line.
+	 * @return parsed file name or {code null}.
+	 */
 	public String getFileName(Matcher matcher) {
 		return groupFileName != 0 ? matcher.group(groupFileName) : null;
 	}
 	
+	/**
+	 * @param matcher - matcher to parse the input line.
+	 * @return parsed line number or {@code 0}.
+	 */
 	public int getLineNum(Matcher matcher) {
 		try {
 			return groupLineNum != 0
@@ -98,18 +123,38 @@ public class ErrorPattern {
 		}
 	}
 	
+	/**
+	 * @param matcher - matcher to parse the input line.
+	 * @return parsed description or {code null}.
+	 */
 	public String getDesc(Matcher matcher) {
 		return groupDesc != 0 ? matcher.group(groupDesc) : null;
 	}
 	
+	/**
+	 * @param matcher - matcher to parse the input line.
+	 * @return parsed variable name or {code null}.
+	 */
 	public String getVarName(Matcher matcher) {
 		return groupVarName != 0 ? matcher.group(groupVarName) : null;
 	}
 	
+	/**
+	 * @param matcher - matcher to parse the input line.
+	 * @return severity of the problem.
+	 */
 	public int getSeverity(Matcher matcher) {
 		return severity;
 	}
 	
+	/**
+	 * Parse a line of build output and register error/warning for
+	 * Problems view.
+	 * 
+	 * @param line - one line of output.
+	 * @param eoParser - {@link ErrorParserManager}.
+	 * @return {@code true} if error/warning/info problem was found.
+	 */
 	public boolean processLine(String line, ErrorParserManager eoParser) {
 		Matcher matcher = getMatcher(line);
 		if (!matcher.find())
@@ -118,6 +163,13 @@ public class ErrorPattern {
 		return recordError(matcher, eoParser);
 	}
 	
+	/**
+	 * Register the error in {@link ErrorParserManager}.
+	 * 
+	 * @param matcher - matcher to parse the input line.
+	 * @param eoParser - {@link ErrorParserManager}.
+	 * @return {@code true} indicating that error was found.
+	 */
 	protected boolean recordError(Matcher matcher, ErrorParserManager eoParser) {
 		int severity = getSeverity(matcher);
 		if (severity == -1)
@@ -132,10 +184,7 @@ public class ErrorPattern {
 		
 		IResource file = null;
 		if (fileName != null) {
-			file = eoParser.findFilePath(fileName);
+			file = eoParser.findFileName(fileName);
-			if (file == null) {
-				file = eoParser.findFileName(fileName);
-			}
 
 			if (file == null) {
 				// If the file is not found in the workspace we attach the problem to the project
@@ -152,11 +201,14 @@ public class ErrorPattern {
 	/**
 	 * If the file designated by filename exists, return the IPath representation of the filename
 	 * If it does not exist, try cygpath translation
+	 * 
+	 * @param filename - file name
+	 * @return location (outside of the workspace).
 	 */
 	protected IPath getLocation(String filename)  {
 		IPath path = new Path(filename);
 		File file = path.toFile() ;
-		if (!file.exists())  {
+		if (!file.exists() && isCygwin && path.isAbsolute())  {
 			CygPath cygpath = null ;
 			try {
 				cygpath = new CygPath("cygpath"); //$NON-NLS-1$
@@ -166,6 +218,8 @@ public class ErrorPattern {
 				if (file.exists()) {
 					path = convertedPath;
 				}
+			} catch (UnsupportedOperationException e) {
+				isCygwin = false;
 			} catch (IOException e) {
 			}
 			finally  {