Systemtap: Forbid running invalid launches.

Display an error message when trying to launch a Run
Configuration that is invalid (ie a configuration that
is marked as invalid in the Run Configurations menu).
Also perform some UI fixes to the Graphing menu.

Change-Id: Ifc04a95a95f6849d94a1226e8b79a05d30b30af3
Signed-off-by: Andrew Ferrazzutti <aferrazz@redhat.com>
Reviewed-on: https://git.eclipse.org/r/19210
Reviewed-by: Jeff Johnston <jjohnstn@redhat.com>
IP-Clean: Jeff Johnston <jjohnstn@redhat.com>
Tested-by: Jeff Johnston <jjohnstn@redhat.com>
Reviewed-on: https://git.eclipse.org/r/19337

6 files changed, 124 insertions, 27 deletions

diff --git a/systemtap/org.eclipse.linuxtools.systemtap.graphingapi.ui/src/org/eclipse/linuxtools/systemtap/graphingapi/ui/wizards/graph/SelectGraphAndSeriesWizardPage.java b/systemtap/org.eclipse.linuxtools.systemtap.graphingapi.ui/src/org/eclipse/linuxtools/systemtap/graphingapi/ui/wizards/graph/SelectGraphAndSeriesWizardPage.java
index 227488af25..4f72e37baf 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.graphingapi.ui/src/org/eclipse/linuxtools/systemtap/graphingapi/ui/wizards/graph/SelectGraphAndSeriesWizardPage.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.graphingapi.ui/src/org/eclipse/linuxtools/systemtap/graphingapi/ui/wizards/graph/SelectGraphAndSeriesWizardPage.java
@@ -263,10 +263,15 @@ public class SelectGraphAndSeriesWizardPage extends WizardPage implements Listen
 
 		for(int j,i=0; i<cboYItems.length; i++) {
 			if(cboYItems[i].isVisible() && !deleted[i+1]) {
+
+				// Find duplicates by comparing selection indices. Every combo has an
+				// extra selection before column names (Row Num or NA), except Y-series 0.
+				int offset = (i == 0 ? 1 : 0);
+
 				for(j=i+1; j<cboYItems.length; j++) {
 					try {
-						if(!deleted[j+1] && cboYItems[j].isVisible() && cboYItems[i].getItem(cboYItems[i].getSelectionIndex())
-								.equals(cboYItems[j].getItem(cboYItems[j].getSelectionIndex()))) {
+						if(!deleted[j+1] && cboYItems[j].isVisible()
+									&& cboYItems[i].getSelectionIndex() + offset == cboYItems[j].getSelectionIndex()) {
 								markAsDuplicate(cboYItems[i], true);
 								markAsDuplicate(cboYItems[j], true);
 								foundDuplicate = true;
@@ -275,8 +280,13 @@ public class SelectGraphAndSeriesWizardPage extends WizardPage implements Listen
 						// If a cboYItem has no item selected, don't mark any duplicates. Ignore.
 					}
 				}
+
+				if (deleted[0]) {
+					continue;
+				}
 				try {
-					if(!deleted[0] && cboYItems[i].getItem(cboYItems[i].getSelectionIndex()).equals(cboXItem.getItem(cboXItem.getSelectionIndex()))) {
+					int selection = cboYItems[i].getSelectionIndex() + offset;
+					if(selection != 0 && selection == cboXItem.getSelectionIndex()) {
 						markAsDuplicate(cboYItems[i], true);
 						markAsDuplicate(cboXItem, true);
 						foundDuplicate = true;
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/Messages.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/Messages.java
index 8805beaf50..6928755f27 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/Messages.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/Messages.java
@@ -42,6 +42,7 @@ public class Messages extends NLS {
 	public static String SystemTapScriptGraphOptionsTab_RemoveGraphButtonToolTip;
 	public static String SystemTapScriptGraphOptionsTab_invalidGraph;
 
+	public static String SystemTapScriptGraphOptionsTab_regexErrorMsgFormat;
 	public static String SystemTapScriptGraphOptionsTab_regexLabel;
 	public static String SystemTapScriptGraphOptionsTab_regexTooltip;
 	public static String SystemTapScriptGraphOptionsTab_regexAddNew;
@@ -74,7 +75,7 @@ public class Messages extends NLS {
 	public static String SystemTapScriptLaunchConfigurationTab_runWithChart;
 	public static String SystemTapScriptLaunchShortcut_couldNotFindConfig;
 	public static String SystemTapScriptLaunchShortcut_couldNotLaunchScript;
-	public static String SystemTapScriptLaunchConfigurationTab_couldNotFindFile;
+	public static String SystemTapScriptLaunchConfigurationTab_fileNotFound;
 	public static String SystemTapScriptLaunchConfigurationTab_fileNotStp;
 	public static String SystemTapScriptOptionsTab_0;
 	public static String SystemTapScriptOptionsTab_1;
@@ -85,6 +86,10 @@ public class Messages extends NLS {
 	public static String SystemTapScriptOptionsTab_initializeConfigurationFailed;
 	public static String SystemTapScriptOptionsTab_targetToolTip;
 
+	public static String SystemTapScriptLaunchError_graph;
+	public static String SystemTapScriptLaunchError_fileNotFound;
+	public static String SystemTapScriptLaunchError_fileNotStp;
+
 	static {
 		// initialize resource bundle
 		NLS.initializeMessages(BUNDLE_NAME, Messages.class);
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptGraphOptionsTab.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptGraphOptionsTab.java
index ed4142607e..b078424be6 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptGraphOptionsTab.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptGraphOptionsTab.java
@@ -632,6 +632,7 @@ public class SystemTapScriptGraphOptionsTab extends
 					badGraphs.remove(old_gd);
 					setUpGraphTableItem(selectedTableItem, gd, false);
 					graphsData.set(graphsTable.indexOf(selectedTableItem), gd);
+					checkErrors(selectedRegex);
 					updateLaunchConfigurationDialog();
 				}
 			}
@@ -646,6 +647,7 @@ public class SystemTapScriptGraphOptionsTab extends
 				badGraphs.remove(gd);
 				selectedTableItem.dispose();
 				setSelectionControlsEnabled(false);
+				checkErrors(selectedRegex);
 				updateLaunchConfigurationDialog();
 			}
 		});
@@ -787,7 +789,7 @@ public class SystemTapScriptGraphOptionsTab extends
 	 * @param regex The regular expression to check for validity.
 	 * @return <code>null</code> if the regular expression is valid, or an error message.
 	 */
-	private String checkRegex(String regex) {
+	private static String checkRegex(String regex) {
 		//TODO may add more invalid regexs here, each with its own error message.
 		if (regex.contains("()")){ //$NON-NLS-1$
 			return Messages.SystemTapScriptGraphOptionsTab_6;
@@ -1010,7 +1012,7 @@ public class SystemTapScriptGraphOptionsTab extends
 				setUpGraphTableItem(item, graphData, true);
 			}
 
-			updateRegexSelection(0, true); // Handles all remaining updates.
+			updateRegexSelection(defaultSelectedRegex, true); // Handles all remaining updates.
 			checkAllOtherErrors();
 
 		} catch (CoreException e) {
@@ -1144,14 +1146,30 @@ public class SystemTapScriptGraphOptionsTab extends
 			if (i == selectedRegex) {
 				continue;
 			}
+			checkErrors(i);
+		}
+	}
 
-			String error = findBadGraphs(i);
-			if (error == null) {
-				error = checkRegex(regularExpressionCombo.getItem(i));
-			}
+	/**
+	 * Checks the regular expression of the provided index for errors.
+	 * Sets the associated error message to contain relevant error information.
+	 * @param i The index of the regular expression to check for errors.
+	 */
+	private void checkErrors(int i) {
+		String regex = regularExpressionCombo.getItem(i);
+		try {
+			Pattern.compile(regex);
+		} catch (PatternSyntaxException e) {
+			regexErrorMessages.set(i, e.getMessage());
+			return;
+		}
 
-			regexErrorMessages.set(i, error);
+		String error = findBadGraphs(i);
+		if (error == null) {
+			error = checkRegex(regex);
 		}
+
+		regexErrorMessages.set(i, error);
 	}
 
 	@Override
@@ -1163,10 +1181,11 @@ public class SystemTapScriptGraphOptionsTab extends
 			return true;
 		}
 
-		for (int i = 0, n = getNumberOfRegexs(); i < n; i++) {
-			String regexErrorMessage = regexErrorMessages.get(i);
+		for (int r = 0, n = getNumberOfRegexs(); r < n; r++) {
+			String regexErrorMessage = regexErrorMessages.get(r);
 			if (regexErrorMessage != null){
-				setErrorMessage(String.format("Expression \"%s\": %s", regularExpressionCombo.getItems()[i], regexErrorMessage)); //$NON-NLS-1$
+				setErrorMessage(MessageFormat.format(Messages.SystemTapScriptGraphOptionsTab_regexErrorMsgFormat,
+						regularExpressionCombo.getItems()[r], regexErrorMessage));
 				return false;
 			}
 		}
@@ -1174,6 +1193,52 @@ public class SystemTapScriptGraphOptionsTab extends
 		return true;
 	}
 
+	/**
+	 * Checks if a launch configuration's Systemtap Graphing settings are valid.
+	 * @param launchConfig The launch configuration to check for graph validity.
+	 * @return <code>true</code> if the launch settings are valid, or <code>false</code> if
+	 * its graph settings are invalid in some way.
+	 */
+	public static boolean isValidLaunch(ILaunchConfiguration launchConfig) throws CoreException {
+		// If graphic is disabled then everything is valid.
+		if (!launchConfig.getAttribute(RUN_WITH_CHART, false)){
+			return true;
+		}
+
+		for (int r = 0, n = launchConfig.getAttribute(NUMBER_OF_REGEXS, 1); r < n; r++) {
+			// Check for any invalid regexs.
+			String regex = launchConfig.getAttribute(REGULAR_EXPRESSION + r, (String) null);
+			if (regex == null || checkRegex(regex) != null) {
+				return false;
+			}
+			try {
+				Pattern.compile(regex);
+			} catch (PatternSyntaxException e) {
+				return false;
+			}
+
+			// If graphs are plotted but no data is captured by one of them, report this as a problem.
+			int numberOfColumns = launchConfig.getAttribute(NUMBER_OF_COLUMNS + r, 0);
+			if (numberOfColumns == 0) {
+				return false;
+			}
+
+			// Check for graphs that are missing required data.
+			for (int i = 0, g = launchConfig.getAttribute(NUMBER_OF_GRAPHS + r, 0); i < g; i++) {
+				if (launchConfig.getAttribute(get2DConfigData(GRAPH_X_SERIES, r, i), 0) >= numberOfColumns) {
+					return false;
+				}
+				for (int j = 0, y = launchConfig.getAttribute(get2DConfigData(GRAPH_Y_SERIES_LENGTH, r, i), 0); j < y; j++) {
+					if (launchConfig.getAttribute(get2DConfigData(GRAPH_Y_SERIES, r, i + "_" + j), 0) >= numberOfColumns) { //$NON-NLS-1$
+						return false;
+					}
+				}
+			}
+		}
+
+		return true;
+	}
+
 	@Override
 	public String getName() {
 		return Messages.SystemTapScriptGraphOptionsTab_7;
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
index 4101453916..5890f36ef9 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
@@ -11,6 +11,7 @@
 
 package org.eclipse.linuxtools.internal.systemtap.ui.ide.launcher;
 
+import java.text.MessageFormat;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -20,7 +21,9 @@ import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Path;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.debug.core.ILaunch;
 import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.model.LaunchConfigurationDelegate;
@@ -64,10 +67,18 @@ public class SystemTapScriptLaunchConfigurationDelegate extends
 		return true;
 	}
 
+	private String getPluginID() {
+		return this.getClass().getPackage().getName();
+	}
+
 	@Override
 	public void launch(ILaunchConfiguration configuration, String mode,
 			ILaunch launch, IProgressMonitor monitor) throws CoreException {
 
+		if (!SystemTapScriptGraphOptionsTab.isValidLaunch(configuration)) {
+			throw new CoreException(new Status(IStatus.ERROR, getPluginID(), Messages.SystemTapScriptLaunchError_graph));
+		}
+
 		IPreferenceStore preferenceStore = ConsoleLogPlugin.getDefault().getPreferenceStore();
 
 		RunScriptHandler action;
@@ -87,10 +98,17 @@ public class SystemTapScriptLaunchConfigurationDelegate extends
 		}
 
 		// Path
-		String path = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.SCRIPT_PATH_ATTR, ""); //$NON-NLS-1$
-		if (!path.isEmpty()){
-			action.setPath(new Path(path));
+		IPath scriptPath = new Path(configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.SCRIPT_PATH_ATTR, "")); //$NON-NLS-1$
+		if (!scriptPath.toFile().exists()) {
+			throw new CoreException(new Status(IStatus.ERROR, getPluginID(),
+					MessageFormat.format(Messages.SystemTapScriptLaunchError_fileNotFound, scriptPath.toString())));
+		}
+		String extension = scriptPath.getFileExtension();
+		if (extension == null || !extension.equals("stp")) { //$NON-NLS-1$
+			throw new CoreException(new Status(IStatus.ERROR, getPluginID(),
+					MessageFormat.format(Messages.SystemTapScriptLaunchError_fileNotStp, scriptPath.toString())));
 		}
+		action.setPath(scriptPath);
 
 		// User Name
 		String userName = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.USER_NAME_ATTR, ""); //$NON-NLS-1$
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationTab.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationTab.java
index bf14fa39c6..08d9a6b14e 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationTab.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationTab.java
@@ -285,7 +285,7 @@ public class SystemTapScriptLaunchConfigurationTab extends
 
 		IPath scriptPath = getScriptPath();
 		if (scriptPath == null) {
-			setErrorMessage(MessageFormat.format(Messages.SystemTapScriptLaunchConfigurationTab_couldNotFindFile, scriptPathText.getText()));
+			setErrorMessage(MessageFormat.format(Messages.SystemTapScriptLaunchConfigurationTab_fileNotFound, scriptPathText.getText()));
 			return false;
 		}
 		String extension = scriptPath.getFileExtension();
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/messages.properties b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/messages.properties
index 448a9a9e25..f7894dfc4f 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/messages.properties
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/messages.properties
@@ -11,13 +11,7 @@
 
 SystemTapScriptGraphOptionsTab_1=Unable to initialize launch configuration tab
 SystemTapScriptGraphOptionsTab_2=Graph the output of this script run
-SystemTapScriptGraphOptionsTab_3=Graph the output of the script\n\
-Make sure your scripts outputs data in columns.\n \
-For example:\n \
-  1305  2309  4233\n \
-  2322  3234  4223\n \
-  2321  3123  4533\n \
-and enter parsing instructions below
+SystemTapScriptGraphOptionsTab_3=Graph the output of the script
 SystemTapScriptGraphOptionsTab_4=Output Parsing:
 SystemTapScriptGraphOptionsTab_5=Unable to initialize launch configuration tab
 SystemTapScriptGraphOptionsTab_6=Cannot parse an empty string: remove '()'
@@ -38,6 +32,7 @@ SystemTapScriptGraphOptionsTab_defaultColumnTitleBase=Column {0}
 SystemTapScriptGraphOptionsTab_RemoveGraphButton=Remove
 SystemTapScriptGraphOptionsTab_RemoveGraphButtonToolTip=Remove the selected graph
 SystemTapScriptGraphOptionsTab_invalidGraph=<Invalid>
+SystemTapScriptGraphOptionsTab_regexErrorMsgFormat=Expression "{0}": {1}
 SystemTapScriptGraphOptionsTab_regexLabel=Regular Expression:
 SystemTapScriptGraphOptionsTab_regexTooltip=Enter a regular expression that will match your expected output. Then add brackets around the values you would like to capture.\n\
 For example to extract reads and writes from the string\n\
@@ -72,7 +67,7 @@ SystemTapScriptLaunchConfigurationTab_11=Select Systemtap Script
 SystemTapScriptLaunchConfigurationTab_errorInitializingTab=Could not initialize tab
 SystemTapScriptLaunchConfigurationTab_options=Options:
 SystemTapScriptLaunchConfigurationTab_runWithChart=Run the script with a chart.
-SystemTapScriptLaunchConfigurationTab_couldNotFindFile=No Systemtap script exists with the path {0}.
+SystemTapScriptLaunchConfigurationTab_fileNotFound=No Systemtap script exists at the path {0}.
 SystemTapScriptLaunchConfigurationTab_fileNotStp=Must provide a path to a valid Systemtap script (with extension ".stp").
 SystemTapScriptLaunchShortcut_couldNotFindConfig=Could Not Find configuration
 SystemTapScriptLaunchShortcut_couldNotLaunchScript=Could Not Launch Script
@@ -83,4 +78,8 @@ SystemTapScriptOptionsTab_3=dyninst
 SystemTapScriptOptionsTab_4=When using dyninst you must specify a target PID or a target executable
 SystemTapScriptOptionsTab_5=Please specify a proper pid
 SystemTapScriptOptionsTab_initializeConfigurationFailed=Could not initialize configuration
-SystemTapScriptOptionsTab_targetToolTip=Select a target executable to be passed to systemtap using the -c option
\ No newline at end of file
+SystemTapScriptOptionsTab_targetToolTip=Select a target executable to be passed to systemtap using the -c option
+
+SystemTapScriptLaunchError_graph=An error exists in this launch's graph settings. Please use the Run Configurations->Graphing menu to fix them.
+SystemTapScriptLaunchError_fileNotFound=No Systemtap script exists at the path {0}. Please use the Run Configurations->General menu to set the path of the script to run.
+SystemTapScriptLaunchError_fileNotStp={0} is not a Systemtap script file. Please use the Run Configurations->General menu to select a file with extension ".stp".