Page MenuHomePhabricator

Add potentially problematic assertEquals( 1 ) to the PHPUnitAssertEquals sniff
Closed, ResolvedPublic

Description

$this->assertEquals( true, … ) is already reported as problematic by the sniff, because it would accept numeric values, but should only accept boolean true.

$this->assertEquals( 0, … ) and similar are also reported, because they would accept a wide variety of values that are all considered false in PHP.

What's currently not reported by the sniff are numeric values and strings that can be confused with true:

$this->assertEquals( 1, true );
$this->assertEquals( 1.0, true );
$this->assertEquals( '1', true );
$this->assertEquals( '1.0', true );
$this->assertEquals( '1.00', true );

Note that other numeric values (e.g. '2') are not considered equal to true, because what PHPUnit's ScalarComparator internally does is not simply an == comparison, but (string)$a == (string)$b if one side is a string, or a straight == otherwise.

Event Timeline

Change 576045 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/tools/codesniffer@master] Add problematic values 1 and 1.0 to PHPUnitAssertEqualsSniff

https://gerrit.wikimedia.org/r/576045

Change 576045 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Add problematic values 1 and 1.0 to PHPUnitAssertEqualsSniff

https://gerrit.wikimedia.org/r/576045