From 3900fb89a55c34cc3dab7b25f628e0e65ccd0c4f Mon Sep 17 00:00:00 2001
From: Timm Friebe <thekid@thekid.de>
Date: Fri, 17 Nov 2023 23:00:23 +0100
Subject: [PATCH] Improve command line runner errors

---
 ChangeLog.md                                  |  6 +++
 src/main/php/xp/web/Source.class.php          | 39 ++++++++++++++-----
 .../php/web/unittest/SourceTest.class.php     | 17 ++++++--
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/ChangeLog.md b/ChangeLog.md
index c6742167..d003e29a 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -3,6 +3,12 @@ Web change log
 
 ## ?.?.? / ????-??-??
 
+## 3.9.0 / 2023-11-17
+
+* Improve error messages when class reference given on the command line
+  is not a `web.Application` subclass
+  (@thekid)
+
 ## 3.8.1 / 2023-05-22
 
 * Extended EOF handling inside server protocol handler to include NULL,
diff --git a/src/main/php/xp/web/Source.class.php b/src/main/php/xp/web/Source.class.php
index b1b6e9e0..ded32086 100755
--- a/src/main/php/xp/web/Source.class.php
+++ b/src/main/php/xp/web/Source.class.php
@@ -1,15 +1,41 @@
 <?php namespace xp\web;
 
-use lang\{ClassLoader, XPClass};
+use lang\{ClassLoader, XPClass, IllegalArgumentException, ClassLoadingException};
+use web\Application;
 
 /**
  * An application source
  *
- * @test xp://web.unittest.SourceTest
+ * @test  web.unittest.SourceTest
  */
 class Source {
   private $application;
 
+  /**
+   * Creates a new instance
+   *
+   * @param  string $application
+   * @param  web.Environment $environment
+   * @return web.Application
+   * @throws lang.IllegalArgumentException
+   */
+  private function newInstance($application, $environment) {
+    if ('-' === $application) return new ServeDocumentRootStatically($environment);
+
+    $cl= ClassLoader::getDefault();
+    try {
+      $class= is_file($application) ? $cl->loadUri($application) : $cl->loadClass($application);
+    } catch (ClassLoadingException $e) {
+      throw new IllegalArgumentException('Cannot load class '.$application, $e);
+    }
+
+    if (!$class->isSubclassOf(Application::class)) {
+      throw new IllegalArgumentException($class->getName().' is not a web.Application');
+    }
+
+    return $class->newInstance($environment);
+  }
+
   /**
    * Creates a new application from a given name and environment
    *
@@ -18,14 +44,7 @@ class Source {
    */
   public function __construct($name, $environment) {
     sscanf($name, '%[^+]+%s', $application, $filters);
-
-    if ('-' === $application) {
-      $this->application= new ServeDocumentRootStatically($environment);
-    } else if (is_file($application)) {
-      $this->application= ClassLoader::getDefault()->loadUri($application)->newInstance($environment);
-    } else {
-      $this->application= ClassLoader::getDefault()->loadClass($application)->newInstance($environment);
-    }
+    $this->application= $this->newInstance($application, $environment);
 
     if ($filters) {
       $this->application->install(array_map(
diff --git a/src/test/php/web/unittest/SourceTest.class.php b/src/test/php/web/unittest/SourceTest.class.php
index 9dcb594e..14931a6c 100755
--- a/src/test/php/web/unittest/SourceTest.class.php
+++ b/src/test/php/web/unittest/SourceTest.class.php
@@ -1,15 +1,16 @@
 <?php namespace web\unittest;
 
 use io\Path;
-use lang\XPClass;
-use test\{Assert, Before, Test};
+use lang\{XPClass, IllegalArgumentException};
+use test\{Assert, Before, Expect, Test};
 use web\Environment;
 use xp\web\{ServeDocumentRootStatically, Source};
 
 class SourceTest {
   private $environment;
 
-  public function __construct() {
+  #[Before]
+  public function environment() {
     $this->environment= new Environment('dev', '.', 'static', []);
   }
 
@@ -37,4 +38,14 @@ public function application_class_and_filter() {
     $src= new Source('web.unittest.HelloWorld+xp.web.dev.Console', $this->environment);
     Assert::instance(HelloWorld::class, $src->application());
   }
+
+  #[Test, Expect(class: IllegalArgumentException::class, message: 'Cannot load class not.a.class')]
+  public function non_existant_class() {
+    (new Source('not.a.class', $this->environment))->application();
+  }
+
+  #[Test, Expect(class: IllegalArgumentException::class, message: 'util.Date is not a web.Application')]
+  public function unrelated_class() {
+    (new Source('util.Date', $this->environment))->application();
+  }
 }
\ No newline at end of file