Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TASK: Correctly mark nullable method parameters as nullable #3427

Open
wants to merge 1 commit into
base: 8.3
Choose a base branch
from

Conversation

Benjamin-K
Copy link
Contributor

Review instructions

This PR will correctly mark implicit nullable types as nullable, which will remove deprecation warnings in PHP 8.4.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@dlubitz
Copy link
Contributor

dlubitz commented Jan 8, 2025

Thank you @Benjamin-K. This is great. But we need to check why the tests are failing.

@Benjamin-K
Copy link
Contributor Author

@dlubitz I fixed the PR already. The only thing it now complains about is that I updated the bracket style of functions:

# before
function name() {}

# after
function name()
{
}

which is from the PSR-12: Extended Coding Style

@dlubitz
Copy link
Contributor

dlubitz commented Jan 8, 2025

No, you changed it the other way around:

-    public function collectGarbage(): void
-    {
-    }
+    public function collectGarbage(): void {}

@mhsdesign
Copy link
Member

which i think is correct, we dont use this shortsyntax because its not worth it ^^ having empty method bodys is rare either way and psr 12 prohibits it:

The opening brace MUST go on its own line, and the closing brace MUST go on the next line following the body.

To apply all style ci changes just use this :D

curl https://github.styleci.io/analyses/dj7o6p/diff | git apply -

@Benjamin-K
Copy link
Contributor Author

No, you changed it the other way around:

-    public function collectGarbage(): void
-    {
-    }
+    public function collectGarbage(): void {}

You're right, sry. Seems like vscode does not use the full PSR-12 set. I fixed this now.

@mhsdesign mhsdesign requested a review from bwaidelich January 8, 2025 15:48
@dlubitz dlubitz self-requested a review January 9, 2025 19:43
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. Thank you so much.
Even though you found so many places I found some additional while reviewing.

@@ -606,7 +607,7 @@ public static function getRecursionLimit(): int
* @return void|string if $return is true, the variable dump is returned. By default, the dump is directly displayed, and nothing is returned.
* @api
*/
function var_dump($variable, string $title = null, bool $return = false, bool $plaintext = null)
function var_dump($variable, string $title = null, bool $return = false, ?bool $plaintext = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function var_dump($variable, string $title = null, bool $return = false, ?bool $plaintext = null)
function var_dump($variable, ?string $title = null, bool $return = false, ?bool $plaintext = null)

@@ -70,7 +71,7 @@ class ConfigurationSchemaValidator
* @return \Neos\Error\Messages\Result the result of the validation
* @throws Exception\SchemaValidationException
*/
public function validate(string $configurationType = null, string $path = null, array &$loadedSchemaFiles = []): Result
public function validate(string $configurationType = null, ?string $path = null, array &$loadedSchemaFiles = []): Result
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function validate(string $configurationType = null, ?string $path = null, array &$loadedSchemaFiles = []): Result
public function validate(?string $configurationType = null, ?string $path = null, array &$loadedSchemaFiles = []): Result

@@ -38,7 +39,7 @@ class TypeConverterCommandController extends CommandController
* @param string $target Filter by target type
* @return void
*/
public function listCommand(string $source = null, string $target = null)
public function listCommand(string $source = null, ?string $target = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function listCommand(string $source = null, ?string $target = null)
public function listCommand(?string $source = null, ?string $target = null)

@@ -37,7 +38,7 @@ class SignalCommandController extends CommandController
* @param string $methodName if specified, only signals matching the given method name will be shown. This is only useful in conjunction with the "--class-name" option.
* @return void
*/
public function listConnectedCommand(string $className = null, string $methodName = null): void
public function listConnectedCommand(string $className = null, ?string $methodName = null): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function listConnectedCommand(string $className = null, ?string $methodName = null): void
public function listConnectedCommand(?string $className = null, ?string $methodName = null): void

@@ -83,7 +84,7 @@ public function open(): void
* @param string $methodName
* @return void
*/
public function append(string $message, int $severity = LOG_INFO, $additionalData = null, string $packageKey = null, string $className = null, string $methodName = null): void
public function append(string $message, int $severity = LOG_INFO, $additionalData = null, string $packageKey = null, string $className = null, ?string $methodName = null): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function append(string $message, int $severity = LOG_INFO, $additionalData = null, string $packageKey = null, string $className = null, ?string $methodName = null): void
public function append(string $message, int $severity = LOG_INFO, $additionalData = null, ?string $packageKey = null, ?string $className = null, ?string $methodName = null): void

@@ -196,7 +197,7 @@ public function getEntityStatus(): array
* @param int|null $maxResult
* @return mixed
*/
public function runDql(string $dql, int $hydrationMode = \Doctrine\ORM\Query::HYDRATE_OBJECT, int $firstResult = null, int $maxResult = null)
public function runDql(string $dql, int $hydrationMode = \Doctrine\ORM\Query::HYDRATE_OBJECT, int $firstResult = null, ?int $maxResult = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function runDql(string $dql, int $hydrationMode = \Doctrine\ORM\Query::HYDRATE_OBJECT, int $firstResult = null, ?int $maxResult = null)
public function runDql(string $dql, int $hydrationMode = \Doctrine\ORM\Query::HYDRATE_OBJECT, ?int $firstResult = null, ?int $maxResult = null)

@@ -160,7 +161,7 @@ public function getBaseValidatorConjunction($targetClassName, array $validationG
* @throws Exception\InvalidTypeHintException
* @throws Exception\InvalidValidationOptionsException
*/
public function buildMethodArgumentsValidatorConjunctions($className, $methodName, array $methodParameters = null, array $methodValidateAnnotations = null)
public function buildMethodArgumentsValidatorConjunctions($className, $methodName, array $methodParameters = null, ?array $methodValidateAnnotations = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function buildMethodArgumentsValidatorConjunctions($className, $methodName, array $methodParameters = null, ?array $methodValidateAnnotations = null)
public function buildMethodArgumentsValidatorConjunctions($className, $methodName, ?array $methodParameters = null, ?array $methodValidateAnnotations = null)

@@ -45,7 +46,7 @@ class DocumentationCommandController extends CommandController
* @param string $xsdDomain Domain used in the XSD schema (for example "http://yourdomain.org"). Defaults to "https://neos.io".
* @return void
*/
public function generateXsdCommand(string $phpNamespace, string $xsdNamespace = null, string $targetFile = null, string $xsdDomain = ''): void
public function generateXsdCommand(string $phpNamespace, string $xsdNamespace = null, ?string $targetFile = null, string $xsdDomain = ''): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function generateXsdCommand(string $phpNamespace, string $xsdNamespace = null, ?string $targetFile = null, string $xsdDomain = ''): void
public function generateXsdCommand(string $phpNamespace, ?string $xsdNamespace = null, ?string $targetFile = null, string $xsdDomain = ''): void

@@ -452,7 +453,7 @@ public function expandGenericPathPatternDataProvider()
* @param string $pattern
* @param string $expectedResult
*/
public function expandGenericPathPatternTests($package, $subPackage, $controller, $format, $templateRootPath, array $templateRootPaths = null, $partialRootPath, array $partialRootPaths = null, $layoutRootPath, array $layoutRootPaths = null, $bubbleControllerAndSubpackage, $formatIsOptional, $pattern, $expectedResult)
public function expandGenericPathPatternTests($package, $subPackage, $controller, $format, $templateRootPath, array $templateRootPaths = null, $partialRootPath, array $partialRootPaths = null, $layoutRootPath, ?array $layoutRootPaths = null, $bubbleControllerAndSubpackage, $formatIsOptional, $pattern, $expectedResult)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function expandGenericPathPatternTests($package, $subPackage, $controller, $format, $templateRootPath, array $templateRootPaths = null, $partialRootPath, array $partialRootPaths = null, $layoutRootPath, ?array $layoutRootPaths = null, $bubbleControllerAndSubpackage, $formatIsOptional, $pattern, $expectedResult)
public function expandGenericPathPatternTests($package, $subPackage, $controller, $format, $templateRootPath, ?array $templateRootPaths = null, $partialRootPath, ?array $partialRootPaths = null, $layoutRootPath, ?array $layoutRootPaths = null, $bubbleControllerAndSubpackage, $formatIsOptional, $pattern, $expectedResult)

@@ -15,7 +16,7 @@ trait UploadedFileFactoryTrait
/**
* @inheritDoc
*/
public function createUploadedFile(StreamInterface $stream, int $size = null, int $error = \UPLOAD_ERR_OK, string $clientFilename = null, string $clientMediaType = null): UploadedFileInterface
public function createUploadedFile(StreamInterface $stream, int $size = null, int $error = \UPLOAD_ERR_OK, string $clientFilename = null, ?string $clientMediaType = null): UploadedFileInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function createUploadedFile(StreamInterface $stream, int $size = null, int $error = \UPLOAD_ERR_OK, string $clientFilename = null, ?string $clientMediaType = null): UploadedFileInterface
public function createUploadedFile(StreamInterface $stream, ?int $size = null, int $error = \UPLOAD_ERR_OK, ?string $clientFilename = null, ?string $clientMediaType = null): UploadedFileInterface

@dlubitz
Copy link
Contributor

dlubitz commented Jan 9, 2025

I also stumbled over this rector rule, which should do the migration:
https://getrector.com/rule-detail/explicit-nullable-param-type-rector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants