Changing method naming in FFI Type Reflection API fromArg->Parameter, etc

  115420
July 13, 2021 13:51 tysonandre775@hotmail.com (tyson andre)
Hi internals,

The FFI Type Reflection API mentioned in https://externals.io/message/115336 was recently added

My opinion is that that they should be renamed to use the same naming scheme that PHP's Reflection extension is already using.
Having different ways of naming very similar concepts (different from https://www.php.net/reflectionfunctionabstract) would make the language harder to remember.
I'd brought that up in https://github.com/php/php-src/pull/7217#pullrequestreview-700990479 with no response

What do others think about the name? I was considering holding a short vote
(on getReturnType, getParameterCount, getParameterType) before the feature freeze if there was interest

In particular,

- FFI\CData->getFuncReturnType should be changed to getReturnType - only functions have return types

  This is consistent with https://www.php.net/reflectionfunctionabstract 
- I believe Arg should be renamed to Parameter and Func should be removed from names where redundant.
  E.g. getFuncArgCount should be renamed to getParameterCount (getFuncArgType should be renamed getParameterType) - only functions have parameters,
  and PHP is already already using "Parameter" instead of "Argument" for reflection on types elsewhere.

  Parameter is used to refer to the function declarations (AST_PARAM internally in the AST, ReflectionFunctionAbstract->getParameters(), etc.)
  Argument is used to refer to expressions passed to the functions by the caller (ArgumentCountError, etc.)

  Other languages use similar definitions, e.g. https://developer.mozilla.org/en-US/docs/Glossary/Parameter
- The discussion over where FFI arrays should support Countable::count (and non-arrays should throw) might be contentious so I'd rather keep getArrayLength

Thanks,
Tyson
  115421
July 13, 2021 14:12 larry@garfieldtech.com ("Larry Garfield")
On Tue, Jul 13, 2021, at 8:51 AM, tyson andre wrote:
> Hi internals, > > The FFI Type Reflection API mentioned in > https://externals.io/message/115336 was recently added > > My opinion is that that they should be renamed to use the same naming > scheme that PHP's Reflection extension is already using. > Having different ways of naming very similar concepts (different from > https://www.php.net/reflectionfunctionabstract) would make the language > harder to remember. > I'd brought that up in > https://github.com/php/php-src/pull/7217#pullrequestreview-700990479 > with no response > > What do others think about the name? I was considering holding a short > vote > (on getReturnType, getParameterCount, getParameterType) before the > feature freeze if there was interest > > In particular, > > - FFI\CData->getFuncReturnType should be changed to getReturnType - > only functions have return types > > This is consistent with > https://www.php.net/reflectionfunctionabstract > - I believe Arg should be renamed to Parameter and Func should be > removed from names where redundant. > E.g. getFuncArgCount should be renamed to getParameterCount > (getFuncArgType should be renamed getParameterType) - only functions > have parameters, > and PHP is already already using "Parameter" instead of "Argument" > for reflection on types elsewhere. > > Parameter is used to refer to the function declarations (AST_PARAM > internally in the AST, ReflectionFunctionAbstract->getParameters(), > etc.) > Argument is used to refer to expressions passed to the functions by > the caller (ArgumentCountError, etc.) > > Other languages use similar definitions, e.g. > https://developer.mozilla.org/en-US/docs/Glossary/Parameter > - The discussion over where FFI arrays should support Countable::count > (and non-arrays should throw) might be contentious so I'd rather keep > getArrayLength
This all makes sense to me. Consistent naming is better unless there's a very specific reason to to otherwise. --Larry Garfield
  115422
July 13, 2021 14:18 tysonandre775@hotmail.com (tyson andre)
> > The FFI Type Reflection API mentioned in > > https://externals.io/message/115336 was recently added > > > > My opinion is that that they should be renamed to use the same naming > > scheme that PHP's Reflection extension is already using. > > Having different ways of naming very similar concepts (different from > > https://www.php.net/reflectionfunctionabstract) would make the language > > harder to remember. > > I'd brought that up in > > https://github.com/php/php-src/pull/7217#pullrequestreview-700990479 > > with no response > > > > What do others think about the name? I was considering holding a short > > vote > > (on getReturnType, getParameterCount, getParameterType) before the > > feature freeze if there was interest > > > > In particular, > > > > - FFI\CData->getFuncReturnType should be changed to getReturnType - > > only functions have return types > > > >   This is consistent with > > https://www.php.net/reflectionfunctionabstract > > - I believe Arg should be renamed to Parameter and Func should be > > removed from names where redundant. > >   E.g. getFuncArgCount should be renamed to getParameterCount > > (getFuncArgType should be renamed getParameterType) - only functions > > have parameters, > >   and PHP is already already using "Parameter" instead of "Argument" > > for reflection on types elsewhere. > > > >   Parameter is used to refer to the function declarations (AST_PARAM > > internally in the AST, ReflectionFunctionAbstract->getParameters(), > > etc.) > >   Argument is used to refer to expressions passed to the functions by > > the caller (ArgumentCountError, etc.) > > > >   Other languages use similar definitions, e.g. > > https://developer.mozilla.org/en-US/docs/Glossary/Parameter > > - The discussion over where FFI arrays should support Countable::count > > (and non-arrays should throw) might be contentious so I'd rather keep > > getArrayLength > > This all makes sense to me.  Consistent naming is better unless there's a very specific reason to to otherwise.
Created a PR https://github.com/php/php-src/pull/7236 Actually, looking at this again, I don't see a need to drop the "Func" - there's already getFuncABI. If you look at the current implementation, there's getStruct* for structures, getArray*, getPointer*, meaning `getFunc*` sort of makes sense for a naming scheme to make it easier to find functionality associated with a given func. Still, I find my proposal of Arg->Parameter continues to make sense to me. Thanks, Tyson