From 2f7131c15d6a05dc9426088d8e518af9d28279ee Mon Sep 17 00:00:00 2001 From: Danny Verkade Date: Thu, 20 Apr 2017 17:43:59 +0200 Subject: [PATCH 1/2] Fixed coding standard violations in the Framework\Data namespace, so that it will be checked bij PHP CS and no longer be ignored while doing CI checks. Made the following changes: - Removed @codingStandardsIgnoreFile at the head of the files - Fixed indentation - Changed is_null function call to a === null compare --- .../Framework/Data/Form/Element/Editor.php | 40 +++++---- .../Framework/Data/Form/Element/Gallery.php | 26 ++++-- .../Framework/Data/Form/Element/Time.php | 44 +++++---- .../Magento/Framework/Data/FormFactory.php | 7 +- .../Data/Test/Unit/Collection/DbTest.php | 32 +++++-- .../Unit/Form/Element/AbstractElementTest.php | 89 +++++++++++++------ 6 files changed, 150 insertions(+), 88 deletions(-) diff --git a/lib/internal/Magento/Framework/Data/Form/Element/Editor.php b/lib/internal/Magento/Framework/Data/Form/Element/Editor.php index 594c97b910e1b..187cf6b4e2bf6 100644 --- a/lib/internal/Magento/Framework/Data/Form/Element/Editor.php +++ b/lib/internal/Magento/Framework/Data/Form/Element/Editor.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - namespace Magento\Framework\Data\Form\Element; use Magento\Framework\Escaper; @@ -123,7 +121,15 @@ public function getElementHtml() ' '; + $html .= ''; } $html .= ''; diff --git a/lib/internal/Magento/Framework/Data/Form/Element/Time.php b/lib/internal/Magento/Framework/Data/Form/Element/Time.php index c813200030267..4f7b6615fc5d2 100644 --- a/lib/internal/Magento/Framework/Data/Form/Element/Time.php +++ b/lib/internal/Magento/Framework/Data/Form/Element/Time.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - /** * Form time element * @@ -54,52 +52,52 @@ public function getElementHtml() { $this->addClass('select admin__control-select'); - $value_hrs = 0; - $value_min = 0; - $value_sec = 0; + $valueHrs = 0; + $valueMin = 0; + $valueSec = 0; if ($value = $this->getValue()) { $values = explode(',', $value); if (is_array($values) && count($values) == 3) { - $value_hrs = $values[0]; - $value_min = $values[1]; - $value_sec = $values[2]; + $valueHrs = $values[0]; + $valueMin = $values[1]; + $valueSec = $values[2]; } } $html = '_getUiId() . '/>'; $html .= '' . "\n"; $html .= ': ' . "\n"; $html .= ': ' . "\n"; diff --git a/lib/internal/Magento/Framework/Data/FormFactory.php b/lib/internal/Magento/Framework/Data/FormFactory.php index 6707340f0c936..ce8d39aa1f23a 100644 --- a/lib/internal/Magento/Framework/Data/FormFactory.php +++ b/lib/internal/Magento/Framework/Data/FormFactory.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - namespace Magento\Framework\Data; /** @@ -54,7 +52,10 @@ public function create(array $data = []) $form = $this->_objectManager->create($this->_instanceName, $data); if (!$form instanceof \Magento\Framework\Data\Form) { throw new \Magento\Framework\Exception\LocalizedException( - new \Magento\Framework\Phrase('%1 doesn\'t extend \Magento\Framework\Data\Form', [$this->_instanceName]) + new \Magento\Framework\Phrase( + '%1 doesn\'t extend \Magento\Framework\Data\Form', + [$this->_instanceName] + ) ); } return $form; diff --git a/lib/internal/Magento/Framework/Data/Test/Unit/Collection/DbTest.php b/lib/internal/Magento/Framework/Data/Test/Unit/Collection/DbTest.php index f3c9cd6795a11..af7745e40ff82 100644 --- a/lib/internal/Magento/Framework/Data/Test/Unit/Collection/DbTest.php +++ b/lib/internal/Magento/Framework/Data/Test/Unit/Collection/DbTest.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - namespace Magento\Framework\Data\Test\Unit\Collection; /** @@ -44,10 +42,18 @@ protected function setUp() { $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); $this->fetchStrategyMock = $this->getMock( - \Magento\Framework\Data\Collection\Db\FetchStrategy\Query::class, ['fetchAll'], [], '', false + \Magento\Framework\Data\Collection\Db\FetchStrategy\Query::class, + ['fetchAll'], + [], + '', + false ); $this->entityFactoryMock = $this->getMock( - \Magento\Framework\Data\Collection\EntityFactory::class, ['create'], [], '', false + \Magento\Framework\Data\Collection\EntityFactory::class, + ['create'], + [], + '', + false ); $this->loggerMock = $this->getMock(\Psr\Log\LoggerInterface::class); $this->collection = new \Magento\Framework\Data\Test\Unit\Collection\DbCollection( @@ -375,7 +381,11 @@ public function testFetchItem() $adapterMock = $this->getMock( \Magento\Framework\DB\Adapter\Pdo\Mysql::class, - ['select', 'query'], [], '', false); + ['select', 'query'], + [], + '', + false + ); $selectMock = $this->getMock( \Magento\Framework\DB\Select::class, [], @@ -430,9 +440,9 @@ public function testGetSize() $adapterMock->expects($this->exactly(2)) ->method('quoteInto') ->will($this->returnValueMap([ - ['testField1=?', 'testValue1', null, null, 'testField1=testValue1'], - ['testField4=?', 'testValue4', null, null, 'testField4=testValue4'], - ])); + ['testField1=?', 'testValue1', null, null, 'testField1=testValue1'], + ['testField4=?', 'testValue4', null, null, 'testField4=testValue4'], + ])); $selectMock->expects($this->once()) ->method('orWhere') ->with('testField1=testValue1'); @@ -558,7 +568,11 @@ public function testToOptionHash() $data = [10 => 'test']; $adapterMock = $this->getMock( \Magento\Framework\DB\Adapter\Pdo\Mysql::class, - ['select', 'query'], [], '', false); + ['select', 'query'], + [], + '', + false + ); $selectMock = $this->getMock( \Magento\Framework\DB\Select::class, [], diff --git a/lib/internal/Magento/Framework/Data/Test/Unit/Form/Element/AbstractElementTest.php b/lib/internal/Magento/Framework/Data/Test/Unit/Form/Element/AbstractElementTest.php index c4621953b78a4..51847b967e797 100644 --- a/lib/internal/Magento/Framework/Data/Test/Unit/Form/Element/AbstractElementTest.php +++ b/lib/internal/Magento/Framework/Data/Test/Unit/Form/Element/AbstractElementTest.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - /** * Tests for \Magento\Framework\Data\Form\Element\AbstractElement */ @@ -36,19 +34,29 @@ class AbstractElementTest extends \PHPUnit_Framework_TestCase protected function setUp() { $this->_factoryMock = $this->getMock( - \Magento\Framework\Data\Form\Element\Factory::class, [], [], '', false + \Magento\Framework\Data\Form\Element\Factory::class, + [], + [], + '', + false ); $this->_collectionFactoryMock = $this->getMock( - \Magento\Framework\Data\Form\Element\CollectionFactory::class, [], [], '', false + \Magento\Framework\Data\Form\Element\CollectionFactory::class, + [], + [], + '', + false ); $this->_escaperMock = $this->getMock(\Magento\Framework\Escaper::class, [], [], '', false); $this->_model = $this->getMockForAbstractClass( - \Magento\Framework\Data\Form\Element\AbstractElement::class, [ - $this->_factoryMock, - $this->_collectionFactoryMock, - $this->_escaperMock - ]); + \Magento\Framework\Data\Form\Element\AbstractElement::class, + [ + $this->_factoryMock, + $this->_collectionFactoryMock, + $this->_escaperMock + ] + ); } /** @@ -58,7 +66,13 @@ public function testAddElement() { $elementId = 11; $elementMock = $this->getMockForAbstractClass( - \Magento\Framework\Data\Form\Element\AbstractElement::class, [], '', false, true, true, ['getId'] + \Magento\Framework\Data\Form\Element\AbstractElement::class, + [], + '', + false, + true, + true, + ['getId'] ); $elementMock->expects($this->once()) ->method('getId') @@ -98,7 +112,11 @@ public function testGetHtmlId() $htmlId = 'some_id'; $formMock = $this->getMock( - \Magento\Framework\Data\Form\AbstractForm::class, ['getHtmlIdPrefix', 'getHtmlIdSuffix'], [], '', false + \Magento\Framework\Data\Form\AbstractForm::class, + ['getHtmlIdPrefix', 'getHtmlIdSuffix'], + [], + '', + false ); $formMock->expects($this->any()) ->method('getHtmlIdPrefix') @@ -168,14 +186,22 @@ public function testRemoveField() $elementId = 'element_id'; $formMock = $this->getMock( - \Magento\Framework\Data\Form\AbstractForm::class, ['removeField'], [], '', false + \Magento\Framework\Data\Form\AbstractForm::class, + ['removeField'], + [], + '', + false ); $formMock->expects($this->once()) ->method('removeField') ->with($elementId); $collectionMock = $this->getMock( - \Magento\Framework\Data\Form\Element\Collection::class, ['remove'], [], '', false + \Magento\Framework\Data\Form\Element\Collection::class, + ['remove'], + [], + '', + false ); $collectionMock->expects($this->once()) ->method('remove') @@ -250,7 +276,8 @@ public function testGetEscapedValueWithoutFilter() { $this->_model->setValue('my \'quoted\' string'); $this->assertEquals( - '<a href="#hash_tag">my \'quoted\' string</a>', $this->_model->getEscapedValue() + '<a href="#hash_tag">my \'quoted\' string</a>', + $this->_model->getEscapedValue() ); } @@ -329,8 +356,10 @@ public function testGetHtmlWithoutRenderer() $this->_model->setForm( $this->getMock(\Magento\Framework\Data\Form\AbstractForm::class, [], [], '', false) ); - $expectedHtml = '
' . "\n" - . '
' . "\n"; + $expectedHtml = '
' + . "\n" + . '
' + . "\n"; $this->assertEquals($expectedHtml, $this->_model->getHtml()); $this->assertEquals(' required-entry _required', $this->_model->getClass()); @@ -407,7 +436,11 @@ public function testGetHtmlContainerIdWithFieldContainerIdPrefix() $id = 'id'; $prefix = 'prefix_'; $formMock = $this->getMock( - \Magento\Framework\Data\Form\AbstractForm::class, ['getFieldContainerIdPrefix'], [], '', false + \Magento\Framework\Data\Form\AbstractForm::class, + ['getFieldContainerIdPrefix'], + [], + '', + false ); $formMock->expects($this->once()) ->method('getFieldContainerIdPrefix') @@ -531,7 +564,7 @@ public function testGetDefaultHtmlDataProvider() [ [], '
' . "\n" - . '
' . "\n", + . '' . "\n", ], [ ['default_html' => 'some default html'], @@ -545,10 +578,10 @@ public function testGetDefaultHtmlDataProvider() 'value' => 'some-value', ], '
' . "\n" - . '' . "\n" - . '' - . '
' . "\n" + . '' . "\n" + . '' + . '' . "\n" ], [ [ @@ -559,8 +592,8 @@ public function testGetDefaultHtmlDataProvider() 'no_span' => true, ], '' . "\n" - . '' + . 'some label' . "\n" + . '' ], ]; } @@ -587,7 +620,7 @@ public function getLabelHtmlDataProvider() 'html_id' => 'some-html-id', ], '' . "\n" + . 'some-label' . "\n" ], [ [ @@ -596,7 +629,7 @@ public function getLabelHtmlDataProvider() 'html_id' => 'some-html-id', ], '' . "\n" + . 'some-label' . "\n" ], ]; } @@ -627,7 +660,7 @@ public function getElementHtmlDataProvider() 'before_element_html' => 'some-html', ], '' - . '' + . '' ], [ [ @@ -646,7 +679,7 @@ public function getElementHtmlDataProvider() 'after_element_html' => 'some-html', ], '' - . '' + . '' ] ]; } From 0e70602ad16e2e27f7e6f927654b334dd80d71db Mon Sep 17 00:00:00 2001 From: Danny Verkade Date: Thu, 20 Apr 2017 20:02:03 +0200 Subject: [PATCH 2/2] - Fixed identation violations --- .../Framework/Data/Form/Element/Editor.php | 31 +++++++++---------- .../Framework/Data/Form/Element/Time.php | 24 ++++++-------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/lib/internal/Magento/Framework/Data/Form/Element/Editor.php b/lib/internal/Magento/Framework/Data/Form/Element/Editor.php index 187cf6b4e2bf6..50d553f5e06ab 100644 --- a/lib/internal/Magento/Framework/Data/Form/Element/Editor.php +++ b/lib/internal/Magento/Framework/Data/Form/Element/Editor.php @@ -260,9 +260,9 @@ protected function _getPluginButtonsHtml($visible = true) $buttonsHtml .= $this->_getButtonHtml( [ 'title' => $this->translate('Insert Widget...'), - 'onclick' => "widgetTools.openDialog('" . $this->getConfig( - 'widget_window_url' - ) . "widget_target_id/" . $this->getHtmlId() . "')", + 'onclick' => "widgetTools.openDialog('" + . $this->getConfig('widget_window_url') + . "widget_target_id/" . $this->getHtmlId() . "')", 'class' => 'action-add-widget plugin', 'style' => $visible ? '' : 'display:none', ] @@ -274,13 +274,12 @@ protected function _getPluginButtonsHtml($visible = true) $buttonsHtml .= $this->_getButtonHtml( [ 'title' => $this->translate('Insert Image...'), - 'onclick' => "MediabrowserUtility.openDialog('" . $this->getConfig( - 'files_browser_window_url' - ) . "target_element_id/" . $this->getHtmlId() . "/" . (null !== $this->getConfig( - 'store_id' - ) ? 'store/' . $this->getConfig( - 'store_id' - ) . '/' : '') . "')", + 'onclick' => "MediabrowserUtility.openDialog('" + . $this->getConfig('files_browser_window_url') + . "target_element_id/" . $this->getHtmlId() . "/" + . (null !== $this->getConfig('store_id') ? 'store/' + . $this->getConfig('store_id') . '/' : '') + . "')", 'class' => 'action-add-image plugin', 'style' => $visible ? '' : 'display:none', ] @@ -395,13 +394,11 @@ protected function _wrapIntoContainer($html) return '
' .$html . '
'; } - $html = '
getConfig( - 'no_display' - ) ? ' style="display:none;"' : '') . ($this->getConfig( - 'container_class' - ) ? ' class="admin__control-wysiwig ' . $this->getConfig( - 'container_class' - ) . '"' : '') . '>' . $html . '
'; + $html = '
getConfig('no_display') ? ' style="display:none;"' : '') + . ($this->getConfig('container_class') ? ' class="admin__control-wysiwig ' + . $this->getConfig('container_class') . '"' : '') + . '>' . $html . '
'; return $html; } diff --git a/lib/internal/Magento/Framework/Data/Form/Element/Time.php b/lib/internal/Magento/Framework/Data/Form/Element/Time.php index 4f7b6615fc5d2..68f32823d7a23 100644 --- a/lib/internal/Magento/Framework/Data/Form/Element/Time.php +++ b/lib/internal/Magento/Framework/Data/Form/Element/Time.php @@ -66,11 +66,9 @@ public function getElementHtml() } $html = '_getUiId() . '/>'; - $html .= 'serialize($this->getHtmlAttributes()) + . $this->_getUiId('hour') . '>' . "\n"; for ($i = 0; $i < 24; $i++) { $hour = str_pad($i, 2, '0', STR_PAD_LEFT); $html .= '