bseddon / XPath20

Provides XPath 2.0 statement execution library for PHP
GNU General Public License v3.0
8 stars 2 forks source link

Undefined method DecimalValue::GetTypedValue() #2

Closed joren closed 4 years ago

joren commented 4 years ago

Hi :wave: A colleague of Tim here.

We got another small error with the Belgian tax taxonomy. This time with a sum comparison of decimal values. This is the test:

<va:valueAssertion
    xlink:type="resource"
    xlink:label="f-corp-4880"
    id="f-corp-4880"
    aspectModel="dimensional"
    implicitFiltering="false"
    test="sum (($PreviousCarryOverNonDeductibleProfessionalExpenseExceedingBorrowingCosts,$PositiveDifferenceExceedingBorrowingCost,
    $LimitExceedingBorrowingCost, $ExceedingBorrowingCost, $ExemptionNonDeductibleProfessionalExpenseExceedingBorrowingCosts,
    $NonDeductibleExceedingBorrowingCosts, $CarryOverNonDeductibleProfessionalExpenseExceedingBorrowingCosts)) eq 0 " />

And it crashes with this error message and stack trace:

Uncaught Error: Call to undefined method lyquidity\\XPath2\\Value\\DecimalValue::GetTypedValue() in /var/www/vendor/lyquidity/xpath2/ExtFuncs.php:3249

    Stack trace:
    0 /var/www/vendor/lyquidity/xpath2/ExtFuncs.php(3301): lyquidity\\XPath2\\ExtFuncs::MinValueWithCollation()
    1 /var/www/vendor/lyquidity/xpath2/FunctionTable.php(1568): lyquidity\\XPath2\\ExtFuncs::MinValue()
    2 [internal function]: lyquidity\\XPath2\\FunctionTable->lyquidity\\XPath2\\{closure}()
    3 /var/www/vendor/lyquidity/xpath2/XPathFunctionDef.php(79): call_user_func()
    4 /var/www/vendor/lyquidity/xpath2/AST/FuncNode.php(163): lyquidity\\XPath2\\XPathFunctionDef->Invoke()
    5 /var/www/vendor/lyquidity/xpath2/XPath2Expression.php(456): lyquidity\\XPath2\\AST\\FuncNode->Execute()
    6 /var/www/vendor/lyquidity/xbrl/Formulas/Resources/Resource.php(229): lyquidity\\XPath2\\XPath2Expression->EvaluateWithVars()
    7 /var/www/vendor/lyquidity/xbrl/Formulas/Resources/Variables/GeneralVariable.php(176): XBRL\\Formulas\\Resources\\Resource->evaluateXPathExpression()
    8 /var/www in /var/www/vendor/lyquidity/xpath2/ExtFuncs.php on line 3249

I followed the code around and adding this to the DecimalValue.php seemed like the best solution:

public function getTypedValue()
{
  return $this->_value;
}

It fixed our issues and passed other tests we have. Please let me know if this is or isn't the bets solution here and if I can provide some more info or tests.

bseddon commented 4 years ago

Thanks for the report and suggested change. I've applied your change and have re-run the conformance tests which continue to pass successfully. So in the sense that the change you suggest works, it's great.

However, I think the issue may indicate something has gone wrong elsewhere and I'll explain why below (these notes are really for me so I don't forget). It will be great if you will send me a link to the taxonomy you have been using, a note of the formula that illustrates the issue and, if possible, a sample instance document. Using these I should be able to replicate the issue and examine what's going on more closely. I know that sometimes instance documents cannot be shared. If this is the case then information about the taxonomy and formula will be great.

The function 'getTypedValue' is not implemented on any of the 'value' classes because instances of these classes are what is supposed to be returned by the 'getTypedValue' function. Instead, this function is implemented in XPath2Item.

It is expected in ExtFuncs::MinValueWithCollation() that $iter will be an instance of XPathNavigator or a array of XPath2Item instances. Instead $iter seems to be an array of DecimalValue instances. Given the note in the function, this may be expected. If it is, then the call to CastToNumber(and so the call to getTypedValue) should be called more selectively (only when $item is not a numeric value instance).

bseddon commented 4 years ago

PS. You included the value assertion element from the formula. You can see from the exception trace line 7 that the min() function is used in the test assigned to a 'general' variable defined within the formula definition. It is this feature of the formula that needs to be reviewed.

joren commented 4 years ago

Thanks, at least we can unblock this issue. We're using this taxonomy: https://financien.belgium.be/sites/default/files/downloads/be-tax-2020-04-30_V1.1.0.zip And I've anonymised the test document, this will now generate a lot of validation errors due to bogus values, but it also triggers the actual error. 2020_2.txt

In this file you the error is triggered by this line:

<tax-inc:PreviousCarryOverNonDeductibleProfessionalExpenseExceedingBorrowingCosts contextRef="D0" decimals="2" unitRef="U-EUR">100</tax-inc:PreviousCarryOverNonDeductibleProfessionalExpenseExceedingBorrowingCosts>

And indeed the correct test that cause the error is the min function in be-tax-f-ebc-5532-2020-04-30-assertion.xml

<va:valueAssertion 
    xlink:type="resource" 
    xlink:label="f-ebc-5532" 
    id="f-ebc-5532"
    aspectModel="dimensional"
    implicitFiltering="true"
    test="$ExemptionNonDeductibleProfessionalExpenseExceedingBorrowingCosts eq 
            min(($PositiveDifferenceExceedingBorrowingCost,$PreviousCarryOverNonDeductibleProfessionalExpenseExceedingBorrowingCosts))"/>
bseddon commented 4 years ago

Thank you, I'm really grateful to you for the data and link.

bseddon commented 4 years ago

This is a really a note to myself about an error in the BE 2020 taxonomy. When validating the taxonomy it fails processing 'be-tax-f-rcorp-4101-2020-04-30-assertion.xml' because there is an extra closing curly brace in the French language error message. You can see the additional character after the xfi:concept-label function. It appears after the closing ']' and before the computed text lead by the (= pair of characters. This can be compared with the DE or NL versions of the message. The FR and DE versions are shown below.

Removing the closing curly brace character solves the issue and no further errors are detected.

<msg:message
    xlink:type="resource"
    xlink:label="f-rcorp-4101Message"
    xlink:role="http://www.xbrl.org/2010/role/message"
    xml:lang="fr">Les éléments non imposables [{xfi:concept-label( 
QName('http://www.minfin.fgov.be/be/tax/inc/2020-04-30',
    'DeductibleMiscellaneousExemptions'), '', 'http://www.xbrl.org/2003/role/documentation', 'en')}] } (= {$DeductibleMiscellaneousExemptions}) doivent être déduits par priorité du bénéfice subsistant belge et le solde restant des éléments non imposables peut être déduit du bénéfice subsistant non exonéré suivant sa provenance (= {$CalculatedAmountMiscellaneousExemptionsNoTaxTreaty}).
</msg:message>
<msg:message
    xlink:type="resource"
    xlink:label="f-rcorp-4101Message"
    xlink:role="http://www.xbrl.org/2010/role/message"
    xml:lang="de">Die nicht steuerbaren Bestandteile [{xfi:concept-label( 
QName('http://www.minfin.fgov.be/be/tax/inc/2020-04-30',
    'DeductibleMiscellaneousExemptions'), '', 'http://www.xbrl.org/2003/role/documentation', 'en')}] (= {$DeductibleMiscellaneousExemptions}) müssen vorzugsweise von den belgischen verbleibenden Gewinnen in Abzug gebracht werden und nur der verbleibende Betrag der nicht steuerbaren Elemente darf von den nicht durch Abkommen befreiten verbleibenden Gewinnen in Abzug gebracht werden (= {$CalculatedAmountMiscellaneousExemptionsNoTaxTreaty}).</msg:message>
bseddon commented 4 years ago

It's been possible to replicate the problem you has reported. A different fix has been implemented. Because adding the function to getTypedValue() to the DecimalValue class is not consistent with the semantics of the Value classes, the change is to the ExtFuncs::MinValueWithCollation() function. The change to the Min function is to wrap the call to CoreFuncs::CastToNumber1() so that what is returned is an XPath2Item instance, The XPath2Item class does export a getTypedValue() so the calls to this function work.

I will update this issue shortly when the updates have been pushed to the master branch.

I also encountered an issue processing the formulas:

be-tax-f-nrcorp-1445-2020-04-30-assertion.xml be-tax-f-rcorp-1445-2020-04-30-assertion.xml

This issue arises because the pre-condition functions return a literal string 'true' or 'false' instead of a boolean which means the pre-condition does not return a CoreFuncs::$TrueValue or CoreFuncs::$FalseValue as expected. This has been addressed as well but the fix is to a file in the main XBRL project. This fix will be pushed to the master branch at the same time.

bseddon commented 4 years ago

The updates have been pushed to the master branch of both XPath2 and XBRL. I am able to process all the formulas though because there only limited data in the test instance document many of the formulas only have fallback values to work with so are not completely evaluated.

I've also taken the opportunity to fix an issue affecting a date function if a microseconds property is not available on a DataTime instance. This issue affects the 'v_NumberOfDays' general variable in formula be-tax-f-ace-5304-2020-04-30-assertion.xml.

You will be able to identify the changes based on the push comments associated with the various changes.

bseddon commented 4 years ago

The composer repository has also been updated so it should be possible to get the updates using 'composer update' if composer was used to install the XBRL processor code originally.

joren commented 4 years ago

Thanks a lot! This also helped me understand this new project a bit better. Great to read through your comments and follow the commits to see how you fixed it.