From 5dc4d1a1f6368911de621d67095de705acd468b7 Mon Sep 17 00:00:00 2001
From: Daniel Scherzer <daniel@wikiteq.com>
Date: Wed, 10 Jul 2024 20:29:36 -0700
Subject: [PATCH] Set up initial Phan plugin

Create a Phan plugin for use with MediaWiki extensions or skins that checks
that classes that can have dependencies injected do indeed have those
dependencies injected.

SEL-758
---
 .gitattributes                            |   1 +
 .github/workflows/ci.yml                  |  37 ++++++++
 .gitignore                                |   3 +
 .phpcs.xml                                |   9 ++
 LICENSE                                   |  23 +++++
 MediaWikiServicesCheckPlugin.php          | 100 ++++++++++++++++++++++
 composer.json                             |  35 ++++++++
 phpunit.xml                               |  15 ++++
 tests/PluginTest.php                      |  45 ++++++++++
 tests/cases/Api/NormalApi.php             |  13 +++
 tests/cases/Api/QueryApi.php              |  13 +++
 tests/cases/Api/expected.txt              |   2 +
 tests/cases/SpecialPage/FormSpecial.php   |  13 +++
 tests/cases/SpecialPage/MySpecial.php     |  25 ++++++
 tests/cases/SpecialPage/expected.txt      |   4 +
 tests/cases/ValidUsage/MisleadingName.php |  16 ++++
 tests/cases/ValidUsage/MySpecial.php      |  17 ++++
 tests/cases/ValidUsage/expected.txt       |   0
 tests/cases/ValidUsage/globalContext.php  |   8 ++
 tests/stubs/BaseClasses.php               |  16 ++++
 tests/stubs/MediaWikiServices.php         |  31 +++++++
 tests/test-config.php                     |  16 ++++
 22 files changed, 442 insertions(+)
 create mode 100644 .gitattributes
 create mode 100644 .github/workflows/ci.yml
 create mode 100644 .gitignore
 create mode 100644 .phpcs.xml
 create mode 100644 LICENSE
 create mode 100644 MediaWikiServicesCheckPlugin.php
 create mode 100644 composer.json
 create mode 100644 phpunit.xml
 create mode 100644 tests/PluginTest.php
 create mode 100644 tests/cases/Api/NormalApi.php
 create mode 100644 tests/cases/Api/QueryApi.php
 create mode 100644 tests/cases/Api/expected.txt
 create mode 100644 tests/cases/SpecialPage/FormSpecial.php
 create mode 100644 tests/cases/SpecialPage/MySpecial.php
 create mode 100644 tests/cases/SpecialPage/expected.txt
 create mode 100644 tests/cases/ValidUsage/MisleadingName.php
 create mode 100644 tests/cases/ValidUsage/MySpecial.php
 create mode 100644 tests/cases/ValidUsage/expected.txt
 create mode 100644 tests/cases/ValidUsage/globalContext.php
 create mode 100644 tests/stubs/BaseClasses.php
 create mode 100644 tests/stubs/MediaWikiServices.php
 create mode 100644 tests/test-config.php

diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..fcadb2c
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+* text eol=lf
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..49cf4f2
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,37 @@
+name: Continuous Integration
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  style-php:
+    name: Code Style (PHP)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: wikiteq/php-lint-action@main
+
+  test:
+    name: PHPUnit
+    runs-on: ubuntu-latest
+    steps:
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.1'
+          extensions: mbstring, intl
+          coverage: none
+          tools: composer
+
+      - uses: actions/checkout@v4
+
+      - name: Setup Composer
+        run: composer update
+        shell: bash
+
+      - name: Run PHPUnit
+        uses: php-actions/phpunit@v4
+        with:
+          configuration: phpunit.xml
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..50b321e
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,3 @@
+vendor
+composer.lock
+.phpunit.result.cache
diff --git a/.phpcs.xml b/.phpcs.xml
new file mode 100644
index 0000000..7ed09bb
--- /dev/null
+++ b/.phpcs.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<ruleset>
+	<rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki" />
+	<file>.</file>
+	<arg name="extensions" value="php"/>
+	<arg name="encoding" value="UTF-8"/>
+	<exclude-pattern type="relative">^tests/cases/*</exclude-pattern>
+	<exclude-pattern type="relative">^tests/stubs/*</exclude-pattern>
+</ruleset>
diff --git a/LICENSE b/LICENSE
new file mode 100644
index 0000000..3036411
--- /dev/null
+++ b/LICENSE
@@ -0,0 +1,23 @@
+WikiTeq Proprietary License
+
+Copyright (c) 2024 WikiTeq
+
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are not permitted without prior written consent from WikiTeq.
+
+Neither the name of WikiTeq nor the names of its contributors may be used to
+endorse or promote products derived from this software without specific prior
+written permission.
+
+THIS SOFTWARE IS PROVIDED BY WIKITEQ "AS IS" AND ANY EXPRESS OR IMPLIED
+WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+EVENT SHALL WIKITEQ BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
diff --git a/MediaWikiServicesCheckPlugin.php b/MediaWikiServicesCheckPlugin.php
new file mode 100644
index 0000000..3bb2af2
--- /dev/null
+++ b/MediaWikiServicesCheckPlugin.php
@@ -0,0 +1,100 @@
+<?php
+
+declare( strict_types=1 );
+
+namespace WikiTeq\MediaWikiServicesCheckPlugin;
+
+use ast\Node;
+use Phan\AST\UnionTypeVisitor;
+use Phan\Language\Type;
+use Phan\Language\UnionType;
+use Phan\PluginV3;
+use Phan\PluginV3\PluginAwarePostAnalysisVisitor;
+use Phan\PluginV3\PostAnalyzeNodeCapability;
+
+/**
+ * Plugin to add our MediaWikiServicesVisitor that gets used for every AST node
+ */
+class MediaWikiServicesCheckPlugin extends PluginV3 implements
+	PostAnalyzeNodeCapability
+{
+
+	public static function getPostAnalyzeNodeVisitorClassName(): string {
+		return MediaWikiServicesVisitor::class;
+	}
+}
+
+/**
+ * Visitor that detects MediaWikiServices::getInstance()->getService() and
+ * similar in places where services can (and should) be injected.
+ */
+// phpcs:ignore Generic.Files.OneObjectStructurePerFile.MultipleFound
+class MediaWikiServicesVisitor extends PluginAwarePostAnalysisVisitor {
+
+	public function visitMethodCall( Node $node ): void {
+		$typeUnion = UnionTypeVisitor::unionTypeFromNode(
+			$this->code_base,
+			$this->context,
+			$node->children['expr']
+		);
+
+		// Use UnionType to simplify comparison with the type of expr
+		$mwServicesType = UnionType::fromFullyQualifiedPHPDocString(
+			'\\MediaWiki\\MediaWikiServices'
+		);
+		$isMWServices = $mwServicesType->isEqualTo( $typeUnion );
+		if ( !$isMWServices ) {
+			return;
+		}
+
+		// Check that we are in a non-static method in a class that should
+		// support dependency injection
+		if ( !$this->context->isInClassScope()
+			|| !$this->context->isInFunctionLikeScope()
+		) {
+			return;
+		}
+
+		$funcLike = $this->context->getFunctionLikeInScope( $this->code_base );
+		if ( $funcLike->isStatic() ) {
+			return;
+		}
+
+		$method = $node->children['method'];
+		$methodStart = substr( $method, 0, 3 );
+		if ( $methodStart !== 'get' && $methodStart !== 'has' ) {
+			// Something more complicated like peeking, ignore
+			return;
+		}
+
+		// Map of class to detect extending => placeholder for message
+		// The *FIRST* matching message is used, to allow for more specific
+		// messages
+		$baseClassMap = [
+			'\\SpecialPage' => 'Special pages',
+			'\\ApiQueryBase' => 'API query modules',
+			'\\ApiBase' => 'API modules',
+		];
+
+		$scope = $this->context->getScope();
+		$currentClass = $scope->getClassFQSEN()->asType();
+		foreach ( $baseClassMap as $baseClass => $msg ) {
+			$baseClassType = Type::fromFullyQualifiedString( $baseClass );
+			if ( !$currentClass->isSubclassOf(
+					$baseClassType,
+					$this->code_base
+			) ) {
+				continue;
+			}
+			$this->emit(
+				'MediaWikiServicesAccessed',
+				'%s should have services injected with dependency injection',
+				[ $msg ]
+			);
+			return;
+		}
+	}
+}
+
+// Plugins return an instance of themselves at the end of their definition files
+return new MediaWikiServicesCheckPlugin();
diff --git a/composer.json b/composer.json
new file mode 100644
index 0000000..dded780
--- /dev/null
+++ b/composer.json
@@ -0,0 +1,35 @@
+{
+	"name": "wikiteq/phan-mediawikiservices-check-plugin",
+	"description": "Phan plugin for checking uses of MediaWiki's MediaWikiServices class",
+	"authors": [
+		{
+			"name": "Daniel Scherzer",
+			"email": "daniel@wikiteq.com"
+		}
+	],
+	"require": {
+		"phan/phan": "5.4.3",
+		"php": ">=7.4.0"
+	},
+	"require-dev": {
+		"mediawiki/mediawiki-codesniffer": "43.0.0",
+		"php-parallel-lint/php-console-highlighter": "1.0.0",
+		"php-parallel-lint/php-parallel-lint": "1.4.0",
+		"phpunit/phpunit": "9.6.16"
+	},
+	"scripts": {
+		"test": [
+			"composer phpcs",
+			"phpunit"
+		],
+		"phpcs": "phpcs -p -s",
+		"fix": [
+			"phpcbf"
+		]
+	},
+	"config": {
+		"allow-plugins": {
+			"dealerdirect/phpcodesniffer-composer-installer": true
+		}
+	}
+}
\ No newline at end of file
diff --git a/phpunit.xml b/phpunit.xml
new file mode 100644
index 0000000..dfaecef
--- /dev/null
+++ b/phpunit.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<phpunit
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    convertErrorsToExceptions="true"
+    convertNoticesToExceptions="true"
+    convertWarningsToExceptions="true"
+    xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
+>
+    <coverage/>
+    <testsuites>
+        <testsuite name="MediaWikiServicesCheckPlugin tests">
+            <directory>./tests</directory>
+        </testsuite>
+    </testsuites>
+</phpunit>
diff --git a/tests/PluginTest.php b/tests/PluginTest.php
new file mode 100644
index 0000000..65d4ce4
--- /dev/null
+++ b/tests/PluginTest.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace WikiTeq\MediaWikiServicesCheckPlugin\Tests;
+
+use DirectoryIterator;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \WikiTeq\MediaWikiServicesCheckPlugin\MediaWikiServicesCheckPlugin
+ */
+class PluginTest extends TestCase {
+
+	/** @dataProvider provideTestCases */
+	public function testScenarios( $testCaseDir, $expectedIssues ) {
+		$testDirPath = realpath( './tests' );
+		// $this->assertSame( $testDirPath, '' );
+		// Go back to the main directory
+		chdir( __DIR__ . '/../' );
+		// Build the command to run
+		$cmd = "php vendor/phan/phan/phan" .
+			" --allow-polyfill-parser" .
+			" -d \"$testDirPath\"" .
+			" -k \"test-config.php\"" .
+			" -l \"stubs\"" .
+			" -l \"cases/$testCaseDir\"";
+		// phpcs:ignore MediaWiki.Usage.ForbiddenFunctions.shell_exec
+		$phanOutput = shell_exec( $cmd ) ?? '';
+		// Trim to avoid issues with newlines
+		$this->assertSame( trim( $expectedIssues ), trim( $phanOutput ) );
+	}
+
+	public function provideTestCases() {
+		$dirIter = new DirectoryIterator( __DIR__ . '/cases' );
+		foreach ( $dirIter as $directory ) {
+			if ( !$directory->isDot() ) {
+				$folder = $directory->getPathname();
+				// In `/cases` we have a bunch of sub directories, each with
+				// PHP files to analyze and then `expected.txt` with the results
+				$testName = basename( $folder );
+				$expected = file_get_contents( $folder . '/expected.txt' );
+				yield $testName => [ $testName, $expected ];
+			}
+		}
+	}
+}
diff --git a/tests/cases/Api/NormalApi.php b/tests/cases/Api/NormalApi.php
new file mode 100644
index 0000000..6f88099
--- /dev/null
+++ b/tests/cases/Api/NormalApi.php
@@ -0,0 +1,13 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class NormalApi extends ApiBase {
+
+    public function test1( $par ) {
+        // getService() with arbitrary service name
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/Api/QueryApi.php b/tests/cases/Api/QueryApi.php
new file mode 100644
index 0000000..76af20d
--- /dev/null
+++ b/tests/cases/Api/QueryApi.php
@@ -0,0 +1,13 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class QueryApi extends ApiQueryBase {
+
+    public function test1( $par ) {
+        // getService() with arbitrary service name
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/Api/expected.txt b/tests/cases/Api/expected.txt
new file mode 100644
index 0000000..99391e2
--- /dev/null
+++ b/tests/cases/Api/expected.txt
@@ -0,0 +1,2 @@
+cases/Api/NormalApi.php:9 MediaWikiServicesAccessed API modules should have services injected with dependency injection
+cases/Api/QueryApi.php:9 MediaWikiServicesAccessed API query modules should have services injected with dependency injection
\ No newline at end of file
diff --git a/tests/cases/SpecialPage/FormSpecial.php b/tests/cases/SpecialPage/FormSpecial.php
new file mode 100644
index 0000000..e82bae2
--- /dev/null
+++ b/tests/cases/SpecialPage/FormSpecial.php
@@ -0,0 +1,13 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class MyFormSpecial extends FormSpecialPage {
+
+    public function test1( $par ) {
+        // getService() with arbitrary service name
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/SpecialPage/MySpecial.php b/tests/cases/SpecialPage/MySpecial.php
new file mode 100644
index 0000000..129eef7
--- /dev/null
+++ b/tests/cases/SpecialPage/MySpecial.php
@@ -0,0 +1,25 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class MySpecial extends SpecialPage {
+
+    public function test1( $par ) {
+        // getService() with arbitrary service name
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+    public function test2( $par ) {
+        // get() with arbitrary service name
+        $service = MediaWikiServices::getInstance()->get( 'MyService' );
+        $service->run( $par );
+    }
+
+    public function test3( $par ) {
+        // service-specific getter
+        $service = MediaWikiServices::getInstance()->getUserFactory();
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/SpecialPage/expected.txt b/tests/cases/SpecialPage/expected.txt
new file mode 100644
index 0000000..679dc0b
--- /dev/null
+++ b/tests/cases/SpecialPage/expected.txt
@@ -0,0 +1,4 @@
+cases/SpecialPage/FormSpecial.php:9 MediaWikiServicesAccessed Special pages should have services injected with dependency injection
+cases/SpecialPage/MySpecial.php:9 MediaWikiServicesAccessed Special pages should have services injected with dependency injection
+cases/SpecialPage/MySpecial.php:15 MediaWikiServicesAccessed Special pages should have services injected with dependency injection
+cases/SpecialPage/MySpecial.php:21 MediaWikiServicesAccessed Special pages should have services injected with dependency injection
diff --git a/tests/cases/ValidUsage/MisleadingName.php b/tests/cases/ValidUsage/MisleadingName.php
new file mode 100644
index 0000000..c7226f4
--- /dev/null
+++ b/tests/cases/ValidUsage/MisleadingName.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace FooBar;
+
+use MediaWiki\MediaWikiServices;
+
+// Not the real SpecialPage class since we are in a namespace
+// @phan-suppress-next-line PhanUndeclaredExtendedClass
+class MySpecial extends SpecialPage {
+
+    public function test1() {
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/ValidUsage/MySpecial.php b/tests/cases/ValidUsage/MySpecial.php
new file mode 100644
index 0000000..cb1eecd
--- /dev/null
+++ b/tests/cases/ValidUsage/MySpecial.php
@@ -0,0 +1,17 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class MySpecial extends SpecialPage {
+
+    public function testPeek() {
+        return MediaWikiServices::getInstance()->peekService( 'MyService' );
+    }
+
+    public static function testStatic( $par ) {
+        // Static method should be ignored
+        $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+        $service->run( $par );
+    }
+
+}
\ No newline at end of file
diff --git a/tests/cases/ValidUsage/expected.txt b/tests/cases/ValidUsage/expected.txt
new file mode 100644
index 0000000..e69de29
diff --git a/tests/cases/ValidUsage/globalContext.php b/tests/cases/ValidUsage/globalContext.php
new file mode 100644
index 0000000..8092f08
--- /dev/null
+++ b/tests/cases/ValidUsage/globalContext.php
@@ -0,0 +1,8 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+function test1() {
+    $service = MediaWikiServices::getInstance()->getService( 'MyService' );
+    $service->run( $par );
+}
diff --git a/tests/stubs/BaseClasses.php b/tests/stubs/BaseClasses.php
new file mode 100644
index 0000000..ffb19f7
--- /dev/null
+++ b/tests/stubs/BaseClasses.php
@@ -0,0 +1,16 @@
+<?php
+
+// Stubs for testing
+class SpecialPage {
+
+	public function __construct($a, $b) {
+	}
+}
+
+// For confirming that sub-sub classes also work
+class FormSpecialPage extends SpecialPage {}
+
+class ApiBase {}
+
+class ApiQueryBase extends ApiBase {}
+
diff --git a/tests/stubs/MediaWikiServices.php b/tests/stubs/MediaWikiServices.php
new file mode 100644
index 0000000..7d8f140
--- /dev/null
+++ b/tests/stubs/MediaWikiServices.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace MediaWiki;
+
+// Stub for testing
+class MediaWikiServices {
+
+	public static function getInstance(): self {
+		return new MediaWikiServices();
+	}
+
+	public function get( string $name ) {
+		return $this->getService( $name );
+	}
+	
+	public function getService( string $name ) {
+		return $name;
+	}
+
+	public function peekService( string $name ) {
+		return $name;
+	}
+
+	public function getUserFactory() {
+		return 1;
+	}
+
+	public function getUserOptionsManager() {
+		return 2;
+	}
+}
\ No newline at end of file
diff --git a/tests/test-config.php b/tests/test-config.php
new file mode 100644
index 0000000..2efad52
--- /dev/null
+++ b/tests/test-config.php
@@ -0,0 +1,16 @@
+<?php
+
+// Configuration for tests
+
+$cfg = [];
+
+$cfg['plugins'] = [
+	__DIR__ . '/../MediaWikiServicesCheckPlugin.php'
+];
+
+// Don't break on the nonsense calls to the fake services
+$cfg['suppress_issue_types'] = [
+	'PhanNonClassMethodCall',
+];
+
+return $cfg;