Sundry fixes

- Make use of PHP 7's null coalescing operator
- remove use of static property in Lang class
- Improve code coverage slightly

lib/CLI.php

@@ -23,9 +23,7 @@ USAGE_TEXT;
     }
 
     public function __construct(array $argv = null) {
-        if (is_null($argv)) {
+        $argv = $argv ?? array_slice($_SERVER['argv'], 1);
-            $argv = array_slice($_SERVER['argv'], 1);
-        }
         $this->args = \Docopt::handle($this->usage(), [
             'argv' => $argv,
             'help' => true,
@@ -46,9 +44,7 @@ USAGE_TEXT;
 
     public function dispatch(array $args = null): int {
         // act on command line
-        if (is_null($args)) {
+        $args = $args ?? $this->args;
-            $args = $this->args;
-        }
         if ($this->command("daemon", $args)) {
             $this->loadConf();
             return $this->daemon();

lib/Db/AbstractDriver.php

@@ -36,9 +36,7 @@ abstract class AbstractDriver implements Driver {
     }
 
     public function savepointRelease(int $index = null): bool {
-        if (is_null($index)) {
+        $index = $index ?? $this->transDepth;
-            $index = $this->transDepth;
-        }
         if (array_key_exists($index, $this->transStatus)) {
             switch ($this->transStatus[$index]) {
                 case self::TR_PEND:
@@ -83,9 +81,7 @@ abstract class AbstractDriver implements Driver {
     }
 
     public function savepointUndo(int $index = null): bool {
-        if (is_null($index)) {
+        $index = $index ?? $this->transDepth;
-            $index = $this->transDepth;
-        }
         if (array_key_exists($index, $this->transStatus)) {
             switch ($this->transStatus[$index]) {
                 case self::TR_PEND:

lib/Db/SQLite3/Driver.php

@@ -20,12 +20,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         // check to make sure required extension is loaded
         if (!class_exists("SQLite3")) {
             throw new Exception("extMissing", self::driverName()); // @codeCoverageIgnore
-        }
+         }
-        $dbFile = Arsse::$conf->dbSQLite3File;
+        // if no database file is specified in the configuration, use a suitable default
-        if (is_null($dbFile)) {
+        $dbFile = Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db";
-            // if no database file is specified in the configuration, use a suitable default
-            $dbFile = \JKingWeb\Arsse\BASE."arsse.db";
-        }
         $mode = \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE;
         $timeout = Arsse::$conf->dbSQLite3Timeout * 1000;
         try {
@@ -66,7 +63,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
     public function __destruct() {
         try {
             $this->db->close();
-        } catch (\Exception $e) {
+        } catch (\Exception $e) { // @codeCoverageIgnore
         }
         unset($this->db);
     }

lib/Db/SQLite3/Result.php

@@ -54,7 +54,7 @@ class Result implements \JKingWeb\Arsse\Db\Result {
     public function __destruct() {
         try {
             $this->set->finalize();
-        } catch (\Throwable $e) {
+        } catch (\Throwable $e) { // @codeCoverageIgnore
         }
         unset($this->set);
     }

lib/Db/SQLite3/Statement.php

@@ -36,7 +36,7 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement {
     public function __destruct() {
         try {
             $this->st->close();
-        } catch (\Throwable $e) {
+        } catch (\Throwable $e) { // @codeCoverageIgnore
         }
         unset($this->st);
     }

lib/Lang.php

@@ -16,7 +16,7 @@ class Lang {
     ];
 
     public $path;                               // path to locale files; this is a public property to facilitate unit testing
-    protected static $requirementsMet = false;  // whether the Intl extension is loaded
+    protected $requirementsMet = false;         // whether the Intl extension is loaded
     protected $synched = false;                 // whether the wanted locale is actually loaded (lazy loading is used by default)
     protected $wanted = self::DEFAULT;          // the currently requested locale
     protected $locale = "";                     // the currently loaded locale
@@ -29,8 +29,8 @@ class Lang {
 
     public function set(string $locale, bool $immediate = false): string {
         // make sure the Intl extension is loaded
-        if (!static::$requirementsMet) {
+        if (!$this->requirementsMet) {
-            static::checkRequirements();
+            $this->checkRequirements();
         }
         // if requesting the same locale as already wanted, just return (but load first if we've requested an immediate load)
         if ($locale==$this->wanted) {
@@ -121,14 +121,15 @@ class Lang {
         return \Locale::lookup($list, $locale, true, $default);
     }
 
-    protected static function checkRequirements(): bool {
+    protected function checkRequirements(): bool {
         if (!extension_loaded("intl")) {
             throw new ExceptionFatal("The \"Intl\" extension is required, but not loaded");
         }
-        static::$requirementsMet = true;
+        $this->requirementsMet = true;
         return true;
     }
 
+    /** @codeCoverageIgnore */
     protected function globFiles(string $path): array {
         // we wrap PHP's glob function in this method so that unit tests may override it
         return glob($path."*.php");
@@ -148,8 +149,8 @@ class Lang {
     }
 
     protected function load(): bool {
-        if (!self::$requirementsMet) {
+        if (!$this->requirementsMet) {
-            self::checkRequirements();
+            $this->checkRequirements();
         }
         // if we've requested no locale (""), just load the fallback strings and return
         if ($this->wanted=="") {

lib/REST/NextCloudNews/V1_2.php

@@ -73,11 +73,12 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
             return new Response(405, "", "", ["Allow: ".$e->getMessage()]);
         }
         if (!method_exists($this, $func)) {
-            return new Response(501);
+            return new Response(501); // @codeCoverageIgnore
         }
         // dispatch
         try {
             return $this->$func($req->paths, $data);
+        // @codeCoverageIgnoreStart
         } catch (Exception $e) {
             // if there was a REST exception return 400
             return new Response(400);
@@ -85,6 +86,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
             // if there was any other Arsse exception return 500
             return new Response(500);
         }
+        // @codeCoverageIgnoreEnd
     }
 
     protected function chooseCall(array $url, string $method): string {

lib/REST/Request.php

@@ -12,15 +12,9 @@ class Request {
     public $body = "";
 
     public function __construct(string $method = null, string $url = null, string $body = null, string $contentType = null) {
-        if (is_null($method)) {
+        $method = $method ?? $_SERVER['REQUEST_METHOD'];
-            $method = $_SERVER['REQUEST_METHOD'];
+        $url = $url ?? $_SERVER['REQUEST_URI'];
-        }
+        $body = $body ?? file_get_contents("php://input");
-        if (is_null($url)) {
-            $url = $_SERVER['REQUEST_URI'];
-        }
-        if (is_null($body)) {
-            $body = file_get_contents("php://input");
-        }
         if (is_null($contentType)) {
             if (isset($_SERVER['HTTP_CONTENT_TYPE'])) {
                 $contentType = $_SERVER['HTTP_CONTENT_TYPE'];

tests/Lang/testLangComplex.php

@@ -47,7 +47,7 @@ class TestLangComplex extends Test\AbstractTest {
     }
 
     public function testFetchAMessage() {
-        $this->l->set("de", true);
+        $this->l->set("de");
         $this->assertEquals('und der Stein der Weisen', $this->l->msg('Test.presentText'));
     }
 

tests/REST/NextCloudNews/TestNCNV1_2.php

@@ -737,6 +737,8 @@ class TestNCNV1_2 extends Test\AbstractTest {
         Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), $this->anything())->thenReturn(true);
         Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples
         Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples
+        Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples
+        Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples
         $exp = new Response(422);
         $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple")));
         $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple")));