Merge "Check bounds in IntegerChecker"

i18n/en.json

@@ -164,6 +164,7 @@
 	"wbqc-violation-message-entityType": "The property $1 should not be used on this type of entity, the only valid {{PLURAL:$2|1=entity type is $4.|2=entity types are $4 and $5.|entity types are: $3}}",
 	"wbqc-violation-message-none-of": "The value for $1 should not be {{PLURAL:$2|1=$4.|2=either $4 or $5.|one of the following:$3}}",
 	"wbqc-violation-message-integer": "Values for $1 should be integer, but $2 has a fractional part.",
+	"wbqc-violation-message-integer-bounds": "Values for $1 should be integer, but the bounds of $2 have a fractional part.",
 
 	"wbqc-violation-message-exception": "This entity is a known exception for this constraint and has been marked as such."
 }

i18n/qqq.json

@@ -160,6 +160,7 @@
 	"wbqc-violation-message-units-or-none": "Message for a violation of the “allowed units” constraint, when a property should be used with a certain set of units (including “no value”, permitting unitless values) but the value has a different unit. Parameters:\n* $1 is the property ID of the property to which the constraint applies.\n* $2 is the number of allowed units.\n* $3 is an HTML list of all allowed units.\n* $4, $5 etc. are the individual allowed units.",
 	"wbqc-violation-message-entityType": "Message for a violation of the “entity type” constraint, when a property should only be used in limited number entity types and not all. Parameters:\n* $1 is the property which has the constraint.\n* $2 is the number of acceptable entity types.\n* $3 is an HTML list of all acceptable entity types.\n* $4, $5 etc. are the individual acceptable entity types.",
 	"wbqc-violation-message-none-of": "Message for a violation of the “none of” constraint, when the value is one of the disallowed values. Parameters:\n* $1 is the property of the statement.\n* $2 the number of disallowed values (note that certain constraints might have only a single disallowed value).\n* $3 is an HTML list of all disallowed values.\n* $4, $5 etc. are the individual disallowed values.",
-	"wbqc-violation-message-integer": "Message for a violation of the “integer” constraint, when a property intended to have integer quantities as its value. Parameters:\n* $1 is the property which has the constraint.\n* $2 is the value of the statement.",
+	"wbqc-violation-message-integer": "Message for a violation of the “integer” constraint, when a property should have integer quantities as its value but the value is not integer. Parameters:\n* $1 is the property which has the constraint.\n* $2 is the value of the statement.",
+	"wbqc-violation-message-integer-bounds": "Message for a violation of the “integer” constraint, when a property should have integer quantities as its value and the value is integer but its bounds are not (for example: 10±0.5). Parameters:\n* $1 is the property which has the constraint.\n* $2 is the value of the statement.",
 	"wbqc-violation-message-exception": "Message for a constraint check result on an entity that has been marked as an exception to the constraint. This message only appears on [[Special:ConstraintReport]]; the gadget does not show exception reports."
 }

src/ConstraintCheck/Checker/IntegerChecker.php

@@ -3,6 +3,7 @@
 namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Checker;
 
 use DataValues\DecimalValue;
+use DataValues\QuantityValue;
 use DataValues\UnboundedQuantityValue;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
 use WikibaseQuality\ConstraintReport\Constraint;
@@ -41,39 +42,71 @@ public function getDefaultContextTypes() {
 	}
 
 	public function checkConstraint( Context $context, Constraint $constraint ) {
-		if ( $context->getSnak()->getType() !== 'value' ) {
-			return new CheckResult( $context, $constraint, [], CheckResult::STATUS_COMPLIANCE );
-		}
-
-		/** @var PropertyValueSnak $snak */
 		$snak = $context->getSnak();
 
-		if ( $snak->getDataValue() instanceof DecimalValue ) {
-			return $this->checkDecimalValue( $snak->getDataValue(), $context, $constraint );
-		} elseif ( $snak->getDataValue() instanceof UnboundedQuantityValue ) {
-			return $this->checkDecimalValue( $snak->getDataValue()->getAmount(), $context, $constraint );
-		}
-
-		return new CheckResult( $context, $constraint, [], CheckResult::STATUS_COMPLIANCE );
-	}
-
-	private function checkDecimalValue( DecimalValue $decimalValue, Context $context, Constraint $constraint ) {
-		if ( $decimalValue->getTrimmed()->getFractionalPart() === '' ) {
+		if ( $context->getSnak()->getType() !== 'value' ) {
 			return new CheckResult( $context, $constraint, [], CheckResult::STATUS_COMPLIANCE );
 		}
 
-		$message = ( new ViolationMessage( 'wbqc-violation-message-integer' ) )
-			->withEntityId( $context->getSnak()->getPropertyId(), Role::CONSTRAINT_PROPERTY )
-			->withDataValue( $decimalValue );
+		$violationMessage = $this->checkSnak( $snak );
+
 		return new CheckResult(
 			$context,
 			$constraint,
 			[],
-			CheckResult::STATUS_VIOLATION,
-			$message
+			$violationMessage === null ?
+				CheckResult::STATUS_COMPLIANCE :
+				CheckResult::STATUS_VIOLATION,
+			$violationMessage
 		);
 	}
 
+	/**
+	 * @param PropertyValueSnak $snak
+	 * @return ViolationMessage|null
+	 */
+	public function checkSnak( PropertyValueSnak $snak ) {
+		$dataValue = $snak->getDataValue();
+
+		if ( $dataValue instanceof DecimalValue ) {
+			if ( !$this->isInteger( $dataValue ) ) {
+				return $this->getViolationMessage( 'wbqc-violation-message-integer', $snak );
+			}
+		} elseif ( $dataValue instanceof UnboundedQuantityValue ) {
+			if ( !$this->isInteger( $dataValue->getAmount() ) ) {
+				return $this->getViolationMessage( 'wbqc-violation-message-integer', $snak );
+			} elseif (
+				$dataValue instanceof QuantityValue && (
+					!$this->isInteger( $dataValue->getLowerBound() ) ||
+					!$this->isInteger( $dataValue->getUpperBound() )
+				)
+			) {
+				return $this->getViolationMessage( 'wbqc-violation-message-integer-bounds', $snak );
+			}
+		}
+
+		return null;
+	}
+
+	/**
+	 * @param DecimalValue $decimalValue
+	 * @return bool
+	 */
+	private function isInteger( DecimalValue $decimalValue ) {
+		return $decimalValue->getTrimmed()->getFractionalPart() === '';
+	}
+
+	/**
+	 * @param string $messageKey
+	 * @param PropertyValueSnak $snak
+	 * @return ViolationMessage
+	 */
+	private function getViolationMessage( $messageKey, PropertyValueSnak $snak ) {
+		return ( new ViolationMessage( $messageKey ) )
+			->withEntityId( $snak->getPropertyId(), Role::CONSTRAINT_PROPERTY )
+			->withDataValue( $snak->getDataValue() );
+	}
+
 	public function checkConstraintParameters( Constraint $constraint ) {
 		// no parameters
 		return [];

tests/phpunit/Checker/IntegerCheckerTest.php

@@ -53,6 +53,14 @@ public function provideSnaks() {
 		$quantityValueDecimal = new QuantityValue( $decimalValue, '1', $decimalValue, $decimalValue );
 		$quantitySnakDecimal = new PropertyValueSnak( $p1, $quantityValueDecimal );
 
+		$quantityValueDecimalWithDecimalBounds = new QuantityValue(
+			$decimalValue,
+			'1',
+			new DecimalValue( 725.13 ),
+			new DecimalValue( 725.07 )
+		);
+		$quantitySnakDecimalWithDecimalBounds = new PropertyValueSnak( $p1, $quantityValueDecimalWithDecimalBounds );
+
 		$quantityValueInteger = new QuantityValue( $integerValue, '1', $integerValue, $integerValue );
 		$quantitySnakInteger = new PropertyValueSnak( $p1, $quantityValueInteger );
 
@@ -78,11 +86,12 @@ public function provideSnaks() {
 
 		return [
 			[ $quantitySnakDecimal, 'wbqc-violation-message-integer' ],
+			[ $quantitySnakDecimalWithDecimalBounds, 'wbqc-violation-message-integer' ],
 			[ $unboundedQuantitySnakDecimal, 'wbqc-violation-message-integer' ],
 			[ $quantitySnakInteger, null ],
 			[ $unboundedQuantitySnakInteger, null ],
 			[ $unboundedQuantitySnakDecimalInteger, null ],
-			[ $quantitySnakIntegerWithDecimalBounds, null ],
+			[ $quantitySnakIntegerWithDecimalBounds, 'wbqc-violation-message-integer-bounds' ],
 		];
 	}