Skip to content

Commit

Permalink
MC-19366: Fixes that agruments with directives would result in errone…
Browse files Browse the repository at this point in the history 
…ous output and that arguments could be treated as fields
  • Loading branch information
@jean-bernard-valentaten
jean-bernard-valentaten committed Sep 13, 2019
1 parent d99c922 commit 49f0b0b
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 67 deletions.
163 changes: 122 additions & 41 deletions Magento2/Sniffs/GraphQL/ValidArgumentNameSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,40 @@

namespace Magento2\Sniffs\GraphQL;

use GraphQL\Error\SyntaxError;
use GraphQL\Language\AST\DocumentNode;
use PHP_CodeSniffer\Files\File;

/**
* Detects argument names that are not specified in <kbd>cameCase</kbd>.
*/
class ValidArgumentNameSniff extends AbstractGraphQLSniff
{

/**
* @inheritDoc
*/
public function register()
{
return [T_OPEN_PARENTHESIS];
return [T_VARIABLE];
}

/**
* @inheritDoc
*/
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'],
Expand All @@ -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);
Expand All @@ -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 <var>$tokens</var> in range <var>$startPointer</var> to
* <var>$endPointer</var>.
* Seeks the last token of an argument definition and returns its pointer.
*
* @param int $startPointer
* @param int $endPointer
* Arguments are defined as follows:
* <noformat>
* {ArgumentName}: {ArgumentType}[ = {DefaultValue}][{Directive}]*
* </noformat>
*
* @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 <var>$openParenthesisPointer</var> in <var>$tokens</var>.
*
* @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 <var>$tokens</var> and returns its pointer.
* Seeks the next available {@link T_OPEN_PARENTHESIS} token that comes directly after <var>$stackPointer</var>.
* 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 <var>$tokens</var> in range <var>$startPointer</var> to
* <var>$endPointer</var>.
*
* The returned array uses token pointers as keys and argument names as values.
*
* @param int $startPointer
* @param int $endPointer
* @param array $tokens
* @return array<int, string>
*/
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;
}
}
19 changes: 18 additions & 1 deletion Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.graphqls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,21 @@ interface Foo {
invalid_argument_name_snake_case: Int
5invalidArgumentNameStatingWithNumber: Int
): String
}
}

#
# 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")
}
2 changes: 2 additions & 0 deletions Magento2/Tests/GraphQL/ValidArgumentNameUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ protected function getErrorList()
28 => 1,
29 => 1,
30 => 1,
46 => 1,
47 => 1,
];
}

Expand Down
39 changes: 14 additions & 25 deletions PHP_CodeSniffer/Tokenizers/GRAPHQL.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace PHP_CodeSniffer\Tokenizers;

use GraphQL\Language\Lexer;
use GraphQL\Language\Parser;
use GraphQL\Language\Source;
use GraphQL\Language\Token;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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];
Expand All @@ -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';
Expand Down

0 comments on commit 49f0b0b

Please sign in to comment.