UnusedPrivateElementsSniff - detect read-only properties

slevomat · Feb 5, 2016 · 5258748 · 5258748
1 parent 5dceec0
commit 5258748
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 26 deletions.
diff --git a/SlevomatCodingStandard/Sniffs/Classes/UnusedPrivateElementsSniff.php b/SlevomatCodingStandard/Sniffs/Classes/UnusedPrivateElementsSniff.php
@@ -14,6 +14,8 @@ class UnusedPrivateElementsSniff implements \PHP_CodeSniffer_Sniff
 
 	const CODE_WRITE_ONLY_PROPERTY = 'writeOnlyProperty';
 
+	const CODE_READ_ONLY_PROPERTY = 'readOnlyProperty';
+
 	const CODE_UNUSED_METHOD = 'unusedMethod';
 
 	/** @var string[] */
@@ -73,13 +75,25 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $classPointer)
 		$reportedProperties = $this->getProperties($phpcsFile, $tokens, $classToken);
 		$reportedMethods = $this->getMethods($phpcsFile, $tokens, $classToken);
 
-		if (count($reportedProperties) === 0) {
+		if (count($reportedProperties) === 0 && count($reportedMethods) === 0) {
 			return;
 		}
 
-		$writeOnlyProperties = [];
+		$writtenToProperties = [];
+		$readFromProperties = [];
+
+		foreach ($reportedProperties as $name => $propertyTokenPointer) {
+			$equalsPointer = TokenHelper::findNextNonWhitespace($phpcsFile, $propertyTokenPointer + 1);
+			$equalsToken = $tokens[$equalsPointer];
+			if ($equalsToken['code'] === T_EQUAL) {
+				$writtenToProperties[$name] = $propertyTokenPointer;
+			}
+		}
+
 		$findUsagesStartTokenPointer = $classToken['scope_opener'] + 1;
 
+		$assignmentTokens = array_merge(\PHP_CodeSniffer_Tokens::$assignmentTokens, [T_INC, T_DEC]);
+
 		while (($propertyAccessTokenPointer = $phpcsFile->findNext([T_VARIABLE, T_SELF, T_STATIC], $findUsagesStartTokenPointer, $classToken['scope_closer'])) !== false) {
 			$propertyAccessToken = $tokens[$propertyAccessTokenPointer];
 			if ($propertyAccessToken['content'] === '$this') {
@@ -108,18 +122,20 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $classPointer)
 					continue;
 				}
 
+				if (!isset($reportedProperties[$name])) {
+					$findUsagesStartTokenPointer = $methodCallTokenPointer + 1;
+					continue;
+				}
+
 				$assignTokenPointer = TokenHelper::findNextNonWhitespace($phpcsFile, $propertyNameTokenPointer + 1);
 				$assignToken = $tokens[$assignTokenPointer];
-				if ($assignToken['code'] === T_EQUAL) {
-					// assigning value to a property - note possible write-only property
+				if (in_array($assignToken['code'], $assignmentTokens, true)) {
 					$findUsagesStartTokenPointer = $assignTokenPointer + 1;
-					$writeOnlyProperties[$name] = $propertyNameTokenPointer;
+					$writtenToProperties[$name] = $propertyNameTokenPointer;
 					continue;
 				}
 
-				if (isset($reportedProperties[$name])) {
-					unset($reportedProperties[$name]);
-				}
+				$readFromProperties[$name] = $propertyNameTokenPointer;
 
 				$findUsagesStartTokenPointer = $propertyNameTokenPointer + 1;
 			} elseif (in_array($propertyAccessToken['code'], [T_SELF, T_STATIC], true)) {
@@ -156,26 +172,25 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $classPointer)
 			} else {
 				$findUsagesStartTokenPointer = $propertyAccessTokenPointer + 1;
 			}
-
-			if (count($reportedProperties) + count($reportedMethods) === 0) {
-				return;
-			}
 		}
 
-		if (count($reportedProperties) + count($reportedMethods) === 0) {
-			return;
-		}
+		$writeOnlyProperties = array_diff_key($writtenToProperties, $readFromProperties);
+		$readOnlyProperties = array_diff_key($readFromProperties, $writtenToProperties);
+		$unusedProperties = array_diff_key($reportedProperties, $writtenToProperties, $readFromProperties);
 
 		$classNamePointer = $phpcsFile->findNext(T_STRING, $classPointer);
 		$className = $tokens[$classNamePointer]['content'];
 
-		foreach ($reportedProperties as $name => $propertyTokenPointer) {
-			$phpcsFile->addError(sprintf(
-				'Class %s contains %s property: $%s',
-				$className,
-				isset($writeOnlyProperties[$name]) ? 'write-only' : 'unused',
-				$name
-			), $propertyTokenPointer, isset($writeOnlyProperties[$name]) ? self::CODE_WRITE_ONLY_PROPERTY : self::CODE_UNUSED_PROPERTY);
+		foreach ($writeOnlyProperties as $name => $propertyTokenPointer) {
+			$this->addError($phpcsFile, $className, $name, $reportedProperties[$name], 'write-only', self::CODE_WRITE_ONLY_PROPERTY);
+		}
+
+		foreach ($readOnlyProperties as $name => $propertyTokenPointer) {
+			$this->addError($phpcsFile, $className, $name, $reportedProperties[$name], 'read-only', self::CODE_READ_ONLY_PROPERTY);
+		}
+
+		foreach ($unusedProperties as $name => $propertyTokenPointer) {
+			$this->addError($phpcsFile, $className, $name, $reportedProperties[$name], 'unused', self::CODE_UNUSED_PROPERTY);
 		}
 
 		foreach ($reportedMethods as $name => $methodTokenPointer) {
@@ -187,6 +202,23 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $classPointer)
 		}
 	}
 
+	private function addError(
+		PHP_CodeSniffer_File $phpcsFile,
+		$className,
+		$name,
+		$pointer,
+		$type,
+		$errorCode
+	)
+	{
+		$phpcsFile->addError(sprintf(
+			'Class %s contains %s property: $%s',
+			$className,
+			$type,
+			$name
+		), $pointer, $errorCode);
+	}
+
 	/**
 	 * @param PHP_CodeSniffer_File $phpcsFile
 	 * @param mixed[] $tokens

diff --git a/tests/Sniffs/Classes/UnusedPrivateElementsSniffTest.php b/tests/Sniffs/Classes/UnusedPrivateElementsSniffTest.php
@@ -46,20 +46,26 @@ public function testCheckFile()
 		$this->assertNoSniffError($resultFile, 33);
 		$this->assertSniffError(
 			$resultFile,
-			48,
+			35,
+			UnusedPrivateElementsSniff::CODE_READ_ONLY_PROPERTY,
+			'Class ClassWithSomeUnusedProperties contains read-only property: $readOnlyProperty'
+		);
+		$this->assertSniffError(
+			$resultFile,
+			53,
 			UnusedPrivateElementsSniff::CODE_UNUSED_METHOD,
 			'Class ClassWithSomeUnusedProperties contains unused private method: unusedPrivateMethod'
 		);
-		$this->assertNoSniffError($resultFile, 51);
 		$this->assertNoSniffError($resultFile, 58);
 		$this->assertNoSniffError($resultFile, 63);
+		$this->assertNoSniffError($resultFile, 68);
 		$this->assertSniffError(
 			$resultFile,
-			68,
+			73,
 			UnusedPrivateElementsSniff::CODE_UNUSED_METHOD,
 			'Class ClassWithSomeUnusedProperties contains unused private method: unusedStaticPrivateMethod'
 		);
-		$this->assertNoSniffError($resultFile, 73);
+		$this->assertNoSniffError($resultFile, 78);
 	}
 
 }
diff --git a/tests/Sniffs/Classes/data/ClassWithSomeUnusedProperties.php b/tests/Sniffs/Classes/data/ClassWithSomeUnusedProperties.php
@@ -32,12 +32,17 @@ class ClassWithSomeUnusedProperties extends \Consistence\Object
 	/** @ORM\Column(name="foo") */
 	private $doctrineProperty;
 
+	private $readOnlyProperty;
+
 	public function foo()
 	{
+		$this->usedProperty = foo();
 		$this->usedProperty->foo();
 		$this->writeOnlyProperty = 'foo';
 		$this->unusedPropertyWhichNameIsAlsoAFunction();
 		$this->usedPrivateMethod();
+
+		return $this->readOnlyProperty;
 	}
 
 	private function usedPrivateMethod()