From 2ea9cce689fdc3b03e61ab03d23a71274fc65e2a Mon Sep 17 00:00:00 2001 From: Travis Rowland Date: Sat, 29 Apr 2017 11:23:38 -0700 Subject: [PATCH] Minor updates (annotations and best-practices) (#10534) Combining conditionals and using more succinct idioms. --- src/Auth/DigestAuthenticate.php | 6 ++---- src/Core/ClassLoader.php | 2 +- src/Database/Expression/CaseExpression.php | 10 +++++----- src/Filesystem/File.php | 19 +++++++------------ src/Filesystem/Folder.php | 8 +++----- src/Http/Response.php | 2 +- src/Network/Socket.php | 12 ++++-------- src/ORM/Marshaller.php | 5 ++++- src/Shell/I18nShell.php | 7 ++++++- src/TestSuite/IntegrationTestCase.php | 2 +- src/Validation/ValidationRule.php | 2 +- src/View/ViewBlock.php | 2 +- src/View/Widget/WidgetRegistry.php | 2 +- 13 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/Auth/DigestAuthenticate.php b/src/Auth/DigestAuthenticate.php index c1b0ee076bf..e3ce28ea117 100644 --- a/src/Auth/DigestAuthenticate.php +++ b/src/Auth/DigestAuthenticate.php @@ -225,10 +225,8 @@ public function loginHeaders(ServerRequest $request) ]; $digest = $this->_getDigest($request); - if ($digest && isset($digest['nonce'])) { - if (!$this->validNonce($digest['nonce'])) { - $options['stale'] = true; - } + if ($digest && isset($digest['nonce']) && !$this->validNonce($digest['nonce'])) { + $options['stale'] = true; } $opts = []; diff --git a/src/Core/ClassLoader.php b/src/Core/ClassLoader.php index f35db1d4a96..a5395cf7829 100644 --- a/src/Core/ClassLoader.php +++ b/src/Core/ClassLoader.php @@ -62,7 +62,7 @@ public function addNamespace($prefix, $baseDir, $prepend = false) if ($prepend) { array_unshift($this->_prefixes[$prefix], $baseDir); } else { - array_push($this->_prefixes[$prefix], $baseDir); + $this->_prefixes[$prefix][] = $baseDir; } } diff --git a/src/Database/Expression/CaseExpression.php b/src/Database/Expression/CaseExpression.php index feda7730ba4..f9759b5a75d 100644 --- a/src/Database/Expression/CaseExpression.php +++ b/src/Database/Expression/CaseExpression.php @@ -126,18 +126,18 @@ protected function _addExpressions($conditions, $values, $types) continue; } - array_push($this->_conditions, $c); + $this->_conditions[] = $c; $value = isset($rawValues[$k]) ? $rawValues[$k] : 1; if ($value === 'literal') { $value = $keyValues[$k]; - array_push($this->_values, $value); + $this->_values[] = $value; continue; } if ($value === 'identifier') { $value = new IdentifierExpression($keyValues[$k]); - array_push($this->_values, $value); + $this->_values[] = $value; continue; } @@ -148,11 +148,11 @@ protected function _addExpressions($conditions, $values, $types) } if ($value instanceof ExpressionInterface) { - array_push($this->_values, $value); + $this->_values[] = $value; continue; } - array_push($this->_values, ['value' => $value, 'type' => $type]); + $this->_values[] = ['value' => $value, 'type' => $type]; } } diff --git a/src/Filesystem/File.php b/src/Filesystem/File.php index fa521fc2fda..032d5d7302c 100644 --- a/src/Filesystem/File.php +++ b/src/Filesystem/File.php @@ -106,6 +106,7 @@ public function __destruct() public function create() { $dir = $this->Folder->pwd(); + if (is_dir($dir) && is_writable($dir) && !$this->exists()) { if (touch($this->path)) { return true; @@ -127,10 +128,8 @@ public function open($mode = 'r', $force = false) if (!$force && is_resource($this->handle)) { return true; } - if ($this->exists() === false) { - if ($this->create() === false) { - return false; - } + if ($this->exists() === false && $this->create() === false) { + return false; } $this->handle = fopen($this->path, $mode); @@ -227,10 +226,8 @@ public function write($data, $mode = 'w', $force = false) { $success = false; if ($this->open($mode, $force) === true) { - if ($this->lock !== null) { - if (flock($this->handle, LOCK_EX) === false) { - return false; - } + if ($this->lock !== null && flock($this->handle, LOCK_EX) === false) { + return false; } if (fwrite($this->handle, $data) !== false) { @@ -620,10 +617,8 @@ public function replaceText($search, $replace) return false; } - if ($this->lock !== null) { - if (flock($this->handle, LOCK_EX) === false) { - return false; - } + if ($this->lock !== null && flock($this->handle, LOCK_EX) === false) { + return false; } $replaced = $this->write(str_replace($search, $replace, $this->read()), 'w', true); diff --git a/src/Filesystem/Folder.php b/src/Filesystem/Folder.php index ff7c2254bc6..ffd1813257e 100644 --- a/src/Filesystem/Folder.php +++ b/src/Filesystem/Folder.php @@ -368,7 +368,7 @@ public static function normalizePath($path) */ public static function correctSlashFor($path) { - return (Folder::isWindowsPath($path)) ? '\\' : '/'; + return Folder::isWindowsPath($path) ? '\\' : '/'; } /** @@ -879,10 +879,8 @@ public function move($options) } $options += ['to' => $to, 'from' => $this->path, 'mode' => $this->mode, 'skip' => [], 'recursive' => true]; - if ($this->copy($options)) { - if ($this->delete($options['from'])) { - return (bool)$this->cd($options['to']); - } + if ($this->copy($options) && $this->delete($options['from'])) { + return (bool)$this->cd($options['to']); } return false; diff --git a/src/Http/Response.php b/src/Http/Response.php index c924b8c7baa..e4b480d04f3 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1735,7 +1735,7 @@ protected function _getUTCDate($time = null) } else { $result = new DateTime($time); } - $result->setTimeZone(new DateTimeZone('UTC')); + $result->setTimezone(new DateTimeZone('UTC')); return $result; } diff --git a/src/Network/Socket.php b/src/Network/Socket.php index be1afc315be..3b99baab163 100644 --- a/src/Network/Socket.php +++ b/src/Network/Socket.php @@ -317,10 +317,8 @@ public function setLastError($errNum, $errStr) */ public function write($data) { - if (!$this->connected) { - if (!$this->connect()) { - return false; - } + if (!$this->connected && !$this->connect()) { + return false; } $totalBytes = strlen($data); $written = 0; @@ -344,10 +342,8 @@ public function write($data) */ public function read($length = 1024) { - if (!$this->connected) { - if (!$this->connect()) { - return false; - } + if (!$this->connected && !$this->connect()) { + return false; } if (!feof($this->connection)) { diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php index d9eb37563fa..ccd57fd45b0 100644 --- a/src/ORM/Marshaller.php +++ b/src/ORM/Marshaller.php @@ -356,6 +356,9 @@ public function many(array $data, array $options = []) * @param array $data The data to convert into entities. * @param array $options List of options. * @return array An array of built entities. + * @throws \BadMethodCallException + * @throws \InvalidArgumentException + * @throws \RuntimeException */ protected function _belongsToMany(Association $assoc, array $data, $options = []) { @@ -379,7 +382,7 @@ protected function _belongsToMany(Association $assoc, array $data, $options = [] if (count($keys) === $primaryCount) { $rowConditions = []; foreach ($keys as $key => $value) { - $rowConditions[][$target->aliasfield($key)] = $value; + $rowConditions[][$target->aliasField($key)] = $value; } if ($forceNew && !$target->exists($rowConditions)) { diff --git a/src/Shell/I18nShell.php b/src/Shell/I18nShell.php index 95f945d9f20..60d4fb611d1 100644 --- a/src/Shell/I18nShell.php +++ b/src/Shell/I18nShell.php @@ -43,6 +43,9 @@ class I18nShell extends Shell * Override main() for help message hook * * @return void + * @throws \InvalidArgumentException + * @throws \Cake\Core\Exception\MissingPluginException + * @throws \Cake\Console\Exception\StopException */ public function main() { @@ -80,6 +83,7 @@ public function main() * * @param string|null $language Language code to use. * @return void + * @throws \Cake\Console\Exception\StopException */ public function init($language = null) { @@ -111,7 +115,7 @@ public function init($language = null) } $filename = $fileinfo->getFilename(); $newFilename = $fileinfo->getBasename('.pot'); - $newFilename = $newFilename . '.po'; + $newFilename .= '.po'; $this->createFile($targetFolder . $newFilename, file_get_contents($sourceFolder . $filename)); $count++; @@ -124,6 +128,7 @@ public function init($language = null) * Gets the option parser instance and configures it. * * @return \Cake\Console\ConsoleOptionParser + * @throws \Cake\Console\Exception\ConsoleException */ public function getOptionParser() { diff --git a/src/TestSuite/IntegrationTestCase.php b/src/TestSuite/IntegrationTestCase.php index 85a3712a7e5..61ae6cc7f04 100644 --- a/src/TestSuite/IntegrationTestCase.php +++ b/src/TestSuite/IntegrationTestCase.php @@ -763,7 +763,7 @@ public function assertNoRedirect($message = '') if (!empty($result)) { $message .= ': ' . $result; } - $this->assertTrue(empty($result), $message); + $this->assertEmpty($result, $message); } /** diff --git a/src/Validation/ValidationRule.php b/src/Validation/ValidationRule.php index 7d3d306ec55..e95a88db948 100644 --- a/src/Validation/ValidationRule.php +++ b/src/Validation/ValidationRule.php @@ -169,7 +169,7 @@ protected function _skip($context) $newRecord = $context['newRecord']; if (!empty($this->_on)) { - if ($this->_on === 'create' && !$newRecord || $this->_on === 'update' && $newRecord) { + if (($this->_on === 'create' && !$newRecord) || ($this->_on === 'update' && $newRecord)) { return true; } } diff --git a/src/View/ViewBlock.php b/src/View/ViewBlock.php index d69d1ec4b4a..e1fd95627c2 100644 --- a/src/View/ViewBlock.php +++ b/src/View/ViewBlock.php @@ -86,7 +86,7 @@ class ViewBlock */ public function start($name, $mode = ViewBlock::OVERRIDE) { - if (in_array($name, array_keys($this->_active))) { + if (array_key_exists($name, $this->_active)) { throw new Exception(sprintf("A view block with the name '%s' is already/still open.", $name)); } $this->_active[$name] = $mode; diff --git a/src/View/Widget/WidgetRegistry.php b/src/View/Widget/WidgetRegistry.php index 5a572d4cf85..2590d51b58f 100644 --- a/src/View/Widget/WidgetRegistry.php +++ b/src/View/Widget/WidgetRegistry.php @@ -115,7 +115,7 @@ public function load($file) public function add(array $widgets) { foreach ($widgets as $object) { - if (gettype($object) === 'object' && + if (is_object($object) && !($object instanceof WidgetInterface) ) { throw new RuntimeException(