fixup! AssertEqualsIsDiscouragedRule

phpstan · Dec 28, 2017 · bb05c60 · bb05c60
1 parent bef077d
commit bb05c60
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php
@@ -63,15 +63,29 @@ public function processNode(Node $node, Scope $scope): array
 			];
 		}
 
-		$fileContents = explode("\n", file_get_contents($scope->getFile()));
-		$previousLine = $fileContents[$node->getLine() - 2];
-
-		if (!preg_match('~^\s+//\s+assertEquals because(.*)~', $previousLine)) {
+		if (!$node->hasAttribute('comments')) {
 			return [
 				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
 			];
 		}
 
+		/** @var \PhpParser\Comment[] $comments */
+		$comments = $node->getAttribute('comments');
+		$comment = $comments[count($comments) - 1];
+
+		// the comment should be on the line above the assertEquals()
+		if ($comment->getLine() !== ($node->getLine() - 1)) {
+			return [
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)',
+			];
+		}
+
+		if (!preg_match('~^//\s+assertEquals because(.*)~', $comment->getReformattedText())) {
+			return [
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
+			];
+		}
+
 		return [];
 	}
 

diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php
@@ -32,17 +32,21 @@ public function testRule(): void
 				16,
 			],
 			[
-				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
 				19,
 			],
 			[
 				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
 				21,
 			],
 			[
-				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
 				24,
 			],
+			[
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)',
+				28,
+			],
 		]);
 	}
 

diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php
@@ -23,6 +23,10 @@ public function testAssertEqualsIsDiscouraged()
 		// with incorrect comment
 		$this->assertEquals(1, '1');
 
+		// assertEquals because I want it! But sadly, the comment is not just above the assert.
+
+		$this->assertEquals(1, '1');
+
 		// assertEquals because I want it!
 		$this->assertEquals(1, '1');
 	}