Replace strlen() calls with strict string comparisons
authorThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Wed, 27 Mar 2019 10:13:08 +0000 (11:13 +0100)
committerThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Thu, 28 Mar 2019 11:32:39 +0000 (12:32 +0100)
Note there is an important difference between the two ways to express
this: strlen() does a string cast, but the `=== ''` and `!== ''`
comparisons will only detect empty strings, but not null, false, or any
other falsy value that becomes an empty string when cast to be one.

I am only touching code where I'm sure the variable is guaranteed to be
a string.

This change is done because I find the strict comparisons much more
readable. The code does exactly one thing now, and no magic casts any
more.

Change-Id: I3e908a0c7c7b6c29b0e5a1414f2ba9062a215b93

includes/MovePage.php
includes/SiteConfiguration.php
includes/api/ApiQuerySiteinfo.php
includes/content/WikiTextStructure.php
includes/http/PhpHttpRequest.php
includes/jobqueue/jobs/ThumbnailRenderJob.php
includes/libs/rdbms/database/DatabaseDomain.php
includes/specials/SpecialSearch.php

index bcec0a1..db5750a 100644 (file)
@@ -106,7 +106,7 @@ class MovePage {
 
                $oldid = $this->oldTitle->getArticleID();
 
-               if ( strlen( $this->newTitle->getDBkey() ) < 1 ) {
+               if ( $this->newTitle->getDBkey() === '' ) {
                        $status->fatal( 'articleexists' );
                }
                if (
index 7af80dc..b400797 100644 (file)
@@ -562,7 +562,7 @@ class SiteConfiguration {
                                ->execute();
 
                        $data = trim( $result->getStdout() );
-                       if ( $result->getExitCode() != 0 || !strlen( $data ) ) {
+                       if ( $result->getExitCode() || $data === '' ) {
                                throw new MWException( "Failed to run getConfiguration.php: {$result->getStdout()}" );
                        }
                        $res = unserialize( $data );
index 96fa8a1..82a52b4 100644 (file)
@@ -664,8 +664,8 @@ class ApiQuerySiteinfo extends ApiQueryBase {
                }
 
                $data = [
-                       'url' => strlen( $url ) ? $url : '',
-                       'text' => strlen( $text ) ? $text : '',
+                       'url' => (string)$url,
+                       'text' => (string)$text,
                ];
 
                return $this->getResult()->addValue( 'query', $property, $data );
index 0e03e72..2f3a6f6 100644 (file)
@@ -154,7 +154,7 @@ class WikiTextStructure {
                        'enableSectionEditTokens' => false,
                        'allowTOC' => false,
                ] );
-               if ( strlen( $text ) == 0 ) {
+               if ( $text === '' ) {
                        $this->allText = "";
                        // empty text - nothing to seek here
                        return;
index 30ab181..d2af8c8 100644 (file)
@@ -217,7 +217,7 @@ class PhpHttpRequest extends MWHttpRequest {
                                        break;
                                }
 
-                               if ( strlen( $buf ) ) {
+                               if ( $buf !== '' ) {
                                        call_user_func( $this->callback, $fh, $buf );
                                }
                        }
index 63575eb..eb8b1a2 100644 (file)
@@ -93,7 +93,7 @@ class ThumbnailRenderJob extends Job {
                if ( $wgUploadThumbnailRenderHttpCustomDomain ) {
                        $parsedUrl = wfParseUrl( $thumbUrl );
 
-                       if ( !$parsedUrl || !isset( $parsedUrl['path'] ) || !strlen( $parsedUrl['path'] ) ) {
+                       if ( !isset( $parsedUrl['path'] ) || $parsedUrl['path'] === '' ) {
                                $this->setLastError( __METHOD__ . ": invalid thumb URL: $thumbUrl" );
                                return false;
                        }
index 8d82854..ca57938 100644 (file)
@@ -42,11 +42,11 @@ class DatabaseDomain {
         * @param string $prefix Table prefix
         */
        public function __construct( $database, $schema, $prefix ) {
-               if ( $database !== null && ( !is_string( $database ) || !strlen( $database ) ) ) {
+               if ( $database !== null && ( !is_string( $database ) || $database === '' ) ) {
                        throw new InvalidArgumentException( 'Database must be null or a non-empty string.' );
                }
                $this->database = $database;
-               if ( $schema !== null && ( !is_string( $schema ) || !strlen( $schema ) ) ) {
+               if ( $schema !== null && ( !is_string( $schema ) || $schema === '' ) ) {
                        throw new InvalidArgumentException( 'Schema must be null or a non-empty string.' );
                }
                $this->schema = $schema;
index e6d0632..171566b 100644 (file)
@@ -102,7 +102,7 @@ class SpecialSearch extends SpecialPage {
        /**
         * Entry point
         *
-        * @param string $par
+        * @param string|null $par
         */
        public function execute( $par ) {
                $request = $this->getRequest();
@@ -115,7 +115,7 @@ class SpecialSearch extends SpecialPage {
                // parameter, but also as part of the primary url. This can have PII implications
                // in releasing page view data. As such issue a 301 redirect to the correct
                // URL.
-               if ( strlen( $par ) && !strlen( $term ) ) {
+               if ( $par !== null && $par !== '' && $term === '' ) {
                        $query = $request->getValues();
                        unset( $query['title'] );
                        // Strip underscores from title parameter; most of the time we'll want