Improve test coverage, fix few small bugs

.codeclimate.yml

@@ -10,8 +10,9 @@ engines:
     enabled: true
     exclude_fingerprints:
     - 9d462b7c90c564bf28007ee399340fad # table() NPath is too complex.
-    - 7c90035f65bb3bdbd2c03c648a705aac # we use static for factory, so it's good
-    - 80ef7f404dd4f054ca51d9ee12d9e9dd # we exit from toStrign() because it can't throw exceptions
+    - c96f14f28e3586694e93ad4a7185804a # we use static for factory, so it's good
+    - e4eedf458a989182e975f6cd334d2d12 # we exit from toStrign() because it can't throw exceptions
+    - 31780bbd56396145ce17ca6e15095d99 # ignore Single Responsibility Principle in this case
     checks:
       CyclomaticComplexity:
         enabled: false

docs/expressions.rst

@@ -265,7 +265,7 @@ Magic an Debug Methods
 
 .. php:method:: __debugInfo()
 
-    This method will is used to prepare a sensible information about your query
+    This method is used to prepare a sensible information about your query
     when you are executing ``var_dump($expr)``. The output will be HTML-safe.
 
 .. php:method:: debug()
@@ -309,7 +309,7 @@ circumstances.
 
       $query->field('foo,bar');  // escapes and adds 2 fields to the query
       $query->field($query->escape('foo,bar')); // adds field `foo,bar` to the query
-      $query->field(['foo,bar']);  // adds single field `foo,bar` 
+      $query->field(['foo,bar']);  // adds single field `foo,bar`
 
       $query->order('foo desc');  // escapes and add `foo` desc to the query
       $query->field($query->escape('foo desc')); // adds field `foo desc` to the query

src/Connection.php

@@ -10,10 +10,24 @@
  */
 class Connection
 {
+    /** @var string Query classname */
     protected $query_class = 'atk4\dsql\Query';
+
+    /** @var string Expression classname */
     protected $expression_class = 'atk4\dsql\Expression';
+
+    /** @var Connection Connection object */
     protected $connection = null;
 
+    /**
+     * Connect database
+     *
+     * @param string $dsn
+     * @param string $user
+     * @param string $password
+     * @param array $args
+     * @return Connection
+     */
     public static function connect($dsn, $user = null, $password = null, $args = [])
     {
         if (strpos($dsn, ':') === false) {
@@ -24,28 +38,28 @@ public static function connect($dsn, $user = null, $password = null, $args = [])
         switch (strtolower($driver)) {
             case 'mysql':
                 return new Connection(array_merge([
-                    'connection'=>new \PDO($dsn, $user, $password),
-                    'query_class'=>'atk4\dsql\Query_MySQL'
+                    'connection' => new \PDO($dsn, $user, $password),
+                    'query_class' => 'atk4\dsql\Query_MySQL'
                 ], $args));
             case 'sqlite':
                 return new Connection(array_merge([
-                    'connection'=>new \PDO($dsn, $user, $password),
-                    'query_class'=>'atk4\dsql\Query_SQLite'
+                    'connection' => new \PDO($dsn, $user, $password),
+                    'query_class' => 'atk4\dsql\Query_SQLite'
                 ], $args));
             case 'dumper':
                 return new Connection_Dumper(array_merge([
-                    'connection'=>Connection::connect($rest)
+                    'connection' => Connection::connect($rest)
                 ], $args));
 
             case 'counter':
                 return new Connection_Counter(array_merge([
-                    'connection'=>Connection::connect($rest)
+                    'connection' => Connection::connect($rest)
                 ], $args));
 
                 // let PDO handle the rest
             default:
                 return new Connection(array_merge([
-                    'connection'=>new \PDO($dsn, $user, $password)
+                    'connection' => new \PDO($dsn, $user, $password)
                 ], $args));
 
         }
@@ -55,11 +69,11 @@ public static function connect($dsn, $user = null, $password = null, $args = [])
      * Specifying $attributes to constructors will override default
      * attribute values of this class.
      *
-     * @param array        $attributes
+     * @param array $attributes
      */
     public function __construct($attributes = null)
     {
-        if ($attributes) {
+        if ($attributes !== null) {
             if (!is_array($attributes)) {
                 throw new Exception('Invalid arguments for "new Connection()". Did you mean to call Connection::connect()?');
             }
@@ -70,6 +84,12 @@ public function __construct($attributes = null)
         }
     }
 
+    /**
+     * Returns new Query object with connection already set
+     *
+     * @param array $properties
+     * @return Query
+     */
     public function dsql($properties = [])
     {
         $c = $this->query_class;
@@ -79,6 +99,13 @@ public function dsql($properties = [])
         return $q;
     }
 
+    /**
+     * Returns Expression object with connection already set
+     *
+     * @param array $properties
+     * @param array $arguments
+     * @return Expression
+     */
     public function expr($properties = [], $arguments = null)
     {
         $c = $this->expression_class;
@@ -88,11 +115,22 @@ public function expr($properties = [], $arguments = null)
         return $e;
     }
 
+    /**
+     * Returns Connection object
+     *
+     * @return Connection
+     */
     public function connection()
     {
         return $this->connection;
     }
 
+    /**
+     * Execute Expression by using this connection
+     *
+     * @param Expression $expr
+     * @return PDOStatement
+     */
     public function execute(Expression $expr)
     {
         // If custom connection is set, execute again using that

src/Exception.php

@@ -16,6 +16,11 @@ class Exception extends \Exception
      */
     private $params = [];
 
+    /**
+     * @param string $message
+     * @param int $code
+     * @param \Throwable $previous
+     */
     public function __construct(
         $message = "",
         $code = 0,
@@ -32,6 +37,8 @@ public function __construct(
 
     /**
      * Follow the getter-style of PHP Exception
+     *
+     * @return array
      */
     public function getParams()
     {

src/Expression.php

@@ -356,6 +356,9 @@ protected function _escape($value)
      */
     protected function _param($value)
     {
+        // @todo Imants: allowing to pass value as array looks wrong.
+        //      See test case in testParam() method.
+        //      Maybe we should add implode(' ', array_map(...)) here ?
         if (is_array($value)) {
             return array_map(__METHOD__, $value);
         }
@@ -396,7 +399,7 @@ function ($matches) use (&$nameless_count) {
 
                     // use rendering only with named tags
                 }
-                    $fx = '_render_'.$identifier;
+                $fx = '_render_'.$identifier;
 
                 // [foo] will attempt to call $this->_render_foo()
 

src/Query.php

@@ -502,8 +502,7 @@ public function _render_join()
      *  ]);
      *
      * The above use of OR conditions rely on orExpr() functionality. See
-     * that method for morer information, but in short it makes use of
-     * Expression_OR class
+     * that method for more information.
      *
      * To specify OR conditions
      *  $q->where($q->orExpr()->where('a',1)->where('b',1));
@@ -1121,7 +1120,7 @@ public function __debugInfo()
         try {
             $arr['R'] = $this->render();
         } catch (\Exception $e) {
-            $arr['R'] = $this->getMessage();
+            $arr['R'] = $e->getMessage();
         }
 
         return $arr;

tests/ExpressionTest.php

@@ -257,6 +257,26 @@ public function testNestedExpressions()
         $this->assertEquals('Hello :a and good night', $s4);
     }
 
+    /**
+     * @covers ::__toString
+     * @expectedException \LogicException
+     */
+    public function testToStringException1()
+    {
+        $e = new MyBadExpression('Hello');
+        $s = (string)$e;
+    }
+
+    /**
+     * @covers ::__toString
+     * @expectedException Exception
+     */
+    public function testToStringException2()
+    {
+        $e = new MyWorstExpression('Hello');
+        $s = (string)$e;
+    }
+
     /**
      * expr() should return new Expression object and inherit connection from it.
      *
@@ -272,6 +292,7 @@ public function testExpr()
      * Fully covers _escape method
      *
      * @covers ::_escape
+     * @covers ::_escapeSoft
      * @covers ::escape
      */
     public function testEscape()
@@ -354,7 +375,6 @@ public function testEscape()
     public function testParam()
     {
         $e = new Expression('hello, [who]', ['who' => 'world']);
-
         $this->assertEquals(
             'hello, :a',
             $e->render()
@@ -364,14 +384,16 @@ public function testParam()
             $e->params
         );
 
-        $e = new Expression('hello, [who]', ['who' => 'world']);
-
+        // @todo Imants: allowing to pass value as array looks wrong.
+        //      See test case in testParam() method.
+        //      Maybe we should add implode(' ', array_map(...)) here ?
+        $e = new Expression('hello, [who]', ['who' => ['cruel', 'world']]);
         $this->assertEquals(
             'hello, :a',
             $e->render()
         );
         $this->assertEquals(
-            [':a'=>'world'],
+            [':a'=>'cruel', ':b'=>'world'],
             $e->params
         );
     }
@@ -394,6 +416,10 @@ public function testConsume()
             123,
             PHPUnitUtil::callProtectedMethod($this->e(), '_consume', [123, 'none'])
         );
+        $this->assertEquals(
+            '(select *)',
+            PHPUnitUtil::callProtectedMethod($this->e(), '_consume', [new Query()])
+        );
 
         $this->assertEquals(
             'hello, `myfield`',
@@ -535,10 +561,26 @@ public function _param($value)
         return json_encode($value);
     }
 }
-class MyField implements Expressionable {
-    function getDSQLExpression($e)
+class MyField implements Expressionable
+{
+    public function getDSQLExpression($e)
     {
         return $e->expr('`myfield`');
     }
 }
+class MyBadExpression extends Expression
+{
+    public function getOne()
+    {
+        return array();
+    }
+}
+class MyWorstExpression extends Expression
+{
+    public function getOne()
+    {
+        throw new Exception('It is Monday');
+    }
+}
+
 // @codingStandardsIgnoreEnd