Use NullableTypeSniff to check for implicitly nullable types in php8.4

Remove the php7.1 check, as that is obsolete Remove test case with required parameter follows by optional, as that is deprecated in php8.0 This includes an autofix to add the missing ? Bug: T362014 Change-Id: Ide15839e98a6229c22584d1c1c88c690982e1d7a
wikimedia · Oct 16, 2024 · 477ac00 · 477ac00
1 parent 6d16a0f
commit 477ac00
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 75 deletions.
diff --git a/MediaWiki/Sniffs/Usage/NullableTypeSniff.php b/MediaWiki/Sniffs/Usage/NullableTypeSniff.php
@@ -24,10 +24,7 @@
 use PHP_CodeSniffer\Sniffs\Sniff;
 
 /**
- * Enforce use of `?MyClass $x` instead of `MyClass $x = null`, which is (correctly)
- * misinterpreted as optional by IDEs and static analysis tools.
- * This is only done for nullable types followed by required parameters.
- * Note that we don't offer an autofix, because changing a signature should be verified carefully.
+ * Enforce use of `?MyClass $x = null` or `?MyClass $x` instead of `MyClass $x = null`, which is deprecated in php8.4.
  */
 class NullableTypeSniff implements Sniff {
 	/**
@@ -45,28 +42,23 @@ public function register(): array {
 	public function process( File $phpcsFile, $stackPtr ) {
 		$params = $phpcsFile->getMethodParameters( $stackPtr );
 
-		$found = $temp = [];
 		foreach ( $params as $param ) {
 			if (
 				$param['type_hint'] &&
 				$param['nullable_type'] === false &&
 				array_key_exists( 'default', $param ) &&
 				$param['default'] === 'null'
 			) {
-				$temp[] = $param;
-			} elseif ( !array_key_exists( 'default', $param ) ) {
-				$found = array_merge( $found, $temp );
-				$temp = [];
+				$fix = $phpcsFile->addFixableError(
+					'Use PHP 8.4 compatible syntax for explicit nullable types ("?%s %s = %s")',
+					$param['type_hint_token'],
+					'ExplicitNullableTypes',
+					[ $param['type_hint'], $param['name'], $param['default'] ]
+				);
+				if ( $fix ) {
+					$phpcsFile->fixer->addContentBefore( $param['type_hint_token'], '?' );
+				}
 			}
 		}
-
-		foreach ( $found as $param ) {
-			$phpcsFile->addError(
-				'Use PHP 7.1 syntax for nullable parameters ("?%s %s")',
-				$param['token'],
-				'PHP71NullableStyle',
-				[ $param['type_hint'], $param['name'] ]
-			);
-		}
 	}
 }
diff --git a/MediaWiki/Tests/files/Usage/nullable_type.php b/MediaWiki/Tests/files/Usage/nullable_type.php
@@ -9,42 +9,24 @@ public function docNullableArgOptional( $foo, MyClass $x = null ) {
 	}
 
 	/**
-	 * @param MyClass|null $x
 	 * @param string $foo
-	 */
-	public function nullableBeforeRequired( MyClass $x = null, $foo ) {
-	}
-
-	/**
 	 * @param MyClass|null $x
-	 * @param MyClass|null $y
-	 * @param string $foo
 	 */
-	public function nullablesBeforeRequired( MyClass $x = null, MyClass $y = null, $foo ) {
+	public function nullableAfterRequired( $foo, MyClass $x = null ) {
 	}
 
 	/**
-	 * @param MyClass|null $x
 	 * @param string $foo
-	 * @param MyClass|null $y
-	 */
-	public function mixedNullablesAndRequired1( MyClass $x = null, $foo, MyClass $y = null ) {
-	}
-
-	/**
 	 * @param MyClass|null $x
-	 * @param string $foo
 	 * @param MyClass|null $y
-	 * @param string $bar
 	 */
-	public function mixedNullablesAndRequired2( MyClass $x = null, $foo, MyClass $y = null, $bar ) {
+	public function nullableAfterRequired2( $foo, MyClass $x = null, MyClass $y = null ) {
 	}
 
 	/**
 	 * @param MyClass|null $x
-	 * @param ?MyClass $y
 	 */
-	public function mixedNullablesAndRequired3( MyClass $x = null, ?MyClass $y ) {
+	public function nullableOnly( MyClass $x = null ) {
 	}
 }
 
@@ -56,31 +38,11 @@ class PassedExamples {
 	public function genericNullable( ?MyClass $x, $foo ) {
 	}
 
-	/**
-	 * @param string $foo
-	 * @param MyClass|null $x
-	 */
-	public function nullableAfterRequired( $foo, MyClass $x = null ) {
-	}
-
-	/**
-	 * @param MyClass|null $x
-	 */
-	public function nullableOnly( MyClass $x = null ) {
-	}
-
 	/**
 	 * We allow this per T218816.
 	 * @param string $foo
 	 * @param MyClass|null $x
 	 */
 	public function docOptionalArgNullable( $foo, ?MyClass $x ) {
 	}
-
-	/**
-	 * @param ?MyClass $x
-	 * @param string $foo
-	 */
-	public function nullableWithDefault( ?MyClass $x = null, string $foo ) {
-	}
 }
diff --git a/MediaWiki/Tests/files/Usage/nullable_type.php.expect b/MediaWiki/Tests/files/Usage/nullable_type.php.expect
@@ -1,16 +1,13 @@
-  8 | ERROR | Use nullable type("?MyClass") for parameters documented as nullable
-    |       | (MediaWiki.Commenting.FunctionComment.PHP71NullableDocOptionalArg)
- 15 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $x")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 23 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $x")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 23 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $y")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 31 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $x")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 40 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $x")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 40 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $y")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
- 47 | ERROR | Use PHP 7.1 syntax for nullable parameters ("?MyClass $x")
-    |       | (MediaWiki.Usage.NullableType.PHP71NullableStyle)
+  8 | ERROR | [x] Use PHP 8.4 compatible syntax for explicit nullable types ("?MyClass $x = null")
+    |       |     (MediaWiki.Usage.NullableType.ExplicitNullableTypes)
+  8 | ERROR | [ ] Use nullable type("?MyClass") for parameters documented as nullable
+    |       |     (MediaWiki.Commenting.FunctionComment.PHP71NullableDocOptionalArg)
+ 15 | ERROR | [x] Use PHP 8.4 compatible syntax for explicit nullable types ("?MyClass $x = null")
+    |       |     (MediaWiki.Usage.NullableType.ExplicitNullableTypes)
+ 23 | ERROR | [x] Use PHP 8.4 compatible syntax for explicit nullable types ("?MyClass $x = null")
+    |       |     (MediaWiki.Usage.NullableType.ExplicitNullableTypes)
+ 23 | ERROR | [x] Use PHP 8.4 compatible syntax for explicit nullable types ("?MyClass $y = null")
+    |       |     (MediaWiki.Usage.NullableType.ExplicitNullableTypes)
+ 29 | ERROR | [x] Use PHP 8.4 compatible syntax for explicit nullable types ("?MyClass $x = null")
+    |       |     (MediaWiki.Usage.NullableType.ExplicitNullableTypes)
+PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
diff --git a/MediaWiki/Tests/files/Usage/nullable_type.php.fixed b/MediaWiki/Tests/files/Usage/nullable_type.php.fixed
@@ -0,0 +1,48 @@
+<?php
+// phpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound
+class FailedExamples {
+	/**
+	 * @param string $foo
+	 * @param ?MyClass $x
+	 */
+	public function docNullableArgOptional( $foo, ?MyClass $x = null ) {
+	}
+
+	/**
+	 * @param string $foo
+	 * @param MyClass|null $x
+	 */
+	public function nullableAfterRequired( $foo, ?MyClass $x = null ) {
+	}
+
+	/**
+	 * @param string $foo
+	 * @param MyClass|null $x
+	 * @param MyClass|null $y
+	 */
+	public function nullableAfterRequired2( $foo, ?MyClass $x = null, ?MyClass $y = null ) {
+	}
+
+	/**
+	 * @param MyClass|null $x
+	 */
+	public function nullableOnly( ?MyClass $x = null ) {
+	}
+}
+
+class PassedExamples {
+	/**
+	 * @param ?MyClass $x
+	 * @param string $foo
+	 */
+	public function genericNullable( ?MyClass $x, $foo ) {
+	}
+
+	/**
+	 * We allow this per T218816.
+	 * @param string $foo
+	 * @param MyClass|null $x
+	 */
+	public function docOptionalArgNullable( $foo, ?MyClass $x ) {
+	}
+}