[WIP] Fixed whitespace issue on argument escaping.

classes/phing/tasks/system/AttribTask.php

@@ -133,7 +133,7 @@ protected function checkConfiguration()
      * @param mixed $e
      * @throws BuildException
      */
-    public function setExecutable($e)
+    public function setExecutable($e): void
     {
         throw new BuildException(
             $this->getTaskType() . ' doesn\'t support the executable attribute',

classes/phing/tasks/system/ExecTask.php

@@ -38,12 +38,6 @@ class ExecTask extends Task
      */
     protected $realCommand;
 
-    /**
-     * Given command
-     * @var string
-     */
-    protected $command;
-
     /**
      * Commandline managing object
      *
@@ -180,11 +174,22 @@ protected function prepare()
             $this->dir = $this->getProject()->getBasedir();
         }
 
+        if ($this->commandline->getExecutable() === null) {
+            throw new BuildException(
+                'ExecTask: Please provide "executable"'
+            );
+        }
+
         // expand any symbolic links first
         try {
+            if (!$this->dir->getCanonicalFile()->exists()) {
+                throw new BuildException(
+                    "The directory '" . (string) $this->dir . "' does not exist"
+                );
+            }
             if (!$this->dir->getCanonicalFile()->isDirectory()) {
                 throw new BuildException(
-                    "'" . (string) $this->dir . "' is not a valid directory"
+                    "'" . (string) $this->dir . "' is not a directory"
                 );
             }
         } catch (IOException $e) {
@@ -194,6 +199,8 @@ protected function prepare()
         }
         $this->currdir = getcwd();
         @chdir($this->dir->getPath());
+
+        $this->commandline->setEscape($this->escape);
     }
 
     /**
@@ -205,47 +212,23 @@ protected function prepare()
      */
     protected function buildCommand()
     {
-        if ($this->command === null && $this->commandline->getExecutable() === null) {
-            throw new BuildException(
-                'ExecTask: Please provide "command" OR "executable"'
-            );
-        } else {
-            if ($this->command === null) {
-                $this->realCommand = Commandline::toString($this->commandline->getCommandline(), $this->escape);
-            } else {
-                if ($this->commandline->getExecutable() === null) {
-                    $this->realCommand = $this->command;
-
-                    //we need to escape the command only if it's specified directly
-                    // commandline takes care of "executable" already
-                    if ($this->escape == true) {
-                        $this->realCommand = escapeshellcmd($this->realCommand);
-                    }
-                } else {
-                    throw new BuildException(
-                        'ExecTask: Either use "command" OR "executable"'
-                    );
-                }
-            }
-        }
-
         if ($this->error !== null) {
             $this->realCommand .= ' 2> ' . escapeshellarg($this->error->getPath());
             $this->log(
-                "Writing error output to: " . $this->error->getPath(),
+                'Writing error output to: ' . $this->error->getPath(),
                 $this->logLevel
             );
         }
 
         if ($this->output !== null) {
             $this->realCommand .= ' 1> ' . escapeshellarg($this->output->getPath());
             $this->log(
-                "Writing standard output to: " . $this->output->getPath(),
+                'Writing standard output to: ' . $this->output->getPath(),
                 $this->logLevel
             );
         } elseif ($this->spawn) {
             $this->realCommand .= ' 1>/dev/null';
-            $this->log("Sending output to /dev/null", $this->logLevel);
+            $this->log('Sending output to /dev/null', $this->logLevel);
         }
 
         // If neither output nor error are being written to file
@@ -260,24 +243,29 @@ protected function buildCommand()
         if ($this->spawn) {
             $this->realCommand .= ' &';
         }
+
+        $this->realCommand = (string) $this->commandline . $this->realCommand;
     }
 
     /**
      * Executes the command and returns return code and output.
      *
      * @return array array(return code, array with output)
+     * @throws \BuildException
      */
     protected function executeCommand()
     {
-        $this->log("Executing command: " . $this->realCommand, $this->logLevel);
+        $cmdl = $this->realCommand;
+
+        $this->log('Executing command: ' . $cmdl, $this->logLevel);
 
         $output = [];
         $return = null;
 
         if ($this->passthru) {
-            passthru($this->realCommand, $return);
+            passthru($cmdl, $return);
         } else {
-            exec($this->realCommand, $output, $return);
+            exec($cmdl, $output, $return);
         }
 
         return [$return, $output];
@@ -295,7 +283,7 @@ protected function executeCommand()
      * @throws BuildException
      * @return void
      */
-    protected function cleanup($return, $output)
+    protected function cleanup($return, $output): void
     {
         if ($this->dir !== null) {
             @chdir($this->currdir);
@@ -306,9 +294,7 @@ protected function cleanup($return, $output)
             $this->log($line, $outloglevel);
         }
 
-        if ($this->returnProperty) {
-            $this->project->setProperty($this->returnProperty, $return);
-        }
+        $this->maybeSetReturnPropertyValue($return);
 
         if ($this->outputProperty) {
             $this->project->setProperty(
@@ -319,8 +305,11 @@ protected function cleanup($return, $output)
 
         $this->setExitValue($return);
 
-        if ($return != 0 && $this->checkreturn) {
-            throw new BuildException("Task exited with code $return");
+        if ($return !== 0) {
+            if ($this->checkreturn) {
+                throw new BuildException($this->getTaskType() . ' returned: ' . $return, $this->getLocation());
+            }
+            $this->log('Result: ' . $return, Project::MSG_ERR);
         }
     }
 
@@ -329,7 +318,7 @@ protected function cleanup($return, $output)
      *
      * @param int $value exit value of the process.
      */
-    protected function setExitValue($value)
+    protected function setExitValue($value): void
     {
         $this->exitValue = $value;
     }
@@ -340,34 +329,41 @@ protected function setExitValue($value)
      * @return int the exit value or self::INVALID if no exit value has
      *             been received.
      */
-    public function getExitValue()
+    public function getExitValue(): int
     {
         return $this->exitValue;
     }
 
     /**
      * The command to use.
      *
-     * @param mixed $command String or string-compatible (e.g. w/ __toString()).
+     * @param string $command String or string-compatible (e.g. w/ __toString()).
      *
      * @return void
+     * @throws \BuildException
      */
-    public function setCommand($command)
+    public function setCommand($command): void
     {
-        $this->command = "" . $command;
+        $this->log("The command attribute is deprecated.\nPlease use the executable attribute and nested arg elements.",
+            Project::MSG_WARN);
+        $this->commandline = new Commandline($command);
+        $this->executable = $this->commandline->getExecutable();
     }
 
     /**
      * The executable to use.
      *
-     * @param mixed $executable String or string-compatible (e.g. w/ __toString()).
+     * @param string|bool $value String or string-compatible (e.g. w/ __toString()).
      *
      * @return void
      */
-    public function setExecutable($executable)
+    public function setExecutable($value): void
     {
-        $this->executable = $executable;
-        $this->commandline->setExecutable((string) $executable);
+        if (is_bool($value)) {
+            $value = $value === true ? 'true' : 'false';
+        }
+        $this->executable = $value;
+        $this->commandline->setExecutable($value);
     }
 
     /**
@@ -377,7 +373,7 @@ public function setExecutable($executable)
      *
      * @return void
      */
-    public function setEscape($escape)
+    public function setEscape($escape): void
     {
         $this->escape = (bool) $escape;
     }
@@ -389,7 +385,7 @@ public function setEscape($escape)
      *
      * @return void
      */
-    public function setDir(PhingFile $dir)
+    public function setDir(PhingFile $dir): void
     {
         $this->dir = $dir;
     }
@@ -401,15 +397,15 @@ public function setDir(PhingFile $dir)
      *
      * @return void
      */
-    public function setOs($os)
+    public function setOs($os): void
     {
         $this->os = (string) $os;
     }
 
     /**
      * List of operating systems on which the command may be executed.
      */
-    public function getOs()
+    public function getOs(): string
     {
         return $this->os;
     }
@@ -418,7 +414,7 @@ public function getOs()
      * Restrict this execution to a single OS Family
      * @param string $osFamily the family to restrict to.
      */
-    public function setOsFamily($osFamily)
+    public function setOsFamily($osFamily): void
     {
         $this->osFamily = strtolower($osFamily);
     }
@@ -438,7 +434,7 @@ public function getOsFamily()
      *
      * @return void
      */
-    public function setOutput(PhingFile $f)
+    public function setOutput(PhingFile $f): void
     {
         $this->output = $f;
     }
@@ -450,7 +446,7 @@ public function setOutput(PhingFile $f)
      *
      * @return void
      */
-    public function setError(PhingFile $f)
+    public function setError(PhingFile $f): void
     {
         $this->error = $f;
     }
@@ -462,7 +458,7 @@ public function setError(PhingFile $f)
      *
      * @return void
      */
-    public function setPassthru($passthru)
+    public function setPassthru($passthru): void
     {
         $this->passthru = $passthru;
     }
@@ -474,7 +470,7 @@ public function setPassthru($passthru)
      *
      * @return void
      */
-    public function setLogoutput($logOutput)
+    public function setLogoutput($logOutput): void
     {
         $this->logOutput = $logOutput;
     }
@@ -486,7 +482,7 @@ public function setLogoutput($logOutput)
      *
      * @return void
      */
-    public function setSpawn($spawn)
+    public function setSpawn($spawn): void
     {
         $this->spawn = $spawn;
     }
@@ -498,7 +494,7 @@ public function setSpawn($spawn)
      *
      * @return void
      */
-    public function setCheckreturn($checkreturn)
+    public function setCheckreturn($checkreturn): void
     {
         $this->checkreturn = $checkreturn;
     }
@@ -510,19 +506,26 @@ public function setCheckreturn($checkreturn)
      *
      * @return void
      */
-    public function setReturnProperty($prop)
+    public function setReturnProperty($prop): void
     {
         $this->returnProperty = $prop;
     }
 
+    protected function maybeSetReturnPropertyValue(int $return)
+    {
+        if ($this->returnProperty) {
+            $this->getProject()->setNewProperty($this->returnProperty, $return);
+        }
+    }
+
     /**
      * The name of property to set to output value from exec() call.
      *
      * @param string $prop Property name
      *
      * @return void
      */
-    public function setOutputProperty($prop)
+    public function setOutputProperty($prop): void
     {
         $this->outputProperty = $prop;
     }
@@ -535,7 +538,7 @@ public function setOutputProperty($prop)
      * @throws BuildException
      * @return void
      */
-    public function setLevel($level)
+    public function setLevel($level): void
     {
         switch ($level) {
             case 'error':
@@ -597,9 +600,9 @@ public function createArg()
      * <li><code>false</code> otherwise.</li>
      * </ul>
      */
-    protected function isValidOs()
+    protected function isValidOs(): bool
     {
-        //hand osfamily off to Os class, if set
+        //hand osfamily off to OsCondition class, if set
         if ($this->osFamily !== null && !OsCondition::isFamily($this->osFamily)) {
             return false;
         }