diff --git a/.coveralls.yml b/.coveralls.yml new file mode 100644 index 0000000..f6eedd5 --- /dev/null +++ b/.coveralls.yml @@ -0,0 +1,2 @@ +coverage_clover: coverage-clover.xml +json_path: coveralls-upload.json diff --git a/.travis.yml b/.travis.yml index 1fc33a9..80c9081 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,7 @@ language: php php: + - 7.1 - 7.2 install: @@ -8,4 +9,7 @@ install: - travis_retry composer update --no-interaction script: - - composer all + - composer all-ci + +after_success: + - travis_retry php vendor/bin/php-coveralls -v diff --git a/CHANGELOG.md b/CHANGELOG.md index 429113c..14bcb48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Unreleased +- Add `SpecifyArgSeparatorFixer` to make sure arg_separator is always defined when using `http_build_query()` function. - Add PHPUnit fixers: - `PhpUnitMockFixer`: Ensure dedicated helper methods `createMock()` and `createPartialMock()` are used where possible instead of `->getMock()`. - `PhpUnitNoExpectationAnnotationFixer`: Use `setExpectedException()` instead of `@expectedException` annotation. diff --git a/composer.json b/composer.json index 81d7b93..b360502 100644 --- a/composer.json +++ b/composer.json @@ -10,20 +10,53 @@ "homepage": "https://github.com/lmc-eu" } ], + "autoload": { + "psr-4": { + "Lmc\\CodingStandard\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "Lmc\\CodingStandard\\": "tests/" + } + }, "require": { "symplify/easy-coding-standard": "^4.0.2" }, "require-dev": { "j13k/yaml-lint": "^1.1", - "mhujer/yaml-sort-checker": "^1.2" + "mhujer/yaml-sort-checker": "^1.2", + "phpstan/phpstan-phpunit": "^0.9.4", + "phpstan/phpstan-shim": "^0.9.2", + "phpunit/phpunit": "^7.1" }, "scripts": { "all": [ - "@lint" + "@lint", + "@analyze", + "@test" + ], + "all-ci": [ + "@lint", + "@analyze", + "@test-ci" ], "lint": [ "for FILE_NAME in *.yml *.yaml; do vendor/bin/yaml-lint \"$FILE_NAME\"; done", "vendor/bin/yaml-sort-checker" + ], + "analyze": [ + "vendor/bin/ecs check src/ tests/ -vvv --ansi", + "vendor/bin/phpstan.phar analyze ./src ./tests -c phpstan.neon --level 7 --ansi" + ], + "test": [ + "./vendor/bin/phpunit --colors=always" + ], + "test-ci": [ + "./vendor/bin/phpunit --colors=always --coverage-clover coverage-clover.xml" ] + }, + "config": { + "sort-packages": true } } diff --git a/easy-coding-standard.yaml b/easy-coding-standard.yaml index 6c134f6..3f8c677 100644 --- a/easy-coding-standard.yaml +++ b/easy-coding-standard.yaml @@ -2,6 +2,9 @@ imports: - { resource: '%vendor_dir%/symplify/easy-coding-standard/config/psr2.yml' } services: + # Function http_build_query() should always have specified `$arg_separator` parameter + Lmc\CodingStandard\Fixer\SpecifyArgSeparatorFixer: ~ + # Class and Interface names should be unique in a project, they should never be duplicated PHP_CodeSniffer\Standards\Generic\Sniffs\Classes\DuplicateClassNameSniff: ~ # Control Structures must have at least one statement inside of the body (empty catch rules is skipped) diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..2e57360 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,2 @@ +includes: + - vendor/phpstan/phpstan-phpunit/extension.neon diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..6c09f1d --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,23 @@ + + + + ./tests/ + + + + + + ./src + + + + + + + + diff --git a/src/Fixer/SpecifyArgSeparatorFixer.php b/src/Fixer/SpecifyArgSeparatorFixer.php new file mode 100644 index 0000000..6848f22 --- /dev/null +++ b/src/Fixer/SpecifyArgSeparatorFixer.php @@ -0,0 +1,152 @@ + 'bar', 'baz' => 'bat']);"), + ], + 'Function `http_build_query()` uses `arg_separator.output` ini directive as default argument separator, ' + . 'however when its default value "&" is changed, query string assembled by the method will be ' + . 'unexpectedly invalid. This Fixer forces you to not rely on ini settings and rather define ' + . '`$arg_separator` in third argument of the function.', + null, + null, + 'Risky when other than default "&" argument separator should be used in query strings.' + ); + } + + public function isCandidate(Tokens $tokens): bool + { + return $tokens->isTokenKindFound(T_STRING); + } + + public function isRisky(): bool + { + return true; + } + + public function fix(\SplFileInfo $file, Tokens $tokens): void + { + foreach ($tokens as $index => $token) { + if ($token->isGivenKind(T_STRING) && $token->getContent() === 'http_build_query') { + if ($this->isFunctionCall($tokens, $index)) { + continue; + } + + $this->fixFunction($tokens, $index); + } + } + } + + public function getName(): string + { + return self::class; + } + + public function getPriority(): int + { + // Should be run after SingleQuoteFixer (priority 0) and ArraySyntaxFixer (priority 1) + return -1; + } + + public function supports(\SplFileInfo $file): bool + { + return true; + } + + private function fixFunction(Tokens $tokens, int $functionIndex): void + { + $openParenthesisIndex = $tokens->getNextTokenOfKind($functionIndex, ['(']); + if ($openParenthesisIndex === null) { + return; + } + + $closeParenthesisIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParenthesisIndex); + + $argumentCount = (new ArgumentsAnalyzer()) + ->countArguments($tokens, $openParenthesisIndex, $closeParenthesisIndex); + + // When third argument is already present and it is null, override its value. + if ($argumentCount >= 3) { + $thirdArgumentType = $this->getThirdArgumentInfo($tokens, $openParenthesisIndex, $closeParenthesisIndex); + if ($thirdArgumentType === null) { + return; + } + + if ($thirdArgumentType->getName() === 'null') { + $this->setArgumentValueToAmp($tokens, $thirdArgumentType->getStartIndex()); + } + + return; + } + + $tokensToInsert = []; + + // Add second argument if not defined + if ($argumentCount === 1) { + $tokensToInsert[] = new Token(','); + $tokensToInsert[] = new Token([T_WHITESPACE, ' ']); + $tokensToInsert[] = new Token([T_STRING, 'null']); + } + + // Add third argument (arg separator): ", &" + if ($argumentCount < 3) { + $tokensToInsert[] = new Token(','); + $tokensToInsert[] = new Token([T_WHITESPACE, ' ']); + $tokensToInsert[] = new Token([T_STRING, "'&'"]); + } + + if (!empty($tokensToInsert)) { + $beforeCloseParenthesisIndex = $tokens->getPrevNonWhitespace($closeParenthesisIndex); + $tokens->insertAt($beforeCloseParenthesisIndex + 1, $tokensToInsert); + } + } + + /** + * Detect if this is most probably function call (and not function import or function definition). + */ + private function isFunctionCall(Tokens $tokens, int $index): bool + { + $previousIndex = $tokens->getPrevMeaningfulToken($index); + + return $previousIndex !== null + && ($tokens[$previousIndex]->isGivenKind(CT::T_FUNCTION_IMPORT) + || $tokens[$previousIndex]->isGivenKind(T_FUNCTION)); + } + + private function getThirdArgumentInfo( + Tokens $tokens, + int $openParenthesisIndex, + int $closeParenthesisIndex + ): ?TypeAnalysis { + $argumentsAnalyzer = new ArgumentsAnalyzer(); + + $arguments = $argumentsAnalyzer->getArguments($tokens, $openParenthesisIndex, $closeParenthesisIndex); + $argumentIndex = array_slice($arguments, 2, 1, true); + $argumentInfo = $argumentsAnalyzer->getArgumentInfo($tokens, key($argumentIndex), reset($argumentIndex)); + + return $argumentInfo->getTypeAnalysis(); + } + + private function setArgumentValueToAmp(Tokens $tokens, int $argumentStartIndex): void + { + $ampToken = new Token([T_STRING, "'&'"]); + $tokens->offsetSet($argumentStartIndex, $ampToken); + } +} diff --git a/tests/Fixer/Fixtures/Correct.txt b/tests/Fixer/Fixtures/Correct.txt new file mode 100644 index 0000000..2051958 --- /dev/null +++ b/tests/Fixer/Fixtures/Correct.txt @@ -0,0 +1,21 @@ + 'bar'], null, '&'); + + $queryString1WithComment = http_build_query(['foo' => 'bar'], /* Comment, with commas, */ null , '&'); + + $queryString1WithObject = http_build_query((object) ['foo' => 'bar'], null, '&'); + + $queryString2 = http_build_query(['foo' => 'bar', 'baz' => 'bat'], null, '&'); + + $queryString3 = http_build_query(['foo' => 'bar'], null, '&'); + + $queryString4 = http_build_query(['foo' => 'bar'], null, ';'); + } +} diff --git a/tests/Fixer/Fixtures/Fixed.txt b/tests/Fixer/Fixtures/Fixed.txt new file mode 100644 index 0000000..8a095a4 --- /dev/null +++ b/tests/Fixer/Fixtures/Fixed.txt @@ -0,0 +1,23 @@ + 'bar'], null, '&'); + + $queryString2 = http_build_query(['foo' => 'bar'], 333, '&'); + + $queryString3 = http_build_query(['foo' => 'bar'], 333, '&'); + + $queryString4 = http_build_query(['foo' => 'bar'], 333, '&', PHP_QUERY_RFC3986); + + $queryString5 = http_build_query(['foo' => 'bar'], null, '&', PHP_QUERY_RFC3986); + + $queryString6 = http_build_query(['foo' => 'bar', 'baz' => 'ban'], null, '&', PHP_QUERY_RFC3986); + + $queryString6 = http_build_query((object) ['foo' => 'bar', 'baz' => 'ban'], null, '&', PHP_QUERY_RFC3986); +} diff --git a/tests/Fixer/Fixtures/Wrong.txt b/tests/Fixer/Fixtures/Wrong.txt new file mode 100644 index 0000000..40f8f5d --- /dev/null +++ b/tests/Fixer/Fixtures/Wrong.txt @@ -0,0 +1,23 @@ + 'bar']); + + $queryString2 = http_build_query(['foo' => 'bar'], 333); + + $queryString3 = http_build_query(['foo' => 'bar'], 333, null); + + $queryString4 = http_build_query(['foo' => 'bar'], 333, null, PHP_QUERY_RFC3986); + + $queryString5 = http_build_query(['foo' => 'bar'], null, null, PHP_QUERY_RFC3986); + + $queryString6 = http_build_query(['foo' => 'bar', 'baz' => 'ban'], null, null, PHP_QUERY_RFC3986); + + $queryString6 = http_build_query((object) ['foo' => 'bar', 'baz' => 'ban'], null, null, PHP_QUERY_RFC3986); +} diff --git a/tests/Fixer/SpecifyArgSeparatorFixerTest.php b/tests/Fixer/SpecifyArgSeparatorFixerTest.php new file mode 100644 index 0000000..4d2adbe --- /dev/null +++ b/tests/Fixer/SpecifyArgSeparatorFixerTest.php @@ -0,0 +1,38 @@ +getRealPath())); + + $fixer->fix($fileInfo, $tokens); + + $this->assertStringEqualsFile( + __DIR__ . '/Fixtures/' . $expectedOutputFile, + $tokens->generateCode() + ); + } + + /** + * @return array[] + */ + public function provideFiles(): array + { + return [ + 'Correct file should not be changed' => ['Correct.txt', 'Correct.txt'], + 'Wrong file should be fixed' => ['Wrong.txt', 'Fixed.txt'], + ]; + } +}