Issues in RecordValueDescriptionDeserializer.php - Fixed Issues - Inspection of "Merge pull request #1809 from SemanticMediaWiki/re..." - SemanticMediaWiki/SemanticMediaWiki - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 9eae06...a41cb5 )

by mw

created 2016-08-24 04:31 UTC

RecordValueDescriptionDeserializer.php (2 issues)

Labels

Bug 2

Severity

Minor 2

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace SMW\Deserializers\DVDescriptionDeserializer;

use InvalidArgumentException;
use SMW\DataValueFactory;
use SMW\Query\Language\Conjunction;
use SMW\Query\Language\SomeProperty;
use SMW\Query\Language\ThingDescription;
use SMWRecordValue as RecordValue;

/**
 * @private
 *
 * @license GNU GPL v2+
 * @since 2.3
 *
 * @author mwjames
 */
class RecordValueDescriptionDeserializer extends DescriptionDeserializer {

	/**
	 * @since 2.3
	 *
	 * {@inheritDoc}
	 */
	public function isDeserializerFor( $serialization ) {
		return $serialization instanceof RecordValue;
	}

	/**
	 * @since 2.3
	 *
	 * @param string $value
	 *
	 * @return Description
	 * @throws InvalidArgumentException
	 */
	public function deserialize( $value ) {

		if ( !is_string( $value ) ) {
			throw new InvalidArgumentException( 'value needs to be a string' );
		}

		if ( $value === '' ) {
			$this->addError( wfMessage( 'smw_novalues' )->text() );
			return new ThingDescription();
		}

		$subdescriptions = array();
		$values = $this->dataValue->getValuesFromString( $value );
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

		$valueIndex = 0; // index in value array
		$propertyIndex = 0; // index in property list

		foreach ( $this->dataValue->getPropertyDataItems() as $diProperty ) {

			// stop if there are no values left
			if ( !is_array( $values ) || !array_key_exists( $valueIndex, $values ) ) {
				break;
			}

			$description = $this->getDescriptionForProperty(
				$diProperty,
				$values,
				$valueIndex,
				$propertyIndex
			);

			if ( $description !== null ) {
				 $subdescriptions[] = $description;
			}

			++$propertyIndex;
		}

		if ( $subdescriptions === array() ) {
			$this->addError( wfMessage( 'smw_novalues' )->text() );
		}

		return $this->getDescriptionFor( $subdescriptions );
	}

	private function getDescriptionFor( $subdescriptions ) {
		switch ( count( $subdescriptions ) ) {
			case 0:
			return new ThingDescription();
			case 1:
			return reset( $subdescriptions );
			default:
			return new Conjunction( $subdescriptions );
		}
	}

	private function getDescriptionForProperty( $diProperty, $values, &$valueIndex, $propertyIndex ) {

		$values[$valueIndex] = str_replace( "-3B", ";", $values[$valueIndex] );
		$beforePrepareValue = $values[$valueIndex];

		$description = null;
		$comparator = SMW_CMP_EQ;

		$this->prepareValue( $values[$valueIndex], $comparator );

		// generating the DVs:
		if ( ( $values[$valueIndex] === '' ) || ( $values[$valueIndex] == '?' ) ) { // explicit omission
			$valueIndex++;
			return $description;
		}

		$dataValue = DataValueFactory::getInstance()->newDataValueByProperty(
			$diProperty,
			$values[$valueIndex],
			false,
			$this->dataValue->getContextPage()
		);

		if ( $dataValue->isValid() ) { // valid DV: keep
			$description = new SomeProperty(
				$diProperty,
				$dataValue->getQueryDescription( $beforePrepareValue )
			);
			$valueIndex++;
		} elseif ( ( count( $values ) - $valueIndex ) == ( count( $this->dataValue->getProperties() ) - $propertyIndex ) ) {
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
			$this->addError( $dataValue->getErrors() );
			++$valueIndex;
		}

		return $description;
	}

}


1		<?php
2
3		namespace SMW\Deserializers\DVDescriptionDeserializer;
4
5		use InvalidArgumentException;
6		use SMW\DataValueFactory;
7		use SMW\Query\Language\Conjunction;
8		use SMW\Query\Language\SomeProperty;
9		use SMW\Query\Language\ThingDescription;
10		use SMWRecordValue as RecordValue;
11
12		/**
13		* @private
14		*
15		* @license GNU GPL v2+
16		* @since 2.3
17		*
18		* @author mwjames
19		*/
20		class RecordValueDescriptionDeserializer extends DescriptionDeserializer {
21
22		/**
23		* @since 2.3
24		*
25		* {@inheritDoc}
26		*/
27	101	public function isDeserializerFor( $serialization ) {
28	101	return $serialization instanceof RecordValue;
29		}
30
31		/**
32		* @since 2.3
33		*
34		* @param string $value
35		*
36		* @return Description
37		* @throws InvalidArgumentException
38		*/
39	12	public function deserialize( $value ) {
40
41	12	if ( !is_string( $value ) ) {
42	1	throw new InvalidArgumentException( 'value needs to be a string' );
43		}
44
45	11	if ( $value === '' ) {
46	1	$this->addError( wfMessage( 'smw_novalues' )->text() );
47	1	return new ThingDescription();
48		}
49
50	10	$subdescriptions = array();
51	10	$values = $this->dataValue->getValuesFromString( $value );
		0 ignored issues – show Bug introduced 2016-01-02 19:30 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `SMWDataValue` as the method `getValuesFromString()` does only exist in the following sub-classes of `SMWDataValue`: `SMWRecordValue`, `SMW\DataValues\MonolingualTextValue`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
52
53	10	$valueIndex = 0; // index in value array
54	10	$propertyIndex = 0; // index in property list
55
56	10	foreach ( $this->dataValue->getPropertyDataItems() as $diProperty ) {
57
58		// stop if there are no values left
59	9	if ( !is_array( $values ) \|\| !array_key_exists( $valueIndex, $values ) ) {
60	1	break;
61		}
62
63	9	$description = $this->getDescriptionForProperty(
64		$diProperty,
65		$values,
66		$valueIndex,
67		$propertyIndex
68		);
69
70	9	if ( $description !== null ) {
71	8	$subdescriptions[] = $description;
72		}
73
74	9	++$propertyIndex;
75		}
76
77	10	if ( $subdescriptions === array() ) {
78	2	$this->addError( wfMessage( 'smw_novalues' )->text() );
79		}
80
81	10	return $this->getDescriptionFor( $subdescriptions );
82		}
83
84	10	private function getDescriptionFor( $subdescriptions ) {
85	10	switch ( count( $subdescriptions ) ) {
86	10	case 0:
87	2	return new ThingDescription();
88	8	case 1:
89	5	return reset( $subdescriptions );
90		default:
91	6	return new Conjunction( $subdescriptions );
92		}
93		}
94
95	9	private function getDescriptionForProperty( $diProperty, $values, &$valueIndex, $propertyIndex ) {
96
97	9	$values[$valueIndex] = str_replace( "-3B", ";", $values[$valueIndex] );
98	9	$beforePrepareValue = $values[$valueIndex];
99
100	9	$description = null;
101	9	$comparator = SMW_CMP_EQ;
102
103	9	$this->prepareValue( $values[$valueIndex], $comparator );
104
105		// generating the DVs:
106	9	if ( ( $values[$valueIndex] === '' ) \|\| ( $values[$valueIndex] == '?' ) ) { // explicit omission
107	4	$valueIndex++;
108	4	return $description;
109		}
110
111	8	$dataValue = DataValueFactory::getInstance()->newDataValueByProperty(
112		$diProperty,
113	8	$values[$valueIndex],
114	8	false,
115	8	$this->dataValue->getContextPage()
116		);
117
118	8	if ( $dataValue->isValid() ) { // valid DV: keep
119	8	$description = new SomeProperty(
120		$diProperty,
121	8	$dataValue->getQueryDescription( $beforePrepareValue )
122		);
123	8	$valueIndex++;
124	1	} elseif ( ( count( $values ) - $valueIndex ) == ( count( $this->dataValue->getProperties() ) - $propertyIndex ) ) {
		0 ignored issues – show Bug introduced 2015-11-23 10:29 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `SMWDataValue` as the method `getProperties()` does only exist in the following sub-classes of `SMWDataValue`: `SMWRecordValue`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
125	1	$this->addError( $dataValue->getErrors() );
126	1	++$valueIndex;
127		}
128
129	8	return $description;
130		}
131
132		}
133

SemanticMediaWiki / SemanticMediaWiki

Push — master ( 9eae06...a41cb5 )

RecordValueDescriptionDeserializer.php (2 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like