Add AssertSameDifferentTypesRuleTest #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

ondrejmirtes merged 1 commit into phpstan:master from mhujer:mh-add-test

Dec 14, 2017

Merged

Add AssertSameDifferentTypesRuleTest #3

ondrejmirtes merged 1 commit into phpstan:master from mhujer:mh-add-test

Dec 14, 2017

Conversation

@mhujer

Copy link

Contributor

@mhujer mhujer commented Dec 11, 2017

I wanted to have a look on how to write PHPStan rules tests and I found none in this repository. So I created one.

ondrejmirtes

ondrejmirtes requested changes

Dec 11, 2017

View reviewed changes

Copy link

Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks! I found a few minor issues 😊

tests/Rules/PHPUnit/data/assert-same.php

$this->assertSame('1', 1);

$this->assertSame('1', new \stdClass());

$this->assertSame(1, $this->returnsString());

$this->assertSame('1', self::returnsInt());

Copy link

Member

@ondrejmirtes ondrejmirtes Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be a few examples of positive cases - comparing int/int and probably also comparing some unions against simple type and unions against unions.

Copy link

Contributor Author

@mhujer mhujer Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added bellow

tests/Rules/PHPUnit/data/assert-same.php Outdated

@@ -0,0 +1,39 @@

<?php

declare(strict_types = 1);

Copy link

Member

@ondrejmirtes ondrejmirtes Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put declare on the same line as <?php :)

tests/Rules/PHPUnit/data/assert-same.php Outdated

declare(strict_types = 1);

namespace PHPUnit;

Copy link

Member

@ondrejmirtes ondrejmirtes Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a different namespace, something like namespace ExampleTestCase is fine 😊

Copy link

Contributor Author

@mhujer mhujer Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention was to make it consistent with https://github.com/phpstan/phpstan-strict-rules/blob/master/tests/Rules/StrictCalls/data/strict-calls.php

Copy link

Member

@ondrejmirtes ondrejmirtes Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but PHPUnit is a well known and used namespace :)

Copy link

Contributor Author

@mhujer mhujer Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok :-)

mhujer

mhujer commented

Dec 11, 2017

View reviewed changes

tests/Rules/PHPUnit/data/assert-same.php

$this->assertSame(1, self::returnsInt());

// this breaks PHPStan

$this->assertSame(['a'], ['a', 1]);

Copy link

Contributor Author

@mhujer mhujer Dec 11, 2017 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ondrejmirtes this line breaks PHPStan

PHPStan\ShouldNotHappenException : Internal error.
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Type\TypeCombinator.php:23
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Type\ArrayType.php:28
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\Scope.php:575
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\Scope.php:323
 C:\devweb\htdocs\phpstan-phpunit\src\Rules\PHPUnit\AssertSameDifferentTypesRule.php:62
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\Analyser.php:162
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:316
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:176
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:686
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:176
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:686
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:176
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:686
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\NodeScopeResolver.php:176
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Analyser\Analyser.php:166
 C:\devweb\htdocs\phpstan-phpunit\vendor\phpstan\phpstan\src\Testing\RuleTestCase.php:64
 C:\devweb\htdocs\phpstan-phpunit\tests\Rules\PHPUnit\AssertSameDifferentTypesRuleTest.php:19