Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No-leaks doesn't play nice with roave/dont #25

Open
Girgias opened this issue Nov 14, 2019 · 9 comments
Open

No-leaks doesn't play nice with roave/dont #25

Girgias opened this issue Nov 14, 2019 · 9 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Girgias
Copy link

Girgias commented Nov 14, 2019

Seems that using use JustDont; in classes makes vendor/bin/roave-no-leaks fail on a lot of tests.
As by removing the usage of roave/dont I stopped getting errors.

If this behaviour is to be expected a note in the Readme would probably do the job.

@Ocramius
Copy link
Member

It could be a genuine leak though. I don't think that roave/dont leaks per-se, but it could really be an engine issue

@Girgias
Copy link
Author

Girgias commented Nov 14, 2019

I ran this on PHP 7.4.0RC4 if that's of any help:

PHP 7.4.0RC4 (cli) (built: Oct 26 2019 10:48:28) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.0RC4, Copyright (c), by Zend Technologies
    with Xdebug v2.8.0beta2, Copyright (c) 2002-2019, by Derick Rethans

IIRC PHP 7.4 does have USAN and ASAN checks which should prevent such engine issues (or I'm misunderstanding what you mean by engine).

@Ocramius
Copy link
Member

Yes, talking about that engine.

Otherwise, what I can think of is something in phpunit itself.

@Girgias
Copy link
Author

Girgias commented Nov 14, 2019

Just doubled checked and PHP 7.4. does (or did at one point) have a MSAN, ASAN, UBSAN shceduled build, so maybe it is indeed phpunit which has an issue.

@Girgias
Copy link
Author

Girgias commented Nov 15, 2019

I think I've maybe pinned it down to the fact I'm using a typed property and I'm always getting a variation of this error:

Dont\Exception\NonSettableObject: The given object Girgias\CSSParser\Lexer#000000000b7669430000000065904088 is not designed to allow any undefined or inaccessible properties to be written to.

You tried to write to a property called "inputStream".

Perhaps you made a typo in the property name, or tried to write to an inaccessible property?

With that information would you still think it's something related to PHPUnit or more something on no-leaks's side?

EDIT: Okay I'm confident this is due to typed properties, so it's possibly indeed an engine problem.

@Girgias
Copy link
Author

Girgias commented Nov 22, 2019

So updating to PHP-7.4RC6 fixed most of the issues however now I'm getting one unique class/test-file bugging:

<?php

namespace Girgias\CSSParser\Tokens;

use Dont\JustDont;

final class Hash extends BasicToken implements Token
{
    use JustDont;

    public const UNRESTRICTED = 1;
    public const ID = 2;

    private int $type = self::UNRESTRICTED;

    public function __construct(string $hash, int $type = self::UNRESTRICTED)
    {
        if ($type !== self::UNRESTRICTED && $type !== self::ID) {
            throw new \InvalidArgumentException("Hash token type can only be unrestricted or ID");
        }
        $this->type = $type;
        parent::__construct($hash);
    }

    public function getType(): int
    {
        return $this->type;
    }
}

Fails with:

PHPUnit 8.4.3 by Sebastian Bergmann and contributors.

............................................PHP Fatal error:  Cannot override final method Girgias\CSSParser\Tokens\BasicToken::__get() in /mnt/c/Dev/Libs/css-parser/src/Tokens/Hash.php on line 16
PHP Stack trace:
PHP   1. {main}() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/phpunit:61
PHP   3. PHPUnit\TextUI\Command->run() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/TextUI/Command.php:159
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/TextUI/Command.php:200
PHP   5. PHPUnit\Framework\TestSuite->run() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:616
PHP   6. PHPUnit\Framework\TestSuite->run() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestSuite.php:569
PHP   7. Tests\LexerHashTest->run() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestSuite.php:569
PHP   8. PHPUnit\Framework\TestResult->run() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestCase.php:752
PHP   9. Tests\LexerHashTest->runBare() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestResult.php:691
PHP  10. Tests\LexerHashTest->runTest() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestCase.php:1020
PHP  11. Tests\LexerHashTest->testUnrestrictedHash() /mnt/c/Dev/Libs/css-parser/vendor/phpunit/phpunit/src/Framework/TestCase.php:1400
PHP  12. Girgias\CSSParser\Lexer->readNext() /mnt/c/Dev/Libs/css-parser/tests/LexerHashTest.php:24
PHP  13. spl_autoload_call() /mnt/c/Dev/Libs/css-parser/src/Lexer.php:135
PHP  14. Composer\Autoload\ClassLoader->loadClass() /mnt/c/Dev/Libs/css-parser/src/Lexer.php:135
PHP  15. Composer\Autoload\includeFile() /mnt/c/Dev/Libs/css-parser/vendor/composer/ClassLoader.php:322
PHP  16. include() /mnt/c/Dev/Libs/css-parser/vendor/composer/ClassLoader.php:444

@tux-rampage
Copy link

Not sure if this is the root cause of this issue, but possibly the reason for the one in your latest comment: It seems that you're using JustDont in your BaseToken class as well. This is not possible since the *Dont traits declare methods final to prevent bypassing them via inheritance.

@Girgias
Copy link
Author

Girgias commented Dec 4, 2019

Not sure if this is the root cause of this issue, but possibly the reason for the one in your latest comment: It seems that you're using JustDont in your BaseToken class as well. This is not possible since the *Dont traits declare methods final to prevent bypassing them via inheritance.

Aaa, this is indeed the case, that does make sense. I suppose me blindly copy pasting it into every file wasn't that smart afterall ^^

@Ocramius
Copy link
Member

Ocramius commented Jan 6, 2021

Is this still valid on 8.x and the newest release (1.2.0) of this tool?

@Ocramius Ocramius added bug Something isn't working help wanted Extra attention is needed labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants