Code cleanup:

- Merged CommandLauncher and ExternalToolInvoker
- Moved CppcheckChecker from project org.eclipse.codan.checkers to
org.eclipse.codan.checkers.ui
- Removed unnecessary dependencies on UI code in project
org.eclipse.codan.checkers

codan/org.eclipse.cdt.codan.checkers.ui/OSGI-INF/l10n/bundle.properties

@@ -11,3 +11,12 @@
 #Properties file for org.eclipse.cdt.codan.checkers.ui
 Bundle-Vendor = Eclipse CDT
 Bundle-Name = Codan Checkers Ui
+
+checker.name.Cppcheck = Cppcheck
+problem.description.Cppcheck.Error = Errors reported by Cppcheck (http://cppcheck.sourceforge.net/)
+problem.name.Cppcheck.Error = Errors
+problem.description.Cppcheck.Warning = Warnings reported by Cppcheck (http://cppcheck.sourceforge.net/)
+problem.name.Cppcheck.Warning = Warnings
+problem.description.Cppcheck.Syntax = Syntax problems reported by Cppcheck (http://cppcheck.sourceforge.net/)
+problem.name.Cppcheck.Syntax = Syntax Problems
+problem.messagePattern.Cppcheck.all = {0}

codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml

@@ -47,5 +47,44 @@
        </resolution>       
 
     </extension>
-
+    <extension
+         point="org.eclipse.cdt.codan.core.checkers"
+         id="org.eclipse.cdt.codan.core.internal.checkers">
+      <category
+            id="org.eclipse.cdt.codan.checkers.cppcheck"
+            name="%checker.name.Cppcheck">
+      </category>
+      <checker
+            class="org.eclipse.cdt.codan.internal.checkers.ui.CppcheckChecker"
+            id="org.eclipse.cdt.codan.checkers.CppcheckChecker"
+            name="Cppcheck">
+         <problem
+               category="org.eclipse.cdt.codan.checkers.cppcheck"
+               defaultEnabled="false"
+               defaultSeverity="Error"
+               description="%problem.description.Cppcheck.Error"
+               id="org.eclipse.cdt.codan.checkers.cppcheck.error"
+               messagePattern="%problem.messagePattern.Cppcheck.all"
+               name="%problem.name.Cppcheck.Error">
+         </problem>
+         <problem
+               category="org.eclipse.cdt.codan.checkers.cppcheck"
+               defaultEnabled="false"
+               defaultSeverity="Warning"
+               description="%problem.description.Cppcheck.Warning"
+               id="org.eclipse.cdt.codan.checkers.cppcheck.warning"
+               messagePattern="%problem.messagePattern.Cppcheck.all"
+               name="%problem.name.Cppcheck.Warning">
+         </problem>
+         <problem
+               category="org.eclipse.cdt.codan.checkers.cppcheck"
+               defaultEnabled="false"
+               defaultSeverity="Warning"
+               description="%problem.description.Cppcheck.Syntax"
+               id="org.eclipse.cdt.codan.checkers.cppcheck.style"
+               messagePattern="%problem.messagePattern.Cppcheck.all"
+               name="%problem.name.Cppcheck.Syntax">
+         </problem>
+      </checker>
+    </extension>
 </plugin>

codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/CppcheckChecker.java

@@ -8,7 +8,7 @@
  * Contributors:
  *    Alex Ruiz  - initial API and implementation
  *******************************************************************************/
-package org.eclipse.cdt.codan.internal.checkers;
+package org.eclipse.cdt.codan.internal.checkers.ui;
 
 import static java.util.Collections.singletonList;
 
@@ -21,6 +21,7 @@ import org.eclipse.cdt.codan.core.externaltool.AbstractOutputParser;
 import org.eclipse.cdt.codan.core.externaltool.ConfigurationSettings;
 import org.eclipse.cdt.codan.core.externaltool.InvocationParameters;
 import org.eclipse.cdt.codan.core.model.IProblemLocation;
+import org.eclipse.cdt.codan.internal.checkers.externaltool.CppcheckOutputParser;
 import org.eclipse.cdt.codan.ui.cxx.externaltool.AbstractCxxExternalToolBasedChecker;
 
 /**

codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF

@@ -8,11 +8,10 @@ Require-Bundle: org.eclipse.core.runtime,
  org.eclipse.core.resources;bundle-version="3.5.0",
  org.eclipse.cdt.codan.core;bundle-version="1.0.0",
  org.eclipse.cdt.core;bundle-version="5.1.0",
- org.eclipse.cdt.codan.core.cxx;bundle-version="1.0.0",
+ org.eclipse.cdt.codan.core.cxx;bundle-version="1.0.0"
- org.eclipse.cdt.codan.ui.cxx;bundle-version="3.0.0",
- org.eclipse.cdt.codan.ui;bundle-version="2.0.1"
 Bundle-ActivationPolicy: lazy
 Bundle-RequiredExecutionEnvironment: JavaSE-1.6
 Bundle-Vendor: %Bundle-Vendor
 Export-Package: org.eclipse.cdt.codan.checkers,
- org.eclipse.cdt.codan.internal.checkers;x-friends:="org.eclipse.cdt.codan.checkers.ui,org.eclipse.cdt.codan.core.test"
+ org.eclipse.cdt.codan.internal.checkers;x-friends:="org.eclipse.cdt.codan.checkers.ui,org.eclipse.cdt.codan.core.test",
+ org.eclipse.cdt.codan.internal.checkers.externaltool

codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties

@@ -127,11 +127,3 @@ problem.description.UnusedStaticFunctionProblem = Finds static functions which c
 problem.messagePattern.UnusedStaticFunctionProblem = Unused static function ''{0}''
 problem.name.UnusedStaticFunctionProblem = Unused static function
 
-checker.name.Cppcheck = Cppcheck
-problem.description.Cppcheck.Error = Errors reported by Cppcheck (http://cppcheck.sourceforge.net/)
-problem.name.Cppcheck.Error = Errors
-problem.description.Cppcheck.Warning = Warnings reported by Cppcheck (http://cppcheck.sourceforge.net/)
-problem.name.Cppcheck.Warning = Warnings
-problem.description.Cppcheck.Syntax = Syntax problems reported by Cppcheck (http://cppcheck.sourceforge.net/)
-problem.name.Cppcheck.Syntax = Syntax Problems
-problem.messagePattern.Cppcheck.all = {0}

codan/org.eclipse.cdt.codan.checkers/plugin.xml

@@ -398,41 +398,5 @@
             name="%problem.name.ClassMembersInitialization">
          </problem>
       </checker>
-      <category
-            id="org.eclipse.cdt.codan.checkers.cppcheck"
-            name="%checker.name.Cppcheck">
-      </category>
-      <checker
-            class="org.eclipse.cdt.codan.internal.checkers.CppcheckChecker"
-            id="org.eclipse.cdt.codan.checkers.CppcheckChecker"
-            name="Cppcheck">
-         <problem
-               category="org.eclipse.cdt.codan.checkers.cppcheck"
-               defaultEnabled="false"
-               defaultSeverity="Error"
-               description="%problem.description.Cppcheck.Error"
-               id="org.eclipse.cdt.codan.checkers.cppcheck.error"
-               messagePattern="%problem.messagePattern.Cppcheck.all"
-               name="%problem.name.Cppcheck.Error">
-         </problem>
-         <problem
-               category="org.eclipse.cdt.codan.checkers.cppcheck"
-               defaultEnabled="false"
-               defaultSeverity="Warning"
-               description="%problem.description.Cppcheck.Warning"
-               id="org.eclipse.cdt.codan.checkers.cppcheck.warning"
-               messagePattern="%problem.messagePattern.Cppcheck.all"
-               name="%problem.name.Cppcheck.Warning">
-         </problem>
-         <problem
-               category="org.eclipse.cdt.codan.checkers.cppcheck"
-               defaultEnabled="false"
-               defaultSeverity="Warning"
-               description="%problem.description.Cppcheck.Syntax"
-               id="org.eclipse.cdt.codan.checkers.cppcheck.style"
-               messagePattern="%problem.messagePattern.Cppcheck.all"
-               name="%problem.name.Cppcheck.Syntax">
-         </problem>
-      </checker>
    </extension>
 </plugin>

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/externaltool/CppcheckOutputParser.java

@@ -8,7 +8,7 @@
  * Contributors:
  *    Alex Ruiz  - initial API and implementation
  *******************************************************************************/
-package org.eclipse.cdt.codan.internal.checkers;
+package org.eclipse.cdt.codan.internal.checkers.externaltool;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -25,8 +25,10 @@ import org.eclipse.core.resources.IFile;
  * Parses the output of Cppcheck.
  *
  * @author alruiz@google.com (Alex Ruiz)
+ *
+ * @since 1.1
  */
-class CppcheckOutputParser extends AbstractOutputParser {
+public class CppcheckOutputParser extends AbstractOutputParser {
 	// the pattern for parsing the message is:
 	//
 	// [/src/HelloWorld.cpp:19]: (style) The scope of the variable 'i' can be reduced
@@ -43,7 +45,12 @@ class CppcheckOutputParser extends AbstractOutputParser {
 	private final InvocationParameters parameters;
 	private final IProblemDisplay problemDisplay;
 
-	CppcheckOutputParser(InvocationParameters parameters, IProblemDisplay problemDisplay) {
+	/**
+	 * Constructor.
+	 * @param parameters the parameters to pass to Cppcheck.
+	 * @param problemDisplay displays any problems reported by Cppcheck as markers.
+	 */
+	public CppcheckOutputParser(InvocationParameters parameters, IProblemDisplay problemDisplay) {
 		this.parameters = parameters;
 		this.problemDisplay = problemDisplay;
 	}

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncherTest.java

@@ -1,234 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2012 Google, 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:
- *    Alex Ruiz  - initial API and implementation
- *******************************************************************************/
-package org.eclipse.cdt.codan.internal.core.externaltool;
-
-import static java.util.Arrays.asList;
-import static org.junit.Assert.assertArrayEquals;
-
-import java.io.ByteArrayInputStream;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.util.ArrayList;
-import java.util.List;
-
-import junit.framework.TestCase;
-
-import org.eclipse.cdt.codan.core.externaltool.AbstractOutputParser;
-import org.eclipse.cdt.codan.core.externaltool.IConsolePrinter;
-import org.eclipse.cdt.codan.core.externaltool.IConsolePrinterProvider;
-import org.eclipse.cdt.codan.core.externaltool.InvocationFailure;
-import org.eclipse.core.runtime.IPath;
-import org.eclipse.core.runtime.Path;
-
-/**
- * Tests for <code>{@link CommandLauncher}</code>.
- *
- * @author alruiz@google.com (Alex Ruiz)
- */
-@SuppressWarnings("nls")
-public class CommandLauncherTest extends TestCase {
-	private String externalToolName;
-	private IPath executablePath;
-	private String[] args;
-	private boolean shouldDisplayOutput;
-	private String command;
-	private IPath workingDirectory;
-	private ConsolePrinterProviderStub consolePrinterProvider;
-	private String[] externalToolOutput;
-	private ProcessInvokerStub processInvoker;
-	private OutputParserStub outputParser;
-	private List<AbstractOutputParser> outputParsers;
-
-	private CommandLauncher commandLauncher;
-
-	@Override
-	protected void setUp() throws Exception {
-		externalToolName = "TestTool";
-		executablePath = new Path("/usr/local/testtool");
-		args = new String[] { "--include=all", "--debug=true" };
-		shouldDisplayOutput = true;
-		command = "/usr/local/testtool --include=all --debug=true";
-		workingDirectory = new Path("/usr/local/project");
-		consolePrinterProvider = new ConsolePrinterProviderStub();
-		externalToolOutput = new String[] { "line1", "line2" };
-		processInvoker = new ProcessInvokerStub(externalToolOutput);
-		outputParser = new OutputParserStub();
-		outputParsers = new ArrayList<AbstractOutputParser>();
-		outputParsers.add(outputParser);
-		commandLauncher = new CommandLauncher(consolePrinterProvider, processInvoker);
-	}
-
-	public void testInvokesProcessCorrectly() throws Throwable {
-		commandLauncher.buildAndLaunchCommand(externalToolName, executablePath, args,
-				workingDirectory, shouldDisplayOutput, outputParsers);
-		consolePrinterProvider.assertThatReceivedExternalToolName(externalToolName);
-		consolePrinterProvider.assertThatReceivedShouldDisplayOutputFlag(shouldDisplayOutput);
-		consolePrinterProvider.consolePrinter.assertThatPrinted(command, externalToolOutput);
-		consolePrinterProvider.consolePrinter.assertThatIsClosed();
-		processInvoker.assertThatReceivedCommand(command);
-		processInvoker.assertThatReceivedWorkingDirectory(workingDirectory);
-		processInvoker.process.assertThatIsDestroyed();
-		outputParser.assertThatParsed(externalToolOutput);
-	}
-
-	private static class ConsolePrinterProviderStub implements IConsolePrinterProvider {
-		private String externalToolName;
-		private boolean shouldDisplayOutput;
-
-		final ConsolePrinterStub consolePrinter = new ConsolePrinterStub();
-
-		@Override
-		public ConsolePrinterStub createConsole(String externalToolName, boolean shouldDisplayOutput) {
-			this.externalToolName = externalToolName;
-			this.shouldDisplayOutput = shouldDisplayOutput;
-			return consolePrinter;
-		}
-
-		void assertThatReceivedExternalToolName(String expected) {
-			assertEquals(expected, externalToolName);
-		}
-
-		void assertThatReceivedShouldDisplayOutputFlag(boolean expected) {
-			assertEquals(expected, shouldDisplayOutput);
-		}
-	}
-
-	private static class ConsolePrinterStub implements IConsolePrinter {
-		private final List<String> printed = new ArrayList<String>();
-		private boolean closed;
-
-		@Override
-		public void clear() {
-			printed.clear();
-		}
-
-		@Override
-		public void println(String message) {
-			printed.add(message);
-		}
-
-		@Override
-		public void println() {}
-
-		@Override
-		public void close() {
-			closed = true;
-		}
-
-		void assertThatPrinted(String command, String[] externalToolOutput) {
-			List<String> expected = new ArrayList<String>();
-			expected.add(command);
-			expected.addAll(asList(externalToolOutput));
-			assertEquals(expected, printed);
-		}
-
-		void assertThatIsClosed() {
-			assertTrue(closed);
-		}
-	}
-
-	private static class ProcessInvokerStub extends ProcessInvoker {
-		final ProcessStub process;
-
-		private String command;
-		private IPath workingDirectory;
-
-		ProcessInvokerStub(String[] externalToolOutput) {
-			process = new ProcessStub(externalToolOutput);
-		}
-
-		@Override
-		public ProcessStub invoke(String command, IPath workingDirectory) throws InvocationFailure {
-			this.command = command;
-			this.workingDirectory = workingDirectory;
-			return process;
-		}
-
-		void assertThatReceivedCommand(String expected) {
-			assertEquals(expected, command);
-		}
-
-		void assertThatReceivedWorkingDirectory(IPath expected) {
-			assertSame(expected, workingDirectory);
-		}
-	}
-
-	private static class ProcessStub extends Process {
-		private static final String LINE_SEPARATOR = System.getProperty("line.separator");
-
-		private final InputStream inputStream;
-		private final InputStream errorStream;
-
-		private boolean destroyed;
-
-		ProcessStub(String[] externalToolOutput) {
-			StringBuilder builder = new StringBuilder();
-			for (String s : externalToolOutput) {
-				builder.append(s).append(LINE_SEPARATOR);
-			}
-			inputStream = new ByteArrayInputStream(builder.toString().getBytes());
-			errorStream = new ByteArrayInputStream(new byte[0]);
-		}
-
-		@Override
-		public OutputStream getOutputStream() {
-			throw new UnsupportedOperationException();
-		}
-
-		@Override
-		public InputStream getInputStream() {
-			return inputStream;
-		}
-
-		@Override
-		public InputStream getErrorStream() {
-			return errorStream;
-		}
-
-		@Override
-		public int waitFor() {
-			throw new UnsupportedOperationException();
-		}
-
-		@Override
-		public int exitValue() {
-			throw new UnsupportedOperationException();
-		}
-
-		@Override
-		public void destroy() {
-			destroyed = true;
-		}
-
-		void assertThatIsDestroyed() {
-			assertTrue(destroyed);
-		}
-	}
-
-	private static class OutputParserStub extends AbstractOutputParser {
-		private final List<String> parsed = new ArrayList<String>();
-
-		@Override
-		public boolean parse(String line) throws InvocationFailure {
-			parsed.add(line);
-			return true;
-		}
-
-		@Override
-		public void reset() {
-			throw new UnsupportedOperationException();
-		}
-
-		void assertThatParsed(String[] expected) {
-			assertArrayEquals(expected, parsed.toArray());
-		}
-	}
-}

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvokerTest.java

@@ -13,22 +13,27 @@ package org.eclipse.cdt.codan.internal.core.externaltool;
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertArrayEquals;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.cdt.codan.core.externaltool.AbstractOutputParser;
 import org.eclipse.cdt.codan.core.externaltool.ConfigurationSettings;
 import org.eclipse.cdt.codan.core.externaltool.IArgsSeparator;
+import org.eclipse.cdt.codan.core.externaltool.IConsolePrinter;
+import org.eclipse.cdt.codan.core.externaltool.IConsolePrinterProvider;
 import org.eclipse.cdt.codan.core.externaltool.InvocationFailure;
 import org.eclipse.cdt.codan.core.externaltool.InvocationParameters;
-import org.eclipse.cdt.codan.core.externaltool.SingleConfigurationSetting;
 import org.eclipse.cdt.codan.core.externaltool.SpaceDelimitedArgsSeparator;
 import org.eclipse.cdt.codan.core.param.BasicProblemPreference;
 import org.eclipse.cdt.codan.core.param.IProblemPreference;
 import org.eclipse.cdt.codan.core.param.MapProblemPreference;
 import org.eclipse.cdt.codan.core.test.CodanTestCase;
 import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.Path;
 
 /**
  * Tests for <code>{@link ExternalToolInvoker}</code>.
@@ -37,35 +42,45 @@ import org.eclipse.core.runtime.IPath;
  */
 @SuppressWarnings("nls")
 public class ExternalToolInvokerTest extends CodanTestCase {
+	private String externalToolName;
+	private String externalToolPath;
+	private boolean shouldDisplayOutput;
+	private IPath workingDirectory;
+	private ConsolePrinterProviderStub consolePrinterProvider;
+	private String[] externalToolOutput;
+	private ProcessInvokerStub processInvoker;
+	private OutputParserStub outputParser;
+	private List<AbstractOutputParser> outputParsers;
 	private ConfigurationSettings settings;
 	private IArgsSeparator argsSeparator;
-	private List<AbstractOutputParser> parsers;
-	private CommandLauncherStub launcher;
 
 	private ExternalToolInvoker externalToolInvoker;
 
 	@Override
 	public void setUp() throws Exception {
 		super.setUp();
-		createConfigurationSettings();
+		externalToolName = "TestTool";
-		argsSeparator = new SpaceDelimitedArgsSeparator();
+		externalToolPath = "/usr/local/testtool";
-		parsers = new ArrayList<AbstractOutputParser>();
+		shouldDisplayOutput = true;
-		launcher = new CommandLauncherStub();
+		workingDirectory = new Path("/usr/local/project");
-		externalToolInvoker = new ExternalToolInvoker(launcher);
+		consolePrinterProvider = new ConsolePrinterProviderStub();
-	}
+		externalToolOutput = new String[] { "line1", "line2" };
-
+		processInvoker = new ProcessInvokerStub(externalToolOutput);
-	private void createConfigurationSettings() {
+		outputParser = new OutputParserStub();
-		settings = new ConfigurationSettings("TestTool", new File("testtool"), "", false);
+		outputParsers = new ArrayList<AbstractOutputParser>();
-		// Update current value of ConfigurationSettings from preferences.
+		outputParsers.add(outputParser);
-		MapProblemPreference preferences = createPreferences(new File("usr/local/testtool"),
+		settings = new ConfigurationSettings(externalToolName, new File("testtool"), "", false);
-				"--debug=true --include=all", true);
+		MapProblemPreference preferences = createPreferences(new File(externalToolPath),
+				"--include=all --debug=true", shouldDisplayOutput);
 		settings.updateValuesFrom(preferences);
+		argsSeparator = new SpaceDelimitedArgsSeparator();
+		externalToolInvoker = new ExternalToolInvoker(consolePrinterProvider, processInvoker);
 	}
 
-	private MapProblemPreference createPreferences(File path, String args,
+	private MapProblemPreference createPreferences(File executablePath, String args,
 			boolean shouldDisplayOutput) {
 		MapProblemPreference preferences = new MapProblemPreference();
-		preferences.addChildDescriptor(createPreference(PathSetting.KEY, path));
+		preferences.addChildDescriptor(createPreference(PathSetting.KEY, executablePath));
 		preferences.addChildDescriptor(createPreference(ArgsSetting.KEY, args));
 		preferences.addChildDescriptor(
 				createPreference(ShouldDisplayOutputSetting.KEY, shouldDisplayOutput));
@@ -85,74 +100,190 @@ public class ExternalToolInvokerTest extends CodanTestCase {
 
 	// class C {
 	// };
-	public void testCommandLauncherGetsCalledCorrectly() throws Throwable {
+	public void testInvokesProcessCorrectly() throws Throwable {
 		loadcode(getAboveComment());
+		String expectedCommand = expectedCommand();
 		InvocationParameters parameters = new InvocationParameters(currentIFile, currentIFile,
-				currentIFile.getLocation().toOSString(), cproject.getProject().getLocation());
+				actualFilePath(), workingDirectory);
-		externalToolInvoker.invoke(parameters, settings, argsSeparator, parsers);
+		externalToolInvoker.invoke(parameters, settings, argsSeparator, outputParsers);
-		launcher.assertThatReceivedExternalToolName(settings.getExternalToolName());
+		consolePrinterProvider.assertThatReceivedExternalToolName(externalToolName);
-		launcher.assertThatReceivedExecutablePath(settings.getPath());
+		consolePrinterProvider.assertThatReceivedShouldDisplayOutputFlag(shouldDisplayOutput);
-		launcher.assertThatReceivedArgs(expectedArgs(parameters));
+		consolePrinterProvider.consolePrinter.assertThatPrinted(expectedCommand, externalToolOutput);
-		launcher.assertThatReceivedWorkingDirectory(parameters.getWorkingDirectory());
+		consolePrinterProvider.consolePrinter.assertThatIsClosed();
-		launcher.assertThatReceivedShouldDisplayOutput(
+		processInvoker.assertThatReceivedCommand(expectedCommand);
-				settings.getShouldDisplayOutput());
+		processInvoker.assertThatReceivedWorkingDirectory(workingDirectory);
-		launcher.assertThatReceivedOutputParsers(parsers);
+		processInvoker.process.assertThatIsDestroyed();
+		outputParser.assertThatParsed(externalToolOutput);
+		outputParser.assertThatWasReset();
 	}
 
-	private List<String> expectedArgs(InvocationParameters parameters) {
+	private String expectedCommand() {
-		String[] originalArgs = settings.getArgs().getValue().split("\\s+");
+		StringBuilder commandBuilder = new StringBuilder();
-		List<String> expectedArgs = new ArrayList<String>(asList(originalArgs));
+		commandBuilder.append(externalToolPath).append(" ")
-		expectedArgs.add(0, parameters.getActualFilePath());
+		              .append(actualFilePath()).append(" ")
-		return expectedArgs;
+		              .append(settings.getArgs().getValue());
+		return commandBuilder.toString();
 	}
 
-	private static class CommandLauncherStub extends CommandLauncher {
+	private String actualFilePath() {
+		return currentIFile.getLocation().toOSString();
+	}
+
+	private static class ConsolePrinterProviderStub implements IConsolePrinterProvider {
 		private String externalToolName;
-		private IPath executablePath;
-		private String[] args;
-		private IPath workingDirectory;
 		private boolean shouldDisplayOutput;
-		private List<AbstractOutputParser> parsers;
 
-		public CommandLauncherStub() {
+		final ConsolePrinterStub consolePrinter = new ConsolePrinterStub();
-			super(null);
-		}
 
 		@Override
-		public void buildAndLaunchCommand(String externalToolName, IPath executablePath,
+		public ConsolePrinterStub createConsole(String externalToolName, boolean shouldDisplayOutput) {
-				String[] args, IPath workingDirectory, boolean shouldDisplayOutput,
-				List<AbstractOutputParser> parsers) throws InvocationFailure, Throwable {
 			this.externalToolName = externalToolName;
-			this.executablePath = executablePath;
-			this.args = args;
-			this.workingDirectory = workingDirectory;
 			this.shouldDisplayOutput = shouldDisplayOutput;
-			this.parsers = parsers;
+			return consolePrinter;
 		}
 
 		void assertThatReceivedExternalToolName(String expected) {
 			assertEquals(expected, externalToolName);
 		}
 
-		void assertThatReceivedExecutablePath(SingleConfigurationSetting<File> expected) {
+		void assertThatReceivedShouldDisplayOutputFlag(boolean expected) {
-			String expectedPath = expected.getValue().toString();
+			assertEquals(expected, shouldDisplayOutput);
-			assertEquals(expectedPath, executablePath.toOSString());
+		}
 	}
 
-		void assertThatReceivedArgs(List<String> expected) {
+	private static class ConsolePrinterStub implements IConsolePrinter {
-			assertArrayEquals(expected.toArray(), args);
+		private final List<String> printed = new ArrayList<String>();
+		private boolean closed;
+
+		@Override
+		public void clear() {
+			printed.clear();
+		}
+
+		@Override
+		public void println(String message) {
+			printed.add(message);
+		}
+
+		@Override
+		public void println() {}
+
+		@Override
+		public void close() {
+			closed = true;
+		}
+
+		void assertThatPrinted(String command, String[] externalToolOutput) {
+			List<String> expected = new ArrayList<String>();
+			expected.add(command);
+			expected.addAll(asList(externalToolOutput));
+			assertEquals(expected, printed);
+		}
+
+		void assertThatIsClosed() {
+			assertTrue(closed);
+		}
+	}
+
+	private static class ProcessInvokerStub extends ProcessInvoker {
+		final ProcessStub process;
+
+		private String command;
+		private IPath workingDirectory;
+
+		ProcessInvokerStub(String[] externalToolOutput) {
+			process = new ProcessStub(externalToolOutput);
+		}
+
+		@Override
+		public ProcessStub invoke(String command, IPath workingDirectory) throws InvocationFailure {
+			this.command = command;
+			this.workingDirectory = workingDirectory;
+			return process;
+		}
+
+		void assertThatReceivedCommand(String expected) {
+			assertEquals(expected, command);
 		}
 
 		void assertThatReceivedWorkingDirectory(IPath expected) {
 			assertSame(expected, workingDirectory);
 		}
-
-		void assertThatReceivedShouldDisplayOutput(SingleConfigurationSetting<Boolean> expected) {
-			assertEquals(expected.getValue().booleanValue(), shouldDisplayOutput);
 	}
 
-		void assertThatReceivedOutputParsers(List<AbstractOutputParser> expected) {
+	private static class ProcessStub extends Process {
-			assertSame(expected, parsers);
+		private static final String LINE_SEPARATOR = System.getProperty("line.separator");
+
+		private final InputStream inputStream;
+		private final InputStream errorStream;
+
+		private boolean destroyed;
+
+		ProcessStub(String[] externalToolOutput) {
+			StringBuilder builder = new StringBuilder();
+			for (String s : externalToolOutput) {
+				builder.append(s).append(LINE_SEPARATOR);
+			}
+			inputStream = new ByteArrayInputStream(builder.toString().getBytes());
+			errorStream = new ByteArrayInputStream(new byte[0]);
+		}
+
+		@Override
+		public OutputStream getOutputStream() {
+			throw new UnsupportedOperationException();
+		}
+
+		@Override
+		public InputStream getInputStream() {
+			return inputStream;
+		}
+
+		@Override
+		public InputStream getErrorStream() {
+			return errorStream;
+		}
+
+		@Override
+		public int waitFor() {
+			throw new UnsupportedOperationException();
+		}
+
+		@Override
+		public int exitValue() {
+			throw new UnsupportedOperationException();
+		}
+
+		@Override
+		public void destroy() {
+			destroyed = true;
+		}
+
+		void assertThatIsDestroyed() {
+			assertTrue(destroyed);
+		}
+	}
+
+	private static class OutputParserStub extends AbstractOutputParser {
+		private final List<String> parsed = new ArrayList<String>();
+		private boolean reset;
+
+		@Override
+		public boolean parse(String line) throws InvocationFailure {
+			parsed.add(line);
+			return true;
+		}
+
+		@Override
+		public void reset() {
+			reset = true;
+		}
+
+		void assertThatParsed(String[] expected) {
+			assertArrayEquals(expected, parsed.toArray());
+		}
+
+		void assertThatWasReset() {
+			assertTrue(reset);
 		}
 	}
 }

codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncher.java

@@ -1,124 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2012 Google, 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:
- *    Alex Ruiz  - initial API and implementation
- *******************************************************************************/
-package org.eclipse.cdt.codan.internal.core.externaltool;
-
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.util.List;
-
-import org.eclipse.cdt.codan.core.externaltool.AbstractOutputParser;
-import org.eclipse.cdt.codan.core.externaltool.IConsolePrinter;
-import org.eclipse.cdt.codan.core.externaltool.IConsolePrinterProvider;
-import org.eclipse.cdt.codan.core.externaltool.InvocationFailure;
-import org.eclipse.core.runtime.IPath;
-
-/**
- * Invokes an external tool command.
- *
- * @author alruiz@google.com (Alex Ruiz)
- *
- * @since 2.1
- */
-public class CommandLauncher {
-	private final IConsolePrinterProvider consolePrinterProvider;
-	private final ProcessInvoker processInvoker;
-
-	/**
-	 * Constructor.
-	 * @param consolePrinterProvider creates an Eclipse console that uses the name of an external
-	 *        tool as its own.
-	 */
-	public CommandLauncher(IConsolePrinterProvider consolePrinterProvider) {
-		this(consolePrinterProvider, new ProcessInvoker());
-	}
-
-	/**
-	 * Constructor.
-	 * @param consolePrinterProvider creates an Eclipse console that uses the name of an external
-	 *        tool as its own.
-	 * @param processInvoker executes a command in a separate process.
-	 */
-	public CommandLauncher(IConsolePrinterProvider consolePrinterProvider,
-			ProcessInvoker processInvoker) {
-		this.consolePrinterProvider = consolePrinterProvider;
-		this.processInvoker = processInvoker;
-	}
-
-	/**
-	 * Builds and launches the command necessary to invoke an external tool.
-	 * @param externalToolName the name of the external tool.
-	 * @param executablePath the path and name of the external tool executable.
-	 * @param args the arguments to pass to the external tool executable.
-	 * @param workingDirectory the directory where the external tool should be executed.
-	 * @param shouldDisplayOutput indicates whether the output of the external tools should be
-	 *        displayed in an Eclipse console.
-	 * @param parsers parse the output of the external tool.
-	 * @throws InvocationFailure if the external tool cannot be executed.
-	 * @throws Throwable if the external tool cannot be launched.
-	 */
-	public void buildAndLaunchCommand(String externalToolName, IPath executablePath, String[] args,
-			IPath workingDirectory, boolean shouldDisplayOutput, List<AbstractOutputParser> parsers)
-					throws InvocationFailure, Throwable {
-		String command = buildCommand(executablePath, args);
-		Process process = null;
-		IConsolePrinter console =
-				consolePrinterProvider.createConsole(externalToolName, shouldDisplayOutput);
-		try {
-			console.clear();
-			console.println(command);
-			console.println();
-			process = processInvoker.invoke(command, workingDirectory);
-			processStream(process.getInputStream(), parsers, console);
-			processStream(process.getErrorStream(), parsers, console);
-		} finally {
-			if (process != null) {
-				process.destroy();
-			}
-			console.close();
-		}
-	}
-
-	private String buildCommand(IPath executablePath, String[] args) {
-		StringBuilder builder = new StringBuilder();
-		builder.append(executablePath.toOSString());
-		for (String arg : args) {
-			builder.append(" ").append(arg); //$NON-NLS-1$
-		}
-		return builder.toString();
-	}
-
-	private void processStream(InputStream inputStream, List<AbstractOutputParser> parsers,
-			IConsolePrinter console) throws IOException, InvocationFailure {
-		Reader reader = null;
-		try {
-			reader = new InputStreamReader(inputStream);
-			BufferedReader bufferedReader = new BufferedReader(reader);
-			String line = null;
-			while ((line = bufferedReader.readLine()) != null) {
-				console.println(line);
-				for (AbstractOutputParser parser : parsers) {
-					if (parser.parse(line)) {
-						break;
-					}
-				}
-			}
-		} finally {
-			if (reader != null) {
-				try {
-					reader.close();
-				} catch (IOException ignored) {}
-			}
-		}
-	}
-}

codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvoker.java

@@ -10,12 +10,18 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.core.externaltool;
 
+import java.io.BufferedReader;
 import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
 import java.util.List;
 
 import org.eclipse.cdt.codan.core.externaltool.AbstractOutputParser;
 import org.eclipse.cdt.codan.core.externaltool.ConfigurationSettings;
 import org.eclipse.cdt.codan.core.externaltool.IArgsSeparator;
+import org.eclipse.cdt.codan.core.externaltool.IConsolePrinter;
 import org.eclipse.cdt.codan.core.externaltool.IConsolePrinterProvider;
 import org.eclipse.cdt.codan.core.externaltool.InvocationFailure;
 import org.eclipse.cdt.codan.core.externaltool.InvocationParameters;
@@ -30,7 +36,8 @@ import org.eclipse.core.runtime.Path;
  * @since 2.1
  */
 public class ExternalToolInvoker {
-	private final CommandLauncher commandLauncher;
+	private final IConsolePrinterProvider consolePrinterProvider;
+	private final ProcessInvoker processInvoker;
 
 	/**
 	 * Constructor.
@@ -38,15 +45,19 @@ public class ExternalToolInvoker {
 	 *        tool as its own.
 	 */
 	public ExternalToolInvoker(IConsolePrinterProvider consolePrinterProvider) {
-		this(new CommandLauncher(consolePrinterProvider));
+		this(consolePrinterProvider, new ProcessInvoker());
 	}
 
 	/**
 	 * Constructor.
-	 * @param commandLauncher invokes an external tool command.
+	 * @param consolePrinterProvider creates an Eclipse console that uses the name of an external
+	 *        tool as its own.
+	 * @param processInvoker executes a command in a separate process.
 	 */
-	public ExternalToolInvoker(CommandLauncher commandLauncher) {
+	public ExternalToolInvoker(IConsolePrinterProvider consolePrinterProvider,
-		this.commandLauncher = commandLauncher;
+			ProcessInvoker processInvoker) {
+		this.consolePrinterProvider = consolePrinterProvider;
+		this.processInvoker = processInvoker;
 	}
 
 	/**
@@ -67,9 +78,8 @@ public class ExternalToolInvoker {
 		String[] args = argsToPass(parameters, configurationSettings, argsSeparator);
 		boolean shouldDisplayOutput = configurationSettings.getShouldDisplayOutput().getValue();
 		try {
-			commandLauncher.buildAndLaunchCommand(configurationSettings.getExternalToolName(),
+			buildAndLaunchCommand(configurationSettings.getExternalToolName(), executablePath, args,
-					executablePath, args, parameters.getWorkingDirectory(), shouldDisplayOutput,
+					parameters.getWorkingDirectory(), shouldDisplayOutput, parsers);
-					parsers);
 		} finally {
 			reset(parsers);
 		}
@@ -104,6 +114,60 @@ public class ExternalToolInvoker {
 		return allArgs;
 	}
 
+	private void buildAndLaunchCommand(String externalToolName, IPath executablePath, String[] args,
+			IPath workingDirectory, boolean shouldDisplayOutput, List<AbstractOutputParser> parsers)
+					throws InvocationFailure, Throwable {
+		String command = buildCommand(executablePath, args);
+		Process process = null;
+		IConsolePrinter console =
+				consolePrinterProvider.createConsole(externalToolName, shouldDisplayOutput);
+		try {
+			console.clear();
+			console.println(command);
+			console.println();
+			process = processInvoker.invoke(command, workingDirectory);
+			processStream(process.getInputStream(), parsers, console);
+			processStream(process.getErrorStream(), parsers, console);
+		} finally {
+			if (process != null) {
+				process.destroy();
+			}
+			console.close();
+		}
+	}
+
+	private String buildCommand(IPath executablePath, String[] args) {
+		StringBuilder builder = new StringBuilder();
+		builder.append(executablePath.toOSString());
+		for (String arg : args) {
+			builder.append(" ").append(arg); //$NON-NLS-1$
+		}
+		return builder.toString();
+	}
+
+	private void processStream(InputStream inputStream, List<AbstractOutputParser> parsers,
+			IConsolePrinter console) throws IOException, InvocationFailure {
+		Reader reader = null;
+		try {
+			reader = new InputStreamReader(inputStream);
+			BufferedReader bufferedReader = new BufferedReader(reader);
+			String line = null;
+			while ((line = bufferedReader.readLine()) != null) {
+				console.println(line);
+				for (AbstractOutputParser parser : parsers) {
+					if (parser.parse(line)) {
+						break;
+					}
+				}
+			}
+		} finally {
+			if (reader != null) {
+				try {
+					reader.close();
+				} catch (IOException ignored) {}
+			}
+		}
+	}
 	private void reset(List<AbstractOutputParser> parsers) {
 		for (AbstractOutputParser parser : parsers) {
 			parser.reset();