Additional unit tests for Style Color

src/PhpSpreadsheet/Style/Alignment.php

@@ -189,21 +189,21 @@ class Alignment extends Supervisor
     /**
      * Set Horizontal.
      *
-     * @param string $pValue see self::HORIZONTAL_*
+     * @param string $horizontalAlignment see self::HORIZONTAL_*
      *
      * @return $this
      */
-    public function setHorizontal($pValue)
+    public function setHorizontal(string $horizontalAlignment)
     {
-        if ($pValue == '') {
+        if ($horizontalAlignment == '') {
-            $pValue = self::HORIZONTAL_GENERAL;
+            $horizontalAlignment = self::HORIZONTAL_GENERAL;
         }
 
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['horizontal' => $pValue]);
+            $styleArray = $this->getStyleArray(['horizontal' => $horizontalAlignment]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->horizontal = $pValue;
+            $this->horizontal = $horizontalAlignment;
         }
 
         return $this;
@@ -226,21 +226,21 @@ class Alignment extends Supervisor
     /**
      * Set Vertical.
      *
-     * @param string $pValue see self::VERTICAL_*
+     * @param string $verticalAlignment see self::VERTICAL_*
      *
      * @return $this
      */
-    public function setVertical($pValue)
+    public function setVertical($verticalAlignment)
     {
-        if ($pValue == '') {
+        if ($verticalAlignment == '') {
-            $pValue = self::VERTICAL_BOTTOM;
+            $verticalAlignment = self::VERTICAL_BOTTOM;
         }
 
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['vertical' => $pValue]);
+            $styleArray = $this->getStyleArray(['vertical' => $verticalAlignment]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->vertical = $pValue;
+            $this->vertical = $verticalAlignment;
         }
 
         return $this;
@@ -263,24 +263,24 @@ class Alignment extends Supervisor
     /**
      * Set TextRotation.
      *
-     * @param int $pValue
+     * @param int $rotation
      *
      * @return $this
      */
-    public function setTextRotation($pValue)
+    public function setTextRotation($rotation)
     {
         // Excel2007 value 255 => PhpSpreadsheet value -165
-        if ($pValue == 255) {
+        if ($rotation == 255) {
-            $pValue = -165;
+            $rotation = -165;
         }
 
         // Set rotation
-        if (($pValue >= -90 && $pValue <= 90) || $pValue == -165) {
+        if (($rotation >= -90 && $rotation <= 90) || $rotation == -165) {
             if ($this->isSupervisor) {
-                $styleArray = $this->getStyleArray(['textRotation' => $pValue]);
+                $styleArray = $this->getStyleArray(['textRotation' => $rotation]);
                 $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
             } else {
-                $this->textRotation = $pValue;
+                $this->textRotation = $rotation;
             }
         } else {
             throw new PhpSpreadsheetException('Text rotation should be a value between -90 and 90.');
@@ -306,20 +306,20 @@ class Alignment extends Supervisor
     /**
      * Set Wrap Text.
      *
-     * @param bool $pValue
+     * @param bool $wrapped
      *
      * @return $this
      */
-    public function setWrapText($pValue)
+    public function setWrapText($wrapped)
     {
-        if ($pValue == '') {
+        if ($wrapped == '') {
-            $pValue = false;
+            $wrapped = false;
         }
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['wrapText' => $pValue]);
+            $styleArray = $this->getStyleArray(['wrapText' => $wrapped]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->wrapText = $pValue;
+            $this->wrapText = $wrapped;
         }
 
         return $this;
@@ -342,20 +342,20 @@ class Alignment extends Supervisor
     /**
      * Set Shrink to fit.
      *
-     * @param bool $pValue
+     * @param bool $shrink
      *
      * @return $this
      */
-    public function setShrinkToFit($pValue)
+    public function setShrinkToFit($shrink)
     {
-        if ($pValue == '') {
+        if ($shrink == '') {
-            $pValue = false;
+            $shrink = false;
         }
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['shrinkToFit' => $pValue]);
+            $styleArray = $this->getStyleArray(['shrinkToFit' => $shrink]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->shrinkToFit = $pValue;
+            $this->shrinkToFit = $shrink;
         }
 
         return $this;
@@ -378,26 +378,26 @@ class Alignment extends Supervisor
     /**
      * Set indent.
      *
-     * @param int $pValue
+     * @param int $indent
      *
      * @return $this
      */
-    public function setIndent($pValue)
+    public function setIndent($indent)
     {
-        if ($pValue > 0) {
+        if ($indent > 0) {
             if (
                 $this->getHorizontal() != self::HORIZONTAL_GENERAL &&
                 $this->getHorizontal() != self::HORIZONTAL_LEFT &&
                 $this->getHorizontal() != self::HORIZONTAL_RIGHT
             ) {
-                $pValue = 0; // indent not supported
+                $indent = 0; // indent not supported
             }
         }
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['indent' => $pValue]);
+            $styleArray = $this->getStyleArray(['indent' => $indent]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->indent = $pValue;
+            $this->indent = $indent;
         }
 
         return $this;
@@ -420,20 +420,20 @@ class Alignment extends Supervisor
     /**
      * Set read order.
      *
-     * @param int $pValue
+     * @param int $readOrder
      *
      * @return $this
      */
-    public function setReadOrder($pValue)
+    public function setReadOrder($readOrder)
     {
-        if ($pValue < 0 || $pValue > 2) {
+        if ($readOrder < 0 || $readOrder > 2) {
-            $pValue = 0;
+            $readOrder = 0;
         }
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['readOrder' => $pValue]);
+            $styleArray = $this->getStyleArray(['readOrder' => $readOrder]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->readOrder = $pValue;
+            $this->readOrder = $readOrder;
         }
 
         return $this;

src/PhpSpreadsheet/Style/Border.php

@@ -158,24 +158,24 @@ class Border extends Supervisor
     /**
      * Set Border style.
      *
-     * @param bool|string $pValue
+     * @param bool|string $style
      *                            When passing a boolean, FALSE equates Border::BORDER_NONE
      *                                and TRUE to Border::BORDER_MEDIUM
      *
      * @return $this
      */
-    public function setBorderStyle($pValue)
+    public function setBorderStyle($style)
     {
-        if (empty($pValue)) {
+        if (empty($style)) {
-            $pValue = self::BORDER_NONE;
+            $style = self::BORDER_NONE;
-        } elseif (is_bool($pValue) && $pValue) {
+        } elseif (is_bool($style) && $style) {
-            $pValue = self::BORDER_MEDIUM;
+            $style = self::BORDER_MEDIUM;
         }
         if ($this->isSupervisor) {
-            $styleArray = $this->getStyleArray(['borderStyle' => $pValue]);
+            $styleArray = $this->getStyleArray(['borderStyle' => $style]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
         } else {
-            $this->borderStyle = $pValue;
+            $this->borderStyle = $style;
         }
 
         return $this;
@@ -196,10 +196,10 @@ class Border extends Supervisor
      *
      * @return $this
      */
-    public function setColor(Color $pValue)
+    public function setColor(Color $color)
     {
         // make sure parameter is a real color and not a supervisor
-        $color = $pValue->getIsSupervisor() ? $pValue->getSharedComponent() : $pValue;
+        $color = $color->getIsSupervisor() ? $color->getSharedComponent() : $color;
 
         if ($this->isSupervisor) {
             $styleArray = $this->getColor()->getStyleArray(['argb' => $color->getARGB()]);

src/PhpSpreadsheet/Style/Color.php

@@ -104,26 +104,32 @@ class Color extends Supervisor
      * $spreadsheet->getActiveSheet()->getStyle('B2')->getFont()->getColor()->applyFromArray(['rgb' => '808080']);
      * </code>
      *
-     * @param array $pStyles Array containing style information
+     * @param array $styles Array containing style information
      *
      * @return $this
      */
-    public function applyFromArray(array $pStyles)
+    public function applyFromArray(array $styles)
     {
         if ($this->isSupervisor) {
-            $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($this->getStyleArray($pStyles));
+            $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($this->getStyleArray($styles));
         } else {
-            if (isset($pStyles['rgb'])) {
+            if (isset($styles['rgb'])) {
-                $this->setRGB($pStyles['rgb']);
+                $this->setRGB($styles['rgb']);
             }
-            if (isset($pStyles['argb'])) {
+            if (isset($styles['argb'])) {
-                $this->setARGB($pStyles['argb']);
+                $this->setARGB($styles['argb']);
             }
         }
 
         return $this;
     }
 
+    private function validateColour(string $colorValue, int $size): bool
+    {
+        return in_array(ucfirst($colorValue), self::NAMED_COLORS) ||
+            preg_match(sprintf(self::VALIDATE_COLOR_VALUE, $size), $colorValue);
+    }
+
     /**
      * Get ARGB.
      *
@@ -138,12 +144,6 @@ class Color extends Supervisor
         return $this->argb;
     }
 
-    private function validateColour(string $colorValue, int $size): bool
-    {
-        return in_array(ucfirst($colorValue), self::NAMED_COLORS) ||
-            preg_match(sprintf(self::VALIDATE_COLOR_VALUE, $size), $colorValue);
-    }
-
     /**
      * Set ARGB.
      *
@@ -151,7 +151,7 @@ class Color extends Supervisor
      *
      * @return $this
      */
-    public function setARGB(?string $colorValue)
+    public function setARGB(?string $colorValue = self::COLOR_BLACK)
     {
         if ($colorValue === '' || $colorValue === null) {
             $colorValue = self::COLOR_BLACK;
@@ -190,7 +190,7 @@ class Color extends Supervisor
      *
      * @return $this
      */
-    public function setRGB(?string $colorValue)
+    public function setRGB(?string $colorValue = self::COLOR_BLACK)
     {
         if ($colorValue === '' || $colorValue === null) {
             $colorValue = '000000';
@@ -270,18 +270,18 @@ class Color extends Supervisor
     /**
      * Adjust the brightness of a color.
      *
-     * @param string $hex The colour as an RGBA or RGB value (e.g. FF00CCCC or CCDDEE)
+     * @param string $hexColourValue The colour as an RGBA or RGB value (e.g. FF00CCCC or CCDDEE)
      * @param float $adjustPercentage The percentage by which to adjust the colour as a float from -1 to 1
      *
      * @return string The adjusted colour as an RGBA or RGB value (e.g. FF00CCCC or CCDDEE)
      */
-    public static function changeBrightness($hex, $adjustPercentage)
+    public static function changeBrightness($hexColourValue, $adjustPercentage)
     {
-        $rgba = (strlen($hex) === 8);
+        $rgba = (strlen($hexColourValue) === 8);
 
-        $red = self::getRed($hex, false);
+        $red = self::getRed($hexColourValue, false);
-        $green = self::getGreen($hex, false);
+        $green = self::getGreen($hexColourValue, false);
-        $blue = self::getBlue($hex, false);
+        $blue = self::getBlue($hexColourValue, false);
         if ($adjustPercentage > 0) {
             $red += (255 - $red) * $adjustPercentage;
             $green += (255 - $green) * $adjustPercentage;
@@ -320,16 +320,16 @@ class Color extends Supervisor
     /**
      * Get indexed color.
      *
-     * @param int $pIndex Index entry point into the colour array
+     * @param int $colorIndex Index entry point into the colour array
      * @param bool $background Flag to indicate whether default background or foreground colour
-     *                                            should be returned if the indexed colour doesn't exist
+     *                              should be returned if the indexed colour doesn't exist
      *
-     * @return self
+     * @return string
      */
-    public static function indexedColor($pIndex, $background = false)
+    public static function indexedColor($colorIndex, $background = false)
     {
         // Clean parameter
-        $pIndex = (int) $pIndex;
+        $colorIndex = (int) $colorIndex;
 
         // Indexed colors
         if (self::$indexedColors === null) {
@@ -393,15 +393,11 @@ class Color extends Supervisor
             ];
         }
 
-        if (isset(self::$indexedColors[$pIndex])) {
+        if (isset(self::$indexedColors[$colorIndex])) {
-            return new self(self::$indexedColors[$pIndex]);
+            return new self(self::$indexedColors[$colorIndex]);
         }
 
-        if ($background) {
+        return ($background) ? new self(self::COLOR_WHITE): new self(self::COLOR_BLACK);
-            return new self(self::COLOR_WHITE);
-        }
-
-        return new self(self::COLOR_BLACK);
     }
 
     /**
@@ -409,7 +405,7 @@ class Color extends Supervisor
      *
      * @return string Hash code
      */
-    public function getHashCode()
+    public function getHashCode(): string
     {
         if ($this->isSupervisor) {
             return $this->getSharedComponent()->getHashCode();

tests/PhpSpreadsheetTests/Style/ColorTest.php

@@ -7,6 +7,73 @@ use PHPUnit\Framework\TestCase;
 
 class ColorTest extends TestCase
 {
+    public function testNewColor()
+    {
+        $color = new Color('FF123456');
+        self::assertEquals('FF123456', $color->getARGB());
+        self::assertEquals('123456', $color->getRGB());
+    }
+
+    public function testARGBSetter()
+    {
+        $color = new Color();
+        $color->setARGB('80123456');
+        self::assertEquals('80123456', $color->getARGB());
+        self::assertEquals('123456', $color->getRGB());
+    }
+
+    public function testARGBSetterEmpty()
+    {
+        $color = new Color();
+        $color->setARGB();
+        self::assertEquals(Color::COLOR_BLACK, $color->getARGB());
+    }
+
+    public function testARGBSetterInvalid()
+    {
+        $color = new Color('80123456');
+        $color->setARGB('INVALID COLOR');
+        self::assertEquals('80123456', $color->getARGB());
+    }
+
+    public function testRGBSetter()
+    {
+        $color = new Color();
+        $color->setRGB('123456');
+        self::assertEquals('123456', $color->getRGB());
+        self::assertEquals('FF123456', $color->getARGB());
+    }
+
+    public function testRGBSetterEmpty()
+    {
+        $color = new Color();
+        $color->setRGB();
+        self::assertEquals(Color::COLOR_BLACK, $color->getARGB());
+    }
+
+    public function testRGBSetterInvalid()
+    {
+        $color = new Color('80123456');
+        $color->setRGB('INVALID COLOR');
+        self::assertEquals('123456', $color->getRGB());
+    }
+
+    public function testARGBFromArray()
+    {
+        $color = new Color();
+        $color->applyFromArray(['argb' => '80123456']);
+        self::assertEquals('80123456', $color->getARGB());
+        self::assertEquals('123456', $color->getRGB());
+    }
+
+    public function testRGBFromArray()
+    {
+        $color = new Color();
+        $color->applyFromArray(['rgb' => '123456']);
+        self::assertEquals('123456', $color->getRGB());
+        self::assertEquals('FF123456', $color->getARGB());
+    }
+
     /**
      * @dataProvider providerColorGetRed
      *
@@ -20,7 +87,7 @@ class ColorTest extends TestCase
 
     public function providerColorGetRed()
     {
-        return require 'tests/data/Style/ColorGetRed.php';
+        return require 'tests/data/Style/Color/ColorGetRed.php';
     }
 
     /**
@@ -36,7 +103,7 @@ class ColorTest extends TestCase
 
     public function providerColorGetGreen()
     {
-        return require 'tests/data/Style/ColorGetGreen.php';
+        return require 'tests/data/Style/Color/ColorGetGreen.php';
     }
 
     /**
@@ -52,7 +119,7 @@ class ColorTest extends TestCase
 
     public function providerColorGetBlue()
     {
-        return require 'tests/data/Style/ColorGetBlue.php';
+        return require 'tests/data/Style/Color/ColorGetBlue.php';
     }
 
     /**
@@ -68,6 +135,6 @@ class ColorTest extends TestCase
 
     public function providerColorChangeBrightness()
     {
-        return require 'tests/data/Style/ColorChangeBrightness.php';
+        return require 'tests/data/Style/Color/ColorChangeBrightness.php';
     }
 }