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

Non-variadic parameter causes 'Variadic parameter cannot have a default value' exception #476

Closed
tolgadevsec opened this issue Jul 29, 2021 · 3 comments
Labels

Comments

@tolgadevsec
Copy link

tolgadevsec commented Jul 29, 2021

Hi!
I'm using the Go! AOP framework ("^3.0") in my Laravel project ("^8.40"), however, I'm running into the following exception thrown from the Laminas ParameterGenerator class when the following line is called:

https://github.com/laminas/laminas-code/blob/17fd2af36804f3f61788573cb70196454d6ee1d8/src/Generator/ParameterGenerator.php#L260

$generatedParameter->setVariadic($reflectionParameter->isVariadic());

It seems that the $defaultValue passed to the constructor of the ParameterGenerator in line 62 really needs to be null for optional and non-variadic parameters. This is not the case due to:

$defaultValue = new ValueGenerator(null);

I also have one case where I have a controller method with a default parameter value, this is a non-variadic parameter as well and runs into the same Exception. However, I was able to resolve both issues by explicitly setting the default value after the setVariadic call instead of doing it before. This has the reason that setVariadic is checking whether its default value attribute is set (and throws an Exception if thats the case) before it actually sets the variadic attribute to true or false.

Here is a snippet of what I have changed:

   // ...
   
  $defaultValue = null;

  // Move default value detection code to the bottom (after setVariadic)

  $parameterTypeName = null;
  if (!$useTypeWidening && $reflectionParameter->hasType()) {
      $parameterReflectionType = $reflectionParameter->getType();
      if ($parameterReflectionType instanceof ReflectionNamedType) {
          $parameterTypeName = $parameterReflectionType->getName();
      } else {
          $parameterTypeName = (string) $parameterReflectionType;
      }
  }

  $generatedParameter = new ParameterGenerator(
      $reflectionParameter->getName(),
      $parameterTypeName,
      $defaultValue, // We are passing null here for now
      $reflectionParameter->getPosition(),
      $reflectionParameter->isPassedByReference()
  );
  $generatedParameter->setVariadic($reflectionParameter->isVariadic());
  // The isset($this->defaultValue) wont throw a Exception now

  // Check if parameter is non-variadic and call setDefaultValue 
  if(!$reflectionParameter->isVariadic()){
      if ($reflectionParameter->isDefaultValueAvailable()) {
          $defaultValue = new ValueGenerator($reflectionParameter->getDefaultValue());
      } elseif ($reflectionParameter->isOptional()) {
          $defaultValue = new ValueGenerator(null);
      }
      
      if($defaultValue instanceof ValueGenerator) {
          $generatedParameter->setDefaultValue($defaultValue);
      }
  }

  // ...

Hope this can be of use to anyone running into the same issue. Let me know if further information are required for reproduction - the Laravel project that I'm working on is pretty much an empty project with one HTTP controller.

Kind regards,
Tolga

@func0der
Copy link
Contributor

func0der commented Feb 3, 2022

Same problem with a laminas project here.
It probably worked before this PR got merged: https://github.com/laminas/laminas-code/pull/72/files which was merged into 4.2.0.

Can we find a fix for this, please?
Maybe @tolgadevsec can make a PR for his changes so they can be evaluated against the tests.

@lisachenko
Copy link
Member

Hey, @mchekin, stop breaking the laminas-code API ) Your change has introduced a BC break in minor version and affected my framework.

@sakhunzai
Copy link

@lisachenko the issue in upstream lib is fixed so this issue is also resolved in laminas/laminas-code v4.9.0

lisachenko added a commit that referenced this issue Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants