Re: [PEAR-BUGS] [PEAR-BUG] Bug #19348 [Com]: Missing function parameters in AbstractSniffUnitTest

[email protected]


On 2012年3月25日, at 21:48, "[email protected]" <[email protected]> wrote:

> Edit report at https://pear.php.net/bugs/bug.php?id=19348&edit=1
> 
> ID:               19348
> Comment by:       [email protected]
> Reported By:      jnrbsn at gmail dot com
> Summary:          Missing function parameters in AbstractSniffUnitTest
> Status:           Feedback
> Type:             Bug
> Package:          PHP_CodeSniffer
> Operating System: Mac OS 10.7.3 "Lion"
> Package Version:  1.3.3
> PHP Version:      5.3.8
> Assigned To:      squiz
> Roadmap Versions: 
> New Comment:
> 
> To clarify, solution #1 would be a change to my code, and solutions #2
> and #3 would be changes to PHP_CodeSniffer code.
> 
> Also, another possible solution would be for me to make my own fork of
> the AbstractSniffUnitTest class.
> 
> 
> Previous Comments:
> ------------------------------------------------------------------------
> 
> [2012-03-26 02:48:13] squiz
> 
> Yes, I'm well aware of this issue. But I've never got any errors running
> the test suite 
> and I have differing signatures in some, but not all, sniff unit tests.
> I asked how you 
> were running them because I have no idea why I am not getting this
> error, and why 
> I don't see it on my build systems.
> 
> Here is an example file: 
> https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/G
> eneric/Tests/Commenting/FixmeUnitTest.php
> 
> I used this extensively throughout the test suite, which is why I asked
> how you are 
> running it. I have done this for quite a while on many different PHP
> versions, but 
> never realised I forgot to change the abstract method.
> 
> That's all my comment was asking and why I was asking it. I haven't had
> a chance to 
> play around with the code and figure out what is wrong. Maybe it is a
> php.ini setting. 
> Maybe it is the use of the default value for the argument. I really
> don't know. But it 
> you can run the test suite as it stands right now, you're already
> running a lot of code 
> that does what you want to do.
> 
> ------------------------------------------------------------------------
> 
> [2012-03-26 02:12:32] jnrbsn
> 
> Please forgive me, but are you sure you understand the error that I'm
> getting? The signature of a method that inherits from an abstract method
> must have the same number of arguments as its parent.
> 
> From the "Class Abstraction" page in the PHP manual:
> 
> "When inheriting from an abstract class, all methods marked abstract in
> the parent's class declaration must be defined by the child;
> additionally, these methods must be defined with the same (or a less
> restricted) visibility. For example, if the abstract method is defined
> as protected, the function implementation must be defined as either
> protected or public, but not private. Furthermore the signatures of the
> methods must match, i.e. the type hints and the number of required
> arguments must be the same. This also applies to constructors as of PHP
> 5.4. Before 5.4 constructor signatures could differ."
> 
> The signature of the method "AbstractSniffUnitTest::getErrorList()"
> looks likes this:
> 
>    protected abstract function getErrorList();
> 
> As you can see, it's not declared with any arguments. However, when
> "getErrorList()" is called by "generateFailureMessages()", it passes the
> basename of the test file as an argument. (On a side note, it took me a
> really long time to discover this "feature" because it doesn't seem to
> be documented anywhere, like in the signature of "getErrorList()" as one
> might expect.) There's nothing technically wrong with that part of it,
> because PHP allows you to pass more arguments than what's in the
> function signature (which are retrievable via "func_get_args()"). The
> problem is that I want my method (the one that inherits from the
> abstract one) to look like this:
> 
>    public function getErrorList($testFile) { /* my code here... */ }
> 
> So that I can use the name of the test file to return a specific set of
> line numbers/errors for that specific file. This technique is obviously
> preferred over calling "func_get_args()".
> 
> Does that make sense?
> 
> So the solutions to this (or at least the ones I can think of) are as
> follows:
> 
> 1. Use "func_get_args()" as I described above.
> 2. Change the signature of "AbstractSniffUnitTest::getErrorList()" (and
> "getWarningList()") to include the test file as an argument. This is
> problematic because it would break a lot of people's unit tests, because
> all of the sudden, the abstract method would define one argument, but
> child methods would define zero, breaking PHP's rules for abstract
> method inheritance and triggering the fatal error. The big benefit of
> this is that people would see that the name of test file is available to
> them in their tests, allowing for more robust tests.
> 3. Make change #2 but also make the methods non-abstract. The only
> downside with this is that it would break things if someone writing unit
> tests didn't override them.
> 
> ------------------------------------------------------------------------
> 
> [2012-03-26 00:45:28] squiz
> 
> -Status:      Open
> +Status:      Feedback
> -Assigned To:
> +Assigned To: squiz
> While I see the issue, I don't get any errors (and never have). I have
> tested on 5.2, 
> 5.3 and 5.4. I'm currently running 5.3.8 on Lion, just like you.
> 
> Out of interest, how are you running the tests?
> 
> ------------------------------------------------------------------------
> 
> [2012-03-25 23:50:45] jnrbsn
> 
> Description:
> ------------
> The following abstract methods get a single parameter passed 
> to them during the unit tests (the basename of the test file):
> 
> AbstractSniffUnitTest::getErrorList()
> AbstractSniffUnitTest::getWarningList()
> 
> However, the declarations of those methods in 
> AbstractSniffUnitTest.php do NOT contain any parameters. So 
> if I want to use the parameters in my own tests, I have to use 
> func_get_args(), because if I put the parameter in the 
> declaration of my method, I get a "Declaration of ... must be 
> compatible with that of ..." error.
> 
> This kind of sucks actually, because even if you did "fix" this, 
> it would break everyone's existing unit tests for the same 
> reason. Is that why it's like that?
> 
> Test script:
> ---------------
> <?php
> class MyStandard_Tests_Files_LineEndingsUnitTest extends
> AbstractSniffUnitTest
> {
>    public function getErrorList($testFile)
>    {
>        switch ($testFile) {
>        case 'LineEndingsUnitTest.CRLF.inc':
>            $errors = array(1 => 1);
>            break;
>        // More stuff here...
>        }
>        return $errors;
>    }
> 
>    public function getWarningList($testFile)
>    {
>        return array();
>    }
> }
> 
> Expected result:
> ----------------
> The tests should run, allowing different error lists for different 
> test files. To get it to work, you have to remove $testFile from 
> the method declaration and use func_get_args() to get the name 
> of the test file.
> 
> Actual result:
> --------------
> PHP Fatal error:  Declaration of 
> MyStandard_Tests_Files_LineEndingsUnitTest::getErrorList() must 
> be compatible with that of AbstractSniffUnitTest::getErrorList() in 
> /path/to/file/LineEndingsUnitTest.php on line XXX
> 
> ------------------------------------------------------------------------
> 
> 
> -- 
> Edit this bug report at https://pear.php.net/bugs/bug.php?id=19348&edit=1
> 
> 
> -- 
> PEAR Bugs Mailing List (http://pear.php.net/bugs/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>