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

can't set a new parameter to have no default value #8350

Closed
joachim-n opened this issue Dec 11, 2023 · 3 comments · Fixed by rectorphp/rector-src#5382
Closed

can't set a new parameter to have no default value #8350

joachim-n opened this issue Dec 11, 2023 · 3 comments · Fixed by rectorphp/rector-src#5382
Labels

Comments

@joachim-n
Copy link

joachim-n commented Dec 11, 2023

Bug Report

Subject Details
Rector version 0.18.12

Minimal PHP Code Causing Issue

There doesn't seem to be a way to add a new method or function parameter without a default value. E.g.

    $rectorConfig->ruleWithConfiguration(
      ArgumentAdderRector::class,
      [
        new ArgumentAdder(
          $class,
          'addToGeneratorDefinition',
          0,
          'definition',
          NULL,
          new ObjectType(PropertyDefinition::class),
        ),
      ],
    );

produces this change:

-  public static function addToGeneratorDefinition(): PropertyDefinition {
+  public static function addToGeneratorDefinition(PropertyDefinition $definition = null): PropertyDefinition {

If I remove the type and the NULL like this:

    $rectorConfig->ruleWithConfiguration(
      ArgumentAdderRector::class,
      [
        new ArgumentAdder(
          $class,
          'addToGeneratorDefinition',
          0,
          'definition',
        ),
      ],
    );

it still gets a default:

-  public static function addToGeneratorDefinition(): PropertyDefinition {
+  public static function addToGeneratorDefinition($definition = null): PropertyDefinition {

Expected Behaviour

@joachim-n joachim-n added the bug label Dec 11, 2023
@TomasVotruba
Copy link
Member

Thank you for your report!

We'll need an isolated failing demo link from: http://getrector.com/demo,
that way we can reproduce the bug.

@joachim-n
Copy link
Author

Done: https://getrector.com/demo/17294f15-9968-450f-ad7f-f443fa4e26c6

@samsonasik
Copy link
Member

I think to avoid ambiguosity, it need new value object: ArgumentAdderWithoutDefaultValue, so it will verify instanceof it, then don't add default value.

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

Successfully merging a pull request may close this issue.

3 participants