From d2c68485b9ce5ce6024d41c8542caedda1664365 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 10:46:41 +0200 Subject: [PATCH 1/9] Allow IgnoreByNameSuppressorInspection to ignore inspections that target lists This commit just adds a failing test. --- .../nullable-list-field.graphql.inc | 3 +++ .../IgnoreByNameSuppressorInspectionTest.php | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc create mode 100644 tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php diff --git a/test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc b/test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc new file mode 100644 index 0000000..cd52d34 --- /dev/null +++ b/test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc @@ -0,0 +1,3 @@ +type User { + alwaysTwoItems: [Int]! +} diff --git a/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php b/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php new file mode 100644 index 0000000..7e57f17 --- /dev/null +++ b/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php @@ -0,0 +1,22 @@ +app->get(NonNullableInsideListInspection::class); + $suppressor = $this->app->get(IgnoreByNameSuppressorInspection::class); + $suppressor->configure('User.alwaysTwoEntries'); + + expect($smartFileInfo) + ->toPassInspection($inspection, $suppressor); +})->with(getFixturesForDirectory( + __DIR__ . '/../../../test-resources/Inspections/IgnoreByNameSuppressorInspectionTest' +)); From 6321d13838e60f4e13a64cd5a63184549ab33683 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:10:06 +0200 Subject: [PATCH 2/9] Fix implementation --- .../IgnoreByNameSuppressorInspection.php | 45 ++++++++++--------- src/Utils/NodeNameResolver.php | 5 ++- .../IgnoreByNameSuppressorInspectionTest.php | 2 +- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/Inspections/IgnoreByNameSuppressorInspection.php b/src/Inspections/IgnoreByNameSuppressorInspection.php index 1664196..8abd5e3 100644 --- a/src/Inspections/IgnoreByNameSuppressorInspection.php +++ b/src/Inspections/IgnoreByNameSuppressorInspection.php @@ -23,30 +23,35 @@ public function __construct( public function shouldSuppress(Node $node, array $parents, Inspection $inspection): bool { - $name = $this->nameResolver->getName($node); - - $parent = end($parents); - if ($parent === false || $parent instanceof NodeList) { - $parentName = null; - } else { - $parentName = $this->nameResolver->getName($parent); - } - - if ($name === null) { - return false; + $paths = $this->paths([...$parents, $node]); + foreach ($paths as $path) { + if (in_array($path, $this->names)) { + return true; + } } - // Check if name in names - if (in_array($name, $this->names)) { - return true; - } + return false; + } - // Check if name dotted with parents in names - if (in_array("$parentName.$name", $this->names)) { - return true; + /** + * @param array $nodes A list of nodes where the first parent and the following entries are children + * + * @return list A list of increasingly specific paths, e.g. ['Parent', 'Parent.field', 'Parent.field.argument'] + */ + protected function paths(array $nodes): array + { + $paths = []; + foreach (array_reverse($nodes) as $node) { + $name = $this->nameResolver->getName($node); + if ($name === null) { + continue; + } + foreach ($paths as &$path) { + $path = "$name.$path"; + } + array_unshift($paths, $name); } - - return false; + return $paths; } public function configure(string ...$names): void diff --git a/src/Utils/NodeNameResolver.php b/src/Utils/NodeNameResolver.php index 2fbdade..00b1821 100644 --- a/src/Utils/NodeNameResolver.php +++ b/src/Utils/NodeNameResolver.php @@ -6,12 +6,13 @@ use GraphQL\Language\AST\NameNode; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\NodeList; class NodeNameResolver { - public function getName(?Node $node): ?string + public function getName(Node|NodeList|null $node): ?string { - if ($node === null) { + if (! $node instanceof Node) { return null; } diff --git a/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php b/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php index 7e57f17..bd0d07a 100644 --- a/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php +++ b/tests/Feature/Inspections/IgnoreByNameSuppressorInspectionTest.php @@ -13,7 +13,7 @@ it('ignores list inspection by name', function (SmartFileInfo $smartFileInfo) { $inspection = $this->app->get(NonNullableInsideListInspection::class); $suppressor = $this->app->get(IgnoreByNameSuppressorInspection::class); - $suppressor->configure('User.alwaysTwoEntries'); + $suppressor->configure('User.alwaysTwoItems'); expect($smartFileInfo) ->toPassInspection($inspection, $suppressor); From 3c862726264da34999e84ae1c6aad292de37f597 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:19:10 +0200 Subject: [PATCH 3/9] Simplify implementation --- .../IgnoreByNameSuppressorInspection.php | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/src/Inspections/IgnoreByNameSuppressorInspection.php b/src/Inspections/IgnoreByNameSuppressorInspection.php index 8abd5e3..d493110 100644 --- a/src/Inspections/IgnoreByNameSuppressorInspection.php +++ b/src/Inspections/IgnoreByNameSuppressorInspection.php @@ -23,8 +23,15 @@ public function __construct( public function shouldSuppress(Node $node, array $parents, Inspection $inspection): bool { - $paths = $this->paths([...$parents, $node]); - foreach ($paths as $path) { + $path = ''; + foreach ([...$parents, $node] as $ancestor) { + $name = $this->nameResolver->getName($ancestor); + if ($name === null) { + continue; + } + $path = $path === '' + ? $name + : "$path.$name"; if (in_array($path, $this->names)) { return true; } @@ -33,27 +40,6 @@ public function shouldSuppress(Node $node, array $parents, Inspection $inspectio return false; } - /** - * @param array $nodes A list of nodes where the first parent and the following entries are children - * - * @return list A list of increasingly specific paths, e.g. ['Parent', 'Parent.field', 'Parent.field.argument'] - */ - protected function paths(array $nodes): array - { - $paths = []; - foreach (array_reverse($nodes) as $node) { - $name = $this->nameResolver->getName($node); - if ($name === null) { - continue; - } - foreach ($paths as &$path) { - $path = "$name.$path"; - } - array_unshift($paths, $name); - } - return $paths; - } - public function configure(string ...$names): void { $this->names = $names; From 1b6abc75cc6ec442c4c75dcda568ba268b9cf012 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:19:18 +0200 Subject: [PATCH 4/9] Fix test --- ...ist-field.graphql.inc => nullable-list-field.skip.graphql.inc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/{nullable-list-field.graphql.inc => nullable-list-field.skip.graphql.inc} (100%) diff --git a/test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc b/test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.skip.graphql.inc similarity index 100% rename from test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.graphql.inc rename to test-resources/Inspections/IgnoreByNameSuppressorInspectionTest/nullable-list-field.skip.graphql.inc From aae0f64379e659cfe9f24a117c1829a47324e10e Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:19:22 +0200 Subject: [PATCH 5/9] Fix types --- src/Contracts/SuppressorInspection.php | 3 ++- src/Visitors/VisitorCollector.php | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Contracts/SuppressorInspection.php b/src/Contracts/SuppressorInspection.php index eaafdc4..36095ed 100644 --- a/src/Contracts/SuppressorInspection.php +++ b/src/Contracts/SuppressorInspection.php @@ -5,12 +5,13 @@ namespace Worksome\Graphlint\Contracts; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\NodeList; use Worksome\Graphlint\Inspections\Inspection; interface SuppressorInspection { /** - * @param Node[] $parents + * @param array $parents */ public function shouldSuppress(Node $node, array $parents, Inspection $inspection): bool; } diff --git a/src/Visitors/VisitorCollector.php b/src/Visitors/VisitorCollector.php index 15e929c..05c16ed 100644 --- a/src/Visitors/VisitorCollector.php +++ b/src/Visitors/VisitorCollector.php @@ -15,6 +15,7 @@ use GraphQL\Language\AST\ListTypeNode; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; +use GraphQL\Language\AST\NodeList; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\AST\ScalarTypeDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; @@ -144,7 +145,7 @@ private function wrapper( $key, $parent, $path, - $ancestors, + array $ancestors, ) use ( $closure, $affectedInspections, @@ -171,7 +172,7 @@ private function wrapper( } /** - * @param Node[] $parent + * @param array $parent */ private function shouldSkip(Node $node, array $parent, Inspection $inspection): bool { From 6127869db8d8011f9d9c1c17439878928f2abf7e Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:26:13 +0200 Subject: [PATCH 6/9] generic type for NodeList --- phpstan.neon | 3 --- src/Contracts/SuppressorInspection.php | 2 +- src/Inspections/IgnoreByNameSuppressorInspection.php | 1 - src/Utils/NodeNameResolver.php | 3 +++ src/Visitors/VisitorCollector.php | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 7552167..a172447 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -13,6 +13,3 @@ parameters: - message: "#Undefined variable: \\$this#" path: tests/** - - - message: '#^Instanceof between GraphQL\\Language\\AST\\Node and GraphQL\\Language\\AST\\NodeList will always evaluate to false\.$#' - path: src/Inspections/IgnoreByNameSuppressorInspection.php diff --git a/src/Contracts/SuppressorInspection.php b/src/Contracts/SuppressorInspection.php index 36095ed..2922620 100644 --- a/src/Contracts/SuppressorInspection.php +++ b/src/Contracts/SuppressorInspection.php @@ -11,7 +11,7 @@ interface SuppressorInspection { /** - * @param array $parents + * @param array> $parents */ public function shouldSuppress(Node $node, array $parents, Inspection $inspection): bool; } diff --git a/src/Inspections/IgnoreByNameSuppressorInspection.php b/src/Inspections/IgnoreByNameSuppressorInspection.php index d493110..c185240 100644 --- a/src/Inspections/IgnoreByNameSuppressorInspection.php +++ b/src/Inspections/IgnoreByNameSuppressorInspection.php @@ -5,7 +5,6 @@ namespace Worksome\Graphlint\Inspections; use GraphQL\Language\AST\Node; -use GraphQL\Language\AST\NodeList; use Worksome\Graphlint\Contracts\SuppressorInspection; use Worksome\Graphlint\Utils\NodeNameResolver; diff --git a/src/Utils/NodeNameResolver.php b/src/Utils/NodeNameResolver.php index 00b1821..0fb1b81 100644 --- a/src/Utils/NodeNameResolver.php +++ b/src/Utils/NodeNameResolver.php @@ -10,6 +10,9 @@ class NodeNameResolver { + /** + * @param Node|NodeList|null $node + */ public function getName(Node|NodeList|null $node): ?string { if (! $node instanceof Node) { diff --git a/src/Visitors/VisitorCollector.php b/src/Visitors/VisitorCollector.php index 05c16ed..6565975 100644 --- a/src/Visitors/VisitorCollector.php +++ b/src/Visitors/VisitorCollector.php @@ -172,7 +172,7 @@ private function wrapper( } /** - * @param array $parent + * @param array> $parent */ private function shouldSkip(Node $node, array $parent, Inspection $inspection): bool { From 10c110cd210e65ebd5d3051ba59d51199983a9c0 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:26:39 +0200 Subject: [PATCH 7/9] compact phpstan config --- phpstan.neon | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index a172447..642efd1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,15 +1,11 @@ includes: - - ./vendor/worksome/coding-style/phpstan.neon - +- vendor/worksome/coding-style/phpstan.neon parameters: paths: - - src - - tests - + - src + - tests level: 9 - ignoreErrors: - - '#Call to an undefined method Pest\\Expectation.*#' - - - message: "#Undefined variable: \\$this#" - path: tests/** + - '#Call to an undefined method Pest\\Expectation.*#' + - message: "#Undefined variable: \\$this#" + path: tests/** From 889073be75a89327fd8f5f63b5e753ac70489de8 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 22 Jun 2023 11:26:53 +0200 Subject: [PATCH 8/9] reject --- src/Inspections/MutationFieldArgumentNamedInputInspection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Inspections/MutationFieldArgumentNamedInputInspection.php b/src/Inspections/MutationFieldArgumentNamedInputInspection.php index 0c07637..533b66d 100644 --- a/src/Inspections/MutationFieldArgumentNamedInputInspection.php +++ b/src/Inspections/MutationFieldArgumentNamedInputInspection.php @@ -38,8 +38,8 @@ public function visitObjectTypeDefinition( Collection::make($fields) // Get all arguments of the fields ->flatMap(fn(FieldDefinitionNode $node) => iterator_to_array($node->arguments)) - // Filter down to arguments which are not named `input` - ->filter(fn(InputValueDefinitionNode $node) => $this->nameResolver->getName($node) !== 'input') + // Reject arguments which are named `input` + ->reject(fn(InputValueDefinitionNode $node) => $this->nameResolver->getName($node) === 'input') // Register a problem on each of the arguments ->each(fn(InputValueDefinitionNode $node) => $problemsHolder->registerProblemWithDescription( $node->name, From 6ec138fa2f1248760571a0cd7b2ba3220bcba349 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 3 Jul 2024 09:19:58 +0200 Subject: [PATCH 9/9] Simplify --- src/Inspections/IgnoreByNameSuppressorInspection.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Inspections/IgnoreByNameSuppressorInspection.php b/src/Inspections/IgnoreByNameSuppressorInspection.php index c185240..087eb67 100644 --- a/src/Inspections/IgnoreByNameSuppressorInspection.php +++ b/src/Inspections/IgnoreByNameSuppressorInspection.php @@ -22,16 +22,16 @@ public function __construct( public function shouldSuppress(Node $node, array $parents, Inspection $inspection): bool { - $path = ''; + $path = []; foreach ([...$parents, $node] as $ancestor) { $name = $this->nameResolver->getName($ancestor); if ($name === null) { continue; } - $path = $path === '' - ? $name - : "$path.$name"; - if (in_array($path, $this->names)) { + + $path[] = $name; + $fullName = implode('.', $path); + if (in_array($fullName, $this->names)) { return true; } }