From f405944e2d0434f6dd8b7af1aa052ec8de01c7b7 Mon Sep 17 00:00:00 2001 From: Alex Ruiz Date: Thu, 23 Feb 2012 13:24:18 -0800 Subject: [PATCH] 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 --- .../OSGI-INF/l10n/bundle.properties | 11 +- .../plugin.xml | 41 ++- .../checkers/ui}/CppcheckChecker.java | 3 +- .../META-INF/MANIFEST.MF | 7 +- .../OSGI-INF/l10n/bundle.properties | 8 - .../org.eclipse.cdt.codan.checkers/plugin.xml | 36 --- .../CppcheckOutputParser.java | 13 +- .../externaltool/CommandLauncherTest.java | 234 ----------------- .../externaltool/ExternalToolInvokerTest.java | 243 ++++++++++++++---- .../core/externaltool/CommandLauncher.java | 124 --------- .../externaltool/ExternalToolInvoker.java | 80 +++++- 11 files changed, 324 insertions(+), 476 deletions(-) rename codan/{org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers => org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui}/CppcheckChecker.java (95%) rename codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/{ => externaltool}/CppcheckOutputParser.java (86%) delete mode 100644 codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncherTest.java delete mode 100644 codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncher.java diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers.ui/OSGI-INF/l10n/bundle.properties index e0f2a06a2ea..6b16f775995 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/OSGI-INF/l10n/bundle.properties +++ b/codan/org.eclipse.cdt.codan.checkers.ui/OSGI-INF/l10n/bundle.properties @@ -10,4 +10,13 @@ ############################################################################### #Properties file for org.eclipse.cdt.codan.checkers.ui Bundle-Vendor = Eclipse CDT -Bundle-Name = Codan Checkers Ui \ No newline at end of file +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} diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml b/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml index 0d9a0bfa5c2..aaf3f26c54c 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml @@ -47,5 +47,44 @@ - + + + + + + + + + + + + diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckChecker.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/CppcheckChecker.java similarity index 95% rename from codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckChecker.java rename to codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/CppcheckChecker.java index 68eb51fa26f..1c5f672a890 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckChecker.java +++ b/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; /** diff --git a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF b/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF index 016debe39c0..71773c353f7 100644 --- a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF +++ b/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.ui.cxx;bundle-version="3.0.0", - org.eclipse.cdt.codan.ui;bundle-version="2.0.1" + org.eclipse.cdt.codan.core.cxx;bundle-version="1.0.0" 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 diff --git a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties index 021c95cddec..c5ebf9beed5 100644 --- a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties +++ b/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} diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml index ab04c268e7c..0f3f1c76678 100644 --- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml @@ -398,41 +398,5 @@ name="%problem.name.ClassMembersInitialization"> - - - - - - - - - - diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckOutputParser.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/externaltool/CppcheckOutputParser.java similarity index 86% rename from codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckOutputParser.java rename to codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/externaltool/CppcheckOutputParser.java index 6fc05439ea5..81a237d65a9 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CppcheckOutputParser.java +++ b/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; } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncherTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncherTest.java deleted file mode 100644 index dcd2faba891..00000000000 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncherTest.java +++ /dev/null @@ -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 {@link CommandLauncher}. - * - * @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 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(); - 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 printed = new ArrayList(); - 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 expected = new ArrayList(); - 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 parsed = new ArrayList(); - - @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()); - } - } -} diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvokerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvokerTest.java index 3b804103690..ef86e72194c 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvokerTest.java +++ b/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 {@link ExternalToolInvoker}. @@ -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 outputParsers; private ConfigurationSettings settings; private IArgsSeparator argsSeparator; - private List parsers; - private CommandLauncherStub launcher; private ExternalToolInvoker externalToolInvoker; @Override public void setUp() throws Exception { super.setUp(); - createConfigurationSettings(); - argsSeparator = new SpaceDelimitedArgsSeparator(); - parsers = new ArrayList(); - launcher = new CommandLauncherStub(); - externalToolInvoker = new ExternalToolInvoker(launcher); - } - - private void createConfigurationSettings() { - settings = new ConfigurationSettings("TestTool", new File("testtool"), "", false); - // Update current value of ConfigurationSettings from preferences. - MapProblemPreference preferences = createPreferences(new File("usr/local/testtool"), - "--debug=true --include=all", true); + externalToolName = "TestTool"; + externalToolPath = "/usr/local/testtool"; + shouldDisplayOutput = 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(); + outputParsers.add(outputParser); + settings = new ConfigurationSettings(externalToolName, new File("testtool"), "", false); + 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()); - externalToolInvoker.invoke(parameters, settings, argsSeparator, parsers); - launcher.assertThatReceivedExternalToolName(settings.getExternalToolName()); - launcher.assertThatReceivedExecutablePath(settings.getPath()); - launcher.assertThatReceivedArgs(expectedArgs(parameters)); - launcher.assertThatReceivedWorkingDirectory(parameters.getWorkingDirectory()); - launcher.assertThatReceivedShouldDisplayOutput( - settings.getShouldDisplayOutput()); - launcher.assertThatReceivedOutputParsers(parsers); + actualFilePath(), workingDirectory); + externalToolInvoker.invoke(parameters, settings, argsSeparator, outputParsers); + consolePrinterProvider.assertThatReceivedExternalToolName(externalToolName); + consolePrinterProvider.assertThatReceivedShouldDisplayOutputFlag(shouldDisplayOutput); + consolePrinterProvider.consolePrinter.assertThatPrinted(expectedCommand, externalToolOutput); + consolePrinterProvider.consolePrinter.assertThatIsClosed(); + processInvoker.assertThatReceivedCommand(expectedCommand); + processInvoker.assertThatReceivedWorkingDirectory(workingDirectory); + processInvoker.process.assertThatIsDestroyed(); + outputParser.assertThatParsed(externalToolOutput); + outputParser.assertThatWasReset(); } - private List expectedArgs(InvocationParameters parameters) { - String[] originalArgs = settings.getArgs().getValue().split("\\s+"); - List expectedArgs = new ArrayList(asList(originalArgs)); - expectedArgs.add(0, parameters.getActualFilePath()); - return expectedArgs; + private String expectedCommand() { + StringBuilder commandBuilder = new StringBuilder(); + commandBuilder.append(externalToolPath).append(" ") + .append(actualFilePath()).append(" ") + .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 parsers; - public CommandLauncherStub() { - super(null); - } + final ConsolePrinterStub consolePrinter = new ConsolePrinterStub(); @Override - public void buildAndLaunchCommand(String externalToolName, IPath executablePath, - String[] args, IPath workingDirectory, boolean shouldDisplayOutput, - List parsers) throws InvocationFailure, Throwable { + public ConsolePrinterStub createConsole(String externalToolName, boolean shouldDisplayOutput) { 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 expected) { - String expectedPath = expected.getValue().toString(); - assertEquals(expectedPath, executablePath.toOSString()); + void assertThatReceivedShouldDisplayOutputFlag(boolean expected) { + assertEquals(expected, shouldDisplayOutput); + } + } + + private static class ConsolePrinterStub implements IConsolePrinter { + private final List printed = new ArrayList(); + private boolean closed; + + @Override + public void clear() { + printed.clear(); } - void assertThatReceivedArgs(List expected) { - assertArrayEquals(expected.toArray(), args); + @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 expected = new ArrayList(); + 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 expected) { - assertEquals(expected.getValue().booleanValue(), shouldDisplayOutput); + 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]); } - void assertThatReceivedOutputParsers(List expected) { - assertSame(expected, parsers); + @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 parsed = new ArrayList(); + 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); } } } diff --git a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncher.java b/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncher.java deleted file mode 100644 index 8ab1c9dca53..00000000000 --- a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/CommandLauncher.java +++ /dev/null @@ -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 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 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) {} - } - } - } -} diff --git a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvoker.java b/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvoker.java index 7165a69740c..5d7d0ab8691 100644 --- a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/externaltool/ExternalToolInvoker.java +++ b/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) { - this.commandLauncher = commandLauncher; + public ExternalToolInvoker(IConsolePrinterProvider consolePrinterProvider, + 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(), - executablePath, args, parameters.getWorkingDirectory(), shouldDisplayOutput, - parsers); + buildAndLaunchCommand(configurationSettings.getExternalToolName(), executablePath, args, + parameters.getWorkingDirectory(), shouldDisplayOutput, 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 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 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 parsers) { for (AbstractOutputParser parser : parsers) { parser.reset();