From 9a7a567f322ab7c16d72cec64bbd291451706f38 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thiemo=20M=C3=A4ttig?= <thiemo.maettig@wikimedia.de>
Date: Fri, 23 Jun 2017 10:15:31 +0200
Subject: [PATCH] Better documentation for the DataValue::newFromArray contract

https://phabricator.wikimedia.org/T168681
---
 src/DataValues/DecimalValue.php           | 27 ++++++++++-------------
 src/DataValues/UnboundedQuantityValue.php | 19 +++++++++++-----
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/DataValues/DecimalValue.php b/src/DataValues/DecimalValue.php
index 901b8a4..2f4f887 100644
--- a/src/DataValues/DecimalValue.php
+++ b/src/DataValues/DecimalValue.php
@@ -48,7 +48,7 @@ class DecimalValue extends DataValueObject {
 	 * @param string|int|float $value If given as a string, the value must match
 	 *  QUANTITY_VALUE_PATTERN. The leading plus sign is optional.
 	 *
-	 * @throws IllegalValueException
+	 * @throws InvalidArgumentException
 	 */
 	public function __construct( $value ) {
 		if ( is_int( $value ) || is_float( $value ) ) {
@@ -112,8 +112,6 @@ private function convertToDecimal( $number ) {
 	/**
 	 * Compares this DecimalValue to another DecimalValue.
 	 *
-	 * @since 0.1
-	 *
 	 * @param self $that
 	 *
 	 * @throws LogicException
@@ -227,8 +225,6 @@ public function getValue() {
 	/**
 	 * Returns the sign of the amount (+ or -).
 	 *
-	 * @since 0.1
-	 *
 	 * @return string "+" or "-".
 	 */
 	public function getSign() {
@@ -288,8 +284,6 @@ public function computeAbsolute() {
 	 * Returns the integer part of the value, that is, the part before the decimal point,
 	 * without the sign.
 	 *
-	 * @since 0.1
-	 *
 	 * @return string
 	 */
 	public function getIntegerPart() {
@@ -306,8 +300,6 @@ public function getIntegerPart() {
 	 * Returns the fractional part of the value, that is, the part after the decimal point,
 	 * if any.
 	 *
-	 * @since 0.1
-	 *
 	 * @return string
 	 */
 	public function getFractionalPart() {
@@ -342,8 +334,6 @@ public function getTrimmed() {
 	 * Returns the value held by this object, as a float.
 	 * Equivalent to floatval( $this->getvalue() ).
 	 *
-	 * @since 0.1
-	 *
 	 * @return float
 	 */
 	public function getValueFloat() {
@@ -360,13 +350,20 @@ public function getArrayValue() {
 	}
 
 	/**
-	 * Constructs a new instance of the DataValue from the provided data.
-	 * This can round-trip with @see getArrayValue
+	 * Constructs a new instance from the provided data. Required for @see DataValueDeserializer.
+	 * This is expected to round-trip with @see getArrayValue.
+	 *
+	 * @deprecated since 0.8.3. Static DataValue::newFromArray constructors like this are
+	 *  underspecified (not in the DataValue interface), and misleadingly named (should be named
+	 *  newFromArrayValue). Instead, use DataValue builder callbacks in @see DataValueDeserializer.
 	 *
-	 * @param string|int|float $data
+	 * @param mixed $data Warning! Even if this is expected to be a value as returned by
+	 *  @see getArrayValue, callers of this specific newFromArray implementation can not guarantee
+	 *  this. This is not guaranteed to be a string!
 	 *
+	 * @throws InvalidArgumentException if $data is not in the expected format. Subclasses of
+	 *  InvalidArgumentException are expected and properly handled by @see DataValueDeserializer.
 	 * @return self
-	 * @throws IllegalValueException
 	 */
 	public static function newFromArray( $data ) {
 		return new static( $data );
diff --git a/src/DataValues/UnboundedQuantityValue.php b/src/DataValues/UnboundedQuantityValue.php
index 23a14b0..d88c0b4 100644
--- a/src/DataValues/UnboundedQuantityValue.php
+++ b/src/DataValues/UnboundedQuantityValue.php
@@ -64,7 +64,7 @@ public function __construct( DecimalValue $amount, $unit ) {
 	 * @param string $unit A unit identifier. Must not be empty, use "1" for unit-less quantities.
 	 *
 	 * @return self
-	 * @throws IllegalValueException
+	 * @throws InvalidArgumentException
 	 */
 	public static function newFromNumber( $amount, $unit = '1' ) {
 		$amount = self::asDecimalValue( 'amount', $amount );
@@ -82,7 +82,6 @@ public static function newFromNumber( $amount, $unit = '1' ) {
 	 * @param string|int|float|DecimalValue|null $number
 	 * @param DecimalValue|null $default
 	 *
-	 * @throws IllegalValueException
 	 * @throws InvalidArgumentException
 	 * @return DecimalValue
 	 */
@@ -248,13 +247,21 @@ public function getArrayValue() {
 
 	/**
 	 * Static helper capable of constructing both unbounded and bounded quantity value objects,
-	 * depending on the serialization provided. Required for @see DataValueDeserializer. This can
-	 * round-trip with both @see self::getArrayValue as well as @see QuantityValue::getArrayValue.
+	 * depending on the serialization provided. Required for @see DataValueDeserializer.
+	 * This is expected to round-trip with both @see getArrayValue as well as
+	 * @see QuantityValue::getArrayValue.
 	 *
-	 * @param mixed $data
+	 * @deprecated since 0.8.3. Static DataValue::newFromArray constructors like this are
+	 *  underspecified (not in the DataValue interface), and misleadingly named (should be named
+	 *  newFromArrayValue). Instead, use DataValue builder callbacks in @see DataValueDeserializer.
 	 *
+	 * @param mixed $data Warning! Even if this is expected to be a value as returned by
+	 *  @see getArrayValue, callers of this specific newFromArray implementation can not guarantee
+	 *  this. This is not even guaranteed to be an array!
+	 *
+	 * @throws IllegalValueException if $data is not in the expected format. Subclasses of
+	 *  InvalidArgumentException are expected and properly handled by @see DataValueDeserializer.
 	 * @return self|QuantityValue Either an unbounded or bounded quantity value object.
-	 * @throws IllegalValueException
 	 */
 	public static function newFromArray( $data ) {
 		self::requireArrayFields( $data, [ 'amount', 'unit' ] );