framework icon indicating copy to clipboard operation
framework copied to clipboard

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

Open tolgadevsec opened this issue 2 years ago • 3 comments

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

https://github.com/goaop/framework/blob/501d013c78e92057601c17ca0df1a59ab984f210/src/Proxy/Part/FunctionParameterList.php#L66

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: https://github.com/goaop/framework/blob/501d013c78e92057601c17ca0df1a59ab984f210/src/Proxy/Part/FunctionParameterList.php#L46

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

tolgadevsec avatar Jul 29 '21 19:07 tolgadevsec

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.

func0der avatar Feb 03 '22 15:02 func0der

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

lisachenko avatar Feb 04 '22 16:02 lisachenko

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

sakhunzai avatar Mar 06 '23 01:03 sakhunzai