[426329] Cancelling ${string_prompt} doesn't cancel Run

Maven run configurations are executed even if they contain a
${string_prompt} variable that is cancelled or invalid variables.

Changes:
* cancelling the string prompt cancels the run
* variable substitution errors cancel the run AND show an error message
to the user

Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=426329
Change-Id: I4b703504b7ccf9c2ce796fa0fd5fdbcd71b0f9a8
Signed-off-by: Johannes Beichter <jbeichter@gmx.de>

5 files changed, 37 insertions, 13 deletions

diff --git a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/LaunchingUtils.java b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/LaunchingUtils.java
index 92878457..86c756f0 100644
--- a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/LaunchingUtils.java
+++ b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/LaunchingUtils.java
@@ -15,7 +15,12 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.core.variables.VariablesPlugin;
+import org.eclipse.osgi.util.NLS;
+
+import org.eclipse.m2e.actions.MavenLaunchConstants;
 
 
 public class LaunchingUtils {
@@ -25,15 +30,19 @@ public class LaunchingUtils {
   /**
    * Substitute any variable
    */
-  public static String substituteVar(String s) {
+  public static String substituteVar(String s) throws CoreException {
     if(s == null) {
       return s;
     }
     try {
       return VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(s);
     } catch(CoreException e) {
+      if(e.getStatus() != null && e.getStatus().matches(IStatus.CANCEL)) {
+        throw e;
+      }
       log.error("Could not substitute variable {}.", s, e);
-      return null;
+      throw new CoreException(new Status(IStatus.ERROR, MavenLaunchConstants.PLUGIN_ID, -1,
+          NLS.bind(Messages.MavenLaunchUtils_error_could_not_substitute_variable, s), e));
     }
   }
 
diff --git a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/MavenLaunchDelegate.java b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/MavenLaunchDelegate.java
index 7be80ca4..d59f776a 100644
--- a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/MavenLaunchDelegate.java
+++ b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/MavenLaunchDelegate.java
@@ -153,7 +153,7 @@ public class MavenLaunchDelegate extends JavaLaunchDelegate implements MavenLaun
   /**
    * Construct string with properties to pass to JVM as system properties
    */
-  private String getProperties(ILaunchConfiguration configuration) {
+  private String getProperties(ILaunchConfiguration configuration) throws CoreException {
     StringBuffer sb = new StringBuffer();
 
     try {
@@ -179,6 +179,7 @@ public class MavenLaunchDelegate extends JavaLaunchDelegate implements MavenLaun
     } catch(CoreException e) {
       String msg = "Exception while getting configuration attribute " + ATTR_PROPERTIES;
       log.error(msg, e);
+      throw e;
     }
 
     try {
@@ -189,6 +190,7 @@ public class MavenLaunchDelegate extends JavaLaunchDelegate implements MavenLaun
     } catch(CoreException ex) {
       String msg = "Exception while getting configuration attribute " + ATTR_PROFILES;
       log.error(msg, ex);
+      throw ex;
     }
 
     return sb.toString();
diff --git a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/Messages.java b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/Messages.java
index a244d168..ae0ab91c 100644
--- a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/Messages.java
+++ b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/Messages.java
@@ -77,6 +77,8 @@ public class Messages extends NLS {
 
   public static String MavenLaunchExtensionsTab_lblExtensions;
 
+  public static String MavenLaunchUtils_error_could_not_substitute_variable;
+
   public static String MavenLaunchUtils_error_no_maven_install;
 
   public static String launchPomGroup;
@@ -113,6 +115,8 @@ public class Messages extends NLS {
 
   public static String launchPomDirectoryDoesntExist;
 
+  public static String launchErrorEvaluatingBaseDirectory;
+
   public static String MavenLaunchMainTab_lblUserSettings_text;
 
   public static String MavenLaunchMainTab_btnUserSettings_text;
diff --git a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/messages.properties b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/messages.properties
index 60f1f5fc..d899e5c3 100644
--- a/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/messages.properties
+++ b/org.eclipse.m2e.launching/src/org/eclipse/m2e/internal/launch/messages.properties
@@ -31,6 +31,7 @@ MavenLaunchMainTab_lblThreads=&Threads
 MavenLaunchMainTab_lblUserSettings_text=User settings\:
 MavenLaunchMainTab_property_dialog_edit_title=Edit Parameter
 MavenLaunchMainTab_property_dialog_title=Add Parameter
+MavenLaunchUtils_error_could_not_substitute_variable=Could not substitute variable {0}
 MavenLaunchUtils_error_no_maven_install=Can't find Maven installation {0}
 MavenLaynchDelegate_unsupported_source_locator=Unknown or unsupported source locator {0}
 launchBrowseFs=Browse File Syste&m...
@@ -41,6 +42,7 @@ launchGoals=&Select...
 launchGoalsLabel=&Goals\:
 launchMainTabName=Main
 launchPomDirectoryDoesntExist=Base directory doesn't exist or can't be read
+launchErrorEvaluatingBaseDirectory=Base directory name can't be evaluated
 launchPomDirectoryEmpty=Base directory should be specified
 launchPomGroup=Base directory\:
 launchProfilesLabel=&Profiles\:
diff --git a/org.eclipse.m2e.launching/src/org/eclipse/m2e/ui/internal/launch/MavenLaunchMainTab.java b/org.eclipse.m2e.launching/src/org/eclipse/m2e/ui/internal/launch/MavenLaunchMainTab.java
index a121e719..eb089a76 100644
--- a/org.eclipse.m2e.launching/src/org/eclipse/m2e/ui/internal/launch/MavenLaunchMainTab.java
+++ b/org.eclipse.m2e.launching/src/org/eclipse/m2e/ui/internal/launch/MavenLaunchMainTab.java
@@ -606,7 +606,9 @@ public class MavenLaunchMainTab extends AbstractLaunchConfigurationTab implement
       return false;
     }
     if(!isDirectoryExist(pomFileName)) {
-      setErrorMessage(Messages.launchPomDirectoryDoesntExist);
+      if(getErrorMessage() == null) {
+        setErrorMessage(Messages.launchPomDirectoryDoesntExist);
+      }
       return false;
     }
     return true;
@@ -616,15 +618,20 @@ public class MavenLaunchMainTab extends AbstractLaunchConfigurationTab implement
     if(name == null || name.trim().length() == 0) {
       return false;
     }
-    String dirName = LaunchingUtils.substituteVar(name);
-    if(dirName == null) {
-      return false;
-    }
-    File pomDir = new File(dirName);
-    if(!pomDir.exists()) {
-      return false;
-    }
-    if(!pomDir.isDirectory()) {
+    try {
+      String dirName = LaunchingUtils.substituteVar(name);
+      if(dirName == null) {
+        return false;
+      }
+      File pomDir = new File(dirName);
+      if(!pomDir.exists()) {
+        return false;
+      }
+      if(!pomDir.isDirectory()) {
+        return false;
+      }
+    } catch(CoreException e) {
+      setErrorMessage(Messages.launchErrorEvaluatingBaseDirectory);
       return false;
     }
     return true;