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';