jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/351795 )
Change subject: Use a consistent code style to check for flags in a bit field
......................................................................
Use a consistent code style to check for flags in a bit field
Our current code base uses a bunch of different code styles to check
for flags in a bit field:
* ( $var & CONST ) === CONST
* ( $var & CONST ) !== 0
* ( $var & CONST ) > 0
* (bool)( $var & CONST )
* $var & CONST
I find, for example, the first one quite confusing. (Can you quickly
say if it checks if a flag is set or not set?) I also find it confusing
to have so many styles next to each other.
This patch proposes:
* Stick to the "!== 0" style in case it's critical to have a boolean
result. This is more specific than a vague "> 0". It avoids repeating
the const name, which is a source of mistakes. It's also close to other
styles we already use, for example "!== null" or "=== []" for empty
array checks.
* In an if() use the shortest possible "if ( $var & CONST )". This is
what most code already does.
This patch directly follows the comments I wrote in I5fc2557.
Change-Id: I7f7bd6a445f6b0c3b9b33ee7d0d7e6adb03209c0
---
M lib/tests/phpunit/MockRepository.php
M repo/WikibaseRepo.entitytypes.php
M repo/includes/Api/EntitySavingHelper.php
M repo/includes/Content/EntityContent.php
M repo/includes/Store/Sql/WikiPageEntityStore.php
M repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php
M repo/tests/phpunit/includes/Rdf/RdfBuilderTest.php
7 files changed, 12 insertions(+), 12 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
Aleksey Bekh-Ivanov (WMDE): Looks good to me, but someone else must approve
jenkins-bot: Verified
Jforrester: Looks good to me, approved
diff --git a/lib/tests/phpunit/MockRepository.php
b/lib/tests/phpunit/MockRepository.php
index 40ed64a..ab67ef0 100644
--- a/lib/tests/phpunit/MockRepository.php
+++ b/lib/tests/phpunit/MockRepository.php
@@ -494,11 +494,11 @@
$status = Status::newGood();
- if ( ( $flags & EDIT_NEW ) > 0 && $entityId &&
$this->hasEntity( $entityId ) ) {
+ if ( ( $flags & EDIT_NEW ) && $entityId && $this->hasEntity(
$entityId ) ) {
$status->fatal( 'edit-already-exists' );
}
- if ( ( $flags & EDIT_UPDATE ) > 0 && !$this->hasEntity(
$entityId ) ) {
+ if ( ( $flags & EDIT_UPDATE ) && !$this->hasEntity( $entityId )
) {
$status->fatal( 'edit-gone-missing' );
}
diff --git a/repo/WikibaseRepo.entitytypes.php
b/repo/WikibaseRepo.entitytypes.php
index 953066f..e3dea3e 100644
--- a/repo/WikibaseRepo.entitytypes.php
+++ b/repo/WikibaseRepo.entitytypes.php
@@ -70,7 +70,7 @@
$mentionedEntityTracker,
$dedupe
) {
- if ( ( $flavorFlags & RdfProducer::PRODUCE_SITELINKS )
!== 0 ) {
+ if ( $flavorFlags & RdfProducer::PRODUCE_SITELINKS ) {
$sites =
WikibaseRepo::getDefaultInstance()->getSiteLookup()->getSites();
// Since the only extra mapping needed for
Items are site links,
// we just return the SiteLinksRdfBuilder
directly,
diff --git a/repo/includes/Api/EntitySavingHelper.php
b/repo/includes/Api/EntitySavingHelper.php
index 4296076..893b6b1 100644
--- a/repo/includes/Api/EntitySavingHelper.php
+++ b/repo/includes/Api/EntitySavingHelper.php
@@ -390,11 +390,11 @@
$editError = $value['errorFlags'];
}
- if ( ( $editError & EditEntityHandler::TOKEN_ERROR ) >
0 ) {
+ if ( $editError & EditEntityHandler::TOKEN_ERROR ) {
$errorCode = 'badtoken';
- } elseif ( ( $editError &
EditEntityHandler::EDIT_CONFLICT_ERROR ) > 0 ) {
+ } elseif ( $editError &
EditEntityHandler::EDIT_CONFLICT_ERROR ) {
$errorCode = 'editconflict';
- } elseif ( ( $editError & EditEntityHandler::ANY_ERROR
) > 0 ) {
+ } elseif ( $editError & EditEntityHandler::ANY_ERROR ) {
$errorCode = 'failed-save';
}
}
diff --git a/repo/includes/Content/EntityContent.php
b/repo/includes/Content/EntityContent.php
index 41a05c1..5e10817 100644
--- a/repo/includes/Content/EntityContent.php
+++ b/repo/includes/Content/EntityContent.php
@@ -659,10 +659,10 @@
$status = parent::prepareSave( $page, $flags, $baseRevId, $user
);
if ( $status->isOK() ) {
- if ( !$this->isRedirect() && ( $flags &
self::EDIT_IGNORE_CONSTRAINTS ) === 0 ) {
+ if ( !$this->isRedirect() && !( $flags &
self::EDIT_IGNORE_CONSTRAINTS ) ) {
/* @var EntityHandler $handler */
$handler = $this->getContentHandler();
- $validators = $handler->getOnSaveValidators( (
$flags & EDIT_NEW ) > 0 );
+ $validators = $handler->getOnSaveValidators( (
$flags & EDIT_NEW ) !== 0 );
$status = $this->applyValidators( $validators );
}
}
diff --git a/repo/includes/Store/Sql/WikiPageEntityStore.php
b/repo/includes/Store/Sql/WikiPageEntityStore.php
index 838e35b..359763e 100644
--- a/repo/includes/Store/Sql/WikiPageEntityStore.php
+++ b/repo/includes/Store/Sql/WikiPageEntityStore.php
@@ -170,7 +170,7 @@
$baseRevId = false
) {
if ( $entity->getId() === null ) {
- if ( ( $flags & EDIT_NEW ) !== EDIT_NEW ) {
+ if ( !( $flags & EDIT_NEW ) ) {
throw new StorageException( Status::newFatal(
'edit-gone-missing' ) );
}
@@ -259,7 +259,7 @@
) {
$page = $this->getWikiPageForEntity(
$entityContent->getEntityId() );
- if ( ( $flags & EDIT_NEW ) === EDIT_NEW ) {
+ if ( $flags & EDIT_NEW ) {
$title = $page->getTitle();
if ( $title->exists() ) {
throw new StorageException( Status::newFatal(
'edit-already-exists' ) );
diff --git a/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php
b/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php
index 450d574..58d8511 100644
--- a/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php
+++ b/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php
@@ -128,7 +128,7 @@
$mentionedEntityTracker,
$dedupe
) use ( $siteLookup ) {
- if ( ( $flavorFlags &
RdfProducer::PRODUCE_SITELINKS ) !== 0 ) {
+ if ( $flavorFlags &
RdfProducer::PRODUCE_SITELINKS ) {
$sites = $siteLookup->getSites();
$builder = new SiteLinksRdfBuilder(
$vocabulary, $writer, $sites );
$builder->setDedupeBag( $dedupe );
diff --git a/repo/tests/phpunit/includes/Rdf/RdfBuilderTest.php
b/repo/tests/phpunit/includes/Rdf/RdfBuilderTest.php
index 926e164..e35f6c8 100644
--- a/repo/tests/phpunit/includes/Rdf/RdfBuilderTest.php
+++ b/repo/tests/phpunit/includes/Rdf/RdfBuilderTest.php
@@ -96,7 +96,7 @@
$mentionedEntityTracker,
$dedupe
) use ( $siteLookup ) {
- if ( ( $flavorFlags &
RdfProducer::PRODUCE_SITELINKS ) !== 0 ) {
+ if ( $flavorFlags &
RdfProducer::PRODUCE_SITELINKS ) {
$sites = $siteLookup->getSites();
$builder = new SiteLinksRdfBuilder(
$vocabulary, $writer, $sites );
$builder->setDedupeBag( $dedupe );
--
To view, visit https://gerrit.wikimedia.org/r/351795
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7f7bd6a445f6b0c3b9b33ee7d0d7e6adb03209c0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
