From af9c04119912bb96ccd5e067c97772d7169f53b1 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 17 Sep 2019 10:28:30 +0200 Subject: [PATCH] MC-19366: Fixes that list type and non null arguments would not be parsed as expected and would thus lead to false positives --- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 17 ++++++++++++++--- .../GraphQL/ValidArgumentNameUnitTest.graphqls | 12 +++++++++++- .../Tests/GraphQL/ValidArgumentNameUnitTest.php | 9 +++++++-- PHP_CodeSniffer/Tokenizers/GRAPHQL.php | 7 +++++-- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 337bf9ff..7741b675 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -85,10 +85,21 @@ public function process(File $phpcsFile, $stackPtr) */ private function getArgumentDefinitionEndPointer($argumentDefinitionStartPointer, array $tokens) { - $colonPointer = $this->seekToken(T_COLON, $tokens, $argumentDefinitionStartPointer); + $endPointer = $this->seekToken(T_COLON, $tokens, $argumentDefinitionStartPointer); + + //the colon is always followed by the type, which we can consume. it could be a list type though, thus we check + if ($tokens[$endPointer + 1]['code'] === T_OPEN_SQUARE_BRACKET) { + //consume everything up to closing bracket + $endPointer = $tokens[$endPointer + 1]['bracket_closer']; + } else { + //consume everything up to type + ++$endPointer; + } - //the colon is always followed by a type so we can consume the token after the colon - $endPointer = $colonPointer + 1; + //the type may be non null, meaning that it is followed by an exclamation mark, which we consume + if ($tokens[$endPointer + 1]['code'] === T_BOOLEAN_NOT) { + ++$endPointer; + } //if argument has a default value, we advance to the default definition end if ($tokens[$endPointer + 1]['code'] === T_EQUAL) { diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls index 5fa3c277..8c1e12c0 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls @@ -41,9 +41,19 @@ type Foo @doc(descripton: "Foo Bar Baz") { # A comment description: "This is a valid argument, spanned over multiple lines." ) @foo - varlidArgumentWithDefaultValue: Int = 20 @doc(description: "This is another valid argument with a default value.") + validArgumentWithDefaultValue: Int = 20 @doc(description: "This is another valid argument with a default value.") + validArgumentListType: [String] @doc(description: "This is a valid argument that uses a list type.") + validArgumentNonNullType: String! @doc(description: "This is a valid argument that uses a non null type.") + validArgumentNonNullListType: [String]! @doc(description: "This is a valid argument that uses a non null type.") + validArgumentNonNullTypeInListType: [String!] @doc(description: "This is a valid argument that uses a non null type within a list.") + validArgumentNonNullTypeInNonNullListType: [String!]! @doc(description: "This is a valid argument that uses a non null type within a non null list type.") # Invalid argument invalid_argument: String @doc(description: "This is an invalid argument."), invalid_argument_with_default_value: Int = 20 @doc(description: "This is another invalid argument with a default value") + invalid_argument_list_type: [String] @doc(description: "This is an invalid argument that uses a list type.") + invalid_argument_non_null_type: String! @doc(description: "This is an invalid argument that uses a non null type.") + invalid_argument_non_null_list_type: [String]! @doc(description: "This is an invalid argument that uses a non null type.") + invalid_argument_non_null_type_in_list_type: [String!] @doc(description: "This is an invalid argument that uses a non null type within a list.") + invalid_argument_non_null_type_in_non_null_list_type: [String!]! @doc(description: "This is a valid argument that uses a non null type within a non null list type.") ): String @doc(description: "Foo Bar") } diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php index 8ed55cfa..c0f41e5b 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php @@ -24,8 +24,13 @@ protected function getErrorList() 28 => 1, 29 => 1, 30 => 1, - 46 => 1, - 47 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, + 56 => 1, + 57 => 1, ]; } diff --git a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php index 9de922df..8dd5fcae 100644 --- a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php +++ b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php @@ -27,12 +27,12 @@ class GRAPHQL extends Tokenizer */ private $tokenTypeMap = [ Token::AT => 'T_DOC_COMMENT_TAG', - Token::BANG => null, + Token::BANG => 'T_BOOLEAN_NOT', Token::BLOCK_STRING => 'T_COMMENT', Token::BRACE_L => 'T_OPEN_CURLY_BRACKET', Token::BRACE_R => 'T_CLOSE_CURLY_BRACKET', Token::BRACKET_L => 'T_OPEN_SQUARE_BRACKET', - Token::BRACKET_R => 'T_CLOSE_CURLY_BRACKET', + Token::BRACKET_R => 'T_CLOSE_SQUARE_BRACKET', Token::COLON => 'T_COLON', Token::COMMENT => 'T_COMMENT', Token::DOLLAR => 'T_DOLLAR', @@ -112,6 +112,9 @@ protected function tokenize($string) case Token::PAREN_L: case Token::PAREN_R: case Token::COLON: + case Token::BRACKET_L: + case Token::BRACKET_R: + case Token::BANG: $value = $kind; break; default: