diff --git a/.gitignore b/.gitignore index 6586246..3f9bc5e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,6 @@ +.phpcs-cache composer.phar composer.lock phpcs.xml +phpstan.neon vendor/ diff --git a/.travis.yml b/.travis.yml index b12d6c4..95f327c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,10 +6,6 @@ php: - 7.2 - nightly -matrix: - allow_failures: - - php: nightly - cache: directories: - $HOME/.composer/cache @@ -18,10 +14,7 @@ before_install: - mv ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini{,.disabled} || echo "xdebug not available" - travis_retry composer self-update -install: - - composer require react/promise:2.* - - composer require psr/http-message:1.* - - travis_retry composer update --prefer-dist +install: travis_retry composer update --prefer-dist script: ./vendor/bin/phpunit --group default,ReactPromise @@ -46,8 +39,15 @@ jobs: - wget https://scrutinizer-ci.com/ocular.phar - php ocular.phar code-coverage:upload --format=php-clover clover.xml - - stage: Coding standard + - stage: Code Quality php: 7.1 + env: CODING_STANDARD install: travis_retry composer install --prefer-dist script: - ./vendor/bin/phpcs + + - stage: Code Quality + php: 7.1 + env: STATIC_ANALYSIS + install: travis_retry composer install --prefer-dist + script: composer static-analysis diff --git a/composer.json b/composer.json index d0a9d34..152bcdf 100644 --- a/composer.json +++ b/composer.json @@ -15,8 +15,11 @@ }, "require-dev": { "doctrine/coding-standard": "^4.0", + "phpstan/phpstan": "^0.10.3", + "phpstan/phpstan-phpunit": "^0.10.0", "phpunit/phpunit": "^7.2", - "psr/http-message": "^1.0" + "psr/http-message": "^1.0", + "react/promise": "2.*" }, "config": { "preferred-install": "dist", @@ -39,6 +42,7 @@ "psr/http-message": "To use standard GraphQL server" }, "scripts": { - "lint" : "phpcs" + "lint" : "phpcs", + "static-analysis": "phpstan analyse --ansi -l 1 -c phpstan.neon.dist src tests" } } diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..b2b86dc --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,3 @@ +includes: + - vendor/phpstan/phpstan-phpunit/extension.neon + - vendor/phpstan/phpstan-phpunit/rules.neon diff --git a/src/Executor/Promise/Promise.php b/src/Executor/Promise/Promise.php index 8079663..2f060e1 100644 --- a/src/Executor/Promise/Promise.php +++ b/src/Executor/Promise/Promise.php @@ -6,6 +6,7 @@ namespace GraphQL\Executor\Promise; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Utils\Utils; +use React\Promise\Promise as ReactPromise; /** * Convenience wrapper for promises represented by Promise Adapter diff --git a/src/Language/Visitor.php b/src/Language/Visitor.php index e266f0f..94c83e7 100644 --- a/src/Language/Visitor.php +++ b/src/Language/Visitor.php @@ -263,7 +263,8 @@ class Visitor $visitFn = self::getVisitFn($visitor, $node->kind, $isLeaving); if ($visitFn) { - $result = call_user_func($visitFn, $node, $key, $parent, $path, $ancestors); + $result = call_user_func($visitFn, $node, $key, $parent, $path, $ancestors); + $editValue = null; if ($result !== null) { if ($result instanceof VisitorOperation) { diff --git a/src/Server/StandardServer.php b/src/Server/StandardServer.php index c631a83..92a8f3d 100644 --- a/src/Server/StandardServer.php +++ b/src/Server/StandardServer.php @@ -8,7 +8,7 @@ use GraphQL\Error\FormattedError; use GraphQL\Error\InvariantViolation; use GraphQL\Executor\ExecutionResult; use GraphQL\Executor\Promise\Promise; -use GraphQL\Utils; +use GraphQL\Utils\Utils; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\StreamInterface; diff --git a/src/Type/Definition/InputObjectField.php b/src/Type/Definition/InputObjectField.php index 793994b..5ceb3f4 100644 --- a/src/Type/Definition/InputObjectField.php +++ b/src/Type/Definition/InputObjectField.php @@ -21,7 +21,7 @@ class InputObjectField /** @var string|null */ public $description; - /** @var callback|InputType */ + /** @var callable|InputType */ public $type; /** @var InputValueDefinitionNode|null */ diff --git a/src/Type/Definition/InputObjectType.php b/src/Type/Definition/InputObjectType.php index 0e39c05..4b4bb52 100644 --- a/src/Type/Definition/InputObjectType.php +++ b/src/Type/Definition/InputObjectType.php @@ -12,7 +12,6 @@ use function is_array; use function is_callable; use function is_string; use function sprintf; -use function spritnf; /** * Class InputObjectType @@ -70,7 +69,7 @@ class InputObjectType extends Type implements InputType, NamedType if (! is_array($fields)) { throw new InvariantViolation( - spritnf('%s fields must be an array or a callable which returns such an array.', $this->name) + sprintf('%s fields must be an array or a callable which returns such an array.', $this->name) ); } diff --git a/src/Validator/Rules/KnownDirectives.php b/src/Validator/Rules/KnownDirectives.php index ecccafd..e9a0b8c 100644 --- a/src/Validator/Rules/KnownDirectives.php +++ b/src/Validator/Rules/KnownDirectives.php @@ -7,7 +7,9 @@ namespace GraphQL\Validator\Rules; use GraphQL\Error\Error; use GraphQL\Language\AST\DirectiveNode; use GraphQL\Language\AST\InputObjectTypeDefinitionNode; +use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; +use GraphQL\Language\AST\NodeList; use GraphQL\Language\DirectiveLocation; use GraphQL\Validator\ValidationContext; use function count; @@ -59,7 +61,9 @@ class KnownDirectives extends ValidationRule } /** - * @param (Node|NodeList)[] $ancestors + * @param Node[]|NodeList[] $ancestors The type is actually (Node|NodeList)[] but this PSR-5 syntax is so far not supported by most of the tools + * + * @return string */ private function getDirectiveLocationForASTPath(array $ancestors) { diff --git a/src/Validator/Rules/VariablesDefaultValueAllowed.php b/src/Validator/Rules/VariablesDefaultValueAllowed.php index 64808fc..4f2b5cf 100644 --- a/src/Validator/Rules/VariablesDefaultValueAllowed.php +++ b/src/Validator/Rules/VariablesDefaultValueAllowed.php @@ -44,10 +44,10 @@ class VariablesDefaultValueAllowed extends ValidationRule return Visitor::skipNode(); }, - NodeKind::SELECTION_SET => function (SelectionSetNode $node) use ($context) { + NodeKind::SELECTION_SET => function (SelectionSetNode $node) { return Visitor::skipNode(); }, - NodeKind::FRAGMENT_DEFINITION => function (FragmentDefinitionNode $node) use ($context) { + NodeKind::FRAGMENT_DEFINITION => function (FragmentDefinitionNode $node) { return Visitor::skipNode(); }, ]; diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index f462c15..64b6eb7 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -329,7 +329,7 @@ class ExecutorTest extends TestCase 'fields' => [ 'a' => [ 'type' => Type::string(), - 'resolve' => function ($context) use ($doc, &$gotHere) { + 'resolve' => function ($context) use (&$gotHere) { $this->assertEquals('thing', $context['contextThing']); $gotHere = true; }, diff --git a/tests/Executor/Promise/ReactPromiseAdapterTest.php b/tests/Executor/Promise/ReactPromiseAdapterTest.php index e28ba3c..e007ae3 100644 --- a/tests/Executor/Promise/ReactPromiseAdapterTest.php +++ b/tests/Executor/Promise/ReactPromiseAdapterTest.php @@ -31,26 +31,24 @@ class ReactPromiseAdapterTest extends TestCase { $reactAdapter = new ReactPromiseAdapter(); - $this->assertSame( - true, + $this->assertTrue( $reactAdapter->isThenable(new ReactPromise(function () { })) ); - $this->assertSame(true, $reactAdapter->isThenable(new FulfilledPromise())); - $this->assertSame(true, $reactAdapter->isThenable(new RejectedPromise())); - $this->assertSame( - true, + $this->assertTrue($reactAdapter->isThenable(new FulfilledPromise())); + $this->assertTrue($reactAdapter->isThenable(new RejectedPromise())); + $this->assertTrue( $reactAdapter->isThenable(new LazyPromise(function () { })) ); - $this->assertSame(false, $reactAdapter->isThenable(false)); - $this->assertSame(false, $reactAdapter->isThenable(true)); - $this->assertSame(false, $reactAdapter->isThenable(1)); - $this->assertSame(false, $reactAdapter->isThenable(0)); - $this->assertSame(false, $reactAdapter->isThenable('test')); - $this->assertSame(false, $reactAdapter->isThenable('')); - $this->assertSame(false, $reactAdapter->isThenable([])); - $this->assertSame(false, $reactAdapter->isThenable(new \stdClass())); + $this->assertFalse($reactAdapter->isThenable(false)); + $this->assertFalse($reactAdapter->isThenable(true)); + $this->assertFalse($reactAdapter->isThenable(1)); + $this->assertFalse($reactAdapter->isThenable(0)); + $this->assertFalse($reactAdapter->isThenable('test')); + $this->assertFalse($reactAdapter->isThenable('')); + $this->assertFalse($reactAdapter->isThenable([])); + $this->assertFalse($reactAdapter->isThenable(new \stdClass())); } public function testConvertsReactPromisesToGraphQlOnes() : void diff --git a/tests/Language/ParserTest.php b/tests/Language/ParserTest.php index 360e5a1..49f461a 100644 --- a/tests/Language/ParserTest.php +++ b/tests/Language/ParserTest.php @@ -331,7 +331,7 @@ GRAPHQL '); $result = Parser::parse($source); - $loc = function ($start, $end) use ($source) { + $loc = function ($start, $end) { return [ 'start' => $start, 'end' => $end, @@ -442,7 +442,7 @@ GRAPHQL '); $result = Parser::parse($source); - $loc = function ($start, $end) use ($source) { + $loc = function ($start, $end) { return [ 'start' => $start, 'end' => $end, diff --git a/tests/Language/SchemaPrinterTest.php b/tests/Language/SchemaPrinterTest.php index 760d036..8c90401 100644 --- a/tests/Language/SchemaPrinterTest.php +++ b/tests/Language/SchemaPrinterTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace GraphQL\Tests; +namespace GraphQL\Tests\Language; use GraphQL\Language\AST\NameNode; use GraphQL\Language\AST\ScalarTypeDefinitionNode; diff --git a/tests/Language/SerializationTest.php b/tests/Language/SerializationTest.php index a55a509..403dbfb 100644 --- a/tests/Language/SerializationTest.php +++ b/tests/Language/SerializationTest.php @@ -51,7 +51,7 @@ class SerializationTest extends TestCase $expectedVars = get_object_vars($expected); $actualVars = get_object_vars($actual); - $this->assertSame(count($expectedVars), count($actualVars), $err); + $this->assertCount(count($expectedVars), $actualVars, $err); $this->assertEquals(array_keys($expectedVars), array_keys($actualVars), $err); foreach ($expectedVars as $name => $expectedValue) { diff --git a/tests/Language/TokenTest.php b/tests/Language/TokenTest.php index cb78c4e..54fc245 100644 --- a/tests/Language/TokenTest.php +++ b/tests/Language/TokenTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace GraphQL\Tests; +namespace GraphQL\Tests\Language; use GraphQL\Language\Token; use PHPUnit\Framework\TestCase; diff --git a/tests/Language/VisitorTest.php b/tests/Language/VisitorTest.php index b3c81f7..5c62397 100644 --- a/tests/Language/VisitorTest.php +++ b/tests/Language/VisitorTest.php @@ -1222,7 +1222,7 @@ class VisitorTest extends ValidatorTestCase $ast, Visitor::visitInParallel([ [ - 'enter' => function ($node) use (&$visited, $ast) { + 'enter' => function ($node) use ($ast) { $this->checkVisitorFnArgs($ast, func_get_args()); if ($node->kind === 'Field' && isset($node->name->value) && $node->name->value === 'b') { return Visitor::removeNode(); @@ -1292,7 +1292,7 @@ class VisitorTest extends ValidatorTestCase $ast, Visitor::visitInParallel([ [ - 'leave' => function ($node) use (&$visited, $ast) { + 'leave' => function ($node) use ($ast) { $this->checkVisitorFnArgs($ast, func_get_args(), true); if ($node->kind === 'Field' && isset($node->name->value) && $node->name->value === 'b') { return Visitor::removeNode(); diff --git a/tests/Server/QueryExecutionTest.php b/tests/Server/QueryExecutionTest.php index b6fc44e..2cde99f 100644 --- a/tests/Server/QueryExecutionTest.php +++ b/tests/Server/QueryExecutionTest.php @@ -66,7 +66,7 @@ class QueryExecutionTest extends ServerTestCase $query = '{f1'; $result = $this->executeQuery($query); - $this->assertSame(null, $result->data); + $this->assertNull($result->data); $this->assertCount(1, $result->errors); $this->assertContains( 'Syntax Error: Expected Name, found ', @@ -202,7 +202,7 @@ class QueryExecutionTest extends ServerTestCase $called1 = false; $called2 = false; - $this->config->setValidationRules(function (OperationParams $params) use ($q1, $q2, &$called1, &$called2) { + $this->config->setValidationRules(function (OperationParams $params) use ($q1, &$called1, &$called2) { if ($params->query === $q1) { $called1 = true; @@ -376,7 +376,7 @@ class QueryExecutionTest extends ServerTestCase 'Persistent query loader must return query string or instance of GraphQL\Language\AST\DocumentNode ' . 'but got: {"err":"err"}' ); - $this->config->setPersistentQueryLoader(function ($queryId, OperationParams $params) use (&$called) { + $this->config->setPersistentQueryLoader(function () { return ['err' => 'err']; }); $this->executePersistedQuery('some-id'); diff --git a/tests/Server/ServerConfigTest.php b/tests/Server/ServerConfigTest.php index 1fc2be8..b02eb38 100644 --- a/tests/Server/ServerConfigTest.php +++ b/tests/Server/ServerConfigTest.php @@ -17,17 +17,17 @@ class ServerConfigTest extends TestCase public function testDefaults() : void { $config = ServerConfig::create(); - $this->assertEquals(null, $config->getSchema()); - $this->assertEquals(null, $config->getContext()); - $this->assertEquals(null, $config->getRootValue()); - $this->assertEquals(null, $config->getErrorFormatter()); - $this->assertEquals(null, $config->getErrorsHandler()); - $this->assertEquals(null, $config->getPromiseAdapter()); - $this->assertEquals(null, $config->getValidationRules()); - $this->assertEquals(null, $config->getFieldResolver()); - $this->assertEquals(null, $config->getPersistentQueryLoader()); - $this->assertEquals(false, $config->getDebug()); - $this->assertEquals(false, $config->getQueryBatching()); + $this->assertNull($config->getSchema()); + $this->assertNull($config->getContext()); + $this->assertNull($config->getRootValue()); + $this->assertNull($config->getErrorFormatter()); + $this->assertNull($config->getErrorsHandler()); + $this->assertNull($config->getPromiseAdapter()); + $this->assertNull($config->getValidationRules()); + $this->assertNull($config->getFieldResolver()); + $this->assertNull($config->getPersistentQueryLoader()); + $this->assertFalse($config->getDebug()); + $this->assertFalse($config->getQueryBatching()); } public function testAllowsSettingSchema() : void @@ -166,10 +166,10 @@ class ServerConfigTest extends TestCase $config = ServerConfig::create(); $config->setDebug(true); - $this->assertSame(true, $config->getDebug()); + $this->assertTrue($config->getDebug()); $config->setDebug(false); - $this->assertSame(false, $config->getDebug()); + $this->assertFalse($config->getDebug()); } public function testAcceptsArray() : void @@ -204,8 +204,8 @@ class ServerConfigTest extends TestCase $this->assertSame($arr['validationRules'], $config->getValidationRules()); $this->assertSame($arr['fieldResolver'], $config->getFieldResolver()); $this->assertSame($arr['persistentQueryLoader'], $config->getPersistentQueryLoader()); - $this->assertSame(true, $config->getDebug()); - $this->assertSame(true, $config->getQueryBatching()); + $this->assertTrue($config->getDebug()); + $this->assertTrue($config->getQueryBatching()); } public function testThrowsOnInvalidArrayKey() : void diff --git a/tests/Type/EnumTypeTest.php b/tests/Type/EnumTypeTest.php index 55e508d..8445d60 100644 --- a/tests/Type/EnumTypeTest.php +++ b/tests/Type/EnumTypeTest.php @@ -132,7 +132,7 @@ class EnumTypeTest extends TestCase 'type' => Type::boolean(), ], ], - 'resolve' => function ($value, $args) use ($Complex1, $Complex2) { + 'resolve' => function ($value, $args) use ($Complex2) { if (! empty($args['provideGoodValue'])) { // Note: this is one of the references of the internal values which // ComplexEnum allows. diff --git a/tests/Type/ResolutionTest.php b/tests/Type/ResolutionTest.php index 352372c..741b13e 100644 --- a/tests/Type/ResolutionTest.php +++ b/tests/Type/ResolutionTest.php @@ -287,7 +287,7 @@ class ResolutionTest extends TestCase ]; $this->assertEquals($expectedDescriptor, $eagerTypeResolution->getDescriptor()); - $this->assertSame(null, $eagerTypeResolution->resolveType('User')); + $this->assertNull($eagerTypeResolution->resolveType('User')); $this->assertSame([], $eagerTypeResolution->resolvePossibleTypes($this->node)); $this->assertSame([], $eagerTypeResolution->resolvePossibleTypes($this->content)); $this->assertSame([], $eagerTypeResolution->resolvePossibleTypes($this->mention)); diff --git a/tests/Type/ScalarSerializationTest.php b/tests/Type/ScalarSerializationTest.php index fa89e01..fce19bc 100644 --- a/tests/Type/ScalarSerializationTest.php +++ b/tests/Type/ScalarSerializationTest.php @@ -186,13 +186,13 @@ class ScalarSerializationTest extends TestCase { $boolType = Type::boolean(); - $this->assertSame(true, $boolType->serialize('string')); - $this->assertSame(false, $boolType->serialize('')); - $this->assertSame(true, $boolType->serialize('1')); - $this->assertSame(true, $boolType->serialize(1)); - $this->assertSame(false, $boolType->serialize(0)); - $this->assertSame(true, $boolType->serialize(true)); - $this->assertSame(false, $boolType->serialize(false)); + $this->assertTrue($boolType->serialize('string')); + $this->assertFalse($boolType->serialize('')); + $this->assertTrue($boolType->serialize('1')); + $this->assertTrue($boolType->serialize(1)); + $this->assertFalse($boolType->serialize(0)); + $this->assertTrue($boolType->serialize(true)); + $this->assertFalse($boolType->serialize(false)); // TODO: how should it behave on '0'? } diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index b96f46d..362574b 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -1974,7 +1974,7 @@ class ValidationTest extends TestCase public function testRejectsDifferentInstancesOfTheSameType() : void { // Invalid: always creates new instance vs returning one from registry - $typeLoader = function ($name) use (&$typeLoader) { + $typeLoader = function ($name) { switch ($name) { case 'Query': return new ObjectType([ diff --git a/tests/Utils/AstFromValueTest.php b/tests/Utils/AstFromValueTest.php index da5156c..2f56a53 100644 --- a/tests/Utils/AstFromValueTest.php +++ b/tests/Utils/AstFromValueTest.php @@ -126,7 +126,7 @@ class AstFromValueTest extends TestCase */ public function testDoesNotConvertsNonNullValuestoNullValue() : void { - $this->assertSame(null, AST::astFromValue(null, Type::nonNull(Type::boolean()))); + $this->assertNull(AST::astFromValue(null, Type::nonNull(Type::boolean()))); } /** @@ -141,10 +141,10 @@ class AstFromValueTest extends TestCase ); // Note: case sensitive - $this->assertEquals(null, AST::astFromValue('hello', $this->myEnum())); + $this->assertNull(AST::astFromValue('hello', $this->myEnum())); // Note: Not a valid enum value - $this->assertEquals(null, AST::astFromValue('VALUE', $this->myEnum())); + $this->assertNull(AST::astFromValue('VALUE', $this->myEnum())); } /**