From fbcc0ed8b8cdc1a0a7c642750533867f0cbe7635 Mon Sep 17 00:00:00 2001 From: Samuel Hilson Date: Thu, 20 Sep 2018 17:22:21 -0500 Subject: [PATCH 1/2] Add license comment sniff Add basic unit testing --- .../Sniffs/Commenting/LicenseCommentSniff.php | 284 ++++++++++++++++++ .../WhiteSpace/NoSpaceAfterNotSniff.php | 15 +- HydraWiki/ruleset.xml | 1 + composer.json | 17 +- phpunit.xml | 7 + tests/BaseTest.php | 18 ++ .../Commenting/LicenseCommentSniffTest.php | 86 ++++++ .../WhiteSpace/NoSpaceAfterNotSniffTest.php | 78 +++++ tests/bootstrap.php | 9 + 9 files changed, 504 insertions(+), 11 deletions(-) create mode 100644 HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php create mode 100644 phpunit.xml create mode 100644 tests/BaseTest.php create mode 100644 tests/Unit/Commenting/LicenseCommentSniffTest.php create mode 100644 tests/Unit/WhiteSpace/NoSpaceAfterNotSniffTest.php create mode 100644 tests/bootstrap.php diff --git a/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php b/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php new file mode 100644 index 0000000..24c24ba --- /dev/null +++ b/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php @@ -0,0 +1,284 @@ + replacement + */ + private $replacements = [ + 'GNU General Public Licen[sc]e 2(\.0)? or later' => 'GPL-2.0-or-later', + 'GNU GPL v2\+' => 'GPL-2.0-or-later', + ]; + + /** + * Returns an array of tokens this test wants to listen for. + * + * @return array + */ + public function register() { + return [ T_DOC_COMMENT_OPEN_TAG ]; + } + + /** + * Initialize + * + * @param File $phpcsFile + * @param int $stackPtr + * @param SpdxLicenses|null $spdx + * @return void + */ + public function initialize(File $phpcsFile, $stackPtr, SpdxLicenses $spdx = null) { + $this->file = $phpcsFile; + $this->tokens = $this->file->getTokens(); + $this->end = $this->tokens[$stackPtr]['comment_closer']; + $this->spdx = $spdx; + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @return void + */ + public function process(File $phpcsFile, $stackPtr) { + $this->initialize($phpcsFile, $stackPtr); + + foreach ($this->tokens[$stackPtr]['comment_tags'] as $tag) { + $this->processDocTag($tag); + } + } + + /** + * handle a single doc line + * + * @param string $tag + * @return mixed + */ + private function processDocTag($tag) { + if ($this->isCorrectTag($tag)) { + return; + } + + // Only allow license on file comments + $this->isValidTagLocation($tag); + + // Validate text behind license + $next = $this->file->findNext([T_DOC_COMMENT_WHITESPACE], $tag + 1, $this->end, true); + + if ($this->hasTextAfterTag($tag, $next)) { + return; + } + + // Get license text + $license = $this->getLicenseFromTag($next); + + // Allow for proprietary licensing to be used + if ($this->isValidProprietaryLicense($license)) { + return; + } + + // Flag deprecated licenses + if ($this->isLicenseDeprecated($tag, $license)) { + return; + } + + // Validate licenses + $this->handleInvalidLicense($tag, $next, $license); + } + + /** + * get or initialize SpdxLicenses library + * + * @return SpdxLicenses + */ + private function getLicenseValidator() { + if ($this->spdx === null) { + $this->spdx = new SpdxLicenses(); + } + return $this->spdx; + } + + /** + * normalize license text + * + * @param string $next + * @return string + */ + private function getLicenseFromTag($next) { + $license = $this->tokens[$next]['content']; + + // license can contain a url, use the text behind it + if (preg_match('/^https?:\/\/[^\s]+\s+(.*)/', $license, $match)) { + $license = $match[1]; + } + return $license; + } + + /** + * check if we are on the correct license tag + * + * @param string $tag + * @return boolean + */ + private function isCorrectTag($tag) { + $tagText = $this->tokens[$tag]['content']; + switch ($tagText) { + case '@licence': + $fix = $this->file->addFixableWarning( + 'Incorrect spelling of @license', + $tag, + 'LicenceTag' + ); + if ($fix) { + $this->file->fixer->replaceToken($tag, '@license'); + } + break; + case '@license': + break; + default: + return true; + } + + return false; + } + + /** + * check the tag location is in file level doc comment + * + * @param string $tag + * @return boolean + */ + private function isValidTagLocation($tag) { + if ($this->tokens[$tag]['level'] !== 0) { + $this->file->addWarning( + '@license should only be used on file comments', + $tag, + 'LicenseTagNonFileComment' + ); + } + } + + /** + * check that tag has some text after @license + * + * @param string $tag + * @param string $next + * @return boolean + */ + private function hasTextAfterTag($tag, $next) { + if ($this->tokens[$next]['code'] !== T_DOC_COMMENT_STRING) { + $this->file->addWarning( + '@license not followed by a license', + $tag, + 'LicenseTagEmpty' + ); + return true; + } + } + + /** + * check for a private license + * + * @param string $license + * @return boolean + */ + private function isValidProprietaryLicense($license) { + if (strtolower($license) == 'proprietary') { + return true; + } + } + + /** + * check if the license is marked as deprecated + * + * @param string $tag + * @param string $license + * @return boolean + */ + private function isLicenseDeprecated($tag, $license) { + // Initialize the spdx license validator + $spdx = $this->getLicenseValidator(); + + $valid = $spdx->validate($license); + // Check for Deprecated licensing + if ($valid && $spdx->isDeprecatedByIdentifier($license)) { + $this->file->addWarning( + 'Deprecated SPDX license identifier "%s", see ', + $tag, + 'DeprecatedLicenseTag', + [$license] + ); + return true; + } + } + + /** + * checks that the license is a valid license from spdx + * + * @param string $tag + * @param string $next + * @param string $license + * @return boolean + */ + private function handleInvalidLicense($tag, $next, $license) { + // Initialize the spdx license validator + $spdx = $this->getLicenseValidator(); + + $valid = $spdx->validate($license); + if ($valid) { + return; + } + + $fixable = null; + foreach ($this->replacements as $regex => $identifier) { + // Make sure the entire license matches the regex, and + // then a sanity check that the new replacement is valid too + if (preg_match("/^$regex$/", $license) === 1 + && $spdx->validate($identifier) + ) { + $fixable = $identifier; + break; + } + } + + // handle fixable license problem + if ($fixable !== null) { + $fix = $this->file->addFixableWarning( + 'Invalid SPDX license identifier "%s", see ', + $tag, + 'InvalidLicenseTag', + [$license] + ); + if ($fix) { + $this->file->fixer->replaceToken($next, $fixable); + } + return; + } + + // report un-fixable problems + $this->file->addWarning( + 'Invalid SPDX license identifier "%s", see ', + $tag, + 'InvalidLicenseTag', + [$license] + ); + } +} diff --git a/HydraWiki/Sniffs/WhiteSpace/NoSpaceAfterNotSniff.php b/HydraWiki/Sniffs/WhiteSpace/NoSpaceAfterNotSniff.php index 673a0f2..9c2ed9d 100644 --- a/HydraWiki/Sniffs/WhiteSpace/NoSpaceAfterNotSniff.php +++ b/HydraWiki/Sniffs/WhiteSpace/NoSpaceAfterNotSniff.php @@ -1,5 +1,14 @@ getTokens(); $spacing = 0; if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) { diff --git a/HydraWiki/ruleset.xml b/HydraWiki/ruleset.xml index 773f256..ab41507 100644 --- a/HydraWiki/ruleset.xml +++ b/HydraWiki/ruleset.xml @@ -4,6 +4,7 @@ + diff --git a/composer.json b/composer.json index e8a96ce..0e9a8b4 100644 --- a/composer.json +++ b/composer.json @@ -8,16 +8,21 @@ ], "homepage": "https://www.gamepedia.com", "license": "MIT", - "require": { - "php": ">= 7.1", - "composer/semver": "1.4.2", - "mediawiki/mediawiki-codesniffer": "21.0.0" - }, "autoload": { "psr-4": { "HydraWiki\\": "HydraWiki" } }, + "autoload-dev": { + "psr-4": { + "Tests\\": "tests/" + } + }, + "require": { + "php": ">= 7.1", + "composer/semver": "1.4.2", + "mediawiki/mediawiki-codesniffer": "21.0.0" + }, "require-dev": { "jakub-onderka/php-parallel-lint": "1.0.0", "jakub-onderka/php-console-highlighter": "0.3.2", @@ -27,7 +32,7 @@ "scripts": { "test": [ "parallel-lint . --exclude vendor", - "phpunit $PHPUNIT_ARGS", + "phpunit", "phpcs -p -s", "minus-x check ." ], diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..0e95973 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,7 @@ + + + + ./tests/Unit + + + diff --git a/tests/BaseTest.php b/tests/BaseTest.php new file mode 100644 index 0000000..5732c86 --- /dev/null +++ b/tests/BaseTest.php @@ -0,0 +1,18 @@ +fileMock = $this->createMock(File::class); + $fixer = $this->createMock(Fixer::class); + $this->fileMock->fixer = $fixer; + } +} diff --git a/tests/Unit/Commenting/LicenseCommentSniffTest.php b/tests/Unit/Commenting/LicenseCommentSniffTest.php new file mode 100644 index 0000000..bb6d18e --- /dev/null +++ b/tests/Unit/Commenting/LicenseCommentSniffTest.php @@ -0,0 +1,86 @@ +sniff = new LicenseCommentSniff(); + + $this->spdxMock = $this->createMock(SpdxLicenses::class); + + $this->sniff->initialize($this->fileMock, 0, $this->spdxMock); + } + + /** + * test sniff when no whitespace is present + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithInvalidLicense() { + $tokens = [ + 0 => [ + 'content' => '/**', + 'code' => 'PHPCS_T_DOC_COMMENT_OPEN_TAG', + 'type' => 'T_DOC_COMMENT_OPEN_TAG', + 'comment_tags' => [6], + 'comment_closer' => 11, + 'line' => 2, + 'column' => 1, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ], + 6 => [ + 'content' => '@license', + 'code' => 'PHPCS_T_DOC_COMMENT_TAG', + 'type' => 'T_DOC_COMMENT_TAG', + 'line' => 3, + 'column' => 4, + 'length' => 8, + 'level' => 0, + 'conditions' => [] + ], + 8 => [ + 'content' => 'MIT2', + 'code' => 'PHPCS_T_DOC_COMMENT_STRING', + 'type' => 'T_DOC_COMMENT_STRING', + 'line' => 3, + 'column' => 14, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ] + ]; + + $this->fileMock->method('getTokens') + ->willReturn($tokens); + + $this->fileMock->method('findNext') + ->willReturn(8); + + $this->fileMock->expects($this->once()) + ->method('addWarning') + ->with( + $this->stringContains('Invalid SPDX license identifier "%s", see '), + $this->equalTo(6), + $this->equalTo('InvalidLicenseTag'), + $this->identicalTo(['MIT2']) + ) + ->willReturn(false); + $this->sniff->process($this->fileMock, 0); + } +} diff --git a/tests/Unit/WhiteSpace/NoSpaceAfterNotSniffTest.php b/tests/Unit/WhiteSpace/NoSpaceAfterNotSniffTest.php new file mode 100644 index 0000000..d77600c --- /dev/null +++ b/tests/Unit/WhiteSpace/NoSpaceAfterNotSniffTest.php @@ -0,0 +1,78 @@ +sniff = new NoSpaceAfterNotSniff(); + } + /** + * test sniff when no whitespace is present + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithNoWhitespace() { + $this->fileMock->method('getTokens') + ->willReturn(['', ['code' => '', 'length' => 0]]); + + $this->fileMock->expects($this->exactly(0)) + ->method('addFixableError'); + + $result = $this->sniff->process($this->fileMock, 0); + + $this->assertNull($result); + } + + /** + * test sniff when whitespace is present + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithOneWhitespace() { + $this->fileMock->method('getTokens') + ->willReturn(['', ['code' => T_WHITESPACE, 'length' => 1]]); + + $this->fileMock->expects($this->once()) + ->method('addFixableError') + ->with( + $this->stringContains('There must not be a space after a NOT operator; %s found'), + $this->equalTo(0), + $this->equalTo('Incorrect'), + $this->identicalTo([1]) + ) + ->willReturn(false); + + $this->sniff->process($this->fileMock, 0); + } + + /** + * test sniff when whitespace is present + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithOneWhitespaceFixer() { + $this->fileMock->method('getTokens') + ->willReturn(['', ['code' => T_WHITESPACE, 'length' => 1]]); + + $this->fileMock->expects($this->once()) + ->method('addFixableError') + ->willReturn(true); + + $this->fileMock->fixer->expects($this->once()) + ->method('replaceToken'); + + $this->sniff->process($this->fileMock, 0); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..cad41f6 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,9 @@ + Date: Mon, 24 Sep 2018 13:59:39 -0500 Subject: [PATCH 2/2] Adding more test --- .../Sniffs/Commenting/LicenseCommentSniff.php | 12 +- .../Commenting/LicenseCommentSniffTest.php | 129 +++++++++++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php b/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php index 24c24ba..7d6c610 100644 --- a/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php +++ b/HydraWiki/Sniffs/Commenting/LicenseCommentSniff.php @@ -1,6 +1,13 @@ file = $phpcsFile; $this->tokens = $this->file->getTokens(); $this->end = $this->tokens[$stackPtr]['comment_closer']; - $this->spdx = $spdx; + if ($spdx !== null) { + $this->spdx = $spdx; + } } /** @@ -243,6 +252,7 @@ private function handleInvalidLicense($tag, $next, $license) { $spdx = $this->getLicenseValidator(); $valid = $spdx->validate($license); + if ($valid) { return; } diff --git a/tests/Unit/Commenting/LicenseCommentSniffTest.php b/tests/Unit/Commenting/LicenseCommentSniffTest.php index bb6d18e..78e36f3 100644 --- a/tests/Unit/Commenting/LicenseCommentSniffTest.php +++ b/tests/Unit/Commenting/LicenseCommentSniffTest.php @@ -26,7 +26,7 @@ protected function setUp() { } /** - * test sniff when no whitespace is present + * Test sniff with invalid license * @covers NoSpaceAfterNotSniff::process * @return void */ @@ -81,6 +81,133 @@ public function testProcessWithInvalidLicense() { $this->identicalTo(['MIT2']) ) ->willReturn(false); + + $this->sniff->process($this->fileMock, 0); + } + + /** + * Test sniff with a valid license + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithValidLicense() { + $tokens = [ + 0 => [ + 'content' => '/**', + 'code' => 'PHPCS_T_DOC_COMMENT_OPEN_TAG', + 'type' => 'T_DOC_COMMENT_OPEN_TAG', + 'comment_tags' => [6], + 'comment_closer' => 11, + 'line' => 2, + 'column' => 1, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ], + 6 => [ + 'content' => '@license', + 'code' => 'PHPCS_T_DOC_COMMENT_TAG', + 'type' => 'T_DOC_COMMENT_TAG', + 'line' => 3, + 'column' => 4, + 'length' => 8, + 'level' => 0, + 'conditions' => [] + ], + 8 => [ + 'content' => 'MIT', + 'code' => 'PHPCS_T_DOC_COMMENT_STRING', + 'type' => 'T_DOC_COMMENT_STRING', + 'line' => 3, + 'column' => 14, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ] + ]; + + $this->fileMock->method('getTokens') + ->willReturn($tokens); + + $this->fileMock->method('findNext') + ->willReturn(8); + + $this->spdxMock->expects($this->exactly(1)) + ->method('isDeprecatedByIdentifier') + ->willReturn(false); + + $this->spdxMock->expects($this->exactly(2)) + ->method('validate') + ->willReturn(true); + + $this->sniff->process($this->fileMock, 0); + } + + /** + * Test sniff with a Deprecated license + * @covers NoSpaceAfterNotSniff::process + * @return void + */ + public function testProcessWithValidDeprecatedLicense() { + $tokens = [ + 0 => [ + 'content' => '/**', + 'code' => 'PHPCS_T_DOC_COMMENT_OPEN_TAG', + 'type' => 'T_DOC_COMMENT_OPEN_TAG', + 'comment_tags' => [6], + 'comment_closer' => 11, + 'line' => 2, + 'column' => 1, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ], + 6 => [ + 'content' => '@license', + 'code' => 'PHPCS_T_DOC_COMMENT_TAG', + 'type' => 'T_DOC_COMMENT_TAG', + 'line' => 3, + 'column' => 4, + 'length' => 8, + 'level' => 0, + 'conditions' => [] + ], + 8 => [ + 'content' => 'GPL-2.0+', + 'code' => 'PHPCS_T_DOC_COMMENT_STRING', + 'type' => 'T_DOC_COMMENT_STRING', + 'line' => 3, + 'column' => 14, + 'length' => 3, + 'level' => 0, + 'conditions' => [] + ] + ]; + + $this->fileMock->method('getTokens') + ->willReturn($tokens); + + $this->fileMock->method('findNext') + ->willReturn(8); + + $this->spdxMock->expects($this->exactly(1)) + ->method('isDeprecatedByIdentifier') + ->willReturn(true); + + $this->spdxMock->expects($this->exactly(1)) + ->method('validate') + ->willReturn(true); + + $this->fileMock->expects($this->once()) + ->method('addWarning') + ->with( + $this->stringContains('Deprecated SPDX license identifier "%s", see '), + $this->equalTo(6), + $this->equalTo('DeprecatedLicenseTag'), + $this->identicalTo(['GPL-2.0+']) + ) + ->willReturn(false); + $this->sniff->process($this->fileMock, 0); } }