global constant vs. class constant

[janpieper]

An optional method parameter with a constant as default value (e.g. SORT_ASC) will result in

  <parameter name="sort" optional="true" byreference="false">
    <default>3</default>
  </parameter>

but if it is using a class constant like self::DETECT it will result in

  <parameter name="type" optional="true" byreference="false">
    <default>'__StaticReflectionConstantValue(self::DETECT)'</default>
  </parameter>

Is this the expected behavior?

[theseer]

No, that is clearly a bug.

[LouisLandry]

After a cursory inspection I think the issue actually may lie upstream in the pdepend AST ... or at least how phpdox is interacting with it. I'm actually not entirely sure what the expected behavior should be, but I suspect it should be evaluating the constant and storing that in the XML.

[LouisLandry]

OK, so I have tracked it to here: https://github.com/manuelpichler/staticReflection/blob/master/src/main/php/pdepend/reflection/parser/Parser.php#L1296

If you run phpdox against the following folder in the Joomla Platform: https://github.com/joomla/joomla-platform/tree/master/libraries/joomla/log you can see the problem in action. Essentially in the static reflection parser the result of defined(JLog::ALL) or any of the other constants is coming back false. I'm not really sure why that would be, but in the 20 minutes I played around with it I was unsuccessful in figuring out how to get those values.

Hope that helps.

[LouisLandry]

It is already logged: https://www.pivotaltracker.com/story/show/9356191

[theseer]

I've been talking to Manuel about that issue a few times. Hence his ticket :)

We so far didn't fully agree though on what the actual expected behaviour in my context (==phpdox) vs. the expected behaviour in a Reflection API would be: From a documentation point of view it might be nice to have the actual value but it's probably more important to know the constant name since usually nobody uses the real value anyway as it is an implementation detail of the defining class. On the other hand in an static reflection, there might not even be code available to resolve the constant in question.

Any recommendation from your perspective?

[LouisLandry]

It is a good question actually. From my point of view the most valuable piece of information is in fact the constant name, not it's value as you pointed out. The whole point of constants to me is to not have to specifically care about what the value is. That being said, I can see where the static reflection API might be equally if not more interested in the value.

Using the example above, assuming self::DETECT has a value of 1:

<parameter name="type" optional="true" byreference="false">
    <default>'__StaticReflectionConstantValue(self::DETECT)'</default>
</parameter>

I would rather see it look like:

<parameter name="type" optional="true" byreference="false" value="1">
    <default>self::DETECT</default>
</parameter>

In the case where the constant cannot be resolved just leave out the value attribute. Ideally that wouldn't ever happen, but it would provide a stop gap for the time being since you will always have the actual constant name throughout parsing.

[theseer]

I like that approach - comes close to what I had in mind already - and I guess i'll try to talk Manuel into providing support like that. Until that happens, I'll probably just hack around the string and strip out the '__StaticRelectionConstanValue( as well as the closing )' to get at least a somewhat usable output. Thanx for the input!

[theseer]

Fixed with a workaround by @LouisLandry in commit 32102ca