Jonaskeutel has submitted this change and it was merged.
Change subject: code cleanup
......................................................................
code cleanup
added more type hinting
fixed one of checker test accordingly
minor fix in sorting of check result
removed unreachable code
Change-Id: I81ff1923d5c109fd2ff344f2a4877108c9924ec4
---
A .coveralls.yml
A .gitreview
M constraint-report/includes/ConstraintCheck/Checker/ConnectionChecker.php
M constraint-report/includes/ConstraintCheck/ConstraintChecker.php
M constraint-report/includes/ConstraintCheck/Result/CheckResult.php
M constraint-report/specials/SpecialConstraintReport.php
M constraint-report/tests/phpunit/Checker/OneOfChecker/OneOfCheckerTest.php
M specials/SpecialCheckResultPage.php
8 files changed, 47 insertions(+), 38 deletions(-)
Approvals:
Jonaskeutel: Verified; Looks good to me, approved
diff --git a/.coveralls.yml b/.coveralls.yml
new file mode 100644
index 0000000..8a42d12
--- /dev/null
+++ b/.coveralls.yml
@@ -0,0 +1,2 @@
+coverage_clover: build/logs/clover.xml
+src_dir: .
diff --git a/.gitreview b/.gitreview
new file mode 100644
index 0000000..deef200
--- /dev/null
+++ b/.gitreview
@@ -0,0 +1,6 @@
+[gerrit]
+host=gerrit.wikimedia.org
+port=29418
+project=mediawiki/extensions/WikidataQuality.git
+defaultbranch=master
+defaultrebase=0
diff --git
a/constraint-report/includes/ConstraintCheck/Checker/ConnectionChecker.php
b/constraint-report/includes/ConstraintCheck/Checker/ConnectionChecker.php
index 37d6b62..3e05c2c 100644
--- a/constraint-report/includes/ConstraintCheck/Checker/ConnectionChecker.php
+++ b/constraint-report/includes/ConstraintCheck/Checker/ConnectionChecker.php
@@ -321,7 +321,7 @@
*
* @param array $statementsArray
* @param string $propertyIdSerialization
- * @param mixed string|array $itemIdSerializationOrArray
+ * @param string|array $itemIdSerializationOrArray
*
* @return boolean
*/
diff --git a/constraint-report/includes/ConstraintCheck/ConstraintChecker.php
b/constraint-report/includes/ConstraintCheck/ConstraintChecker.php
index f44637c..d3caf08 100644
--- a/constraint-report/includes/ConstraintCheck/ConstraintChecker.php
+++ b/constraint-report/includes/ConstraintCheck/ConstraintChecker.php
@@ -2,11 +2,15 @@
namespace WikidataQuality\ConstraintReport\ConstraintCheck;
-use Wikibase\Repo\WikibaseRepo;
+use Wikibase\Lib\Store\EntityLookup;
+use LoadBalancer;
use Wikibase\Repo\Store;
use Wikibase\DataModel\Statement;
+use Wikibase\DataModel\Statement\StatementList;
use Wikibase\DataModel\Snak;
use Wikibase\Datamodel\Entity;
+use Wikibase\DataModel\Entity\PropertyId;
+use DataValues\DataValue;
use WikidataQuality\ConstraintReport\ConstraintCheck\Checker\ValueCountChecker;
use
WikidataQuality\ConstraintReport\ConstraintCheck\Checker\CommonsLinkChecker;
use WikidataQuality\ConstraintReport\ConstraintCheck\Checker\ConnectionChecker;
@@ -32,14 +36,14 @@
/**
* Wikibase entity lookup.
*
- * @var \Wikibase\Lib\Store\EntityLookup
+ * @var EntityLookup
*/
private $entityLookup;
/**
* Wikibase load balancer for database connections.
*
- * @var \LoadBalancer
+ * @var LoadBalancer
*/
private $loadBalancer;
@@ -113,7 +117,7 @@
*/
private $statements;
- public function __construct( $lookup ) {
+ public function __construct( EntityLookup $lookup ) {
// Get entity lookup
$this->entityLookup = $lookup;
@@ -129,9 +133,9 @@
* Starts the whole constraint-check process.
* Statements of the entity will be checked against every constraint
that is defined on the property.
*
- * @param Entity\Entity $entity - Entity that shall be checked against
constraints
+ * @param Entity $entity - Entity that shall be checked against
constraints
*
- * @return Array|null
+ * @return array|null
*/
public function execute( $entity ) {
if ( $entity ) {
@@ -175,7 +179,6 @@
return $result;
}
-
private function checkConstraintsForStatementOnEntity( $constraints,
$entity, $statement ) {
$result = array();
foreach ( $constraints as $row ) {
@@ -192,17 +195,11 @@
return $result;
}
-
/**
- * @param $propertyId
- * @param $dataValue
+ * @param Statement $statement
+ * @param string $constraintQid
* @param $constraintParameters
- * @param $constraintQid
- * @param $classArray
- * @param $itemArray
- * @param $propertyArray
- * @param $entity
- * @param $statement
+ * @param Entity $entity
*
* @return CheckResult
*/
@@ -235,11 +232,9 @@
case 'Inverse':
return $this->getConnectionChecker()
->checkInverseConstraint( $propertyId, $dataValue,
$entity->getId()->getSerialization(), $this->helper->getPropertyOfJson(
$constraintParameters, 'property' ) );
- break;
case 'Conflicts with':
return $this->getConnectionChecker()
->checkConflictsWithConstraint( $propertyId, $dataValue,
$constraintParameters->property, $itemArray );
- break;
case 'Item':
return $this->getConnectionChecker()
->checkItemConstraint(
$propertyId, $dataValue, $constraintParameters->property, $itemArray );
@@ -265,8 +260,8 @@
// Type Checkers
case 'Type':
- return
$this->getTypeChecker()->checkTypeConstraint( $propertyId, $dataValue,
$this->statements, $classArray, $this->helper->getPropertyOfJson(
$constraintParameters, 'relation' ) );
- break;
+ return $this->getTypeChecker()
+ ->checkTypeConstraint(
$propertyId, $dataValue, $this->statements, $classArray,
$this->helper->getPropertyOfJson( $constraintParameters, 'relation' ) );
case 'Value type':
return $this->getTypeChecker()
->checkValueTypeConstraint( $propertyId, $dataValue, $classArray,
$this->helper->getPropertyOfJson( $constraintParameters, 'relation' ) );
@@ -290,17 +285,17 @@
private function queryConstraintsForProperty( $dbr, $prop ) {
return $dbr->select(
- CONSTRAINT_TABLE,
// table name
- array ( 'pid', 'constraint_qid',
'constraint_parameters' ), // columns of the table
- ( "pid = $prop" ),
// conditions
- __METHOD__,
// $fname = 'Database::select',
- array ( '' )
// $options = array()
+ CONSTRAINT_TABLE,
+ array ( 'pid', 'constraint_qid',
'constraint_parameters' ),
+ ( "pid = $prop" ),
+ __METHOD__,
+ array ( '' )
);
}
private function sortResult( $result ) {
$sortFunction = function ( $a, $b ) {
- $order = array ( 'compliance' => 0, 'exception' => 1,
'todo' => 2, 'violation' => 3, 'other' => 4 );
+ $order = array ( 'other' => 4, 'compliance' => 3,
'exception' => 2, 'violation' => 1 );
$statusA = $a->getStatus();
$statusB = $b->getStatus();
@@ -311,7 +306,7 @@
if ( $orderA === $orderB ) {
return 0;
} else {
- return ( $orderA < $orderB ) ? 1 : -1;
+ return ( $orderA > $orderB ) ? 1 : -1;
}
};
diff --git a/constraint-report/includes/ConstraintCheck/Result/CheckResult.php
b/constraint-report/includes/ConstraintCheck/Result/CheckResult.php
index e31b707..ce35140 100644
--- a/constraint-report/includes/ConstraintCheck/Result/CheckResult.php
+++ b/constraint-report/includes/ConstraintCheck/Result/CheckResult.php
@@ -32,7 +32,7 @@
private $constraintName;
/**
- * @var Array
+ * @var array
* Includes arrays of ItemIds or PropertyIds or strings.
*/
private $parameters;
@@ -47,7 +47,7 @@
*/
private $message;
- public function __construct( $propertyId, $dataValue, $constraintName,
$parameters = array (), $status = 'todo', $message = '' ) {
+ public function __construct( PropertyId $propertyId, DataValue
$dataValue, $constraintName, $parameters = array (), $status = 'todo', $message
= '' ) {
$this->propertyId = $propertyId;
$this->dataValue = $dataValue;
$this->constraintName = $constraintName;
@@ -78,7 +78,7 @@
}
/**
- * @return Array
+ * @return array
*/
public function getParameters() {
return $this->parameters;
diff --git a/constraint-report/specials/SpecialConstraintReport.php
b/constraint-report/specials/SpecialConstraintReport.php
index 89050e3..1099505 100644
--- a/constraint-report/specials/SpecialConstraintReport.php
+++ b/constraint-report/specials/SpecialConstraintReport.php
@@ -3,6 +3,7 @@
namespace WikidataQuality\ConstraintReport\Specials;
use DataValues;
+use DataValues\DataValue;
use Html;
use Wikibase\DataModel;
use Wikibase\DataModel\Entity\EntityDocument;
@@ -207,7 +208,7 @@
/**
* Formats values of constraints.
*
- * @param string|ItemId|PropertyId|DataValues\DataValue $value
+ * @param string|ItemId|PropertyId|DataValue $value
* @param bool $linking
*
* @return string
diff --git
a/constraint-report/tests/phpunit/Checker/OneOfChecker/OneOfCheckerTest.php
b/constraint-report/tests/phpunit/Checker/OneOfChecker/OneOfCheckerTest.php
index 11de216..ceb17c9 100644
--- a/constraint-report/tests/phpunit/Checker/OneOfChecker/OneOfCheckerTest.php
+++ b/constraint-report/tests/phpunit/Checker/OneOfChecker/OneOfCheckerTest.php
@@ -5,6 +5,7 @@
use DataValues\StringValue;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
use WikidataQuality\ConstraintReport\ConstraintCheck\Checker\OneOfChecker;
use
WikidataQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintReportHelper;
@@ -39,26 +40,26 @@
$valueIn = new EntityIdValue( new ItemId( 'Q1' ) );
$valueNotIn = new EntityIdValue( new ItemId( 'Q9' ) );
$values = array ( 'Q1', 'Q2', 'Q3' );
- $this->assertEquals( 'compliance',
$this->oneOfChecker->checkOneOfConstraint( 123, $valueIn, $values
)->getStatus(), 'check should comply' );
- $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( 123, $valueNotIn, $values
)->getStatus(), 'check should not comply' );
+ $this->assertEquals( 'compliance',
$this->oneOfChecker->checkOneOfConstraint( new PropertyId( 'P123' ), $valueIn,
$values )->getStatus(), 'check should comply' );
+ $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( new PropertyId( 'P123' ),
$valueNotIn, $values )->getStatus(), 'check should not comply' );
}
public function testCheckOneOfConstraintWrongType() {
$value = new StringValue( 'Q1' );
$values = array ( 'Q1', 'Q2', 'Q3' );
- $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( 123, $value, $values )->getStatus(),
'check should not comply' );
+ $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( new PropertyId( 'P123' ), $value,
$values )->getStatus(), 'check should not comply' );
}
public function testCheckOneOfConstraintEmptyArray() {
$value = new EntityIdValue( new ItemId( 'Q1' ) );
$values = array ( '' );
- $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( 123, $value, $values )->getStatus(),
'check should not comply' );
+ $this->assertEquals( 'violation',
$this->oneOfChecker->checkOneOfConstraint( new PropertyId( 'P123' ), $value,
$values )->getStatus(), 'check should not comply' );
}
public function testCheckOneOfConstraintArrayWithSomevalue() {
$value = new EntityIdValue( new ItemId( 'Q1' ) );
$values = array ( 'Q1', 'Q2', 'Q3', 'somevalue' );
- $this->assertEquals( 'compliance',
$this->oneOfChecker->checkOneOfConstraint( 123, $value, $values )->getStatus(),
'check should comply' );
+ $this->assertEquals( 'compliance',
$this->oneOfChecker->checkOneOfConstraint( new PropertyId( 'P123' ), $value,
$values )->getStatus(), 'check should comply' );
}
}
\ No newline at end of file
diff --git a/specials/SpecialCheckResultPage.php
b/specials/SpecialCheckResultPage.php
index a9dbeae..f803ed3 100644
--- a/specials/SpecialCheckResultPage.php
+++ b/specials/SpecialCheckResultPage.php
@@ -111,7 +111,7 @@
return;
}
- // Check, if entity exists
+ // Check if entity exists
if ( !$entity ) {
$out->addHTML(
$this->buildNotice( $this->msg(
'wikidataquality-checkresult-not-existent-entity' )->text(), true )
@@ -206,6 +206,8 @@
* Returns html text of the result header
*
* @param EntityId
+ *
+ * @return string
*/
protected function buildResultHeader( EntityId $entityId ) {
$entityLink = sprintf( '%s (%s)',
@@ -291,6 +293,8 @@
* Formats given status to html.
*
* @param string $status
+ *
+ * @return string
*/
protected function formatStatus( $status ) {
// Get i18n message
--
To view, visit https://gerrit.wikimedia.org/r/203810
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I81ff1923d5c109fd2ff344f2a4877108c9924ec4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQuality
Gerrit-Branch: master
Gerrit-Owner: Andreasburmeister <[email protected]>
Gerrit-Reviewer: Jonaskeutel <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
