Skip to content
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.

Sign up for GitHub

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

DX: remove PHPStan exceptions for "tests" from phpstan.neon #5661

Merged
merged 1 commit into from Apr 28, 2021
Merged

DX: remove PHPStan exceptions for "tests" from phpstan.neon #5661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 0 additions & 15 deletions phpstan.neon
View file
Open in desktop
Expand Up @@ -35,32 +35,17 @@ parameters:
paths:
- src/Config.php
- src/Tokenizer/Token.php
-
message: '/^Parameter #1 \$fixers of method PhpCsFixer\\Config::registerCustomFixers\(\) expects iterable<PhpCsFixer\\Fixer\\FixerInterface>, string given\.$/'
path: tests/ConfigTest.php
-
message: '/^Parameter #1 \$options of method PhpCsFixer\\FixerConfiguration\\FixerConfigurationResolverRootless::resolve\(\) expects array<string, mixed>, array<int, string> given\.$/'
path: tests/FixerConfiguration/FixerConfigurationResolverRootlessTest.php
-
message: '/^Parameter #1 \$function of function register_shutdown_function expects callable\(\): void, array\(\$this\(PhpCsFixer\\FileRemoval\), ''clean''\) given\.$/'
path: src/FileRemoval.php
-
message: '/^Parameter #1 \$finder of method PhpCsFixer\\Config::setFinder\(\) expects iterable<string>, int given\.$/'
path: tests/ConfigTest.php
- # https://github.com/phpstan/phpstan/issues/1215
message: '/^Strict comparison using === between false and string will always evaluate to false\.$/'
path: src/Fixer/StringNotation/NoTrailingWhitespaceInStringFixer.php
-
message: '/^Property .*::\$indicator .* does not accept null\.$/'
path: tests/Indicator/PhpUnitTestCaseIndicatorTest.php
-
message: '/^Constant T_ATTRIBUTE not found\.$/'
path: src/Tokenizer/Transformer/AttributeTransformer.php
-
message: '/^\$this\(PhpCsFixer\\Tokenizer\\Tokens\) does not accept PhpCsFixer\\Tokenizer\\Token\|null\.$/'
path: src/Tokenizer/Tokens.php
-
message: '/^Class Test\dConfig not found\.$/'
path: tests/Console/ConfigurationResolverTest.php

tipsOfTheDay: false
4 changes: 2 additions & 2 deletions tests/ConfigTest.php
View file
Open in desktop
Expand Up @@ -193,7 +193,7 @@ public function testRegisterCustomFixersWithInvalidArgument()
$this->expectExceptionMessageMatches('/^Argument must be an array or a Traversable, got "\w+"\.$/');

$config = new Config();
$config->registerCustomFixers('foo');
$config->registerCustomFixers('foo'); // @phpstan-ignore-line to avoid `expects iterable<PhpCsFixer\Fixer\FixerInterface>, string given`
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the point of this change, it's moving issue handling from one place to another, while on 3.0 we already have the fix in ConfigInterface directly:
public function registerCustomFixers(iterable $fixers): self;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is not having such exceptions in phpstan.neon - this is not error to ignore, but expected wrong usage.

Copy link
Member

Choose a reason for hiding this comment

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

my point was that whatever you produce here, it is out on 3.0 line, because we enforced proper use on typehint level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is not bad, is it? What's important to me it is out of phpstan.neon file, so that file can be improved futher.

Copy link
Member

Choose a reason for hiding this comment

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

i'm saying that I don't see the value of this change, because what you aim is to move the warning behind this line out of phpstan.neon file, but that warning is already removed in phpstan.neon file on 3.0 line: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/3.0/phpstan.neon

so, doing the change on 2.x is not bringing any benefit (we have better change already on 3.0), but only causing the conflict for 2.x->3.0 merge.

}

/**
Expand Down Expand Up @@ -259,7 +259,7 @@ public function testSetInvalidFinder()
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/^Argument must be an array or a Traversable, got "integer"\.$/');

$config->setFinder(123);
$config->setFinder(123); // @phpstan-ignore-line to avoid `expects iterable<string>, int given`
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/Console/ConfigurationResolverTest.php
View file
Open in desktop
Expand Up @@ -205,7 +205,7 @@ public function testResolveConfigFileByPathOfFile()
$resolver = $this->createConfigurationResolver(['path' => [$dir.\DIRECTORY_SEPARATOR.'foo.php']]);

static::assertSame($dir.\DIRECTORY_SEPARATOR.'.php_cs.dist', $resolver->getConfigFile());
static::assertInstanceOf(\Test1Config::class, $resolver->getConfig());
static::assertInstanceOf(\Test1Config::class, $resolver->getConfig()); // @phpstan-ignore-line to avoid `Class Test1Config not found.`
}

public function testResolveConfigFileSpecified()
Expand All @@ -215,7 +215,7 @@ public function testResolveConfigFileSpecified()
$resolver = $this->createConfigurationResolver(['config' => $file]);

static::assertSame($file, $resolver->getConfigFile());
static::assertInstanceOf(\Test4Config::class, $resolver->getConfig());
static::assertInstanceOf(\Test4Config::class, $resolver->getConfig()); // @phpstan-ignore-line to avoid `Class Test4Config not found.`
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/FixerConfiguration/FixerConfigurationResolverRootlessTest.php
View file
Open in desktop
Expand Up @@ -44,6 +44,6 @@ public function testResolveWithMappedRoot()

static::assertSame($options, $configuration->getOptions());

$configuration->resolve(['baz', 'qux']);
$configuration->resolve(['baz' => 'qux']);
Copy link
Member

Choose a reason for hiding this comment

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

wasn't goal of this test to have non-assoc array passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently not - test is still passing. According to expected deprecation message it tests config for creating resolver, not resolving.

}
}
2 changes: 1 addition & 1 deletion tests/Indicator/PhpUnitTestCaseIndicatorTest.php
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@
final class PhpUnitTestCaseIndicatorTest extends TestCase
{
/**
* @var PhpUnitTestCaseIndicator
* @var null|PhpUnitTestCaseIndicator
*/
private $indicator;

Expand Down