From 0a52ca2baebcf32bbe60a86dfa9c5a9108f315c3 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sun, 29 Jun 2025 18:22:25 -0700 Subject: [PATCH 1/8] Special:MobileDiff --- .gitignore | 152 +++++++++++ composer.json | 19 ++ includes/Hooks.php | 4 +- tests/phpunit/namespaced-stubs.php | 55 ++++ tests/phpunit/stubs.php | 17 ++ tests/phpunit/unit/HooksTest.php | 394 ++++++++++++++++++++--------- 6 files changed, 515 insertions(+), 126 deletions(-) create mode 100644 .gitignore create mode 100644 composer.json create mode 100644 tests/phpunit/namespaced-stubs.php create mode 100644 tests/phpunit/stubs.php 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..e60d3af --- /dev/null +++ b/composer.json @@ -0,0 +1,19 @@ +{ + "require-dev": { + "phpunit/phpunit": "^9.0" + }, + "autoload": { + "psr-4": { + "MediaWiki\\Extension\\CrawlerProtection\\": "includes/" + }, + "files": [ + "tests/phpunit/stubs.php", + "tests/phpunit/namespaced-stubs.php" + ] + }, + "autoload-dev": { + "psr-4": { + "MediaWiki\\Extension\\CrawlerProtection\\Tests\\": "tests/phpunit/unit/" + } + } +} diff --git a/includes/Hooks.php b/includes/Hooks.php index 436d7e0..50f7e1e 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -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; diff --git a/tests/phpunit/namespaced-stubs.php b/tests/phpunit/namespaced-stubs.php new file mode 100644 index 0000000..488c45a --- /dev/null +++ b/tests/phpunit/namespaced-stubs.php @@ -0,0 +1,55 @@ +createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'revision' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( false ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->once() )->method( 'denyAccess' ); - - $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); - $this->assertFalse( $result ); - } - - /** - * @covers ::onMediaWikiPerformAction - */ - public function testRevisionTypeAllowsLoggedIn() { - $output = $this->createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'revision' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( true ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->never() )->method( 'denyAccess' ); - - $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); - $this->assertTrue( $result ); - } - - /** - * @covers ::onMediaWikiPerformAction - */ - public function testNonRevisionTypeAlwaysAllowed() { - $output = $this->createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'view' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( false ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->never() )->method( 'denyAccess' ); - - $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); - $this->assertTrue( $result ); - } + /** @var string */ + private static string $actionEntryPointClassName; + + /** @var string */ + private static string $articleClassName; + + /** @var string */ + private static string $outputPageClassName; + + /** @var string */ + private static string $specialPageClassName; + + /** @var string */ + private static string $titleClassName; + + /** @var string */ + private static string $userClassName; + + /** @var string */ + private static string $webRequestClassName; + + public static function setUpBeforeClass(): void { + self::$actionEntryPointClassName = class_exists( '\MediaWiki\Actions\ActionEntryPoint' ) + ? '\MediaWiki\Actions\ActionEntryPoint' + : '\MediaWiki'; + + self::$articleClassName = class_exists( '\MediaWiki\Page\Article' ) + ? '\MediaWiki\Page\Article' + : '\Article'; + + self::$outputPageClassName = class_exists( '\MediaWiki\Output\OutputPage' ) + ? '\MediaWiki\Output\OutputPage' + : '\OutputPage'; + + self::$specialPageClassName = class_exists( '\MediaWiki\SpecialPage\SpecialPage' ) + ? '\MediaWiki\SpecialPage\SpecialPage' + : '\SpecialPage'; + + self::$titleClassName = class_exists( '\MediaWiki\Title\Title' ) + ? '\MediaWiki\Title\Title' + : '\Title'; + + self::$userClassName = class_exists( '\MediaWiki\User\User' ) + ? '\MediaWiki\User\User' + : '\User'; + + self::$webRequestClassName = class_exists( '\MediaWiki\Request\WebRequest' ) + ? '\MediaWiki\Request\WebRequest' + : '\WebRequest'; + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testRevisionTypeBlocksAnonymous() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'revision' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->once() )->method( 'denyAccess' ); + + $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); + $this->assertFalse( $result ); + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testRevisionTypeAllowsLoggedIn() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'revision' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( true ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); + $this->assertTrue( $result ); + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testNonRevisionTypeAlwaysAllowed() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'view' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); + $this->assertTrue( $result ); + } + + /** + * @covers ::onSpecialPageBeforeExecute + * @dataProvider provideBlockedSpecialPages + */ + public function testSpecialPageBlocksAnonymous( $specialPageName ) { + $output = $this->createMock( self::$outputPageClassName ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $context = new class( $user, $output ) { + private $user; + private $output; + + public function __construct( $user, $output ) { + $this->user = $user; + $this->output = $output; + } + + public function getUser() { + return $this->user; + } + + public function getOutput() { + return $this->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 + */ + public function testSpecialPageAllowsLoggedIn( $specialPageName ) { + $output = $this->createMock( self::$outputPageClassName ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( true ); + + $context = new class( $user, $output ) { + private $user; + private $output; + + public function __construct( $user, $output ) { + $this->user = $user; + $this->output = $output; + } + + public function getUser() { + return $this->user; + } + + public function getOutput() { + return $this->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 = new class( $user, $output ) { + private $user; + private $output; + + public function __construct( $user, $output ) { + $this->user = $user; + $this->output = $output; + } + + public function getUser() { + return $this->user; + } + + public function getOutput() { + return $this->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 ); + } + + /** + * Data provider for blocked special pages + */ + 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' ], + ]; + } } From c0a68e1e494ba1ab0665056b63700db34a9a7d35 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sat, 25 Oct 2025 21:04:28 -0700 Subject: [PATCH 2/8] Add coding conventions files for GitHub Copilot (#11) * Add coding conventions * Add applyTo for PHP instructions * Add extension-specific coding standards --- .github/copilot-instructions.md | 0 .../mediawiki-extensions.instructions.md | 118 ++++ .../instructions/mediawiki.instructions.md | 436 ++++++++++++ .github/instructions/php.instructions.md | 629 ++++++++++++++++++ 4 files changed, 1183 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/mediawiki-extensions.instructions.md create mode 100644 .github/instructions/mediawiki.instructions.md create mode 100644 .github/instructions/php.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..e69de29 diff --git a/.github/instructions/mediawiki-extensions.instructions.md b/.github/instructions/mediawiki-extensions.instructions.md new file mode 100644 index 0000000..ffd5cab --- /dev/null +++ b/.github/instructions/mediawiki-extensions.instructions.md @@ -0,0 +1,118 @@ +--- +applyTo: "*" +--- + +# MediaWiki Extension Best Practices + +## Code architecture + +* **MUST:** Use structured logging, with a channel name specific to your extension and with appropriate use of message severity levels. This aids debugging. For Wikimedia Foundation deployment, this also helps your team in monitoring the extension. See also: [Logstash on Wikitech](https://wikitech.wikimedia.org/wiki/OpenSearch_Dashboards). +* **SHOULD:** Provide the same functionality through the API (Action API or REST API) and the graphical interface (`index.php`). + * **OPTIONAL:** The functionality should be implemented without significant code duplication (e.g. shared backend service class with light API and SpecialPage bindings.) +* **SHOULD:** Use Dependency Injection ("DI") and service classes registered in service wiring. +* **SHOULD:** Avoid global state, especially singletons or other hidden state via static class variable (unless for debug logs and other state intrinsically about the current process and not about any particular wiki or caller). Avoid static calls for anything other than stateless utility methods. +* **SHOULD:** Avoid use of older unrefactored global or static functions from unrelated classes when writing new DI-based service classes. +* **SHOULD:** Only throw exceptions for situations that callers should not handle and thus are meant to bubble up. +* **SHOULD:** Don't hardcode wikitext and assumptions about templates, especially in a way that's not configurable for each website. +* **SHOULD:** Code should be readable by someone who is familiar in that area. +* **SHOULD:** Have a clear separation of concerns between what it actually does, and how it is presented to the user. +* **SHOULD:** Think twice before adding new public APIs that must remain for content compatibility, such as new wikitext syntax functionality. +* **SHOULD:** Not tightly integrate skin functionality with extension functionality. +* **SHOULD:** Not add new user preferences, unless you have a really good reason for doing so. +* **OPTIONAL:** Expose JavaScript methods for use by user scripts and gadgets, and to enable easy debugging from the console. + +## File structure + +Overall, the extension's file layout should be organized: consistent naming, directory structure that is logical and not messy. + +* **MUST:** Using the following standard directory layout: + * `src/` (when using PSR4 name-spacing, preferred) or `includes/`: Contains all (and only) PHP classes. + * i18n files for special page alias, magic words or namespaces located in root folder. + * **SHOULD:** Use [PSR-4](https://www.php-fig.org/psr/psr-4/) structure for classes and files, using AutoloadNamespaces in `extension.json` instead of listing all classes. + * **SHOULD:** Classes in `MediaWiki\Extension\ExtensionName` namespace (or `MediaWiki\Skin\SkinName` if a skin). + * **SHOULD:** One class per file. + * `modules/` (or `resources`) - Contains JavaScript and CSS for ResourceLoader. + * `maintenance/` command-line maintenance scripts + * `i18n/` - Contains localised messages in JSON files. + * `sql/` - SQL files for database modifications (e.g. called by LoadExtensionSchemaUpdates hook) + * `tests/`: + * `tests/parser/` - Contains parser test files. + * `tests/phpunit/` - Contains PHPUnit test cases. + * `tests/phpunit/unit/` - Contains test cases extending `MediaWikiUnitTestCase` + * `tests/phpunit/integration/` - Contains test cases extending `MediaWikiIntegrationTestCase` + * `tests/qunit/` - Contains QUnit test cases. + * `tests/selenium/` - Contains Selenium browser test files. + * `COPYING` or `LICENSE` - Contains full copy of the relevant license the code is released under. +* **SHOULD:** Avoid having many files in the root level directory. +* **SHOULD:** Avoid having dozens of nested directories that all only contain one or two things. +* **SHOULD:** Avoid having very large files, or very many tiny files (but keep following one class per file pattern – many tiny classes may be a sign of something else going wrong). +* **SHOULD:** Write a README file that summarizes the docs and gives detailed installation instructions. +* **SHOULD:** Declare foreign resources in a `foreign-resources.yaml` file. + +## Database + +* **MUST:** If adding database tables, use the LoadExtensionSchemaUpdates hook to ensure update.php works. +* **MUST:** Uses the Wikimedia-Rdbms library for all database access. Doing so avoids most SQL injection attack vectors, takes care of ensuring transactional correctness, and follows performance best practices. +* **SHOULD:** Work well in a distributed environment (concurrency, multiple databases, clustering). +* **SHOULD:** If it needs persistence, create nice SQL (primary keys, indexes where needed) and uses some caching mechanism where/if necessary. +* **SHOULD:** Never add fields to the core tables nor alter them in any way. To persist data associated with core tables, create a dedicated table for the extension and reference the core table's primary key. This makes it easier to remove an extension. +* **SHOULD:** It should use abstract schema. +* **OPTIONAL:** If the extension persists data and supports uninstalling, provide a maintenance script that automates this (e.g. drop tables, prune relevant log entries and page properties). + +## Coding conventions + +Overall, follow the MediaWiki coding conventions for PHP, JavaScript, CSS, and any other languages that are in-use and have applicable code conventions. + +* **SHOULD:** Run MediaWiki-CodeSniffer to enforce PHP conventions (check CI Entry points). +* **SHOULD:** Run Phan for PHP static analysis (check CI Entry points). +* **SHOULD:** Run ESLint for JavaScript conventions and static analysis (check CI Entry points). +* **SHOULD:** Run Stylelint for CSS conventions (check CI Entry points). +* **SHOULD:** Avoid writing all code into one large function (in JavaScript especially). +* **OPTIONAL:** Use code comments generally to document why the code exists, not what the code does. In long blocks of code, adding comments stating what each paragraph does is nice for easy parsing, but generally, comments should focus on the questions that can't be answered by just reading the code. + +## Testing + +* **SHOULD:** Have and run PHPUnit and QUnit tests. + * **OPTIONAL:** Split out integration and unit tests (see T87781). +* **SHOULD:** If there are parser functions or tags, have and run parser tests. +* **OPTIONAL:** Have and run browser tests. +* **OPTIONAL:** Test against right-to-left (RTL) languages! (how to verify?). +* **OPTIONAL:** Test against language converter languages! (how to verify?). + +## Language + +Various aspects of language support are also known as Localisation (L10n), internationalization (i18n), multilingualization, and globalization. +Overall, your extension should be fully usable and compatible with non-English and non-left-to-right languages. + +* **MUST:** Use the proper Localisation functions (wfMessage), and not have hardcoded non-translatable strings in your code. +* **MUST:** Use the standard internationalization systems in MediaWiki. +* **MUST:** Use a clear and unique prefix named after the extension for all interface messages. +* **MUST:** Add `qqq.json` message documentation for all messages that exist in `en.json` +* **SHOULD:** Escape parameters to localisation messages as close to output as possible. Document whether functions take/accept wikitext vs. HTML. +* **OPTIONAL:** If an extension uses particular terms, write a glossary of these terms, and link to it from the message documentation. Example: Abstract Wikipedia/Glossary. + +## Security + +See also Security for developers. + +* **MUST:** Shelling out should escape arguments. +* **MUST:** All write actions must be protected against cross-site request forgery (CSRF). +* **MUST:** Make sure privacy related issues (checkuser, revision and log suppression and deletion) are still covered when refactoring or writing new code. +* **SHOULD:** Use the standard MediaWiki CSRF token system. +* **SHOULD:** Don't modify HTML after it has been sanitized (common pattern is to use regex, but that's bad). +* **SHOULD:** Don't load any resources from external domains. This is also needed for privacy and improves performance. + +## Don't reinvent / abuse MediaWiki + +As a general principle, do not re-implement or compete with functionality already provided by MediaWiki core. + +* **MUST:** Use MediaWiki functionality/wrappers for things like WebRequest vs. `$_GET`, etc. +* **MUST:** Use hooks where possible as opposed to workarounds or novel ways of modifying, injecting, or extending functionality. +* **MUST:** Use MediaWiki's validation/sanitization methods e.g. those in the Html and Sanitizer classes. +* **MUST:** Don't disable parser cache unless you have a really good reason. +* **MUST:** Use Composer for 3rd party PHP library management. +* **SHOULD:** Don't reimplement the wheel. Prefer stable and well-maintained libraries when they exist. +* **SHOULD:** Don't disable OutputPage. (T140664) +* **SHOULD:** If an abstraction exists (e.g. ContentHandler), use that instead of hooks. +* **SHOULD:** Don't make things harder for yourself – use standard functionality like extension.json's tests/PHPUnit auto-discovery stuff. +* **SHOULD:** Use global MediaWiki configuration such as read-only mode. \ No newline at end of file diff --git a/.github/instructions/mediawiki.instructions.md b/.github/instructions/mediawiki.instructions.md new file mode 100644 index 0000000..d5406c0 --- /dev/null +++ b/.github/instructions/mediawiki.instructions.md @@ -0,0 +1,436 @@ +--- +applyTo: "**/*" +--- + +# MediaWiki Coding Conventions +This lists **general** conventions that apply to all MediaWiki code, whatever language it is written in. + +## Code structure + +### File formatting + +#### Tab size + +Lines should be indented with **a single tab character per indenting level**. You should make no assumptions about the number of spaces per tab. Most MediaWiki developers find 4 spaces per tab to be best for readability, but many systems are configured to use 8 spaces per tab and some developers might use 2 spaces per tab. + +For vim users, one way to establish these settings is to add the following to `$HOME/.vimrc`: + +``` +autocmd Filetype php setlocal ts=4 sw=4 +``` + +with similar lines for CSS, HTML, and JavaScript. + +However, for Python, instead follow the whitespace guidelines from [PEP 8](http://www.python.org/dev/peps/pep-0008/), which recommends spaces for new projects. + +#### Newlines + +All files should use Unix-style newlines (single LF character, not a CR+LF combination). + +* git on Windows will (by default) convert CR+LF newlines to LF during committing. + +All files should have a newline at the end. + +* It makes sense since all other lines have a newline character at the end. +* It makes passing data around in non-binary formats (like diffs) easier. +* Command-line tools like cat and wc don't handle files without one well (or at least, not in the way that one would like or expect). + +#### Encoding + +All text files **must** be encoded with UTF-8 without a Byte Order Mark. + +Do not use Microsoft Notepad to edit files, as it always inserts a BOM. A BOM will stop PHP files from working since it is a special character at the very top of the file and will be output by the web browser to the client. + +In short, make sure your editor supports UTF-8 without BOM. + +#### Trailing whitespace + +When using an IDE, pressing the Home and End keys (among other keyboard shortcuts) usually ignores trailing whitespace and instead jumps to the end of the code, which is intended. In non-IDE text editors, though, pressing End will jump to the very end of the line, which means the developer must backspace through the trailing whitespace to get to the spot where they actually want to type. + +Removing trailing whitespace is a trivial operation in most text editors. Developers should avoid adding trailing whitespace, primarily on lines that contain other visible code. + +Some tools make it easier: + +* nano: GNU nano 3.2; +* Komodo Edit: in the Save Options from menu "Edit > Preferences", enable "Clean trailing whitespace and EOL markers" and "Only clean changed lines"; +* Kate: you can see trailing spaces by enabling the option "Highlight trailing spaces". This option can be found in "Settings > Configure Kate > Appearance". You can also tell Kate to cleanup trailing spaces on save in "Settings > Configure Kate > Open/Save". +* vim: various automatic cleanup plugins; +* Sublime Text: [TrailingSpaces plugin](https://github.com/SublimeText/TrailingSpaces). + +#### Keywords + +Do not use parentheses with keywords (e.g. `require_once`, `require`) where they are not necessary. + +### Indenting and alignment + +#### General style + +MediaWiki's indenting style is similar to the so-called "One True Brace Style". Braces are placed on the same line as the start of the function, conditional, loop, etc. The else/elseif is placed on the same line as the previous closing brace. + +```php +function wfTimestampOrNull( $outputtype = TS_UNIX, $ts = null ) { + if ( $ts === null ) { + return null; + } else { + return wfTimestamp( $outputtype, $ts ); + } +} +``` + +Multi-line statements are written with the second and subsequent lines being indented by one extra level: + +Use indenting and line breaks to clarify the logical structure of your code. Expressions which nest multiple levels of parentheses or similar structures may begin a new indenting level with each nesting level: + +```php +$wgAutopromote = [ + 'autoconfirmed' => [ '&', + [ APCOND_EDITCOUNT, &$wgAutoConfirmCount ], + [ APCOND_AGE, &$wgAutoConfirmAge ], + ], +]; +``` + +#### Vertical alignment + +Avoid vertical alignment. It tends to create diffs which are hard to interpret, since the width allowed for the left column constantly has to be increased as more items are added. + +> **Note:** Most diff tools provide options to ignore whitespace changes. +> Git: `git diff -w` + +When needed, create mid-line vertical alignment with spaces rather than tabs. For instance this: + +```php +$namespaceNames = [ + NS_MEDIA => 'Media', + NS_SPECIAL => 'Special', + NS_MAIN => '', +]; +``` + +Is achieved as follows with spaces rendered as dots: + +``` +$namespaceNames·=·[ + → NS_MEDIA············=>·'Media', + → NS_SPECIAL··········=>·'Special', + → NS_MAIN·············=>·'', +]; +``` + +(If you use the [tabular vim add-on](https://github.com/godlygeek/tabular), entering `:Tabularize /=` will align the '=' signs.) + +#### Line width + +Lines should be broken with a line break at between 80 and 100 columns. There are some rare exceptions to this. Functions which take lots of parameters are not exceptions. The idea is that code should not overflow off the screen when word wrap is turned off. + +The operator separating the two lines should be placed consistently (always at the end or always at the start of the line). Individual languages might have more specific rules. + +```php +return strtolower( $val ) === 'on' + || strtolower( $val ) === 'true' + || strtolower( $val ) === 'yes' + || preg_match( '/^\s*[+-]?0*[1-9]/', $val ); +``` + +```php +$foo->dobar( + Xml::fieldset( wfMessage( 'importinterwiki' )->text() ) . + Xml::openElement( 'form', [ 'method' => 'post', 'action' => $action, 'id' => 'mw-import-interwiki-form' ] ) . + wfMessage( 'import-interwiki-text' )->parse() . + Xml::hidden( 'action', 'submit' ) . + Xml::hidden( 'source', 'interwiki' ) . + Xml::hidden( 'editToken', $wgUser->editToken() ), + 'secondArgument' +); +``` + +The method operator should always be put at the beginning of the next line. + +```php +$this->getMockBuilder( Message::class )->setMethods( [ 'fetchMessage' ] ) + ->disableOriginalConstructor() + ->getMock(); +``` + +When continuing "if" statements, a switch to Allman-style braces makes the separation between the condition and the body clear: + +```javascript +if ( $.inArray( mw.config.get( 'wgNamespaceNumber' ), whitelistedNamespaces ) !== -1 && + mw.config.get( 'wgArticleId' ) > 0 && + ( mw.config.get( 'wgAction' ) == 'view' || mw.config.get( 'wgAction' ) == 'purge' ) && + mw.util.getParamValue( 'redirect' ) !== 'no' && + mw.util.getParamValue( 'printable' ) !== 'yes' +) { + … +} +``` + +Opinions differ on the amount of indentation that should be used for the conditional part. Using an amount of indentation different to that used by the body makes it more clear that the conditional part is not the body, but this is not universally observed. + +Continuation of conditionals and very long expressions tend to be ugly whichever way you do them. So it's sometimes best to break them up by means of temporary variables. + +#### Braceless control structures + +Do not write "blocks" as a single-line. They reduce the readability of the code by moving important statements away from the left margin, where the reader is looking for them. Remember that making code shorter doesn't make it simpler. The goal of coding style is to communicate effectively with humans, not to fit computer-readable text into a small space. + +```php +// No: +if ( $done ) return; + +// No: +if ( $done ) { return; } + +// Yes: +if ( $done ) { + return; +} +``` + +This avoids a common logic error, which is especially prevalent when the developer is using a text editor which does not have a "smart indenting" feature. The error occurs when a single-line block is later extended to two lines: + +```php +if ( $done ) + return; +``` + +Later changed to: + +```php +if ( $done ) + $this->cleanup(); + return; +``` + +This has the potential to create subtle bugs. + +#### emacs style + +In emacs, using `php-mode.el` from [nXHTML mode](https://web.archive.org/web/20121213222615/http://ourcomments.org/Emacs/nXhtml/doc/nxhtml.html), you can set up a MediaWiki minor mode in your `.emacs` file: + +```elisp +(defconst mw-style + '((indent-tabs-mode . t) + (tab-width . 4) + (c-basic-offset . 4) + (c-offsets-alist . ((case-label . +) + (arglist-cont-nonempty . +) + (arglist-close . 0) + (cpp-macro . (lambda(x) (cdr x))) + (comment-intro . 0))) + (c-hanging-braces-alist + (defun-open after) + (block-open after) + (defun-close)))) + +(c-add-style "MediaWiki" mw-style) + +(define-minor-mode mah/mw-mode + "tweak style for mediawiki" + nil " MW" nil + (delete-trailing-whitespace) + (tabify (point-min) (point-max)) + (subword-mode 1)) ;; If this gives an error, try (c-subword-mode 1)), which is the earlier name for it + +;; Add other sniffers as needed +(defun mah/sniff-php-style (filename) + "Given a filename, provide a cons cell of + (style-name . function) +where style-name is the style to use and function +sets the minor-mode" + (cond ((string-match "/\\(mw[^/]*\\|mediawiki\\)/" + filename) + (cons "MediaWiki" 'mah/mw-mode)) + (t + (cons "cc-mode" (lambda (n) t))))) + +(add-hook 'php-mode-hook (lambda () (let ((ans (when (buffer-file-name) + (mah/sniff-php-style (buffer-file-name))))) + (c-set-style (car ans)) + (funcall (cdr ans) 1)))) +``` + +The above `mah/sniff-php-style` function will check your path when `php-mode` is invoked to see if it contains "mw" or "mediawiki" and set the buffer to use the `mw-mode` minor mode for editing MediaWiki source. You will know that the buffer is using `mw-mode` because you'll see something like "PHP MW" or "PHP/lw MW" in the mode line. + +### Data manipulation + +#### Constructing URLs + +Never build URLs manually with string concatenation or similar. Always use the full URL format for requests made by your code (especially POST and background requests). + +You can use the appropriate Linker or Title method in PHP, the fullurl magic word in wikitext, the `mw.util.getUrl()` method in JavaScript, and similar methods in other languages. You'll avoid issues with unexpected short URL configuration and more. + +## File naming + +Files which contain server-side code should be named in *UpperCamelCase*. This is also our naming convention for extensions. Name the file after the most important class it contains; most files will contain only one class, or a base class and a number of descendants. For example, `Title.php` contains only the `Title` class; `WebRequest.php` contains the `WebRequest` class, and also its descendants `FauxRequest` and `DerivativeRequest`. + +### Access point files + +Name "access point" files, such as SQL, and PHP entry points such as `index.php` and `foobar.sql`, in *lowercase*. Maintenance scripts are generally in *lowerCamelCase*, although this varies somewhat. Files intended for the site administrator, such as readmes, licenses and changelogs, are usually in *UPPERCASE*. + +Never include spaces in file names or directories, and never use non-ASCII characters. For lowercase titles, hyphens are preferred to underscores. + +### JS, CSS, and media files + +For JavaScript, CSS and other frontend files (usually registered via ResourceLoader) should be placed in directory named after the module bundle in which they are registered. For example, module `mediawiki.foo` might have files `mediawiki.foo/Bar.js` and `mediawiki.foo/baz.css` + +JavaScript files that define classes should match exactly the name of the class they define. The class `TitleWidget` should be in a file named as, or ending with, `TitleWidget.js`. This allows for rapid navigation in text editors by navigating to files named after a selected class name (such as "Goto Anything [P]" in Sublime, or "Find File [P]" in Atom). + +Large projects may have classes in a hierarchy with names that would overlap or be ambiguous without some additional way of organizing files. We generally approach this with subdirectories like `ext.foo/bar/TitleWidget.js` (for Package files), or longer class and file names like `mw.foo.bar.TitleWidget` in `ext.foo/bar.TitleWidget.js`. + +Modules bundles registered by extensions should follow names like `ext.myExtension`, for example `MyExtension/modules/ext.myExtension/index.js`. This makes it easy to get started with working on a module in a text editor, by directly finding the source code files from only the public module name. + +## Documentation + +The language-specific subpages have more information on the exact syntax for code comments in files, e.g. comments in PHP for doxygen. Using precise syntax allows us to generate documentation from source code at [doc.wikimedia.org](https://doc.wikimedia.org). + +High level concepts, subsystems, and data flows should be documented in the `/docs` folder. + +### Source file headers + +In order to be compliant with most licenses you should have something similar to the following (specific to GPLv2 PHP applications) at the top of every source file. + +```php +msg( 'templatesused-' . ( $section ? 'section' : 'page' ) ); +``` + +**Positive example:** +```php +// Yes: Prefer full message keys +$context->msg( $section ? 'templatesused-section' : 'templatesused-page' ); +``` + +```javascript +// If needed, concatenate and write explicit references in a comment + +// Messages: +// * myextension-connect-success +// * myextension-connect-warning +// * myextension-connect-error +const text = mw.msg( 'myextension-connect-' + status ); +``` + +```javascript +// The following classes are used here: +// * mw-editfont-monospace +// * mw-editfont-sans-serif +// * mw-editfont-serif +$texarea.addClass( 'mw-editfont-' + mw.user.options.get( 'editfont' ) ); +``` + +```php +// Load example/foo.json, or example/foo.php +$thing->load( "$path/foo.$ext" ); +``` + +## Release notes + +You must document all significant changes (including all fixed bug reports) to the core software which might affect wiki users, server administrators, or extension authors in the `RELEASE-NOTES-N.NN` file. + +`RELEASE-NOTES-N.NN` is in development; on every release we move the past release notes into the `HISTORY` file and start afresh. `RELEASE-NOTES-N.NN` is generally divided into three sections: + +* **Configuration changes** is the place to put changes to accepted default behavior, backwards-incompatible changes, or other things which need a server administrator to look at and decide "is this change right for my wiki?". Try to include a brief explanation of how the previous functionality can be recovered if desired. +* **Bug fixes** is the place to note changes which fix behavior which is accepted to be problematic or undesirable. These will often be issues reported in Phabricator, but needn't necessarily. +* **New features** is, unsurprisingly, to note the addition of new functionality. + +There may be additional sections for specific components (e.g. the Action API) or for miscellaneous changes that don't fall into one of the above categories. + +In all cases, if your change is in response to one or more issues reported in Phabricator, include the task ID(s) at the start of the entry. Add new entries in chronological order at the end of the section. + +## System messages + +When creating a new system message, use hyphens (-) where possible instead of CamelCase or snake_case. So for example, `some-new-message` is a good name, while `someNewMessage` and `some_new_message` are not. + +If the message is going to be used as a label which can have a colon (:) after it, don't hardcode the colon; instead, put the colon inside the message text. Some languages (such as French which require a space before) need to handle colons in a different way, which is impossible if the colon is hardcoded. The same holds for several other types of interpunctuation. + +Try to use message keys "whole" in code, rather than building them on the fly; as this makes it easier to search for them in the codebase. For instance, the following shows how a search for `templatesused-section` will not find this use of the message key if they are not used as a whole. + +```php +// No: +return wfMessage( 'templatesused-' . ( $section ? 'section' : 'page' ) ); + +// Yes: +$msgKey = $section ? 'templatesused-section' : 'templatesused-page'; +return wfMessage( $msgKey ); +``` + +If you feel that you have to build messages on the fly, put a comment with all possible whole messages nearby: + +```php +// Messages that can be used here: +// * myextension-connection-success +// * myextension-connection-warning +// * myextension-connection-error +$text = wfMessage( 'myextension-connection-' . $status )->parse(); +``` + +See Localisation for more conventions about creating, using, documenting and maintaining message keys. + +### Preferred spelling + +It is just as important to have consistent spelling in the UI and codebase as it is to have consistent UI. By long standing history, 'American English' is the preferred spelling for English language messages, comments, and documentation. + +### Abbreviations in message keys + +* **ph**: placeholder (text in input fields) +* **tip**: tooltip text +* **tog-xx**: toggle options in user preferences + +### Punctuation + +Non-title error messages are considered as sentences and should have punctuation. + +## Improve the core + +If you need some additional functionality from a MediaWiki core component (PHP class, JS module etc.), or you need a function that does something similar but slightly different, prefer to improve the core component. Avoid duplicating the code to an extension or elsewhere in core and modifying it there. + +## Refactoring + +Refactor code as changes are made: don't let the code keep getting worse with each change. + +However, use separate commits if the refactoring is large. See also Architecture guidelines (draft). + +## HTML + +MediaWiki HTTP responses output HTML that can be generated by one of two sources. The MediaWiki PHP code is a trusted source for the user interface, it can output any arbitrary HTML. The Parser converts user-generated wikitext into HTML, this is an untrusted source. Complex HTML created by users via wikitext is often found in the "Template" namespace. HTML produced by the Parser is subject to sanitization before output. + +Most `data-*` attributes are allowed to be used by users in wikitext and templates. But, the following prefixes have been restricted and are not allowed in wikitext and will be removed from the output HTML. This enables client JavaScript code to determine whether a DOM element came from a trusted source: + +* `data-ooui` – This attribute is present in HTML generated by OOUI widgets. +* `data-parsoid` – reserved attribute for internal use by Parsoid. +* `data-mw` and `data-mw-...` – reserved attribute for internal use by MediaWiki core, skins and extensions. The `data-mw` attribute is used by Parsoid; other core code should use `data-mw-*`. + +When selecting elements in JavaScript, one can specify an attribute key/value to ensure only DOM elements from the intended trusted source are considered. Example: Only trigger 'wikipage.diff' hook diff --git a/.github/instructions/php.instructions.md b/.github/instructions/php.instructions.md new file mode 100644 index 0000000..ea1648d --- /dev/null +++ b/.github/instructions/php.instructions.md @@ -0,0 +1,629 @@ +--- +applyTo: "**/*.php" +--- + +# PHP Coding Conventions + +This page describes the coding conventions used within files of the MediaWiki codebase written in PHP. + +See also the general conventions that apply to all program languages, including PHP. If you would like a short checklist to help you review your commits, try using the Pre-commit checklist. + +Most of the code style rules can be automatically fixed, or at least detected, by PHP_CodeSniffer (also known as PHPCS), using a custom ruleset for MediaWiki. For more information, see Continuous integration/PHP CodeSniffer. + +## Code structure + +### Spaces + +MediaWiki favors a heavily-spaced style for optimum readability. + +Indent with tabs, not spaces. Limit lines to 120 characters (given a tab-width of 4 characters). + +Put spaces on either side of binary operators, for example: + +```php +// No: +$a=$b+$c; + +// Yes: +$a = $b + $c; +``` + +Put spaces next to parentheses on the inside, except where the parentheses are empty. Do not put a space following a function name. + +```php +$a = getFoo( $b ); +$c = getBar(); +``` + +Put a space after the `:` in the function return type hint, but not before: + +```php +function square( int $x ): int { + return $x * $x; +} +``` + +Put spaces in brackets when declaring an array, except where the array is empty. Do not put spaces in brackets when accessing array elements. + +```php +// Yes +$a = [ 'foo', 'bar' ]; +$c = $a[0]; +$x = []; + +// No +$a = ['foo', 'bar']; +$c = $a[ 0 ]; +$x = [ ]; +``` + +Control structures such as `if`, `while`, `for`, `foreach`, `switch`, as well as the `catch` keyword, should be followed by a space: + +```php +// Yes +if ( isFoo() ) { + $a = 'foo'; +} + +// No +if( isFoo() ) { + $a = 'foo'; +} +``` + +When type casting, do not use a space within or after the cast operator: + +```php +// Yes +(int)$foo; + +// No +(int) $bar; +( int )$bar; +( int ) $bar; +``` + +In comments there should be one space between the `#` or `//` character and the comment. + +```php +// Yes: Proper inline comment +//No: Missing space +/***** Do not comment like this ***/ +``` + +### Ternary operator + +The ternary operator can be used profitably if the expressions are very short and obvious: + +```php +$title = $page ? $page->getTitle() : Title::newMainPage(); +``` + +But if you're considering a multi-line expression with a ternary operator, please consider using an `if ()` block instead. Remember, disk space is cheap, code readability is everything, "if" is English and "?:" is not. If you are using a multi-line ternary expression, the question mark and colon should go at the beginning of the second and third lines and not the end of the first and second (in contrast to MediaWiki's JavaScript convention). + +Since MediaWiki requires PHP 7.2 or later, use of the shorthand ternary operator (`?:`) also known as the elvis operator, introduced in PHP 5.3, is allowed. + +Since PHP 7.0 the null coalescing operator is also available and can replace the ternary operator in some use cases. For example, instead of: +```php +$wiki = isset( $this->mParams['wiki'] ) ? $this->mParams['wiki'] : false; +``` +you could instead write the following: +```php +$wiki = $this->mParams['wiki'] ?? false; +``` + +### String literals + +Single quotes are preferred in all cases where they are equivalent to double quotes. Code using single quotes is less error-prone and easier to review, as it cannot accidentally contain escape sequences or variables. For example, the regular expression `"/\\n+/"` requires an extra backslash, making it slightly more confusing and error-prone than `'/\n+/'`. Also for people using US/UK qwerty keyboards, they are easier to type, since it avoids the need to press shift. + +However, do not be afraid of using PHP's double-quoted string interpolation feature: +```php +$elementId = "myextension-$index"; +``` + +This has slightly better performance characteristics than the equivalent using the concatenation (dot) operator, and it looks nicer too. + +Heredoc-style strings are sometimes useful: + +```php +$s = << +$boxContents + +EOT; +``` + +Some authors like to use END as the ending token, which is also the name of a PHP function. + +### Functions and parameters + +Avoid passing huge numbers of parameters to functions or constructors: + +```php +// Constructor for Block.php from 1.17 to 1.26. DO NOT do this! +function __construct( $address = '', $user = 0, $by = 0, $reason = '', + $timestamp = 0, $auto = 0, $expiry = '', $anonOnly = 0, $createAccount = 0, $enableAutoblock = 0, + $hideName = 0, $blockEmail = 0, $allowUsertalk = 0 +) { + ... +} +``` + +It quickly becomes impossible to remember the order of parameters, and you will inevitably end up having to hardcode all the defaults in callers just to customise a parameter at the end of the list. If you are tempted to code a function like this, consider passing an associative array of named parameters instead. + +In general, using boolean parameters is discouraged in functions. In `$object->getSomething( $input, true, true, false )`, without looking up the documentation for `MyClass::getSomething()`, it is impossible to know what those parameters are meant to indicate. Much better is to either use class constants, and make a generic flag parameter: + +```php +$myResult = MyClass::getSomething( $input, MyClass::FROM_DB | MyClass::PUBLIC_ONLY ); +``` + +Or to make your function accept an array of named parameters: + +```php +$myResult = MyClass::getSomething( $input, [ 'fromDB', 'publicOnly' ] ); +``` + +Try not to repurpose variables over the course of a function, and avoid modifying the parameters passed to a function (unless they're passed by reference and that's the whole point of the function, obviously). + +### Assignment expressions + +Using assignment as an expression is surprising to the reader and looks like an error. Do not write code like this: + +```php +if ( $a = foo() ) { + bar(); +} +``` + +Space is cheap, and you're a fast typist, so instead use: + +```php +$a = foo(); +if ( $a ) { + bar(); +} +``` + +Using assignment in a `while()` clause used to be legitimate, for iteration: + +```php +$res = $dbr->query( 'SELECT foo, bar FROM some_table' ); +while ( $row = $dbr->fetchObject( $res ) ) { + showRow( $row ); +} +``` + +This is unnecessary in new code; instead use: + +```php +$res = $dbr->query( 'SELECT foo, bar FROM some_table' ); +foreach ( $res as $row ) { + showRow( $row ); +} +``` + +### C borrowings + +The PHP language was designed by people who love C and wanted to bring souvenirs from that language into PHP. But PHP has some important differences from C. + +In C, constants are implemented as preprocessor macros and are fast. In PHP, they are implemented by doing a runtime hashtable lookup for the constant name, and are slower than just using a string literal. In most places where you would use an enum or enum-like set of macros in C, you can use string literals in PHP. + +PHP has three special literals for which upper-/lower-/mixed-case is insignificant in the language (since PHP 5.1.3), but for which our convention is always lowercase: `true`, `false` and `null`. + +Use `elseif` not `else if`. They have subtly different meanings: + +```php +// This: +if ( $foo === 'bar' ) { + echo 'Hello world'; +} else if ( $foo === 'Bar' ) { + echo 'Hello world'; +} else if ( $baz === $foo ) { + echo 'Hello baz'; +} else { + echo 'Eh?'; +} + +// Is actually equivalent to: +if ( $foo === 'bar' ) { + echo 'Hello world'; +} else { + if ( $foo == 'Bar' ) { + echo 'Hello world'; + } else { + if ( $baz == $foo ) { + echo 'Hello baz'; + } else { + echo 'Eh?'; + } + } +} +``` + +And the latter has poorer performance. + +### Alternative syntax for control structures + +PHP offers an alternative syntax for control structures using colons and keywords such as `endif`, `endwhile`, etc.: + +```php +if ( $foo == $bar ): + echo "
Hello world
"; +endif; +``` + +This syntax should be avoided, as it prevents many text editors from automatically matching and folding braces. Braces should be used instead: + +```php +if ( $foo == $bar ) { + echo "
Hello world
"; +} +``` + +### Brace placement + +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 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.) + +Scalar typehints are allowed as of MediaWiki 1.35, following the switch to PHP 7.2. + +Use PHP 7.1 syntax for nullable parameters: choose + +```php +public function foo ( ?MyClass $mc ) {} +``` + +instead of + +```php +public function foo ( MyClass $mc = null ) {} +``` + +The former conveys precisely the nullability of a parameter, without risking any ambiguity with optional parameters. IDEs and static analysis tools will also recognize it as such, and will not complain if a non-nullable parameter follows a nullable one. + +Do not add PHPDoc comments that only repeat the native types. Add PHPDoc comments if they document types that can't be expressed using native types (e.g. `string[]` where the native type is `array`), or if they document something useful about the value beyond what the type and parameter/function name already says. + +## Naming + +| Element | Convention | Notes | +|---------|------------|-------| +| Files | UpperCamelCase | PHP files should be named after the class they contain, which is UpperCamelCase. For instance, `WebRequest.php` contains the `WebRequest` class. See also Manual:Coding conventions#File naming | +| Namespaces | UpperCamelCase | | +| Classes | UpperCamelCase | Use UpperCamelCase when naming classes. For example: `class ImportantClass` | +| Constants | Uppercase with underscores | Use uppercase with underscores for global and class constants: `DB_PRIMARY`, `IDBAccessObject::READ_LATEST` | +| Functions | lowerCamelCase | Use lowerCamelCase when naming functions. For example: `private function doSomething( $userPrefs, $editSummary )` | +| Function variables | lowerCamelCase | Use lowerCamelCase when naming function variables. Avoid using underscores in variable names. | + +### Prefixes + +There are also some prefixes that can be used in different places: + +#### Function names + +* `wf` (wiki functions) – top-level functions, e.g. `function wfFuncname() { ... }` +* `ef` (extension functions) – global functions in extensions, although "in most cases modern style puts hook functions as static methods on a class, leaving few or no raw top-level functions to be so named." + +Verb phrases are preferred: use `getReturnText()` instead of `returnText()`. When exposing functions for use in testing, mark these as `@internal` per the Stable interface policy. Misuse or unofficial reliance on these is more problematic than most internal methods, and as such we tend to make these throw if they run outside of a test environment. + +```php +/** + * Reset example data cache. + * + * @internal For testing only + */ +public static function clearCacheForTest(): void { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new RuntimeException( 'Not allowed outside tests' ); + } + self::$exampleDataCache = []; +} +``` + +#### Variable names + +* `$wg` – global variables, e.g. `$wgTitle`. Always use this for new globals, so that it's easy to spot missing `global $wgFoo` declarations. In extensions, the extension name should be used as a namespace delimiter. For example, `$wgAbuseFilterConditionLimit`, **not** `$wgConditionLimit`. +* Global declarations should be at the beginning of a function so dependencies can be determined without having to read the whole function. + +It is common to work with an instance of the `Database` class; we have a naming convention for these which helps keep track of the nature of the server to which we are connected. This is of particular importance in replicated environments, such as Wikimedia and other large wikis; in development environments, there is usually no difference between the two types, which can conceal subtle errors. + +* `$dbw` – a `Database` object for writing (a primary connection) +* `$dbr` – a `Database` object for non-concurrency-sensitive reading (this may be a read-only replica, slightly behind primary state, so don't ever try to write to the database with it, or get an "authoritative" answer to important queries like permissions and block status) + +The following may be seen in old code but are discouraged in new code: + +* `$ws` – Session variables, e.g. `$_SESSION['wsSessionName']` +* `$wc` – Cookie variables, e.g. `$_COOKIE['wcCookieName']` +* `$wp` – Post variables (submitted via form fields), e.g. `$wgRequest->getText( 'wpLoginName' )` +* `$m` – object member variables: `$this->mPage`. This is **discouraged in new code**, but try to stay consistent within a class. + +## Pitfalls + +### `empty()` + +The `empty()` function should only be used when you want to suppress errors. Otherwise just use `!` (boolean conversion). + +* `empty( $var )` essentially does `!isset( $var ) || !$var`. + Common use case: Optional boolean configuration keys that default to `false`. `$this->enableFoo = !empty( $options['foo'] );` +* Beware of boolean conversion pitfalls. +* It suppresses errors about undefined properties and variables. If only intending to test for undefined, use `!isset()`. If only intending to test for "empty" values (e.g. `false`, `0`, `[]`, etc.), use `!`. + +### `isset()` + +Do not use `isset()` to test for `null`. Using `isset()` in this situation could introduce errors by hiding misspelled variable names. Instead, use `$var === null`. + +Testing whether a typed property that cannot be null but has no default value has been initialized is a valid use of `isset()`, but confuses the PHP static analysis tool Phan. You can often avoid this by using `??` / `??=`. + +### Boolean conversion + +```php +if ( !$var ) { + … +} +``` + +* Do not use `!` or `empty()` to test if a string or array is empty, because PHP considers `'0'` to be falsy – but `'0'` is a valid title and valid user name in MediaWiki. Use `=== ''` or `=== []` instead. +* Study the rules for conversion to boolean. Be careful when converting strings to boolean. + +### Other + +* Array plus does not renumber the keys of numerically-indexed arrays, so `[ 'a' ] + [ 'b' ] === [ 'a' ]`. If you want keys to be renumbered, use `array_merge()`: `array_merge( [ 'a' ], [ 'b' ] ) === [ 'a', 'b' ]` +* Make sure you have `error_reporting()` set to `-1`. This will notify you of undefined variables and other subtle gotchas that stock PHP will ignore. See also Manual:How to debug. +* When working in a pure PHP file (e.g. not an HTML template), omit any trailing `?>` tags. These tags often cause issues with trailing white-space and "headers already sent" error messages. It is conventional in version control for files to have a new line at end-of-file (which editors may add automatically), which would then trigger this error. +* Do not use the `goto` syntax introduced in 5.3. PHP may have introduced the feature, but that does not mean we should use it. +* Do not pass by reference when traversing an array with `foreach` unless you *have to*. Even then, be aware of the consequences. +* PHP lets you declare static variables even within a non-static method of a class. This has led to subtle bugs in some cases, as the variables are shared between instances. Where you would not use a `private static` property, do not use a static variable either. + +### Equality operators + +Be careful with double-equals comparison operators. Triple-equals (`===`) is generally more intuitive and should be preferred unless you have a reason to use double-equals (`==`). + +* `'000' == '0'` is `true` (!) +* `'000' === '0'` is `false` +* To check if two scalars that are supposed to be numeric are equal, use `==`, e.g. `5 == "5"` is true. +* To check if two variables are both of type 'string' and are the same sequence of characters, use `===`, e.g. `"1.e6" === "1.0e6"` is false. + +Watch out for internal functions and constructs that use weak comparisons; for instance, provide the third parameter to `in_array`, and don't mix scalar types in `switch` constructs. + +Do not use Yoda conditionals. + +### JSON number precision + +JSON uses JavaScript's type system, so all numbers are represented as 64bit IEEE floating point numbers. This means that numbers lose precision when getting bigger, to the point where some whole numbers become indistinguishable: Numbers beyond 2^52 will have a precision worse than ±0.5, so a large integer may end up changing to a different integer. To avoid this issue, represent potentially large integers as strings in JSON. + +## Dos and don'ts + +### Don't use built in serialization + +PHP's built in serialization mechanism (the `serialize()` and `unserialize()` functions) should not be used for data stored (or read from) outside of the current process. Use JSON based serialization instead (however, beware the pitfalls). This is policy established by RFC T161647. + +The reason is twofold: (1) data serialized with this mechanism cannot reliably be unserialized with a later version of the same class. And (2) crafted serialized data can be used to execute malicious code, posing a serious security risk. + +Sometimes, your code will not control the serialization mechanism, but will be using some library or driver that uses it internally. In such cases, steps should be taken to mitigate risk. The first issue mentioned above can be mitigated by converting any data to arrays or plain anonymous objects before serialization. The second issue can perhaps be mitigated using the whitelisting feature PHP 7 introduces for unserialization. + +Although for trivial classes PHP's JsonSerializable interface may suffice, more complex examples will likely find the wikimedia/json-codec package useful when serializing to/from JSON. It contains facilities to integrate with services and dependency injection, as well as to integrate with external classes which don't natively support serialization. The `JsonCodec` service in core extends the codec provided by `wikimedia/json-codec`. + +### Don't add type declarations for "big" legacy classes + +MediaWiki contains some big classes that are going to be split up or replaced sooner or later. This will be done in a way that keeps code compatible for a transition period, but it can break extension code that expects the legacy classes in parameter types, return types, property types, or similar. For instance, a hook handler's `$title` parameter may be passed some kind of `MockTitleCompat` class instead of a real `Title`. + +Such big legacy classes should therefore not be used in type hints, only in PHPDoc. The classes include: + +* `Title` +* `Article` +* `WikiPage` +* `User` +* `MediaWiki` +* `OutputPage` +* `WebRequest` +* `EditPage` + +### Don't add type declarations for DOM classes + +PHP 8.4 introduces a new `\Dom\Document` class which is not-quite-compatible with the older `\DOMDocument` class used in PHP <= 8.3. The `Wikimedia\Parsoid\Utils\DOMCompat` class in `wikimedia/parsoid` contains functions to bridge the gap between the two implementations, and to generally provide standards-compliant implementations of features missing in one or the other implementation. It is recommended to either omit explicit type declarations for DOM classes (allowing either `\Dom\Document` or `\DOMDocument` classes to be passed at runtime) or to use the `Wikimedia\Parsoid\DOM\Document` aliases provided by Parsoid in type hints, which will resolve to `\DOMDocument` before PHP 8.4 and `\Dom\Document` after. + +## Comments and documentation + +It is essential that your code be well documented so that other developers and bug fixers can easily navigate the logic of your code. New classes, methods, and member variables should include comments providing brief descriptions of their functionality (unless it is obvious), even if private. In addition, all new methods should document their parameters and return values. + +We use the Doxygen documentation style (it is very similar to PHPDoc for the subset that we use) to produce auto-generated documentation from code comments (see Manual:mwdocgen.php). Begin a block of Doxygen comments with `/**`, instead of the Qt-style formatting `/*!`. Doxygen structural commands start with `@tagname`. (Use `@` rather than `\` as the escape character – both styles work in Doxygen, but for backwards and future compatibility MediaWiki has chosen the `@param` style.) They organize the generated documentation (using `@ingroup`) and identify authors (using `@author` tags). + +They describe a function or method, the parameters it takes (using `@param`), and what the function returns (using `@return`). The format for parameters is: + +``` +@param type $paramName Description of parameter +``` + +If a parameter can be of multiple types, separate them with the pipe '|' character, for example: + +``` +@param string|Language|bool $lang Language for the ToC title, defaults to user language +``` + +Continue sentences belonging to an annotation on the next line, indented with one additional space. + +For every public interface (method, class, variable, whatever) you add or change, provide a `@since VERSION` tag, so people extending the code via this interface know they are breaking compatibility with older versions of the code. + +```php +class Foo { + + /** + * @var array Description here + * @example [ 'foo' => Bar, 'quux' => Bar, .. ] + */ + protected $bar; + + /** + * Description here, following by documentation of the parameters. + * + * Some example: + * @code + * ... + * @endcode + * + * @since 1.24 + * @param FooContext $context context for decoding Foos + * @param array|string $options Optionally pass extra options. + * Either a string or an array of strings. + * @return Foo|null New instance of Foo or null if quuxification failed. + */ + public function makeQuuxificatedFoo( FooContext $context = null, $options = [] ) { + /* .. */ + } + +} +``` + +FIXME usually means something is bad or broken. TODO means that improvements are needed; it does not necessarily mean that the person adding the comment is going to do it. HACK means that a quick but inelegant, awkward or otherwise suboptimal solution to an immediate problem was made, and that eventually a more thorough rewrite of the code should be done. + +### Source file headers + +In order to be compliant with most licenses you should have something similar to the following (specific to GPLv2 applications) at the top of every source file. + +```php +get*( 'param' )` instead; there are various functions depending on what type of value you want. You can get a `WebRequest` from the nearest `RequestContext`, or if absolutely necessary `RequestContext::getMain()`. Equally, do not access `$_SERVER` directly; use `$request->getIP()` if you want to get the IP address of the current user. + +### Static methods + +Code using static methods should be written so that all method calls inside a class use Late Static Bindings, which basically means that calls to overridable static methods are resolved in the same way as calls to overridable instance methods. Specifically: + +* When calling static methods that may be overridden by subclasses from inside the class, use `static::func()`. This will call the override methods defined in subclasses if they exist, just like `$this->func()` does for instance methods. +* When calling static methods that may not be overridden (especially private methods), use `self::func()`. This will only call the methods of the class where it is used and its parent classes. +* When calling a parent method from an override of a static method, use `parent::func()`. +* If you ever think you need to call a grandparent class's version of a static method, or a child class's, think about it again, and use `forward_static_call()` if you don't come up with any better ideas. + +Do not write out the class name like `ClassName::func()` in the above cases, as that will cause all method calls inside that method to ignore overrides of that class's members in subclasses. This is only a problem for static methods, it works like you'd expect in instance methods, but avoid that syntax in instance methods too to avoid confusion about what the call will do. + +These complications are annoying. Best avoid static methods so that you don't have to think about this. + +### Calling methods + +For clarity, the method call syntax should match the method type: + +* Calls to static methods should always use `::`, even though PHP lets you use `->` sometimes. +* Calls to instance methods should always use `->`, even though PHP lets you use `::` sometimes. (`self::` and `parent::` may be used when needed.) + +### Classes + +Encapsulate your code in an object-oriented class, or add functionality to existing classes; do not add new global functions or variables. Try to be mindful of the distinction between 'backend' classes, which represent entities in the database (e.g. `User`, `Block`, `RevisionRecord`, etc.), and 'frontend' classes, which represent pages or interfaces visible to the user (`SpecialPage`, `Article`, `ChangesList`, etc.). Even if your code is not obviously object-oriented, you can put it in a static class (e.g. `IP` or `Html`). + +As a holdover from PHP 4's lack of private class members and methods, older code will be marked with comments such as `/** @private */` to indicate the intention; respect this as if it were enforced by the interpreter. + +Mark new code with proper visibility modifiers, including `public` if appropriate, but **do not** add visibility to existing code without first checking, testing and refactoring as required. It's generally a good idea to avoid visibility changes unless you're making changes to the function which would break old uses of it anyway. + +## Error handling + +In general, you should not suppress PHP errors. The proper method of handling errors is to *actually handle the errors*. + +For example, if you are thinking of using an error suppression operator to suppress an invalid array index warning, you should instead perform an `isset()` check on the array index before trying to access it. When possible, *always* catch or naturally prevent PHP errors. + +Only if there is a situation where you are expecting an unavoidable PHP warning, you may use PHP's `@` operator. This is for cases where: + +1. It is impossible to anticipate the error that is about to occur; and +2. You are planning on handling the error in an appropriate manner after it occurs. + +We use PHPCS to warn against use of the at-operator. If you really need to use it, you'll also need to instruct PHPCS to make an exemption, like so: + +```php +// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged +$content = @file_get_contents( $path ); +``` + +An example use case is opening a file with `fopen()`. You can try to predict the error by calling `file_exists()` and `is_readable()`, but unlike `isset()`, such file operations add significant overhead and make for unstable code. For example, the file may be deleted or changed between the check and the actual `fopen()` call (see TOC/TOU). + +In this case, write the code to just try the main operation you need to do. Then handle the case of the file failing to open, by using the `@` operator to prevent PHP from being noisy, and then check the result afterwards. For `fopen()` and `filemtime()`, that means checking for a boolean false return, and then performing a fallback, or throw an exception. + +### AtEase + +For PHP 5 and earlier, MediaWiki developers discouraged use of the `@` operator due to it causing unlogged and unexplained fatal errors. Instead, we used custom `AtEase::suppressWarnings()` and `AtEase::restoreWarnings()` methods from the at-ease library. The reason is that the at-operator caused PHP to not provide error messages or stack traces upon fatal errors. While the at-operator is mainly intended for non-fatal errors (not exceptions or fatals), if a fatal were to happen it would make for a very poor developer experience. + +```php +use Wikimedia\AtEase\AtEase; + +AtEase::suppressWarnings(); +$content = file_get_contents( $path ); +AtEase::restoreWarnings(); +``` + +In PHP 7, the exception handler was fixed to always provide such errors, including a stack trace, regardless of error suppression. In 2020, use of AtEase started a phase out, reinstating the at-operator. + +## Exception handling + +Exceptions can be checked (meaning callers are expected to catch them) or unchecked (meaning callers must not catch them). + +Unchecked exceptions are commonly used for programming errors, such as invalid arguments passed to a function. These exceptions should generally use (either directly or by subclassing) the SPL exception classes, and must not be documented with `@throws` annotations. Nonetheless, the conditions that lead to these exceptions being thrown should be documented in prose in the doc comment when they're part of a method's contract (for example, a string argument that must not be empty, or an integer argument that must be non-negative). + +Checked exceptions, on the other hand, must always be documented with `@throws` annotations. When calling a method that can throw a checked exception, said exception must either be caught, or documented in the caller's doc comment. Checked exceptions should generally use dedicated exception classes extending `Exception`. It's recommended not to use SPL exceptions as base classes for checked exceptions, so that correct usage of exception classes can be enforced with static code analyzers. + +The base `Exception` class must never be thrown directly: use more specific exception classes instead. It can be used in a `catch` clause if the intention is to catch all possible exceptions, but `Throwable` is usually more correct for that purpose. + +In legacy code it is relatively common to throw or subclass the `MWException` class. This class must be avoided in new code, as it does not provide any advantage, and could actually be confusing. + +When creating a new exception class, consider implementing `INormalizedException` if the exception message contains variable parts, and `ILocalizedException` if the exception message is shown to users. + +If you're not sure what exception class to use, you can throw a `LogicException` for problems that indicate bugs in the code (e.g. function called with wrong arguments, or an unreachable branch being reached), and `RuntimeException` for anything else (e.g. an external server being down). From e0b610ae7a0ae71599c85f6e07785387b2279262 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sun, 26 Oct 2025 04:33:13 +0000 Subject: [PATCH 3/8] Refactoring --- .github/instructions/php.instructions.md | 9 +++ includes/Hooks.php | 2 +- tests/phpunit/unit/HooksTest.php | 90 +++++++++++------------- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/.github/instructions/php.instructions.md b/.github/instructions/php.instructions.md index ea1648d..190924a 100644 --- a/.github/instructions/php.instructions.md +++ b/.github/instructions/php.instructions.md @@ -266,6 +266,15 @@ 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/includes/Hooks.php b/includes/Hooks.php index 50f7e1e..dfef451 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -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/unit/HooksTest.php b/tests/phpunit/unit/HooksTest.php index 3e97074..b11803f 100644 --- a/tests/phpunit/unit/HooksTest.php +++ b/tests/phpunit/unit/HooksTest.php @@ -144,6 +144,7 @@ public function testNonRevisionTypeAlwaysAllowed() { /** * @covers ::onSpecialPageBeforeExecute * @dataProvider provideBlockedSpecialPages + * @param string $specialPageName */ public function testSpecialPageBlocksAnonymous( $specialPageName ) { $output = $this->createMock( self::$outputPageClassName ); @@ -151,23 +152,7 @@ public function testSpecialPageBlocksAnonymous( $specialPageName ) { $user = $this->createMock( self::$userClassName ); $user->method( 'isRegistered' )->willReturn( false ); - $context = new class( $user, $output ) { - private $user; - private $output; - - public function __construct( $user, $output ) { - $this->user = $user; - $this->output = $output; - } - - public function getUser() { - return $this->user; - } - - public function getOutput() { - return $this->output; - } - }; + $context = $this->createMockContext( $user, $output ); $special = $this->createMock( self::$specialPageClassName ); $special->method( 'getName' )->willReturn( $specialPageName ); @@ -185,6 +170,7 @@ public function getOutput() { /** * @covers ::onSpecialPageBeforeExecute * @dataProvider provideBlockedSpecialPages + * @param string $specialPageName */ public function testSpecialPageAllowsLoggedIn( $specialPageName ) { $output = $this->createMock( self::$outputPageClassName ); @@ -192,23 +178,7 @@ public function testSpecialPageAllowsLoggedIn( $specialPageName ) { $user = $this->createMock( self::$userClassName ); $user->method( 'isRegistered' )->willReturn( true ); - $context = new class( $user, $output ) { - private $user; - private $output; - - public function __construct( $user, $output ) { - $this->user = $user; - $this->output = $output; - } - - public function getUser() { - return $this->user; - } - - public function getOutput() { - return $this->output; - } - }; + $context = $this->createMockContext( $user, $output ); $special = $this->createMock( self::$specialPageClassName ); $special->method( 'getName' )->willReturn( $specialPageName ); @@ -232,39 +202,65 @@ public function testUnblockedSpecialPageAllowsAnonymous() { $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; } }; - - $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 ); + return $context; } /** - * Data provider for blocked special pages + * Data provider for blocked special pages. + * + * @return array */ public function provideBlockedSpecialPages() { return [ From d8d3ac558072211df1071cd56af96ae58e76d261 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sat, 25 Oct 2025 21:35:57 -0700 Subject: [PATCH 4/8] Update tests/phpunit/stubs.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/phpunit/stubs.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/stubs.php b/tests/phpunit/stubs.php index c6664f0..4fffecb 100644 --- a/tests/phpunit/stubs.php +++ b/tests/phpunit/stubs.php @@ -3,7 +3,7 @@ // Basic stubs for MediaWiki testing // Stub constant - set to newer version to avoid class_alias issues -if (!defined('MW_VERSION')) { +if ( !defined('MW_VERSION') ) { define('MW_VERSION', '1.45.0'); } From 7611390a4bbfc03eff7ed92cb24d85f3d5247052 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sat, 25 Oct 2025 21:36:04 -0700 Subject: [PATCH 5/8] Update tests/phpunit/stubs.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/phpunit/stubs.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/stubs.php b/tests/phpunit/stubs.php index 4fffecb..82fd043 100644 --- a/tests/phpunit/stubs.php +++ b/tests/phpunit/stubs.php @@ -8,8 +8,8 @@ } // Stub function for wfMessage -if (!function_exists('wfMessage')) { - function wfMessage($key) { +if ( !function_exists( 'wfMessage' ) ) { + function wfMessage( $key ) { return new class { public function plain() { return 'Mock message'; } }; From fb98d561896b07434c81fb931626bc7d31ddc1e9 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sat, 25 Oct 2025 21:36:11 -0700 Subject: [PATCH 6/8] Update tests/phpunit/stubs.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/phpunit/stubs.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/stubs.php b/tests/phpunit/stubs.php index 82fd043..9982266 100644 --- a/tests/phpunit/stubs.php +++ b/tests/phpunit/stubs.php @@ -11,7 +11,9 @@ if ( !function_exists( 'wfMessage' ) ) { function wfMessage( $key ) { return new class { - public function plain() { return 'Mock message'; } + public function plain() { + return 'Mock message'; + } }; } } From 8481efbf1daa8e3db3af436e6e9a40df4fde4f90 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sun, 26 Oct 2025 04:42:38 +0000 Subject: [PATCH 7/8] Fix stub issue --- composer.json | 10 +++++----- tests/phpunit/stubs.php | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/composer.json b/composer.json index e60d3af..e757cbf 100644 --- a/composer.json +++ b/composer.json @@ -5,15 +5,15 @@ "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" ] - }, - "autoload-dev": { - "psr-4": { - "MediaWiki\\Extension\\CrawlerProtection\\Tests\\": "tests/phpunit/unit/" - } } } diff --git a/tests/phpunit/stubs.php b/tests/phpunit/stubs.php index 9982266..fa2ca5c 100644 --- a/tests/phpunit/stubs.php +++ b/tests/phpunit/stubs.php @@ -3,17 +3,17 @@ // Basic stubs for MediaWiki testing // Stub constant - set to newer version to avoid class_alias issues -if ( !defined('MW_VERSION') ) { - define('MW_VERSION', '1.45.0'); +if ( !defined( 'MW_VERSION' ) ) { + define( 'MW_VERSION', '1.45.0' ); } -// Stub function for wfMessage +// Stub function for wfMessage - only define if not already defined if ( !function_exists( 'wfMessage' ) ) { - function wfMessage( $key ) { - return new class { - public function plain() { - return 'Mock message'; - } - }; - } + function wfMessage( $key ) { + return new class { + public function plain() { + return 'Mock message'; + } + }; + } } From 37a75fe28f76841a94cdedc069c153e383922956 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Sun, 26 Oct 2025 04:50:08 +0000 Subject: [PATCH 8/8] Fix code styles --- includes/Hooks.php | 4 +- tests/phpunit/namespaced-stubs.php | 65 ++-- tests/phpunit/stubs.php | 2 +- tests/phpunit/unit/HooksTest.php | 528 ++++++++++++++--------------- 4 files changed, 307 insertions(+), 292 deletions(-) diff --git a/includes/Hooks.php b/includes/Hooks.php index dfef451..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() diff --git a/tests/phpunit/namespaced-stubs.php b/tests/phpunit/namespaced-stubs.php index 488c45a..4f288a4 100644 --- a/tests/phpunit/namespaced-stubs.php +++ b/tests/phpunit/namespaced-stubs.php @@ -2,54 +2,69 @@ // Hook interfaces namespace MediaWiki\Hook { - interface MediaWikiPerformActionHook { - public function onMediaWikiPerformAction($output, $article, $title, $user, $request, $mediaWiki); - } + interface MediaWikiPerformActionHook { + public function onMediaWikiPerformAction( $output, $article, $title, $user, $request, $mediaWiki ); + } } namespace MediaWiki\SpecialPage\Hook { - interface SpecialPageBeforeExecuteHook { - public function onSpecialPageBeforeExecute($special, $subPage); - } + interface SpecialPageBeforeExecuteHook { + public function onSpecialPageBeforeExecute( $special, $subPage ); + } } // Core classes in their proper namespaces namespace MediaWiki\Output { - class OutputPage { - public function setStatusCode($code) {} - public function addWikiTextAsInterface($text) {} - public function setPageTitle($title) {} - public function setPageTitleMsg($msg) {} - } + class OutputPage { + public function setStatusCode( $code ) { + } + public function addWikiTextAsInterface( $text ) { + } + public function setPageTitle( $title ) { + } + public function setPageTitleMsg( $msg ) { + } + } } namespace MediaWiki\SpecialPage { - class SpecialPage { - public function getName() { return ''; } - public function getContext() { return null; } - } + class SpecialPage { + public function getName() { + return ''; + } + public function getContext() { + return null; + } + } } namespace MediaWiki\User { - class User { - public function isRegistered() { return false; } - } + class User { + public function isRegistered() { + return false; + } + } } namespace MediaWiki\Request { - class WebRequest { - public function getVal($name, $default = null) { return $default; } - } + class WebRequest { + public function getVal( $name, $default = null ) { + return $default; + } + } } namespace MediaWiki\Title { - class Title {} + class Title { + } } namespace MediaWiki\Page { - class Article {} + class Article { + } } namespace MediaWiki\Actions { - class ActionEntryPoint {} + class ActionEntryPoint { + } } diff --git a/tests/phpunit/stubs.php b/tests/phpunit/stubs.php index fa2ca5c..1efe727 100644 --- a/tests/phpunit/stubs.php +++ b/tests/phpunit/stubs.php @@ -10,7 +10,7 @@ // Stub function for wfMessage - only define if not already defined if ( !function_exists( 'wfMessage' ) ) { function wfMessage( $key ) { - return new class { + return new class() { public function plain() { return 'Mock message'; } diff --git a/tests/phpunit/unit/HooksTest.php b/tests/phpunit/unit/HooksTest.php index b11803f..30b34b0 100644 --- a/tests/phpunit/unit/HooksTest.php +++ b/tests/phpunit/unit/HooksTest.php @@ -9,270 +9,270 @@ * @coversDefaultClass \MediaWiki\Extension\CrawlerProtection\Hooks */ class HooksTest extends TestCase { - /** @var string */ - private static string $actionEntryPointClassName; + /** @var string */ + private static string $actionEntryPointClassName; - /** @var string */ - private static string $articleClassName; + /** @var string */ + private static string $articleClassName; - /** @var string */ - private static string $outputPageClassName; - - /** @var string */ - private static string $specialPageClassName; - - /** @var string */ - private static string $titleClassName; - - /** @var string */ - private static string $userClassName; - - /** @var string */ - private static string $webRequestClassName; - - public static function setUpBeforeClass(): void { - self::$actionEntryPointClassName = class_exists( '\MediaWiki\Actions\ActionEntryPoint' ) - ? '\MediaWiki\Actions\ActionEntryPoint' - : '\MediaWiki'; - - self::$articleClassName = class_exists( '\MediaWiki\Page\Article' ) - ? '\MediaWiki\Page\Article' - : '\Article'; - - self::$outputPageClassName = class_exists( '\MediaWiki\Output\OutputPage' ) - ? '\MediaWiki\Output\OutputPage' - : '\OutputPage'; - - self::$specialPageClassName = class_exists( '\MediaWiki\SpecialPage\SpecialPage' ) - ? '\MediaWiki\SpecialPage\SpecialPage' - : '\SpecialPage'; - - self::$titleClassName = class_exists( '\MediaWiki\Title\Title' ) - ? '\MediaWiki\Title\Title' - : '\Title'; - - self::$userClassName = class_exists( '\MediaWiki\User\User' ) - ? '\MediaWiki\User\User' - : '\User'; - - self::$webRequestClassName = class_exists( '\MediaWiki\Request\WebRequest' ) - ? '\MediaWiki\Request\WebRequest' - : '\WebRequest'; - } - - /** - * @covers ::onMediaWikiPerformAction - */ - public function testRevisionTypeBlocksAnonymous() { - $output = $this->createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'revision' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( false ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->once() )->method( 'denyAccess' ); - - $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); - $this->assertFalse( $result ); - } - - /** - * @covers ::onMediaWikiPerformAction - */ - public function testRevisionTypeAllowsLoggedIn() { - $output = $this->createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'revision' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( true ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->never() )->method( 'denyAccess' ); - - $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); - $this->assertTrue( $result ); - } - - /** - * @covers ::onMediaWikiPerformAction - */ - public function testNonRevisionTypeAlwaysAllowed() { - $output = $this->createMock( self::$outputPageClassName ); - - $request = $this->createMock( self::$webRequestClassName ); - $request->method( 'getVal' )->willReturnMap( [ - [ 'type', null, 'view' ], - ] ); - - $user = $this->createMock( self::$userClassName ); - $user->method( 'isRegistered' )->willReturn( false ); - - $article = $this->createMock( self::$articleClassName ); - $title = $this->createMock( self::$titleClassName ); - $wiki = $this->createMock( self::$actionEntryPointClassName ); - - $runner = $this->getMockBuilder( Hooks::class ) - ->onlyMethods( [ 'denyAccess' ] ) - ->getMock(); - $runner->expects( $this->never() )->method( 'denyAccess' ); - - $result = $runner->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' ], - ]; - } + /** @var string */ + private static string $outputPageClassName; + + /** @var string */ + private static string $specialPageClassName; + + /** @var string */ + private static string $titleClassName; + + /** @var string */ + private static string $userClassName; + + /** @var string */ + private static string $webRequestClassName; + + public static function setUpBeforeClass(): void { + self::$actionEntryPointClassName = class_exists( '\MediaWiki\Actions\ActionEntryPoint' ) + ? '\MediaWiki\Actions\ActionEntryPoint' + : '\MediaWiki'; + + self::$articleClassName = class_exists( '\MediaWiki\Page\Article' ) + ? '\MediaWiki\Page\Article' + : '\Article'; + + self::$outputPageClassName = class_exists( '\MediaWiki\Output\OutputPage' ) + ? '\MediaWiki\Output\OutputPage' + : '\OutputPage'; + + self::$specialPageClassName = class_exists( '\MediaWiki\SpecialPage\SpecialPage' ) + ? '\MediaWiki\SpecialPage\SpecialPage' + : '\SpecialPage'; + + self::$titleClassName = class_exists( '\MediaWiki\Title\Title' ) + ? '\MediaWiki\Title\Title' + : '\Title'; + + self::$userClassName = class_exists( '\MediaWiki\User\User' ) + ? '\MediaWiki\User\User' + : '\User'; + + self::$webRequestClassName = class_exists( '\MediaWiki\Request\WebRequest' ) + ? '\MediaWiki\Request\WebRequest' + : '\WebRequest'; + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testRevisionTypeBlocksAnonymous() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'revision' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->once() )->method( 'denyAccess' ); + + $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); + $this->assertFalse( $result ); + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testRevisionTypeAllowsLoggedIn() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'revision' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( true ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki ); + $this->assertTrue( $result ); + } + + /** + * @covers ::onMediaWikiPerformAction + */ + public function testNonRevisionTypeAlwaysAllowed() { + $output = $this->createMock( self::$outputPageClassName ); + + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnMap( [ + [ 'type', null, 'view' ], + ] ); + + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + + $article = $this->createMock( self::$articleClassName ); + $title = $this->createMock( self::$titleClassName ); + $wiki = $this->createMock( self::$actionEntryPointClassName ); + + $runner = $this->getMockBuilder( Hooks::class ) + ->onlyMethods( [ 'denyAccess' ] ) + ->getMock(); + $runner->expects( $this->never() )->method( 'denyAccess' ); + + $result = $runner->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' ], + ]; + } }