diff --git a/.github/instructions/php.instructions.md b/.github/instructions/php.instructions.md index ea1648d..8e82006 100644 --- a/.github/instructions/php.instructions.md +++ b/.github/instructions/php.instructions.md @@ -266,6 +266,16 @@ See Manual:Coding conventions#Indenting and alignment. For anonymous functions, prefer arrow functions when the anonymous function consists only of one line. Arrow functions are more concise and readable than regular anonymous functions and neatly side-steps formatting issues that arise with single-line anonymous functions. +### Type declarations for variables +Avoid using PHPDoc comments to declare types for local variables. Instead, use native type declarations for function parameters and return types, and use static analysis tools (like PHPStan or Psalm) to infer types of local variables. + +Example: + +```php +private static string $nameOfVariable = ''; +``` + + ### Type declarations in function parameters Use native type declarations and return type declarations when applicable. (But see #Don't add type declarations for "big" legacy classes below.) diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..d974a53 --- /dev/null +++ b/.gitignore @@ -0,0 +1,152 @@ +# Byte-compiled / optimized / DLL files +__pycache__/ +*.py[cod] +*$py.class + +# C extensions +*.so + +# Distribution / packaging +.Python +build/ +develop-eggs/ +dist/ +downloads/ +eggs/ +.eggs/ +lib/ +lib64/ +parts/ +sdist/ +var/ +wheels/ +pip-wheel-metadata/ +share/python-wheels/ +*.egg-info/ +.installed.cfg +*.egg +MANIFEST + +# PyInstaller +*.manifest +*.spec + +# Unit test / coverage reports +htmlcov/ +.tox/ +.nox/ +.coverage +.coverage.* +.cache +nosetests.xml +coverage.xml +*.cover +*.py,cover +.hypothesis/ +.pytest_cache/ + +# Virtual environments +venv/ +ENV/ +env/ +.venv/ +.env + +# IDE specific files +.idea/ +.vscode/ +*.swp +*.swo +*~ +.project +.pydevproject +.settings/ +*.sublime-project +*.sublime-workspace + +# OS specific files +.DS_Store +Thumbs.db +*.bak + +# Logs +*.log +logs/ +*.log.* + +# Database files +*.db +*.sqlite +*.sqlite3 + +# Environment variables +.env +.env.local +.env.*.local + +# Temporary files +*.tmp +*.temp +tmp/ +temp/ + +# Jupyter Notebook +.ipynb_checkpoints + +# mypy +.mypy_cache/ +.dmypy.json +dmypy.json + +# Pyre type checker +.pyre/ + +# Project specific +data/ +output/ +*.pkl +*.pickle +*.model +*.h5 +*.weights + +# PHP +*.php~ +*.php.swp +*.php.swo + +# Composer +vendor/ +composer.phar +composer.lock + +# PHP error logs +php_errors.log +php_error.log +error_log +access_log + +# PHP sessions +sess_* + +# PHP uploads +uploads/ +upload/ + +# PHPUnit +.phpunit.result.cache +phpunit.xml +.phpunit.cache/ + +# PHP CS Fixer +.php_cs +.php_cs.cache +.php-cs-fixer.cache + +# PHP CodeSniffer +phpcs.xml +.phpcs-cache + +# PHPStan +phpstan.neon +phpstan-baseline.neon \ No newline at end of file diff --git a/composer.json b/composer.json new file mode 100644 index 0000000..e757cbf --- /dev/null +++ b/composer.json @@ -0,0 +1,19 @@ +{ + "require-dev": { + "phpunit/phpunit": "^9.0" + }, + "autoload": { + "psr-4": { + "MediaWiki\\Extension\\CrawlerProtection\\": "includes/" + } + }, + "autoload-dev": { + "psr-4": { + "MediaWiki\\Extension\\CrawlerProtection\\Tests\\": "tests/phpunit/unit/" + }, + "files": [ + "tests/phpunit/stubs.php", + "tests/phpunit/namespaced-stubs.php" + ] + } +} diff --git a/includes/Hooks.php b/includes/Hooks.php index 436d7e0..15971e2 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -61,10 +61,10 @@ public function onMediaWikiPerformAction( $request, $mediaWiki ) { - $type = $request->getVal( 'type' ); + $type = $request->getVal( 'type' ); $action = $request->getVal( 'action' ); $diffId = (int)$request->getVal( 'diff' ); - $oldId = (int)$request->getVal( 'oldid' ); + $oldId = (int)$request->getVal( 'oldid' ); if ( !$user->isRegistered() @@ -83,7 +83,7 @@ public function onMediaWikiPerformAction( } /** - * Block Special:RecentChangesLinked and Special:WhatLinksHere for anonymous users. + * Block Special:RecentChangesLinked, Special:WhatLinksHere, and Special:MobileDiff for anonymous users. * * @param SpecialPage $special * @param string|null $subPage @@ -97,7 +97,7 @@ public function onSpecialPageBeforeExecute( $special, $subPage ) { } $name = strtolower( $special->getName() ); - if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) { + if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere', 'mobilediff' ], true ) ) { $out = $special->getContext()->getOutput(); $this->denyAccess( $out ); return false; @@ -112,7 +112,7 @@ public function onSpecialPageBeforeExecute( $special, $subPage ) { * @param OutputPage $output * @return void */ - protected function denyAccess( OutputPage $output ): void { + protected function denyAccess( $output ): void { $output->setStatusCode( 403 ); $output->addWikiTextAsInterface( wfMessage( 'crawlerprotection-accessdenied-text' )->plain() ); diff --git a/tests/phpunit/namespaced-stubs.php b/tests/phpunit/namespaced-stubs.php new file mode 100644 index 0000000..4f288a4 --- /dev/null +++ b/tests/phpunit/namespaced-stubs.php @@ -0,0 +1,70 @@ +onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); $this->assertTrue( $result ); } + + /** + * @covers ::onSpecialPageBeforeExecute + * @dataProvider provideBlockedSpecialPages + * @param string $specialPageName + */ + public function testSpecialPageBlocksAnonymous( $specialPageName ) { + $output = $this->createMock( self::$outputPageClassName ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $context = $this->createMockContext( $user, $output ); + + $special = $this->createMock( self::$specialPageClassName ); + $special->method( 'getName' )->willReturn( $specialPageName ); + $special->method( 'getContext' )->willReturn( $context ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->once() )->method( 'denyAccess' )->with( $output ); + + $result = $runner->onSpecialPageBeforeExecute( $special, null ); + $this->assertFalse( $result ); + } + + /** + * @covers ::onSpecialPageBeforeExecute + * @dataProvider provideBlockedSpecialPages + * @param string $specialPageName + */ + public function testSpecialPageAllowsLoggedIn( $specialPageName ) { + $output = $this->createMock( self::$outputPageClassName ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( true ); + + $context = $this->createMockContext( $user, $output ); + + $special = $this->createMock( self::$specialPageClassName ); + $special->method( 'getName' )->willReturn( $specialPageName ); + $special->method( 'getContext' )->willReturn( $context ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->onSpecialPageBeforeExecute( $special, null ); + $this->assertTrue( $result ); + } + + /** + * @covers ::onSpecialPageBeforeExecute + */ + public function testUnblockedSpecialPageAllowsAnonymous() { + $output = $this->createMock( self::$outputPageClassName ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $context = $this->createMockContext( $user, $output ); + + $special = $this->createMock( self::$specialPageClassName ); + $special->method( 'getName' )->willReturn( 'Search' ); + $special->method( 'getContext' )->willReturn( $context ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->onSpecialPageBeforeExecute( $special, null ); + $this->assertTrue( $result ); + } + + /** + * Create a mock context object. + * + * @param object $user Mock user object + * @param object $output Mock output object + * @return object Mock context + */ + private function createMockContext( $user, $output ) { + $context = new class( $user, $output ) { + /** @var object */ + private $user; + /** @var object */ + private $output; + + /** + * @param object $user + * @param object $output + */ + public function __construct( $user, $output ) { + $this->user = $user; + $this->output = $output; + } + + /** + * @return object + */ + public function getUser() { + return $this->user; + } + + /** + * @return object + */ + public function getOutput() { + return $this->output; + } + }; + return $context; + } + + /** + * Data provider for blocked special pages. + * + * @return array + */ + public function provideBlockedSpecialPages() { + return [ + 'RecentChangesLinked' => [ 'RecentChangesLinked' ], + 'WhatLinksHere' => [ 'WhatLinksHere' ], + 'MobileDiff' => [ 'MobileDiff' ], + // Test case sensitivity + 'RecentChangesLinked lowercase' => [ 'recentchangeslinked' ], + 'WhatLinksHere lowercase' => [ 'whatlinkshere' ], + 'MobileDiff lowercase' => [ 'mobilediff' ], + // Test mixed case + 'MobileDiff mixed case' => [ 'MoBiLeDiFf' ], + ]; + } }