From ba0bbce3e893c1c1854edb3736a571218ed73cd9 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 6 Sep 2019 16:52:27 +0200 Subject: [PATCH 01/25] MC-19366: Adds a tokenizer for GraphQL schema files --- Magento2/ruleset.xml | 6 ++- PHP_CodeSniffer/Tokenizers/GraphQL.php | 52 ++++++++++++++++++++++++++ composer.json | 5 +++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 PHP_CodeSniffer/Tokenizers/GraphQL.php diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 246b6fe1..adda516c 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -3,7 +3,7 @@ Magento Coding Standard - + @@ -522,6 +522,10 @@ 0 + + 6 + warning + diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php new file mode 100644 index 00000000..2b66e3a0 --- /dev/null +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -0,0 +1,52 @@ + 'T_CLASS', + 'interface' => 'T_CLASS', + 'enum' => 'T_CLASS', + '#' => 'T_COMMENT', + ]; + + /** + * Constructor. + * + * @param string $content + * @param Config $config + * @param string $eolChar + * @throws \PHP_CodeSniffer\Exceptions\TokenizerException + */ + public function __construct($content, Config $config, $eolChar = '\n') + { + //add our token values + $this->tokenValues = array_merge( + $this->tokenValues, + $this->additionalTokenValues + ); + + //let parent do its job (which will start tokenizing) + parent::__construct($content, $config, $eolChar); + } + + /** + * @inheritDoc + */ + public function processAdditional() + { + //NOP Does nothing intentionally + } + +} diff --git a/composer.json b/composer.json index d67474e9..a3684c36 100644 --- a/composer.json +++ b/composer.json @@ -14,6 +14,11 @@ "require-dev": { "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0" }, + "autoload": { + "classmap": [ + "PHP_CodeSniffer/Tokenizers/" + ] + }, "scripts": { "post-install-cmd": "vendor/bin/phpcs --config-set installed_paths ../../..", "post-update-cmd": "vendor/bin/phpcs --config-set installed_paths ../../.." From 6003dcd0a79f0a49cb3bb152ab4939f80aa2f3bc Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 6 Sep 2019 16:59:17 +0200 Subject: [PATCH 02/25] MC-19366: Adds sniff for type names --- .../Sniffs/GraphQL/ValidTypeNameSniff.php | 32 ++++++++ .../GraphQL/ValidTypeNameUnitTest.graphqls | 44 +++++++++++ .../Tests/GraphQL/ValidTypeNameUnitTest.php | 77 +++++++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php create mode 100644 Magento2/Tests/GraphQL/ValidTypeNameUnitTest.graphqls create mode 100644 Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php diff --git a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php new file mode 100644 index 00000000..76af7b7d --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php @@ -0,0 +1,32 @@ +type, interface and enum) that are not specified in + * UpperCamelCase. + */ +class ValidTypeNameSniff extends ValidClassNameSniff +{ + + /** + * Defines the tokenizers that this sniff is using. + * + * @var array + */ + public $supportedTokenizers = ['GraphQL']; + + /** + * @inheritDoc + */ + public function register() + { + return [T_CLASS]; + } + +} diff --git a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.graphqls new file mode 100644 index 00000000..1fba0386 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.graphqls @@ -0,0 +1,44 @@ +# Valid type names. +type ValidCamelCaseType {} +interface ValidCamelCaseInterface {} +enum ValidCamelCaseEnum {} + +# Incorrect usage of camel case. +type invalidCamelCaseType {} +type Invalid_Camel_Case_Type_With_Underscores {} +interface invalidCamelCaseInterface {} +interface Invalid_Camel_Case_Interface_With_Underscores {} +enum invalidCamelCaseEnum {} +enum Invalid_Camel_Case_Enum_With_Underscores {} + +# All lowercase +type invalidlowercasetype {} +interface invalidlowercaseinterface {} +enum invalidlowercaseenum {} + +# All uppercase +type VALIDUPPERCASETYPE {} +type INVALID_UPPERCASE_TYPE_WITH_UNDERSCORES {} +interface VALIDUPPERCASEINTERFACE {} +interface INVALID_UPPERCASE_INTERFACE_WITH_UNDERSCORES {} +enum VALIDUPPERCASECENUM {} +enum INVALID_UPPERCASE_ENUM_WITH_UNDERSCORES {} + +# Mix camel case with uppercase +type ValidCamelCaseTypeWITHUPPERCASE {} +interface ValidCamelCaseInterfaceWITHUPPERCASE {} +enum ValidCamelCaseEnumWITHUPPERCASE {} + +# Usage of numeric characters +type ValidCamelCaseTypeWith1Number {} +type ValidCamelCaseTypeWith12345Numbers {} +type 5InvalidCamelCaseTypeStartingWithNumber {} +type ValidCamelCaseTypeEndingWithNumber4 {} +interface ValidCamelCaseInterfaceWith1Number {} +interface ValidCamelCaseInterfaceWith12345Numbers {} +interface 5InvalidCamelCaseInterfaceStartingWithNumber {} +interface ValidCamelCaseInterfaceEndingWithNumber4 {} +enum ValidCamelCaseEnumWith1Number {} +enum ValidCamelCaseEnumWith12345Numbers {} +enum 5InvalidCamelCaseEnumStartingWithNumber {} +enum ValidCamelCaseEnumEndingWithNumber4 {} \ No newline at end of file diff --git a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php new file mode 100644 index 00000000..0e07c91d --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php @@ -0,0 +1,77 @@ +extensions = array_merge( + $config->extensions, + [ + 'graphqls' => 'GraphQL' + ] + ); + + //and write back to a global that is used in base class + $GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config; + } + + /** + * Returns the lines where errors should occur. + * + * The key of the array should represent the line number and the value + * should represent the number of errors that should occur on that line. + * + * @return array + */ + protected function getErrorList() + { + return [ + 7 => 1, + 8 => 1, + 9 => 1, + 10 => 1, + 11 => 1, + 12 => 1, + 15 => 1, + 16 => 1, + 17 => 1, + 21 => 1, + 23 => 1, + 25 => 1, + 35 => 1, + 39 => 1, + 43 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * The key of the array should represent the line number and the value + * should represent the number of warnings that should occur on that line. + * + * @return array + */ + protected function getWarningList() + { + return []; + } +} + From 8cb90c89e32224b641beed3c42493be61174f836 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Mon, 9 Sep 2019 17:06:02 +0200 Subject: [PATCH 03/25] MC-19366: Improves GraphQL tokenizer by making use of webonyx/graphql-php library --- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 28 +++ .../Sniffs/GraphQL/ValidTypeNameSniff.php | 36 +++- PHP_CodeSniffer/Tokenizers/GraphQL.php | 171 ++++++++++++++++-- composer.json | 3 +- composer.lock | 54 +++++- 5 files changed, 273 insertions(+), 19 deletions(-) create mode 100644 Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php new file mode 100644 index 00000000..e7acd46a --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -0,0 +1,28 @@ + - TechDivision GmbH + * All rights reserved + * + * This product includes proprietary software developed at TechDivision GmbH, Germany + * For more information see https://www.techdivision.com/ + * + * To obtain a valid license for using this software please contact us at license@techdivision.com + */ + +namespace Magento2\Sniffs\GraphQL; + +/** + * Kurze Beschreibung der Klasse + * + * Lange Beschreibung der Klasse (wenn vorhanden)... + * + * @copyright Copyright (c) 2019 TechDivision GmbH - TechDivision GmbH + * @link https://www.techdivision.com/ + * @author Jean-Bernard Valentaten + */ +class ValidArgumentNymeSniff +{ + +} diff --git a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php index 76af7b7d..c6aba7f8 100644 --- a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php @@ -5,13 +5,15 @@ */ namespace Magento2\Sniffs\GraphQL; -use PHP_CodeSniffer\Standards\Squiz\Sniffs\Classes\ValidClassNameSniff; +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Common; /** * Detects types (type, interface and enum) that are not specified in * UpperCamelCase. */ -class ValidTypeNameSniff extends ValidClassNameSniff +class ValidTypeNameSniff implements Sniff { /** @@ -26,7 +28,35 @@ class ValidTypeNameSniff extends ValidClassNameSniff */ public function register() { - return [T_CLASS]; + return [T_CLASS, T_INTERFACE]; } + /** + * @inheritDoc + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + //compose entity name by making use of the next strings the we find until we hit a non-string token + $name = ''; + for ($i=$stackPtr+1; $tokens[$i]['code'] === T_STRING; ++$i) { + $name .= $tokens[$i]['content']; + } + + $valid = Common::isCamelCaps($name, true, true, false); + + if ($valid === false) { + $type = ucfirst($tokens[$stackPtr]['content']); + $error = '%s name "%s" is not in PascalCase format'; + $data = [ + $type, + $name, + ]; + $phpcsFile->addError($error, $stackPtr, 'NotCamelCaps', $data); + $phpcsFile->recordMetric($stackPtr, 'PascalCase class name', 'no'); + } else { + $phpcsFile->recordMetric($stackPtr, 'PascalCase class name', 'yes'); + } + } } diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php index 2b66e3a0..2fbe2634 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -6,19 +6,63 @@ namespace PHP_CodeSniffer\Tokenizers; +use GraphQL\Language\Lexer; +use GraphQL\Language\Source; +use GraphQL\Language\Token; use PHP_CodeSniffer\Config; +use PHP_CodeSniffer\Exceptions\TokenizerException; /** * Implements a tokenizer for GraphQL files. + * + * @todo Reimplement using the official GraphQL implementation */ -class GraphQL extends JS +class GraphQL extends Tokenizer { - protected $additionalTokenValues = [ - 'type' => 'T_CLASS', - 'interface' => 'T_CLASS', - 'enum' => 'T_CLASS', - '#' => 'T_COMMENT', + /** + * Defines how GraphQL token types are mapped to PHP token types. + * + * @var array + */ + private $tokenTypeMap = [ + Token::AMP => null, //TODO + Token::AT => 'T_DOC_COMMENT_TAG', + Token::BANG => null, //TODO + 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::COLON => 'T_COLON', + Token::COMMENT => 'T_COMMENT', + Token::DOLLAR => 'T_DOLLAR', + Token::EOF => 'T_CLOSE_TAG', + Token::EQUALS => 'T_EQUAL', + Token::FLOAT => null, //TODO + Token::INT => null, //TODO + Token::NAME => 'T_STRING', + Token::PAREN_L => 'T_OPEN_PARENTHESIS', + Token::PAREN_R => 'T_CLOSE_PARENTHESIS', + Token::PIPE => null, //TODO + Token::SPREAD => 'T_ELLIPSIS', + Token::SOF => 'T_OPEN_TAG', + Token::STRING => 'T_STRING', + ]; + + /** + * Defines how special keywords are mapped to PHP token types + * + * @var array + */ + private $keywordTokenTypeMap = [ + 'enum' => 'T_CLASS', + 'extend' => 'T_EXTENDS', //TODO This might not be the appropriate equivalent + 'interface' => 'T_INTERFACE', + 'implements' => 'T_IMPLEMENTS', + 'type' => 'T_CLASS', + 'union' => 'T_CLASS', + //TODO Add further types ]; /** @@ -27,17 +71,13 @@ class GraphQL extends JS * @param string $content * @param Config $config * @param string $eolChar - * @throws \PHP_CodeSniffer\Exceptions\TokenizerException + * @throws TokenizerException */ public function __construct($content, Config $config, $eolChar = '\n') { - //add our token values - $this->tokenValues = array_merge( - $this->tokenValues, - $this->additionalTokenValues - ); + //TODO We might want to delete this unless we need the constructor to work totally different - //let parent do its job (which will start tokenizing) + //let parent do its job parent::__construct($content, $config, $eolChar); } @@ -46,7 +86,110 @@ public function __construct($content, Config $config, $eolChar = '\n') */ public function processAdditional() { - //NOP Does nothing intentionally + //NOP: Does nothing intentionally } + /** + * {@inheritDoc} + * + * @throws TokenizerException + */ + protected function tokenize($string) + { + $this->logVerbose('*** START GRAPHQL TOKENIZING ***'); + + $string = str_replace($this->eolChar, "\n", $string); + $tokens = []; + $lexer = new Lexer( + new Source($string) + ); + + do { + $kind = $lexer->token->kind; + $value = $lexer->token->value ?: ''; + + //if we have encountered a keyword, we convert it + //otherwise we translate the token or default it to T_STRING + if ($kind === Token::NAME && isset($this->keywordTokenTypeMap[$value])) { + $tokenType = $this->keywordTokenTypeMap[$value]; + } elseif (isset($this->tokenTypeMap[$kind])) { + $tokenType = $this->tokenTypeMap[$kind]; + } else { + $tokenType = 'T_STRING'; + } + + //some GraphQL tokens need special handling + switch ($kind) { + case Token::AT: + case Token::BRACE_L: + case Token::BRACE_R: + case Token::PAREN_L: + case Token::PAREN_R: + $value = $kind; + break; + default: + //NOP + } + + //finally we create the PHP token + $token = [ + 'code' => constant($tokenType), + 'type' => $tokenType, + 'content' => $value, + ]; + $line = $lexer->token->line; + + $lexer->advance(); + + //if line has changed (and we're not on start of file) we have to append at least one line break to current + //tokens content otherwise PHP_CodeSniffer will screw up line numbers + if ($lexer->token->line !== $line && $kind !== Token::SOF) { + $token['content'] .= $this->eolChar; + } + $tokens[] = $token; + $tokens = array_merge( + $tokens, + $this->getNewLineTokens($line, $lexer->token->line) + ); + } while ($lexer->token->kind !== Token::EOF); + + $this->logVerbose('*** END GRAPHQL TOKENIZING ***'); + return $tokens; + } + + /** + * Returns tokens of empty new lines for the range $lineStart to $lineEnd + * + * @param int $lineStart + * @param int $lineEnd + * @return array + */ + private function getNewLineTokens($lineStart, $lineEnd) + { + $amount = ($lineEnd - $lineStart) - 1; + $tokens = []; + + for ($i = 0; $i < $amount; ++$i) { + $tokens[] = [ + 'code' => T_WHITESPACE, + 'type' => 'T_WHITESPACE', + 'content' => $this->eolChar, + ]; + } + + return $tokens; + } + + /** + * Logs $message if {@link PHP_CODESNIFFER_VERBOSITY} is greater than $level. + * + * @param string $message + * @param int $level + */ + private function logVerbose($message, $level = 1) + { + if (PHP_CODESNIFFER_VERBOSITY > $level) { + printf("\t%s" . PHP_EOL, $message); + } + } } diff --git a/composer.json b/composer.json index a3684c36..889a9f4e 100644 --- a/composer.json +++ b/composer.json @@ -9,7 +9,8 @@ "version": "4", "require": { "php": ">=5.6.0", - "squizlabs/php_codesniffer": "^3.4" + "squizlabs/php_codesniffer": "^3.4", + "webonyx/graphql-php": "^0.13.8" }, "require-dev": { "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0" diff --git a/composer.lock b/composer.lock index 416f08c7..377b98d9 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "dd5642c8fa31b4c4fdcf19135002185f", + "content-hash": "c407f30fd32fc53345205fe5c6d3aa4d", "packages": [ { "name": "squizlabs/php_codesniffer", @@ -56,6 +56,58 @@ "standards" ], "time": "2019-04-10T23:49:02+00:00" + }, + { + "name": "webonyx/graphql-php", + "version": "v0.13.8", + "source": { + "type": "git", + "url": "https://github.com/webonyx/graphql-php.git", + "reference": "6829ae58f4c59121df1f86915fb9917a2ec595e8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/webonyx/graphql-php/zipball/6829ae58f4c59121df1f86915fb9917a2ec595e8", + "reference": "6829ae58f4c59121df1f86915fb9917a2ec595e8", + "shasum": "" + }, + "require": { + "ext-json": "*", + "ext-mbstring": "*", + "php": "^7.1||^8.0" + }, + "require-dev": { + "doctrine/coding-standard": "^6.0", + "phpbench/phpbench": "^0.14.0", + "phpstan/phpstan": "^0.11.4", + "phpstan/phpstan-phpunit": "^0.11.0", + "phpstan/phpstan-strict-rules": "^0.11.0", + "phpunit/phpcov": "^5.0", + "phpunit/phpunit": "^7.2", + "psr/http-message": "^1.0", + "react/promise": "2.*" + }, + "suggest": { + "psr/http-message": "To use standard GraphQL server", + "react/promise": "To leverage async resolving on React PHP platform" + }, + "type": "library", + "autoload": { + "psr-4": { + "GraphQL\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "A PHP port of GraphQL reference implementation", + "homepage": "https://github.com/webonyx/graphql-php", + "keywords": [ + "api", + "graphql" + ], + "time": "2019-08-25T10:32:47+00:00" } ], "packages-dev": [ From 586244ad0718b730d03eaf532a2b2a12218e3f67 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 10 Sep 2019 14:38:08 +0200 Subject: [PATCH 04/25] MC-19366: Adds sniff for field names --- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 28 ----- .../Sniffs/GraphQL/ValidFieldNameSniff.php | 65 +++++++++++ .../Sniffs/GraphQL/ValidTypeNameSniff.php | 2 +- .../AbstractGraphQLSniffUnitTestCase.php | 35 ++++++ .../GraphQL/ValidFieldNameUnitTest.graphqls | 31 ++++++ .../Tests/GraphQL/ValidFieldNameUnitTest.php | 45 ++++++++ .../Tests/GraphQL/ValidTypeNameUnitTest.php | 24 +--- PHP_CodeSniffer/Tokenizers/GraphQL.php | 103 ++++++++++++------ 8 files changed, 249 insertions(+), 84 deletions(-) delete mode 100644 Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php create mode 100644 Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php create mode 100644 Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php create mode 100644 Magento2/Tests/GraphQL/ValidFieldNameUnitTest.graphqls create mode 100644 Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php deleted file mode 100644 index e7acd46a..00000000 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ /dev/null @@ -1,28 +0,0 @@ - - TechDivision GmbH - * All rights reserved - * - * This product includes proprietary software developed at TechDivision GmbH, Germany - * For more information see https://www.techdivision.com/ - * - * To obtain a valid license for using this software please contact us at license@techdivision.com - */ - -namespace Magento2\Sniffs\GraphQL; - -/** - * Kurze Beschreibung der Klasse - * - * Lange Beschreibung der Klasse (wenn vorhanden)... - * - * @copyright Copyright (c) 2019 TechDivision GmbH - TechDivision GmbH - * @link https://www.techdivision.com/ - * @author Jean-Bernard Valentaten - */ -class ValidArgumentNymeSniff -{ - -} diff --git a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php new file mode 100644 index 00000000..c8aef8c0 --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php @@ -0,0 +1,65 @@ +snake_case. + */ +class ValidFieldNameSniff implements Sniff +{ + + /** + * Defines the tokenizers that this sniff is using. + * + * @var array + */ + public $supportedTokenizers = ['GraphQL']; + + /** + * @inheritDoc + */ + public function register() + { + return [T_VARIABLE]; + } + + /** + * @inheritDoc + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + $name = $tokens[$stackPtr]['content']; + + if (!$this->isSnakeCase($name)) { + $type = 'Field'; + $error = '%s name "%s" is not in snake_case format'; + $data = [ + $type, + $name, + ]; + $phpcsFile->addError($error, $stackPtr, 'NotSnakeCase', $data); + $phpcsFile->recordMetric($stackPtr, 'SnakeCase field name', 'no'); + } else { + $phpcsFile->recordMetric($stackPtr, 'SnakeCase field name', 'yes'); + } + } + + /** + * Returns whether $name is strictly lower case, potentially separated by underscores. + * + * @param string $name + * @return bool + */ + private function isSnakeCase($name) + { + return preg_match('/^[a-z][a-z0-9_]*$/', $name); + } +} diff --git a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php index c6aba7f8..6b639294 100644 --- a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php @@ -38,7 +38,7 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - //compose entity name by making use of the next strings the we find until we hit a non-string token + //compose entity name by making use of the next strings that we find until we hit a non-string token $name = ''; for ($i=$stackPtr+1; $tokens[$i]['code'] === T_STRING; ++$i) { $name .= $tokens[$i]['content']; diff --git a/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php new file mode 100644 index 00000000..83f858b0 --- /dev/null +++ b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php @@ -0,0 +1,35 @@ +extensions = array_merge( + $config->extensions, + [ + 'graphqls' => 'GraphQL' + ] + ); + + //and write back to a global that is used in base class + $GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config; + } + +} diff --git a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.graphqls new file mode 100644 index 00000000..54b496ac --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.graphqls @@ -0,0 +1,31 @@ +type FooType { + # Valid snake case field names + valid_snake_case_field: String + validlowercasefield: String + valid_snake_case_field_ending_with_number_5: String + valid_snake_case_field_with_ending_numbers_12345: String + valid_snake_case_field_5_with_number5_inline: String + + # Incorrect use of snake case + INVALIDUPPERCASEFIELD: String + INVALID_SCREAMING_SNAKE_CASE_FIELD: String + Invalid_Upper_Snake_Case_Field: String + invalid_mixed_case_FIELD: String + InvalidCameCaseFieldName: String +} + +interface FooInterface { + # Valid snake case field names + valid_snake_case_field: String + validlowercasefield: String + valid_snake_case_field_ending_with_number_5: String + valid_snake_case_field_with_ending_numbers_12345: String + valid_snake_case_field_5_with_number5_inline: String + + # Incorrect use of snake case + INVALIDUPPERCASEFIELD: String + INVALID_SCREAMING_SNAKE_CASE_FIELD: String + Invalid_Upper_Snake_Case_Field: String + invalid_mixed_case_FIELD: String + InvalidCameCaseFieldName: String +} diff --git a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php new file mode 100644 index 00000000..eebd53e3 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php @@ -0,0 +1,45 @@ + + */ + protected function getErrorList() + { + return [ + 10 => 1, + 11 => 1, + 12 => 1, + 13 => 1, + 14 => 1, + 26 => 1, + 27 => 1, + 28 => 1, + 29 => 1, + 30 => 1, + ]; + } + + /** + * @inheritDoc + */ + protected function getWarningList() + { + return []; + } +} diff --git a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php index 0e07c91d..a04fb314 100644 --- a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php @@ -6,32 +6,12 @@ namespace Magento2\Tests\GraphQL; use PHP_CodeSniffer\Config; -use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Covers {@link \agento2\Sniffs\GraphQL\ValidTypeNameSniff}. + * Covers {@link \Magento2\Sniffs\GraphQL\ValidTypeNameSniff}. */ -class ValidTypeNameUnitTest extends AbstractSniffUnitTest +class ValidTypeNameUnitTest extends AbstractGraphQLSniffUnitTestCase { - - protected function setUp() - { - //let parent do its job - parent::setUp(); - - //generate a config that allows ro use our GraphQL tokenizer - $config = new Config(); - $config->extensions = array_merge( - $config->extensions, - [ - 'graphqls' => 'GraphQL' - ] - ); - - //and write back to a global that is used in base class - $GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config; - } - /** * Returns the lines where errors should occur. * diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php index 2fbe2634..d3d03ecc 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -3,19 +3,14 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace PHP_CodeSniffer\Tokenizers; use GraphQL\Language\Lexer; use GraphQL\Language\Source; use GraphQL\Language\Token; -use PHP_CodeSniffer\Config; -use PHP_CodeSniffer\Exceptions\TokenizerException; /** * Implements a tokenizer for GraphQL files. - * - * @todo Reimplement using the official GraphQL implementation */ class GraphQL extends Tokenizer { @@ -26,9 +21,9 @@ class GraphQL extends Tokenizer * @var array */ private $tokenTypeMap = [ - Token::AMP => null, //TODO + Token::AMP => null, //TODO Should we map this to a specific type Token::AT => 'T_DOC_COMMENT_TAG', - Token::BANG => null, //TODO + Token::BANG => null, //TODO Should we map this to a specific type Token::BLOCK_STRING => 'T_COMMENT', Token::BRACE_L => 'T_OPEN_CURLY_BRACKET', Token::BRACE_R => 'T_CLOSE_CURLY_BRACKET', @@ -39,12 +34,12 @@ class GraphQL extends Tokenizer Token::DOLLAR => 'T_DOLLAR', Token::EOF => 'T_CLOSE_TAG', Token::EQUALS => 'T_EQUAL', - Token::FLOAT => null, //TODO - Token::INT => null, //TODO + Token::FLOAT => null, //TODO Should we map this to a specific type + Token::INT => null, //TODO Should we map this to a specific type Token::NAME => 'T_STRING', Token::PAREN_L => 'T_OPEN_PARENTHESIS', Token::PAREN_R => 'T_CLOSE_PARENTHESIS', - Token::PIPE => null, //TODO + Token::PIPE => null, //TODO Should we map this to a specific type Token::SPREAD => 'T_ELLIPSIS', Token::SOF => 'T_OPEN_TAG', Token::STRING => 'T_STRING', @@ -62,37 +57,45 @@ class GraphQL extends Tokenizer 'implements' => 'T_IMPLEMENTS', 'type' => 'T_CLASS', 'union' => 'T_CLASS', - //TODO Add further types + //TODO We may have to add further types ]; - /** - * Constructor. - * - * @param string $content - * @param Config $config - * @param string $eolChar - * @throws TokenizerException - */ - public function __construct($content, Config $config, $eolChar = '\n') - { - //TODO We might want to delete this unless we need the constructor to work totally different - - //let parent do its job - parent::__construct($content, $config, $eolChar); - } - /** * @inheritDoc */ public function processAdditional() { - //NOP: Does nothing intentionally + $this->logVerbose('*** START ADDITIONAL GRAPHQL PROCESSING ***'); + + $processingEntity = false; + $numTokens = count($this->tokens); + $entityTypes = [T_CLASS, T_INTERFACE]; + + for ($i = 0; $i < $numTokens; ++$i) { + $tokenCode = $this->tokens[$i]['code']; + + //have we found a new entity or its end? + if (in_array($tokenCode, $entityTypes) && $this->tokens[$i]['content'] !== 'enum') { + $processingEntity = true; + continue; + } elseif ($tokenCode === T_CLOSE_CURLY_BRACKET) { + $processingEntity = false; + continue; + } + + //if we are processing an entity, are we currently seeing a field? + if ($processingEntity && $this->isFieldToken($i)) { + $this->tokens[$i]['code'] = T_VARIABLE; + $this->tokens[$i]['type'] = 'T_VARIABLE'; + continue; + } + } + + $this->logVerbose('*** END ADDITIONAL GRAPHQL PROCESSING ***'); } /** - * {@inheritDoc} - * - * @throws TokenizerException + * @inheritDoc */ protected function tokenize($string) { @@ -125,6 +128,7 @@ protected function tokenize($string) case Token::BRACE_R: case Token::PAREN_L: case Token::PAREN_R: + case Token::COLON: $value = $kind; break; default: @@ -142,12 +146,12 @@ protected function tokenize($string) $lexer->advance(); //if line has changed (and we're not on start of file) we have to append at least one line break to current - //tokens content otherwise PHP_CodeSniffer will screw up line numbers + //token's content otherwise PHP_CodeSniffer will screw up line numbers if ($lexer->token->line !== $line && $kind !== Token::SOF) { $token['content'] .= $this->eolChar; } $tokens[] = $token; - $tokens = array_merge( + $tokens = array_merge( $tokens, $this->getNewLineTokens($line, $lexer->token->line) ); @@ -180,6 +184,39 @@ private function getNewLineTokens($lineStart, $lineEnd) return $tokens; } + /** + * Returns whether the token under $stackPointer is a field. + * + * We consider a token to be a field if: + *
    + *
  • its direct successor is of type {@link T_COLON}
    • + *
  • it has a list of arguments followed by a {@link T_COLON}
    • + *
+ * + * @param int $stackPointer + * @return bool + */ + private function isFieldToken($stackPointer) + { + $nextToken = $this->tokens[$stackPointer + 1]; + + //if next token is an opening parenthesis, we seek for the closing parenthesis + if ($nextToken['code'] === T_OPEN_PARENTHESIS) { + $nextPointer = $stackPointer + 1; + $numTokens = count($this->tokens); + + for ($i=$nextPointer; $i<$numTokens; ++$i) { + if ($this->tokens[$i]['code'] === T_CLOSE_PARENTHESIS) { + $nextToken = $this->tokens[$i + 1]; + break; + } + } + } + + //return whether next token is a colon + return $nextToken['code'] === T_COLON; + } + /** * Logs $message if {@link PHP_CODESNIFFER_VERBOSITY} is greater than $level. * From 27e99de30626d032604f30203e46a29f84630e25 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 11 Sep 2019 10:41:28 +0200 Subject: [PATCH 05/25] MC-19366: Adds sniff for argument names --- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 145 ++++++++++++++++++ .../Sniffs/GraphQL/ValidFieldNameSniff.php | 1 - .../ValidArgumentNameUnitTest.graphqls | 32 ++++ .../GraphQL/ValidArgumentNameUnitTest.php | 39 +++++ .../Tests/GraphQL/ValidFieldNameUnitTest.php | 9 +- PHP_CodeSniffer/Tokenizers/GraphQL.php | 79 ++++++---- 6 files changed, 270 insertions(+), 35 deletions(-) create mode 100644 Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php create mode 100644 Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls create mode 100644 Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php new file mode 100644 index 00000000..6faeefb6 --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -0,0 +1,145 @@ +cameCase. + */ +class ValidArgumentNameSniff implements Sniff +{ + + /** + * Defines the tokenizers that this sniff is using. + * + * @var array + */ + public $supportedTokenizers = ['GraphQL']; + + /** + * @inheritDoc + */ + public function register() + { + return [T_OPEN_PARENTHESIS]; + } + + /** + * @inheritDoc + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + $closeParenthesisPointer = $this->getCloseParenthesisPointer($stackPtr, $tokens); + + //if we could not find the closing parenthesis pointer, we add a warning and terminate + if ($closeParenthesisPointer === false) { + $error = 'Possible parse error: Missing closing parenthesis for argument list in line %d'; + $data = [ + $tokens[$stackPtr]['line'], + ]; + $phpcsFile->addWarning($error, $stackPtr, 'UnclosedArgumentList', $data); + return; + } + + $arguments = $this->getArguments($stackPtr, $closeParenthesisPointer, $tokens); + + foreach ($arguments as $argument) { + $pointer = $argument[0]; + $name = $argument[1]; + + if (!$this->isCamelCase($name)) { + $type = 'Argument'; + $error = '%s name "%s" is not in CamelCase format'; + $data = [ + $type, + $name, + ]; + + $phpcsFile->addError($error, $pointer, 'NotCamelCase', $data); + $phpcsFile->recordMetric($pointer, 'CamelCase argument name', 'no'); + } else { + $phpcsFile->recordMetric($pointer, 'CamelCase argument name', 'yes'); + } + } + + //return stack pointer of closing parenthesis + return $closeParenthesisPointer; + } + + /** + * Finds all argument names contained in $tokens range $startPointer and + * $endPointer. + * + * @param int $startPointer + * @param int $endPointer + * @param array $tokens + * @return array[] + */ + private function getArguments($startPointer, $endPointer, array $tokens) + { + $argumentTokenPointer = null; + $argument = ''; + $names = []; + $skipTypes = [T_COMMENT, T_WHITESPACE]; + + for ($i = $startPointer + 1; $i < $endPointer; ++$i) { + //skip comment tokens + if (in_array($tokens[$i]['code'], $skipTypes)) { + continue; + } + $argument .= $tokens[$i]['content']; + + if ($argumentTokenPointer === null) { + $argumentTokenPointer = $i; + } + + if (preg_match('/^.+:.+$/', $argument)) { + list($name, $type) = explode(':', $argument); + $names[] = [$argumentTokenPointer, $name]; + $argument = ''; + $argumentTokenPointer = null; + } + } + + return $names; + } + + /** + * Seeks the next available token of type {@link T_CLOSE_PARENTHESIS} in $tokens and returns its pointer. + * + * @param int $stackPointer + * @param array $tokens + * @return bool|int + */ + private function getCloseParenthesisPointer($stackPointer, array $tokens) + { + $numTokens = count($tokens); + + for ($i = $stackPointer + 1; $i < $numTokens; ++$i) { + if ($tokens[$i]['code'] === T_CLOSE_PARENTHESIS) { + return $i; + } + } + + //if we came here we could not find the closing parenthesis + return false; + } + + /** + * Returns whether $name starts with a lower case character and is written in camel case. + * + * @param string $argumentName + * @return bool + */ + private function isCamelCase($argumentName) + { + return (preg_match('/^[a-z][a-zA-Z0-9]+$/', $argumentName) !== 0); + } +} diff --git a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php index c8aef8c0..23d7cb1c 100644 --- a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php @@ -7,7 +7,6 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; -use PHP_CodeSniffer\Util\Common; /** * Detects field names the are not specified in snake_case. diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls new file mode 100644 index 00000000..83b4f23c --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls @@ -0,0 +1,32 @@ +type Foo { + bar( + # Valid argument names + validArgumentName: Int + validArgumentNameWith1Number: Int + validArgumentNameWith12345Numbers: Int + validArgumentNameEndingWithNumber5: Int + validArgumentNameWithUPPERCASE: Int + validargumentwithlowercase: Int + # Invalid argument names + InvalidArgumentNameUpperCamelCase: Int + INVALIDARGUMENTNAMEUPPERCASE: Int + invalid_argument_name_snake_case: Int + 5invalidArgumentNameStatingWithNumber: Int + ): String +} +interface Foo { + bar( + # Valid argument names + validArgumentName: Int + validArgumentNameWith1Number: Int + validArgumentNameWith12345Numbers: Int + validArgumentNameEndingWithNumber5: Int + validArgumentNameWithUPPERCASE: Int + validargumentwithlowercase: Int + # Invalid argument names + InvalidArgumentNameUpperCamelCase: Int + INVALIDARGUMENTNAMEUPPERCASE: Int + invalid_argument_name_snake_case: Int + 5invalidArgumentNameStatingWithNumber: Int + ): String +} \ No newline at end of file diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php new file mode 100644 index 00000000..746c867e --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php @@ -0,0 +1,39 @@ + 1, + 12 => 1, + 13 => 1, + 14 => 1, + 27 => 1, + 28 => 1, + 29 => 1, + 30 => 1, + ]; + } + + /** + * @inheritDoc + */ + protected function getWarningList() + { + return []; + } +} + diff --git a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php index eebd53e3..3ded35bb 100644 --- a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php @@ -6,18 +6,13 @@ namespace Magento2\Tests\GraphQL; /** - * Covers {@link \Magento2\Sniffs\GraphQL\ValidFieldNameUnitTest}. + * Covers {@link \Magento2\Sniffs\GraphQL\ValidFieldNameSniff}. */ class ValidFieldNameUnitTest extends AbstractGraphQLSniffUnitTestCase { /** - * Returns the lines where errors should occur. - * - * The key of the array should represent the line number and the value - * should represent the number of errors that should occur on that line. - * - * @return array + * @inheritDoc */ protected function getErrorList() { diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php index d3d03ecc..0e49cd01 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -3,6 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ + namespace PHP_CodeSniffer\Tokenizers; use GraphQL\Language\Lexer; @@ -67,29 +68,7 @@ public function processAdditional() { $this->logVerbose('*** START ADDITIONAL GRAPHQL PROCESSING ***'); - $processingEntity = false; - $numTokens = count($this->tokens); - $entityTypes = [T_CLASS, T_INTERFACE]; - - for ($i = 0; $i < $numTokens; ++$i) { - $tokenCode = $this->tokens[$i]['code']; - - //have we found a new entity or its end? - if (in_array($tokenCode, $entityTypes) && $this->tokens[$i]['content'] !== 'enum') { - $processingEntity = true; - continue; - } elseif ($tokenCode === T_CLOSE_CURLY_BRACKET) { - $processingEntity = false; - continue; - } - - //if we are processing an entity, are we currently seeing a field? - if ($processingEntity && $this->isFieldToken($i)) { - $this->tokens[$i]['code'] = T_VARIABLE; - $this->tokens[$i]['type'] = 'T_VARIABLE'; - continue; - } - } + $this->processFields(); $this->logVerbose('*** END ADDITIONAL GRAPHQL PROCESSING ***'); } @@ -203,9 +182,9 @@ private function isFieldToken($stackPointer) //if next token is an opening parenthesis, we seek for the closing parenthesis if ($nextToken['code'] === T_OPEN_PARENTHESIS) { $nextPointer = $stackPointer + 1; - $numTokens = count($this->tokens); + $numTokens = count($this->tokens); - for ($i=$nextPointer; $i<$numTokens; ++$i) { + for ($i = $nextPointer; $i < $numTokens; ++$i) { if ($this->tokens[$i]['code'] === T_CLOSE_PARENTHESIS) { $nextToken = $this->tokens[$i + 1]; break; @@ -213,8 +192,8 @@ private function isFieldToken($stackPointer) } } - //return whether next token is a colon - return $nextToken['code'] === T_COLON; + //return whether current token is a string and next token is a colon + return $this->tokens[$stackPointer]['code'] === T_STRING && $nextToken['code'] === T_COLON; } /** @@ -229,4 +208,50 @@ private function logVerbose($message, $level = 1) printf("\t%s" . PHP_EOL, $message); } } + + /** + * Processes field tokens, setting their type to {@link T_VARIABLE}. + */ + private function processFields() + { + $processingArgumentList = false; + $processingEntity = false; + $numTokens = count($this->tokens); + $entityTypes = [T_CLASS, T_INTERFACE]; + $skipTypes = [T_COMMENT, T_WHITESPACE]; + + for ($i = 0; $i < $numTokens; ++$i) { + $tokenCode = $this->tokens[$i]['code']; + + //process current token + switch (true) { + case in_array($tokenCode, $skipTypes): + //we have hit a token that needs to be skipped -> NOP + break; + case in_array($tokenCode, $entityTypes) && $this->tokens[$i]['content'] !== 'enum': + //we have hit an entity declaration + $processingEntity = true; + break; + case $tokenCode === T_CLOSE_CURLY_BRACKET: + //we have hit the end of an entity declaration + $processingEntity = false; + break; + case $tokenCode === T_OPEN_PARENTHESIS: + //we have hit an argument list + $processingArgumentList = true; + break; + case $tokenCode === T_CLOSE_PARENTHESIS: + //we have hit an argument list end + $processingArgumentList = false; + break; + case $processingEntity && !$processingArgumentList && $this->isFieldToken($i): + //we have hit a field + $this->tokens[$i]['code'] = T_VARIABLE; + $this->tokens[$i]['type'] = 'T_VARIABLE'; + break; + default: + //NOP All operations have already been executed + } + } + } } From 47fb2030cfcf97d50142c53454505f073d1a3255 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 11 Sep 2019 13:10:47 +0200 Subject: [PATCH 06/25] MC-19366: Adds sniff for top level fields --- .../Sniffs/GraphQL/AbstractGraphQLSniff.php | 52 +++++++++++++++++++ .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 21 +------- .../Sniffs/GraphQL/ValidFieldNameSniff.php | 20 +------ .../GraphQL/ValidTopLevelFieldNameSniff.php | 51 ++++++++++++++++++ .../Sniffs/GraphQL/ValidTypeNameSniff.php | 10 +--- .../ValidTopLevelFieldNameUnitTest.graphqls | 35 +++++++++++++ .../ValidTopLevelFieldNameUnitTest.php | 39 ++++++++++++++ Magento2/ruleset.xml | 12 +++++ PHP_CodeSniffer/Tokenizers/GraphQL.php | 2 + 9 files changed, 194 insertions(+), 48 deletions(-) create mode 100644 Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php create mode 100644 Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php create mode 100644 Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.graphqls create mode 100644 Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php new file mode 100644 index 00000000..6d52dcfd --- /dev/null +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -0,0 +1,52 @@ + - TechDivision GmbH + * All rights reserved + * + * This product includes proprietary software developed at TechDivision GmbH, Germany + * For more information see https://www.techdivision.com/ + * + * To obtain a valid license for using this software please contact us at license@techdivision.com + */ +namespace Magento2\Sniffs\GraphQL; + +use PHP_CodeSniffer\Sniffs\Sniff; + +/** + * Defines an abstract base class for GraphQL sniffs. + */ +abstract class AbstractGraphQLSniff implements Sniff +{ + + /** + * Defines the tokenizers that this sniff is using. + * + * @var array + */ + public $supportedTokenizers = ['GraphQL']; + + /** + * Returns whether $name starts with a lower case character and is written in camel case. + * + * @param string $name + * @return bool + */ + protected function isCamelCase($name) + { + return (preg_match('/^[a-z][a-zA-Z0-9]+$/', $name) !== 0); + } + + /** + * Returns whether $name is strictly lower case, potentially separated by underscores. + * + * @param string $name + * @return bool + */ + protected function isSnakeCase($name) + { + return preg_match('/^[a-z][a-z0-9_]*$/', $name); + } + +} diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 6faeefb6..5a96ec77 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -3,25 +3,16 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\Sniff; /** * Detects argument names that are not specified in cameCase. */ -class ValidArgumentNameSniff implements Sniff +class ValidArgumentNameSniff extends AbstractGraphQLSniff { - /** - * Defines the tokenizers that this sniff is using. - * - * @var array - */ - public $supportedTokenizers = ['GraphQL']; - /** * @inheritDoc */ @@ -132,14 +123,4 @@ private function getCloseParenthesisPointer($stackPointer, array $tokens) return false; } - /** - * Returns whether $name starts with a lower case character and is written in camel case. - * - * @param string $argumentName - * @return bool - */ - private function isCamelCase($argumentName) - { - return (preg_match('/^[a-z][a-zA-Z0-9]+$/', $argumentName) !== 0); - } } diff --git a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php index 23d7cb1c..91743afa 100644 --- a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php @@ -6,21 +6,13 @@ namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\Sniff; /** * Detects field names the are not specified in snake_case. */ -class ValidFieldNameSniff implements Sniff +class ValidFieldNameSniff extends AbstractGraphQLSniff { - /** - * Defines the tokenizers that this sniff is using. - * - * @var array - */ - public $supportedTokenizers = ['GraphQL']; - /** * @inheritDoc */ @@ -51,14 +43,4 @@ public function process(File $phpcsFile, $stackPtr) } } - /** - * Returns whether $name is strictly lower case, potentially separated by underscores. - * - * @param string $name - * @return bool - */ - private function isSnakeCase($name) - { - return preg_match('/^[a-z][a-z0-9_]*$/', $name); - } } diff --git a/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php new file mode 100644 index 00000000..cbdf642b --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php @@ -0,0 +1,51 @@ +cameCase. + */ +class ValidTopLevelFieldNameSniff extends AbstractGraphQLSniff +{ + + /** + * @inheritDoc + */ + public function register() + { + return [T_FUNCTION]; + } + + /** + * @inheritDoc + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + //compose function name by making use of the next strings that we find until we hit a non-string token + $name = ''; + for ($i=$stackPtr+1; $tokens[$i]['code'] === T_STRING; ++$i) { + $name .= $tokens[$i]['content']; + } + + if (strlen($name) > 0 && !$this->isCamelCase($name)) { + $type = ucfirst($tokens[$stackPtr]['content']); + $error = '%s name "%s" is not in PascalCase format'; + $data = [ + $type, + $name, + ]; + $phpcsFile->addError($error, $stackPtr, 'NotCamelCase', $data); + $phpcsFile->recordMetric($stackPtr, 'CamelCase top level field name', 'no'); + } else { + $phpcsFile->recordMetric($stackPtr, 'CamelCase top level field name', 'yes'); + } + } + +} diff --git a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php index 6b639294..ca153340 100644 --- a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php @@ -6,23 +6,15 @@ namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Common; /** * Detects types (type, interface and enum) that are not specified in * UpperCamelCase. */ -class ValidTypeNameSniff implements Sniff +class ValidTypeNameSniff extends AbstractGraphQLSniff { - /** - * Defines the tokenizers that this sniff is using. - * - * @var array - */ - public $supportedTokenizers = ['GraphQL']; - /** * @inheritDoc */ diff --git a/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.graphqls new file mode 100644 index 00000000..39d16c92 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.graphqls @@ -0,0 +1,35 @@ +# Valid names +query validCamelCaseQuery {} +mutation validCamelCaseMutation {} + +# Incorrect use of camel cyse +query InvalidCamelCaseQuery {} +query invalid_Camel_Case_Query_With_Underscores {} +mutation InvalidCamelCaseMutation {} +mutation invalid_Camel_Case_Mutation_With_Underscores {} + +# All lower case +query validlowercasequery {} +mutation validlowercasemutation {} + +# All upper case +query INVALIDUPPERCASEQUERY {} +mutation INVALIDUPPERCASEMUTATION {} + +# Mix camel case with uppercase +query validCamelCaseQueryWithUPPERCASE {} +mutation validCamelCaseMutationWithUPPERCASE {} + +# Usage of numeric characters +query validCamelCaseQueryWith1Number {} +query validCamelCaseQueryWith12345Numbers {} +query validCamelCaseQueryEndingWithNumber4 {} +query 5invalidCamelCaseQueryStartingWithNumber {} +mutation validCamelCaseMutationWith1Number {} +mutation validCamelCaseMutationWith12345Numbers {} +mutation validCamelCaseMutationEndingWithNumber4 {} +mutation 5invalidCamelCaseMutationStartingWithNumber {} + +# Empty names (valid by definition of GraphQL) +query {} +mutation {} \ No newline at end of file diff --git a/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php new file mode 100644 index 00000000..759e9e18 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php @@ -0,0 +1,39 @@ + 1, + 7 => 1, + 8 => 1, + 9 => 1, + 16 => 1, + 17 => 1, + 27 => 1, + 31 => 1, + ]; + } + + /** + * @inheritDoc + */ + protected function getWarningList() + { + return []; + } + +} diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index adda516c..d31750fe 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -526,6 +526,18 @@ 6 warning + + 6 + warning + + + 6 + warning + + + 6 + warning + diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php index 0e49cd01..508660fd 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -58,6 +58,8 @@ class GraphQL extends Tokenizer 'implements' => 'T_IMPLEMENTS', 'type' => 'T_CLASS', 'union' => 'T_CLASS', + 'query' => 'T_FUNCTION', + 'mutation' => 'T_FUNCTION', //TODO We may have to add further types ]; From 6f5d136ce1a4d98598ccbefd043158de35bd3ef2 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 11 Sep 2019 13:31:36 +0200 Subject: [PATCH 07/25] MC-19366: Downgrade of package webonyx/graphql-php to match lowest php version supported by coding standard --- PHP_CodeSniffer/Tokenizers/GraphQL.php | 1 - composer.json | 2 +- composer.lock | 23 ++++++++--------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GraphQL.php index 508660fd..3d2c30b1 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GraphQL.php @@ -22,7 +22,6 @@ class GraphQL extends Tokenizer * @var array */ private $tokenTypeMap = [ - Token::AMP => null, //TODO Should we map this to a specific type Token::AT => 'T_DOC_COMMENT_TAG', Token::BANG => null, //TODO Should we map this to a specific type Token::BLOCK_STRING => 'T_COMMENT', diff --git a/composer.json b/composer.json index 889a9f4e..492357e1 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ "require": { "php": ">=5.6.0", "squizlabs/php_codesniffer": "^3.4", - "webonyx/graphql-php": "^0.13.8" + "webonyx/graphql-php": ">=0.12.6 <1.0" }, "require-dev": { "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0" diff --git a/composer.lock b/composer.lock index 377b98d9..de4d5a34 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c407f30fd32fc53345205fe5c6d3aa4d", + "content-hash": "560d5eca9cf59f965d8f3b98d2ee2a0a", "packages": [ { "name": "squizlabs/php_codesniffer", @@ -59,31 +59,24 @@ }, { "name": "webonyx/graphql-php", - "version": "v0.13.8", + "version": "v0.12.6", "source": { "type": "git", "url": "https://github.com/webonyx/graphql-php.git", - "reference": "6829ae58f4c59121df1f86915fb9917a2ec595e8" + "reference": "4c545e5ec4fc37f6eb36c19f5a0e7feaf5979c95" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/webonyx/graphql-php/zipball/6829ae58f4c59121df1f86915fb9917a2ec595e8", - "reference": "6829ae58f4c59121df1f86915fb9917a2ec595e8", + "url": "https://api.github.com/repos/webonyx/graphql-php/zipball/4c545e5ec4fc37f6eb36c19f5a0e7feaf5979c95", + "reference": "4c545e5ec4fc37f6eb36c19f5a0e7feaf5979c95", "shasum": "" }, "require": { - "ext-json": "*", "ext-mbstring": "*", - "php": "^7.1||^8.0" + "php": ">=5.6" }, "require-dev": { - "doctrine/coding-standard": "^6.0", - "phpbench/phpbench": "^0.14.0", - "phpstan/phpstan": "^0.11.4", - "phpstan/phpstan-phpunit": "^0.11.0", - "phpstan/phpstan-strict-rules": "^0.11.0", - "phpunit/phpcov": "^5.0", - "phpunit/phpunit": "^7.2", + "phpunit/phpunit": "^4.8", "psr/http-message": "^1.0", "react/promise": "2.*" }, @@ -107,7 +100,7 @@ "api", "graphql" ], - "time": "2019-08-25T10:32:47+00:00" + "time": "2018-09-02T14:59:54+00:00" } ], "packages-dev": [ From 7a5b2661b42a88aff31f23924060147ab274fb32 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 11 Sep 2019 14:18:20 +0200 Subject: [PATCH 08/25] MC-19366: Fixes code style issues --- Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php | 2 -- Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php | 2 -- Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php | 2 -- Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php | 2 -- Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php | 1 - Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php | 2 -- Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php | 1 - Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php | 1 - Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php | 2 -- Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php | 1 - 10 files changed, 16 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index 6d52dcfd..b03cffe8 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -19,7 +19,6 @@ */ abstract class AbstractGraphQLSniff implements Sniff { - /** * Defines the tokenizers that this sniff is using. * @@ -48,5 +47,4 @@ protected function isSnakeCase($name) { return preg_match('/^[a-z][a-z0-9_]*$/', $name); } - } diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 5a96ec77..ffb1f8d4 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -12,7 +12,6 @@ */ class ValidArgumentNameSniff extends AbstractGraphQLSniff { - /** * @inheritDoc */ @@ -122,5 +121,4 @@ private function getCloseParenthesisPointer($stackPointer, array $tokens) //if we came here we could not find the closing parenthesis return false; } - } diff --git a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php index 91743afa..f638c228 100644 --- a/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidFieldNameSniff.php @@ -12,7 +12,6 @@ */ class ValidFieldNameSniff extends AbstractGraphQLSniff { - /** * @inheritDoc */ @@ -42,5 +41,4 @@ public function process(File $phpcsFile, $stackPtr) $phpcsFile->recordMetric($stackPtr, 'SnakeCase field name', 'yes'); } } - } diff --git a/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php index cbdf642b..eacbc10d 100644 --- a/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidTopLevelFieldNameSniff.php @@ -12,7 +12,6 @@ */ class ValidTopLevelFieldNameSniff extends AbstractGraphQLSniff { - /** * @inheritDoc */ @@ -47,5 +46,4 @@ public function process(File $phpcsFile, $stackPtr) $phpcsFile->recordMetric($stackPtr, 'CamelCase top level field name', 'yes'); } } - } diff --git a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php index ca153340..b80ada40 100644 --- a/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidTypeNameSniff.php @@ -14,7 +14,6 @@ */ class ValidTypeNameSniff extends AbstractGraphQLSniff { - /** * @inheritDoc */ diff --git a/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php index 83f858b0..f767a25f 100644 --- a/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php +++ b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php @@ -13,7 +13,6 @@ */ abstract class AbstractGraphQLSniffUnitTestCase extends AbstractSniffUnitTest { - protected function setUp() { //let parent do its job @@ -31,5 +30,4 @@ protected function setUp() //and write back to a global that is used in base class $GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config; } - } diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php index 746c867e..9c9cfa1d 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php @@ -10,7 +10,6 @@ */ class ValidArgumentNameUnitTest extends AbstractGraphQLSniffUnitTestCase { - /** * @inheritDoc */ diff --git a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php index 3ded35bb..6e55e399 100644 --- a/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidFieldNameUnitTest.php @@ -10,7 +10,6 @@ */ class ValidFieldNameUnitTest extends AbstractGraphQLSniffUnitTestCase { - /** * @inheritDoc */ diff --git a/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php index 759e9e18..be37ddd2 100644 --- a/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidTopLevelFieldNameUnitTest.php @@ -10,7 +10,6 @@ */ class ValidTopLevelFieldNameUnitTest extends AbstractGraphQLSniffUnitTestCase { - /** * @inheritDoc */ @@ -35,5 +34,4 @@ protected function getWarningList() { return []; } - } diff --git a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php index a04fb314..035934e0 100644 --- a/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidTypeNameUnitTest.php @@ -54,4 +54,3 @@ protected function getWarningList() return []; } } - From e310c5c80eed552966ba677a41d945ecbd7e021c Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 11 Sep 2019 14:25:05 +0200 Subject: [PATCH 09/25] MC-19366: Fixes code style issue --- Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php index 9c9cfa1d..091a017d 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php @@ -35,4 +35,3 @@ protected function getWarningList() return []; } } - From 2a8d52183bb612c3e4b520fb9b6d8a664932d9a9 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Thu, 12 Sep 2019 08:59:40 +0200 Subject: [PATCH 10/25] MC-19366: Deactivates field name validation --- Magento2/ruleset.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index d31750fe..9c524658 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -526,10 +526,11 @@ 6 warning - - 6 - warning - + + + + + 6 warning From 505c9b25c8e974a8f430992aba3f9a8adcc6958d Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Thu, 12 Sep 2019 11:33:18 +0200 Subject: [PATCH 11/25] MC-19366: Adds sniff for valid enum values --- .../Sniffs/GraphQL/AbstractGraphQLSniff.php | 32 ++++- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 16 +-- .../Sniffs/GraphQL/ValidEnumValueSniff.php | 134 ++++++++++++++++++ .../GraphQL/ValidEnumValueUnitTest.graphqls | 19 +++ .../Tests/GraphQL/ValidEnumValueUnitTest.php | 34 +++++ 5 files changed, 220 insertions(+), 15 deletions(-) create mode 100644 Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php create mode 100644 Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls create mode 100644 Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index b03cffe8..b9e95fae 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -10,6 +10,7 @@ * * To obtain a valid license for using this software please contact us at license@techdivision.com */ + namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Sniffs\Sniff; @@ -38,13 +39,38 @@ protected function isCamelCase($name) } /** - * Returns whether $name is strictly lower case, potentially separated by underscores. + * Returns whether $name is specified in snake case (either all lower case or all upper case). * * @param string $name + * @param bool $upperCase If set to true checks for all upper case, otherwise all lower case * @return bool */ - protected function isSnakeCase($name) + protected function isSnakeCase($name, $upperCase = false) { - return preg_match('/^[a-z][a-z0-9_]*$/', $name); + $pattern = $upperCase ? '/^[A-Z][A-Z0-9_]*$/' : '/^[a-z][a-z0-9_]*$/'; + return preg_match($pattern, $name); + } + + /** + * Searches for the first token that has $tokenCode in $tokens from position + * $startPointer (excluded). + * + * @param mixed $tokenCode + * @param array $tokens + * @param int $startPointer + * @return bool|int If token was found, returns its pointer, false otherwise + */ + protected function seekToken($tokenCode, array $tokens, $startPointer = 0) + { + $numTokens = count($tokens); + + for ($i = $startPointer + 1; $i < $numTokens; ++$i) { + if ($tokens[$i]['code'] === $tokenCode) { + return $i; + } + } + + //if we came here we could not find the requested token + return false; } } diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index ffb1f8d4..03de433f 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -3,6 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Files\File; @@ -64,7 +65,7 @@ public function process(File $phpcsFile, $stackPtr) } /** - * Finds all argument names contained in $tokens range $startPointer and + * Finds all argument names contained in $tokens in range $startPointer to * $endPointer. * * @param int $startPointer @@ -80,7 +81,7 @@ private function getArguments($startPointer, $endPointer, array $tokens) $skipTypes = [T_COMMENT, T_WHITESPACE]; for ($i = $startPointer + 1; $i < $endPointer; ++$i) { - //skip comment tokens + //skip some tokens if (in_array($tokens[$i]['code'], $skipTypes)) { continue; } @@ -110,15 +111,6 @@ private function getArguments($startPointer, $endPointer, array $tokens) */ private function getCloseParenthesisPointer($stackPointer, array $tokens) { - $numTokens = count($tokens); - - for ($i = $stackPointer + 1; $i < $numTokens; ++$i) { - if ($tokens[$i]['code'] === T_CLOSE_PARENTHESIS) { - return $i; - } - } - - //if we came here we could not find the closing parenthesis - return false; + return $this->seekToken(T_CLOSE_PARENTHESIS, $tokens, $stackPointer); } } diff --git a/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php b/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php new file mode 100644 index 00000000..b7c702c7 --- /dev/null +++ b/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php @@ -0,0 +1,134 @@ +SCREAMING_SNAKE_CASE. + */ +class ValidEnumValueSniff extends AbstractGraphQLSniff +{ + + /** + * @inheritDoc + */ + public function register() + { + return [T_CLASS]; + } + + /** + * @inheritDoc + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + //bail out if we're not inspecting an enum + if ($tokens[$stackPtr]['content'] !== 'enum') { + return; + } + + $openingCurlyPointer = $this->getOpeningCurlyBracketPointer($stackPtr, $tokens); + $closingCurlyPointer = $this->getClosingCurlyBracketPointer($stackPtr, $tokens); + + //if we could not find the closing curly bracket pointer, we add a warning and terminate + if ($openingCurlyPointer === false || $closingCurlyPointer === false) { + $error = 'Possible parse error: %s missing opening or closing brace'; + $data = [$tokens[$stackPtr]['content']]; + $phpcsFile->addWarning($error, $stackPtr, 'MissingBrace', $data); + return; + } + + $values = $this->getValues($openingCurlyPointer, $closingCurlyPointer, $tokens, $phpcsFile->eolChar); + + foreach ($values as $value) { + $pointer = $value[0]; + $name = $value[1]; + + if (!$this->isSnakeCase($name, true)) { + $type = 'Enum value'; + $error = '%s "%s" is not in SCREAMING_SNAKE_CASE format'; + $data = [ + $type, + $name, + ]; + + $phpcsFile->addError($error, $pointer, 'NotScreamingSnakeCase', $data); + $phpcsFile->recordMetric($pointer, 'SCREAMING_SNAKE_CASE enum value', 'no'); + } else { + $phpcsFile->recordMetric($pointer, 'SCREAMING_SNAKE_CASE enum value', 'yes'); + } + } + + return $closingCurlyPointer; + } + + /** + * Seeks the next available token of type {@link T_CLOSE_CURLY_BRACKET} in $tokens and returns its + * pointer. + * + * @param int $startPointer + * @param array $tokens + * @return bool|int + */ + private function getClosingCurlyBracketPointer($startPointer, array $tokens) + { + return $this->seekToken(T_CLOSE_CURLY_BRACKET, $tokens, $startPointer); + } + + /** + * Seeks the next available token of type {@link T_OPEN_CURLY_BRACKET} in $tokens and returns its + * pointer. + * + * @param $startPointer + * @param array $tokens + * @return bool|int + */ + private function getOpeningCurlyBracketPointer($startPointer, array $tokens) + { + return $this->seekToken(T_OPEN_CURLY_BRACKET, $tokens, $startPointer); + } + + /** + * Finds all enum values contained in $tokens in range $startPointer to + * $endPointer. + * + * @param int $startPointer + * @param int $endPointer + * @param array $tokens + * @param string $eolChar + * @return array[] + */ + private function getValues($startPointer, $endPointer, array $tokens, $eolChar) + { + $valueTokenPointer = null; + $enumValue = ''; + $values = []; + $skipTypes = [T_COMMENT, T_WHITESPACE]; + + for ($i = $startPointer + 1; $i < $endPointer; ++$i) { + //skip some tokens + if (in_array($tokens[$i]['code'], $skipTypes)) { + continue; + } + $enumValue .= $tokens[$i]['content']; + + if ($valueTokenPointer === null) { + $valueTokenPointer = $i; + } + + if (strpos($enumValue, $eolChar) !== false) { + $values[] = [$valueTokenPointer, rtrim($enumValue, $eolChar)]; + $enumValue = ''; + $valueTokenPointer = null; + } + } + + return $values; + } +} diff --git a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls new file mode 100644 index 00000000..994e75f2 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls @@ -0,0 +1,19 @@ +enum Foo { + # Valid values + VALID_SCREAMING_SNAKE_CASE_VALUE + VALID_SCREAMING_SNAKE_CASE_VALUE_WITH_1_NUMBER + VALID_SCREAMING_SNAKE_CASE_VALUE_WITH_12345_NUMBERS + VALID_SCREAMING_SNAKE_CASE_VALUE_ENDING_WITH_NUMBER_5 + VALIDUPPERCASEVALUE + + # Invalid values + 1_INVALID_SCREAMING_SNAKE_CASE_VALUE_STARTING_WITH_NUMBER + invalidCamelCaseValue + InvalidCamelCaseCapsValue + invalidlowercasevalue +} + +# Ignored although it triggers a T_CLASS token +type Bar { + some_field: String +} \ No newline at end of file diff --git a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php new file mode 100644 index 00000000..9d577f68 --- /dev/null +++ b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php @@ -0,0 +1,34 @@ + 1, + 11 => 1, + 12 => 1, + 13 => 1, + ]; + } + + /** + * @inheritDoc + */ + protected function getWarningList() + { + return []; + } +} From 19cdbecfce7889dc09d4941f85649bcd42b352b8 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Thu, 12 Sep 2019 11:41:02 +0200 Subject: [PATCH 12/25] MC-19366: Activates sniff for enum values --- Magento2/ruleset.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 9c524658..8d404b05 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -539,6 +539,10 @@ 6 warning + + 6 + warning + From 01eee92d2c72a2433e298a17682d5aa739e7ba50 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Thu, 12 Sep 2019 15:14:22 +0200 Subject: [PATCH 13/25] MC-19366: Fixes file doc --- Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php | 12 ++---------- Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php | 1 - 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index b9e95fae..1012f372 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -1,16 +1,8 @@ - TechDivision GmbH - * All rights reserved - * - * This product includes proprietary software developed at TechDivision GmbH, Germany - * For more information see https://www.techdivision.com/ - * - * To obtain a valid license for using this software please contact us at license@techdivision.com + * Copyright © Magento. All rights reserved. + * See COPYING.txt for license details. */ - namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Sniffs\Sniff; diff --git a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php index 9d577f68..aaa3e55c 100644 --- a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php @@ -3,7 +3,6 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento2\Tests\GraphQL; /** From d99c92283aa303d1c4fbb7a6af6853cb4bffc34e Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 13 Sep 2019 09:22:25 +0200 Subject: [PATCH 14/25] MC-19366: Fixes that tokenizer for GraphQL files could not be loaded due to PHP_CodeSniffer uppercasing tokenizer names --- Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php | 2 +- Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php | 2 +- PHP_CodeSniffer/Tokenizers/{GraphQL.php => GRAPHQL.php} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename PHP_CodeSniffer/Tokenizers/{GraphQL.php => GRAPHQL.php} (99%) diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index 1012f372..6309df08 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -17,7 +17,7 @@ abstract class AbstractGraphQLSniff implements Sniff * * @var array */ - public $supportedTokenizers = ['GraphQL']; + public $supportedTokenizers = ['GRAPHQL']; /** * Returns whether $name starts with a lower case character and is written in camel case. diff --git a/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php index f767a25f..19bcd586 100644 --- a/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php +++ b/Magento2/Tests/GraphQL/AbstractGraphQLSniffUnitTestCase.php @@ -23,7 +23,7 @@ protected function setUp() $config->extensions = array_merge( $config->extensions, [ - 'graphqls' => 'GraphQL' + 'graphqls' => 'GRAPHQL' ] ); diff --git a/PHP_CodeSniffer/Tokenizers/GraphQL.php b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php similarity index 99% rename from PHP_CodeSniffer/Tokenizers/GraphQL.php rename to PHP_CodeSniffer/Tokenizers/GRAPHQL.php index 3d2c30b1..17c30581 100644 --- a/PHP_CodeSniffer/Tokenizers/GraphQL.php +++ b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php @@ -13,7 +13,7 @@ /** * Implements a tokenizer for GraphQL files. */ -class GraphQL extends Tokenizer +class GRAPHQL extends Tokenizer { /** From 49f0b0b09924067373cfc86b2e8a210f546393c6 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 13 Sep 2019 13:38:10 +0200 Subject: [PATCH 15/25] MC-19366: Fixes that agruments with directives would result in erroneous output and that arguments could be treated as fields --- .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 163 +++++++++++++----- .../ValidArgumentNameUnitTest.graphqls | 19 +- .../GraphQL/ValidArgumentNameUnitTest.php | 2 + PHP_CodeSniffer/Tokenizers/GRAPHQL.php | 39 ++--- 4 files changed, 156 insertions(+), 67 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 03de433f..39cf76df 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -6,6 +6,8 @@ namespace Magento2\Sniffs\GraphQL; +use GraphQL\Error\SyntaxError; +use GraphQL\Language\AST\DocumentNode; use PHP_CodeSniffer\Files\File; /** @@ -13,12 +15,13 @@ */ class ValidArgumentNameSniff extends AbstractGraphQLSniff { + /** * @inheritDoc */ public function register() { - return [T_OPEN_PARENTHESIS]; + return [T_VARIABLE]; } /** @@ -26,11 +29,17 @@ public function register() */ public function process(File $phpcsFile, $stackPtr) { - $tokens = $phpcsFile->getTokens(); - $closeParenthesisPointer = $this->getCloseParenthesisPointer($stackPtr, $tokens); + $tokens = $phpcsFile->getTokens(); + + //get the pointer to the argument list opener or bail out if none was found since then the field does not have arguments + $openArgumentListPointer = $this->getArgumentListOpenPointer($stackPtr, $tokens); + if ($openArgumentListPointer === false) { + return; + } - //if we could not find the closing parenthesis pointer, we add a warning and terminate - if ($closeParenthesisPointer === false) { + //get the pointer to the argument list closer or add a warning and terminate as we have an unbalanced file + $closeArgumentListPointer = $this->getArgumentListClosePointer($openArgumentListPointer, $tokens); + if ($closeArgumentListPointer === false) { $error = 'Possible parse error: Missing closing parenthesis for argument list in line %d'; $data = [ $tokens[$stackPtr]['line'], @@ -39,18 +48,15 @@ public function process(File $phpcsFile, $stackPtr) return; } - $arguments = $this->getArguments($stackPtr, $closeParenthesisPointer, $tokens); + $arguments = $this->getArguments($openArgumentListPointer, $closeArgumentListPointer, $tokens); - foreach ($arguments as $argument) { - $pointer = $argument[0]; - $name = $argument[1]; - - if (!$this->isCamelCase($name)) { + foreach ($arguments as $pointer => $argument) { + if (!$this->isCamelCase($argument)) { $type = 'Argument'; $error = '%s name "%s" is not in CamelCase format'; $data = [ $type, - $name, + $argument, ]; $phpcsFile->addError($error, $pointer, 'NotCamelCase', $data); @@ -61,56 +67,131 @@ public function process(File $phpcsFile, $stackPtr) } //return stack pointer of closing parenthesis - return $closeParenthesisPointer; + return $closeArgumentListPointer; } /** - * Finds all argument names contained in $tokens in range $startPointer to - * $endPointer. + * Seeks the last token of an argument definition and returns its pointer. * - * @param int $startPointer - * @param int $endPointer + * Arguments are defined as follows: + * + * {ArgumentName}: {ArgumentType}[ = {DefaultValue}][{Directive}]* + * + * + * @param int $argumentDefinitionStartPointer * @param array $tokens - * @return array[] + * @return int */ - private function getArguments($startPointer, $endPointer, array $tokens) + private function getArgumentDefinitionEndPointer($argumentDefinitionStartPointer, array $tokens) { - $argumentTokenPointer = null; - $argument = ''; - $names = []; - $skipTypes = [T_COMMENT, T_WHITESPACE]; + $colonPointer = $this->seekToken(T_COLON, $tokens, $argumentDefinitionStartPointer); - for ($i = $startPointer + 1; $i < $endPointer; ++$i) { - //skip some tokens - if (in_array($tokens[$i]['code'], $skipTypes)) { - continue; - } - $argument .= $tokens[$i]['content']; + //the colon is always followed by a type so we can consume the token after the colon + $endPointer = $colonPointer + 1; - if ($argumentTokenPointer === null) { - $argumentTokenPointer = $i; - } + //if argument has a default value, we advance to the default definition end + if ($tokens[$endPointer + 1]['code'] === T_EQUAL) { + $endPointer += 2; + } + + //while next token starts a directive, we advance to the end of the directive + while ($tokens[$endPointer + 1]['code'] === T_DOC_COMMENT_TAG) { + //consume next two tokens + $endPointer += 2; - if (preg_match('/^.+:.+$/', $argument)) { - list($name, $type) = explode(':', $argument); - $names[] = [$argumentTokenPointer, $name]; - $argument = ''; - $argumentTokenPointer = null; + //if next token is an opening parenthesis, we consume everything up to the closing parenthesis + if ($tokens[$endPointer + 1]['code'] === T_OPEN_PARENTHESIS) { + $endPointer = $tokens[$endPointer + 1]['parenthesis_closer']; } } - return $names; + return $endPointer; + } + + /** + * Returns the closing parenthesis for the token found at $openParenthesisPointer in $tokens. + * + * @param int $openParenthesisPointer + * @param array $tokens + * @return bool|int + */ + private function getArgumentListClosePointer($openParenthesisPointer, array $tokens) + { + $openParenthesisToken = $tokens[$openParenthesisPointer]; + return $openParenthesisToken['parenthesis_closer']; } /** - * Seeks the next available token of type {@link T_CLOSE_PARENTHESIS} in $tokens and returns its pointer. + * Seeks the next available {@link T_OPEN_PARENTHESIS} token that comes directly after $stackPointer. + * token. * * @param int $stackPointer * @param array $tokens * @return bool|int */ - private function getCloseParenthesisPointer($stackPointer, array $tokens) + private function getArgumentListOpenPointer($stackPointer, array $tokens) + { + //get next open parenthesis pointer or bail out if none was found + $openParenthesisPointer = $this->seekToken(T_OPEN_PARENTHESIS, $tokens, $stackPointer); + if ($openParenthesisPointer === false) { + return false; + } + + //bail out if open parenthesis does not directly come after current stack pointer + if ($openParenthesisPointer !== $stackPointer + 1) { + return false; + } + + //we have found the appropriate opening parenthesis + return $openParenthesisPointer; + } + + /** + * Finds all argument names contained in $tokens in range $startPointer to + * $endPointer. + * + * The returned array uses token pointers as keys and argument names as values. + * + * @param int $startPointer + * @param int $endPointer + * @param array $tokens + * @return array + */ + private function getArguments($startPointer, $endPointer, array $tokens) { - return $this->seekToken(T_CLOSE_PARENTHESIS, $tokens, $stackPointer); + $argumentTokenPointer = null; + $argument = ''; + $names = []; + $skipTypes = [T_COMMENT, T_WHITESPACE]; + + for ($i = $startPointer + 1; $i < $endPointer; ++$i) { + $tokenCode = $tokens[$i]['code']; + + switch (true) { + case in_array($tokenCode, $skipTypes): + //NOP This is a toke that we have to skip + break; + case $tokenCode === T_COLON: + //we have reached the end of the argument name, thus we store its pointer and value + $names[$argumentTokenPointer] = $argument; + + //advance to end of argument definition + $i = $this->getArgumentDefinitionEndPointer($argumentTokenPointer, $tokens); + + //and reset temporary variables + $argument = ''; + $argumentTokenPointer = null; + break; + default: + //this seems to be part of the argument name + $argument .= $tokens[$i]['content']; + + if ($argumentTokenPointer === null) { + $argumentTokenPointer = $i; + } + } + } + + return $names; } } diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls index 83b4f23c..5fa3c277 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls @@ -29,4 +29,21 @@ interface Foo { invalid_argument_name_snake_case: Int 5invalidArgumentNameStatingWithNumber: Int ): String -} \ No newline at end of file +} + +# +# Make sure that directives on arguments do not lead to false positives +# +type Foo @doc(descripton: "Foo Bar Baz") { + myfield ( + # Valid arguments + validArgument: String = "My fair lady" @doc( + # 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.") + # 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") + ): String @doc(description: "Foo Bar") +} diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php index 091a017d..8ed55cfa 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php @@ -24,6 +24,8 @@ protected function getErrorList() 28 => 1, 29 => 1, 30 => 1, + 46 => 1, + 47 => 1, ]; } diff --git a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php index 17c30581..01345fc3 100644 --- a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php +++ b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php @@ -7,6 +7,7 @@ namespace PHP_CodeSniffer\Tokenizers; use GraphQL\Language\Lexer; +use GraphQL\Language\Parser; use GraphQL\Language\Source; use GraphQL\Language\Token; @@ -81,11 +82,10 @@ protected function tokenize($string) { $this->logVerbose('*** START GRAPHQL TOKENIZING ***'); - $string = str_replace($this->eolChar, "\n", $string); - $tokens = []; - $lexer = new Lexer( - new Source($string) - ); + $string = str_replace($this->eolChar, "\n", $string); + $tokens = []; + $source = new Source($string); + $lexer = new Lexer($source); do { $kind = $lexer->token->kind; @@ -178,19 +178,17 @@ private function getNewLineTokens($lineStart, $lineEnd) */ private function isFieldToken($stackPointer) { + //bail out if current token is nested in a parenthesis, since fields cannot be contained in parenthesises + if (isset($this->tokens[$stackPointer]['nested_parenthesis'])) { + return false; + } + $nextToken = $this->tokens[$stackPointer + 1]; - //if next token is an opening parenthesis, we seek for the closing parenthesis + //if next token is an opening parenthesis, we advance to the token after the closing parenthesis if ($nextToken['code'] === T_OPEN_PARENTHESIS) { - $nextPointer = $stackPointer + 1; - $numTokens = count($this->tokens); - - for ($i = $nextPointer; $i < $numTokens; ++$i) { - if ($this->tokens[$i]['code'] === T_CLOSE_PARENTHESIS) { - $nextToken = $this->tokens[$i + 1]; - break; - } - } + $nextPointer = $nextToken['parenthesis_closer'] + 1; + $nextToken = $this->tokens[$nextPointer]; } //return whether current token is a string and next token is a colon @@ -215,7 +213,6 @@ private function logVerbose($message, $level = 1) */ private function processFields() { - $processingArgumentList = false; $processingEntity = false; $numTokens = count($this->tokens); $entityTypes = [T_CLASS, T_INTERFACE]; @@ -237,15 +234,7 @@ private function processFields() //we have hit the end of an entity declaration $processingEntity = false; break; - case $tokenCode === T_OPEN_PARENTHESIS: - //we have hit an argument list - $processingArgumentList = true; - break; - case $tokenCode === T_CLOSE_PARENTHESIS: - //we have hit an argument list end - $processingArgumentList = false; - break; - case $processingEntity && !$processingArgumentList && $this->isFieldToken($i): + case $processingEntity && $this->isFieldToken($i): //we have hit a field $this->tokens[$i]['code'] = T_VARIABLE; $this->tokens[$i]['type'] = 'T_VARIABLE'; From 0b8eeaad4c41b40a5997ab7e0bf8b77b5248d8d4 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 13 Sep 2019 13:50:41 +0200 Subject: [PATCH 16/25] MC-19366: Fixes that field name validation was executed although it should not --- Magento2/ruleset.xml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 8d404b05..60186f30 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -526,11 +526,15 @@ 6 warning - - - - - + + + + 6 + warning + 6 warning From 571b642071ff71fe5f49f0660939b2b86c20a20d Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 13 Sep 2019 14:06:47 +0200 Subject: [PATCH 17/25] MC-19366: Fixes that field and argument names were marked as entities in case their name was a keyword --- PHP_CodeSniffer/Tokenizers/GRAPHQL.php | 53 +++++++++++++++++++++----- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php index 01345fc3..b9056414 100644 --- a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php +++ b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php @@ -7,7 +7,6 @@ namespace PHP_CodeSniffer\Tokenizers; use GraphQL\Language\Lexer; -use GraphQL\Language\Parser; use GraphQL\Language\Source; use GraphQL\Language\Token; @@ -70,6 +69,7 @@ public function processAdditional() { $this->logVerbose('*** START ADDITIONAL GRAPHQL PROCESSING ***'); + $this->fixErroneousKeywordTokens(); $this->processFields(); $this->logVerbose('*** END ADDITIONAL GRAPHQL PROCESSING ***'); @@ -82,10 +82,10 @@ protected function tokenize($string) { $this->logVerbose('*** START GRAPHQL TOKENIZING ***'); - $string = str_replace($this->eolChar, "\n", $string); - $tokens = []; - $source = new Source($string); - $lexer = new Lexer($source); + $string = str_replace($this->eolChar, "\n", $string); + $tokens = []; + $source = new Source($string); + $lexer = new Lexer($source); do { $kind = $lexer->token->kind; @@ -141,6 +141,41 @@ protected function tokenize($string) return $tokens; } + /** + * Fixes that keywords may be used as field, argument etc. names and could thus have been marked as special tokens + * while tokenizing. + */ + private function fixErroneousKeywordTokens() + { + $processingCodeBlock = false; + $numTokens = count($this->tokens); + + for ($i = 0; $i < $numTokens; ++$i) { + $tokenCode = $this->tokens[$i]['code']; + $tokenContent = $this->tokens[$i]['content']; + + switch (true) { + case $tokenCode === T_OPEN_CURLY_BRACKET: + //we have hit the beginning of a code block + $processingCodeBlock = true; + break; + case $tokenCode === T_CLOSE_CURLY_BRACKET: + //we have hit the end of a code block + $processingCodeBlock = false; + break; + case $processingCodeBlock + && $tokenCode !== T_STRING + && isset($this->keywordTokenTypeMap[$tokenContent]): + //we have hit a keyword within a code block that is of wrong token type + $this->tokens[$i]['code'] = T_STRING; + $this->tokens[$i]['type'] = 'T_STRING'; + break; + default: + //NOP All operations have already been executed + } + } + } + /** * Returns tokens of empty new lines for the range $lineStart to $lineEnd * @@ -213,10 +248,10 @@ private function logVerbose($message, $level = 1) */ private function processFields() { - $processingEntity = false; - $numTokens = count($this->tokens); - $entityTypes = [T_CLASS, T_INTERFACE]; - $skipTypes = [T_COMMENT, T_WHITESPACE]; + $processingEntity = false; + $numTokens = count($this->tokens); + $entityTypes = [T_CLASS, T_INTERFACE]; + $skipTypes = [T_COMMENT, T_WHITESPACE]; for ($i = 0; $i < $numTokens; ++$i) { $tokenCode = $this->tokens[$i]['code']; From c57400364834333181731376a51bfe9951ee73cc Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Fri, 13 Sep 2019 14:19:44 +0200 Subject: [PATCH 18/25] MC-19366: Fixes code style issue --- Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 39cf76df..337bf9ff 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -31,7 +31,8 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - //get the pointer to the argument list opener or bail out if none was found since then the field does not have arguments + //get the pointer to the argument list opener or bail out if none was found + //since then the field does not have arguments $openArgumentListPointer = $this->getArgumentListOpenPointer($stackPtr, $tokens); if ($openArgumentListPointer === false) { return; From 43ef0a4b8d14197f8ec34cc97161a45a6d71b45a Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Mon, 16 Sep 2019 09:07:19 +0200 Subject: [PATCH 19/25] MC-19366: Removes obsolete TODOs --- PHP_CodeSniffer/Tokenizers/GRAPHQL.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php index b9056414..9de922df 100644 --- a/PHP_CodeSniffer/Tokenizers/GRAPHQL.php +++ b/PHP_CodeSniffer/Tokenizers/GRAPHQL.php @@ -19,11 +19,15 @@ class GRAPHQL extends Tokenizer /** * Defines how GraphQL token types are mapped to PHP token types. * + * This is a complete list of all token types supported by webonyx/graphql-php. null values + * are automatically mapped to T_STRING but are noted as null in this list to improve + * maintenance at a glance. + * * @var array */ private $tokenTypeMap = [ Token::AT => 'T_DOC_COMMENT_TAG', - Token::BANG => null, //TODO Should we map this to a specific type + Token::BANG => null, Token::BLOCK_STRING => 'T_COMMENT', Token::BRACE_L => 'T_OPEN_CURLY_BRACKET', Token::BRACE_R => 'T_CLOSE_CURLY_BRACKET', @@ -34,12 +38,12 @@ class GRAPHQL extends Tokenizer Token::DOLLAR => 'T_DOLLAR', Token::EOF => 'T_CLOSE_TAG', Token::EQUALS => 'T_EQUAL', - Token::FLOAT => null, //TODO Should we map this to a specific type - Token::INT => null, //TODO Should we map this to a specific type + Token::FLOAT => null, + Token::INT => null, Token::NAME => 'T_STRING', Token::PAREN_L => 'T_OPEN_PARENTHESIS', Token::PAREN_R => 'T_CLOSE_PARENTHESIS', - Token::PIPE => null, //TODO Should we map this to a specific type + Token::PIPE => null, Token::SPREAD => 'T_ELLIPSIS', Token::SOF => 'T_OPEN_TAG', Token::STRING => 'T_STRING', @@ -52,14 +56,13 @@ class GRAPHQL extends Tokenizer */ private $keywordTokenTypeMap = [ 'enum' => 'T_CLASS', - 'extend' => 'T_EXTENDS', //TODO This might not be the appropriate equivalent + 'extend' => 'T_EXTENDS', 'interface' => 'T_INTERFACE', 'implements' => 'T_IMPLEMENTS', 'type' => 'T_CLASS', 'union' => 'T_CLASS', 'query' => 'T_FUNCTION', 'mutation' => 'T_FUNCTION', - //TODO We may have to add further types ]; /** From c539ed44c3b93284d9bc24e5e58c0271cbaa1d40 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 17 Sep 2019 09:37:32 +0200 Subject: [PATCH 20/25] MC-19366: Fixes order of rules and silences GraphQL field name validation --- Magento2/ruleset.xml | 49 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 60186f30..9d65e166 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -364,6 +364,30 @@ 6 warning + + 6 + warning + + + 6 + warning + + + + 0 + warning + + + 6 + warning + + + 6 + warning + 6 warning @@ -522,31 +546,6 @@ 0 - - 6 - warning - - - - - 6 - warning - - - 6 - warning - - - 6 - warning - - - 6 - warning - From af9c04119912bb96ccd5e067c97772d7169f53b1 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 17 Sep 2019 10:28:30 +0200 Subject: [PATCH 21/25] 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: From c4069e0e44e3b16fc75c01df7c131d599ff8f67f Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 17 Sep 2019 11:54:43 +0200 Subject: [PATCH 22/25] MC-19366: Fixes that enum values with directives would be parsed as expected and would thus lead to false positives --- .../Sniffs/GraphQL/AbstractGraphQLSniff.php | 24 +++++++++++ .../Sniffs/GraphQL/ValidArgumentNameSniff.php | 10 +---- .../Sniffs/GraphQL/ValidEnumValueSniff.php | 40 ++++++++++++------- .../ValidArgumentNameUnitTest.graphqls | 2 +- .../GraphQL/ValidEnumValueUnitTest.graphqls | 8 ++++ .../Tests/GraphQL/ValidEnumValueUnitTest.php | 10 +++-- 6 files changed, 67 insertions(+), 27 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index 6309df08..d15ff43a 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -43,6 +43,30 @@ protected function isSnakeCase($name, $upperCase = false) return preg_match($pattern, $name); } + /** + * Returns the pointer to the last token of a directive if the token at $startPointer starts a directive. + * + * @param array $tokens + * @param int $startPointer + * @return int The end of the directive if one is found, the start pointer otherwise + */ + protected function seekEndOfDirective(array $tokens, $startPointer) + { + $endPointer = $startPointer; + + if ($tokens[$startPointer]['code'] === T_DOC_COMMENT_TAG) { + //advance to next token + $endPointer += 1; + + //if next token is an opening parenthesis, we consume everything up to the closing parenthesis + if ($tokens[$endPointer + 1]['code'] === T_OPEN_PARENTHESIS) { + $endPointer = $tokens[$endPointer + 1]['parenthesis_closer']; + } + } + + return $endPointer; + } + /** * Searches for the first token that has $tokenCode in $tokens from position * $startPointer (excluded). diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 7741b675..549130e4 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -108,13 +108,7 @@ private function getArgumentDefinitionEndPointer($argumentDefinitionStartPointer //while next token starts a directive, we advance to the end of the directive while ($tokens[$endPointer + 1]['code'] === T_DOC_COMMENT_TAG) { - //consume next two tokens - $endPointer += 2; - - //if next token is an opening parenthesis, we consume everything up to the closing parenthesis - if ($tokens[$endPointer + 1]['code'] === T_OPEN_PARENTHESIS) { - $endPointer = $tokens[$endPointer + 1]['parenthesis_closer']; - } + $endPointer = $this->seekEndOfDirective($tokens, $endPointer + 1); } return $endPointer; @@ -181,7 +175,7 @@ private function getArguments($startPointer, $endPointer, array $tokens) switch (true) { case in_array($tokenCode, $skipTypes): - //NOP This is a toke that we have to skip + //NOP This is a token that we have to skip break; case $tokenCode === T_COLON: //we have reached the end of the argument name, thus we store its pointer and value diff --git a/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php b/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php index b7c702c7..a9c5f245 100644 --- a/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidEnumValueSniff.php @@ -3,6 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento2\Sniffs\GraphQL; use PHP_CodeSniffer\Files\File; @@ -46,16 +47,14 @@ public function process(File $phpcsFile, $stackPtr) $values = $this->getValues($openingCurlyPointer, $closingCurlyPointer, $tokens, $phpcsFile->eolChar); - foreach ($values as $value) { - $pointer = $value[0]; - $name = $value[1]; + foreach ($values as $pointer => $value) { - if (!$this->isSnakeCase($name, true)) { + if (!$this->isSnakeCase($value, true)) { $type = 'Enum value'; $error = '%s "%s" is not in SCREAMING_SNAKE_CASE format'; $data = [ $type, - $name, + $value, ]; $phpcsFile->addError($error, $pointer, 'NotScreamingSnakeCase', $data); @@ -98,11 +97,13 @@ private function getOpeningCurlyBracketPointer($startPointer, array $tokens) * Finds all enum values contained in $tokens in range $startPointer to * $endPointer. * + * The returned array uses token pointers as keys and value names as values. + * * @param int $startPointer * @param int $endPointer * @param array $tokens * @param string $eolChar - * @return array[] + * @return array */ private function getValues($startPointer, $endPointer, array $tokens, $eolChar) { @@ -112,20 +113,31 @@ private function getValues($startPointer, $endPointer, array $tokens, $eolChar) $skipTypes = [T_COMMENT, T_WHITESPACE]; for ($i = $startPointer + 1; $i < $endPointer; ++$i) { - //skip some tokens if (in_array($tokens[$i]['code'], $skipTypes)) { + //NOP This is a token that we have to skip continue; } - $enumValue .= $tokens[$i]['content']; - if ($valueTokenPointer === null) { - $valueTokenPointer = $i; + //add current tokens content to enum value if we have a string + if ($tokens[$i]['code'] === T_STRING) { + $enumValue .= $tokens[$i]['content']; + + //and store the pointer if we have not done it already + if ($valueTokenPointer === null) { + $valueTokenPointer = $i; + } + } + + //consume directive if we have found one + if ($tokens[$i]['code'] === T_DOC_COMMENT_TAG) { + $i = $this->seekEndOfDirective($tokens, $i); } - if (strpos($enumValue, $eolChar) !== false) { - $values[] = [$valueTokenPointer, rtrim($enumValue, $eolChar)]; - $enumValue = ''; - $valueTokenPointer = null; + //if current token has a line break, we have found the end of the value definition + if (strpos($tokens[$i]['content'], $eolChar) !== false) { + $values[$valueTokenPointer] = trim($enumValue); + $enumValue = ''; + $valueTokenPointer = null; } } diff --git a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls index 8c1e12c0..e2128c9b 100644 --- a/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls +++ b/Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls @@ -40,7 +40,7 @@ type Foo @doc(descripton: "Foo Bar Baz") { validArgument: String = "My fair lady" @doc( # A comment description: "This is a valid argument, spanned over multiple lines." - ) @foo + ) @unparametrizedDirective 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.") diff --git a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls index 994e75f2..5f70754a 100644 --- a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls +++ b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.graphqls @@ -5,12 +5,20 @@ enum Foo { VALID_SCREAMING_SNAKE_CASE_VALUE_WITH_12345_NUMBERS VALID_SCREAMING_SNAKE_CASE_VALUE_ENDING_WITH_NUMBER_5 VALIDUPPERCASEVALUE + VALID_SCREMING_CASE_VALUE_WITH_DIRECTIVE @doc(description: "This is a valid enum value with a directive") + VALID_SCREMING_CASE_VALUE_WITH_TWO_DIRECTIVES @doc( + description: "This is a valid enum value with a directive" + ) @unparametrizedDirective # Invalid values 1_INVALID_SCREAMING_SNAKE_CASE_VALUE_STARTING_WITH_NUMBER invalidCamelCaseValue InvalidCamelCaseCapsValue invalidlowercasevalue + invalidCamelCaseValueWithDirective @doc(description: "This is an invalid enum value with a directive") + invalidCamelCaseValueWithTwoDirectives @doc( + description: "This is an invalid enum value with a directive" + ) @unparametrizedDirective } # Ignored although it triggers a T_CLASS token diff --git a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php index aaa3e55c..57e6ffed 100644 --- a/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php +++ b/Magento2/Tests/GraphQL/ValidEnumValueUnitTest.php @@ -16,10 +16,12 @@ class ValidEnumValueUnitTest extends AbstractGraphQLSniffUnitTestCase protected function getErrorList() { return [ - 10 => 1, - 11 => 1, - 12 => 1, - 13 => 1, + 14 => 1, + 15 => 1, + 16 => 1, + 17 => 1, + 18 => 1, + 19 => 1, ]; } From 874c9d78dc304bb02ce1ef47f41bf2ad29add04d Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Tue, 17 Sep 2019 12:54:17 +0200 Subject: [PATCH 23/25] MC-19366: Fixes code style issue --- Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php index d15ff43a..2c0448f0 100644 --- a/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php +++ b/Magento2/Sniffs/GraphQL/AbstractGraphQLSniff.php @@ -56,7 +56,7 @@ protected function seekEndOfDirective(array $tokens, $startPointer) if ($tokens[$startPointer]['code'] === T_DOC_COMMENT_TAG) { //advance to next token - $endPointer += 1; + ++$endPointer; //if next token is an opening parenthesis, we consume everything up to the closing parenthesis if ($tokens[$endPointer + 1]['code'] === T_OPEN_PARENTHESIS) { From 4d99ee4797c2e881bd1e75bee78e5faeef2f2dd2 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Wed, 18 Sep 2019 14:25:06 +0200 Subject: [PATCH 24/25] MC-19366: Fixes warning when composer install was run --- composer.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.lock b/composer.lock index de4d5a34..3bc96315 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "560d5eca9cf59f965d8f3b98d2ee2a0a", + "content-hash": "397eb37a3ebf83c48685aeb1c77f12b7", "packages": [ { "name": "squizlabs/php_codesniffer", From e252b1946a0ac34c7454bb76359d091dddaac570 Mon Sep 17 00:00:00 2001 From: Jean-Bernard Valentaten Date: Thu, 19 Sep 2019 09:45:15 +0200 Subject: [PATCH 25/25] MC-19366: Fixes invalid tag in PHPDoc --- Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php index 549130e4..d7a19621 100644 --- a/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php +++ b/Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php @@ -75,9 +75,9 @@ public function process(File $phpcsFile, $stackPtr) * Seeks the last token of an argument definition and returns its pointer. * * Arguments are defined as follows: - * + * 
      *   {ArgumentName}: {ArgumentType}[ = {DefaultValue}][{Directive}]*
-     * 
+     *
* * @param int $argumentDefinitionStartPointer * @param array $tokens