Added diff files (library, tests).

Reassign to Ralph for validation.

this issue is stil present in 1.11.2 (and trunk) actually now more broken than ever.

Additionally to faulty formatting and wrongly cut out closing brace of structures like (if, ..) at end of function calls it now messes up the document head. The files arent more declared as PHP.

It removes <?php from beginning of file and adds following code at end of file; <?php /* Zend_CodeGenerator_Php_File-DocblockMarker */

Hope somebody can fix it soon. We are writing bigger applications and have to clean up the classes always after adding a action.

Otherwise Zend Tool is a great help and thank you all for creating it.

The patch doesn't fully solve the issue as getStartLine on ReflectionMethod returns the line number containing the name of the function...a line break in the arguments lists for instance would also break the parsing. The only solution that's a 100% sure to fix the issue (which I have a patch for) is to use token_get_all to extract the method's body.

New patch to fix the bug using the PHP tokenizer to make sure any kind of weird source formatting doesn't break the parsing.

This patch breaks compatibility with the existing implementation, as it includes everything (specifically, the leading and trailing whitespace characters) inside the method brackets. When I run the Zend_CodeGenerator test suite after applying your patch, I get failures like these:

5) Zend_CodeGenerator_Php_MethodTest::testMethodFromReflection
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
     {
+
         /* test test */
+
     }

Yeah I know, but I don't immediately see the point in not preserving the full method body as is - at least that's not something (as an API user) I'd expect.

Do we agree that using the tokenizer is the only sensible way to make sure we get it right regardless of the coding style used in PHP files Zend_Reflection_Method gets called on ? If we agree I can resubmit the patch to restore leading and trailing whitespaces removal, if not, then I just want to emphasize that the previous patch that was attached to this also fails in many cases.

Thanks

I absolutely agree. The current implementation of {{Zend_Reflection_Method::getBody}} blows up when it's given a PHP file which doesn't conform to Zend Framework coding standards; that is a product of it's design, which was primarily driven by it's use in {{Zend_Tool}}/{{Zend_CodeGenerator}}. Your approach is much better than the current implementation, unfortunately the existing narrow behavior is already in the wild and it's highly unlikely to be changed in ZFv1 without risk of breaking backwards-compatibility. I would push for your patch to be integrated into ZF2's Zend\Code\Reflection component.

In the meanwhile, we can probably fix the original poster's issue (which is the root cause of ZF-9501) using the simple fix below:

Index: library/Zend/Reflection/Method.php
===================================================================
--- library/Zend/Reflection/Method.php  (revision 24511)
+++ library/Zend/Reflection/Method.php  (working copy)
@@ -163,6 +163,6 @@
         }

         // just in case we had code on the bracket lines
-        return rtrim(ltrim(implode("\n", $lines), '{'), '}');
+        return implode("\n", $lines);
     }
 }
Index: tests/Zend/Reflection/MethodTest.php
===================================================================
--- tests/Zend/Reflection/MethodTest.php        (revision 24511)
+++ tests/Zend/Reflection/MethodTest.php        (working copy)
@@ -26,6 +26,11 @@
 require_once 'Zend/Reflection/Method.php';

 /**
+ * @see ZF-9018
+ */
+require_once dirname(__FILE__) . '/_files/ZF9018TestClass.php';
+
+/**
  * @category   Zend
  * @package    Zend_Reflection
  * @subpackage UnitTests
@@ -79,6 +84,36 @@
         $reflectionMethod = new Zend_Reflection_Method('Zend_Reflection_TestSampleClass6', 'doSomething');
         $this->assertEquals($body, $reflectionMethod->getBody());
     }
+
+    /**
+     * @group ZF-9018
+     * @group ZF-9501
+     */
+    public function testGetBodyReturnsCorrectBodyWhenContentEndsWithClosingCurlyBrace()
+    {
+        $body = '        if ( true ) {
+            echo "True";
+        } else {
+            echo "False";
+        }';
+        $reflectionMethod = new Zend_Reflection_Method('ZF9018TestClass', 'doSomething');
+        $this->assertEquals($body, $reflectionMethod->getBody());
+    }
+
+    /**
+     * @group ZF-9018
+     * @group ZF-9501
+     */
+    public function testGetBodyReturnsCorrectBodyWhenMethodWithInlineOpenBraceHasBodyWhichEndsWithClosingCurlyBrace()
+    {
+        $body = '        if ( true ) {
+            echo "True";
+        } else {
+            echo "False";
+        }';
+        $reflectionMethod = new Zend_Reflection_Method('ZF9018TestClass', 'doSomethingOpenBraceInline');
+        $this->assertEquals($body, $reflectionMethod->getBody());
+    }

     public function testGetContentsReturnsCorrectContent()
     {
Index: tests/Zend/Reflection/_files/ZF9018TestClass.php
===================================================================
--- tests/Zend/Reflection/_files/ZF9018TestClass.php    (revision 0)
+++ tests/Zend/Reflection/_files/ZF9018TestClass.php    (revision 0)
@@ -0,0 +1,21 @@
+<?php
+
+class ZF9018TestClass
+{
+    public function doSomething($var)
+    {
+        if ( true ) {
+            echo "True";
+        } else {
+            echo "False";
+        }
+    }
+
+    public function doSomethingOpenBraceInline($var) {
+        if ( true ) {
+            echo "True";
+        } else {
+            echo "False";
+        }
+    }
+}
\ No newline at end of file

Property changes on: tests/Zend/Reflection/_files/ZF9018TestClass.php
___________________________________________________________________
Added: svn:keywords
   + Id

As far as I can see, it fixes the issue with ZF-9501 without breaking the existing tested behavior of {{Zend_Reflection}}, {{Zend_Tool}} or {{Zend_CodeGenerator}}.

OK, thanks for the clarification.

Attached file: ZF-9018.patch

Fixed in trunk (1.12.0): r24860