zendframework / zend-code

BSD 3-Clause "New" or "Revised" License
1.68k stars 79 forks source link

Additional blank lines make generated class not PSR2 compliant #135

Open JuJuDropThor opened 6 years ago

JuJuDropThor commented 6 years ago

Hi guys

I saw PSR2 issues when generating classes.

See errors below

92 | ERROR | [x] Expected 1 blank line at end of file; 2 found 92 | ERROR | [x] The closing brace for the class must go on the next line after the body

I post you an exemple which generate these errors

<?php

namespace AppBundle\Domain\Entity;

class MyClass
{
    /**
     * @var int|null
     */
    private $id = null;

    /**
     * @param int $id
     * @return $this
     */
    public function setId($id)
    {
        $id = (int) $id;
        $this->id = $id;

        return $this;
    }

    /**
     * @return int
     */
    public function getId()
    {
        return $this->id;
    }

}
Ocramius commented 6 years ago

@JuJuDropThor can you maybe write a test that verifies PSR-2 compliance of generated code?

JuJuDropThor commented 6 years ago

I'm not sure I understand what you asked. You want to know how I checked if the generated code was PSR-2 compliant ?

I just ran phpcs with --standard=PSR2 argument.

Ocramius commented 6 years ago

@JuJuDropThor yes, what we'd need is a test that automates such a check

JuJuDropThor commented 6 years ago

Sure. See the code below :

<?php
/**
 * composer require zendframework/zend-code
 * php sample_1.php
 * Assuming you got phpcs in your path: phpcs ./Person.php --standard=PSR2
 */

require_once __DIR__ . '/vendor/autoload.php';

use Zend\Code\Generator\FileGenerator;
use Zend\Code\Generator\ClassGenerator;
use Zend\Code\Generator\PropertyGenerator;
use Zend\Code\Generator\DocBlockGenerator;
use Zend\Code\Generator\MethodGenerator;
use Zend\Code\Generator\DocBlock\Tag\ParamTag;
use Zend\Code\Generator\DocBlock\Tag\ReturnTag;
use Zend\Code\Generator\DocBlock\Tag\GenericTag;

$classGenerator = new ClassGenerator();
$classGenerator
    ->setName('Person')
    ->setNamespaceName('Domain\Entity')
    ->addPropertyFromGenerator(
        PropertyGenerator::fromArray([
            'name'         => 'firstName',
            'defaultValue' => null,
            'visibility'   => PropertyGenerator::VISIBILITY_PRIVATE,
            'docblock'     => DocBlockGenerator::fromArray([
                'tags'         => [new GenericTag('var', 'string')]
            ])
        ])
    )
    ->addPropertyFromGenerator(
        PropertyGenerator::fromArray([
            'name'         => 'lastName',
            'defaultValue' => null,
            'visibility'   => PropertyGenerator::VISIBILITY_PRIVATE,
            'docblock'     => DocBlockGenerator::fromArray([
                'tags'         => [new GenericTag('var', 'string')]
            ])
        ])
    )
    ->addMethodFromGenerator(
        MethodGenerator::fromArray([
            'name'       => 'setFirstName',
            'parameters' => ['firstName'],
            'visibility' => MethodGenerator::VISIBILITY_PUBLIC,
            'body'       => '$this->firstName = $firstName;',
            'docblock'   => DocBlockGenerator::fromArray([
                'tags' => [
                    new ParamTag('firstName', ['string']),
                    new ReturnTag(['$this'])
                ]
            ])
        ])
    )
    ->addMethodFromGenerator(
        MethodGenerator::fromArray([
            'name'       => 'setLastName',
            'parameters' => ['lastName'],
            'visibility' => MethodGenerator::VISIBILITY_PUBLIC,
            'body'       => '$this->lastName = $lastName;',
            'docblock'   => DocBlockGenerator::fromArray([
                'tags' => [
                    new ParamTag('lastName', ['string']),
                    new ReturnTag(['$this'])
                ]
            ])
        ])
    );

$fileGenerator = new FileGenerator();
$fileGenerator->setClass($classGenerator);

file_put_contents(__DIR__ . '/Person.php', $fileGenerator->generate());
JuJuDropThor commented 6 years ago

Hello guys

Any news ? Tell me if you need more code.

Ocramius commented 6 years ago

@JuJuDropThor what would be needed is to turn this into a proper test case. We already have phpcs installed as dev dependency: it just needs to run (as part of the test suite) in one of the tests.

allcode commented 6 years ago

Something like this class works for testing various generated code:

use PHPUnit\Framework\TestCase;
use Zend\Code\Generator\ClassGenerator;
use Zend\Code\Generator\FileGenerator;

class Psr2ComplianceTest extends TestCase
{
    public function testEmptyClassCompliance()
    {
        $classGenerator = new ClassGenerator('Bar', 'Foo');
        $fileGenerator = new FileGenerator();
        $fileGenerator->setClass($classGenerator);
        $generatedCode = $fileGenerator->generate();

        $codeSniffer = new \PHP_CodeSniffer();
        $codeSniffer->initStandard('PSR2');
        $report = $codeSniffer->processFile('', $generatedCode);

        $this->assertEquals(0, $report->getErrorCount());
        $this->assertEquals(0, $report->getWarningCount());
    }
}

I'll write some tests for various generated code later.

JuJuDropThor commented 6 years ago

Hello

I added your unit test and ran it with phpunit ./test/Psr2ComplianceTest.

See bellow what I got

PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

F

Time: 157 ms, Memory: 12.00MB

There was 1 failure:

1) ZendTest\Code\Psr2ComplianceTest::testEmptyClassCompliance
Failed asserting that 1 matches expected 0.

/var/www/zend-code/test/Psr2ComplianceTest.php:22

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

$report->getErrorCount() return 1. But it's strange because $report->getErrors() return an empty array

flowl commented 6 years ago

The closing brace for the class must go on the next line after the body

That one is obvious, you have 2 blank lines but it expects 0 blank lines, that is ok.

Expected 1 blank line at end of file; 2 found

This also seems obvious, how many blank lines did you actually have? It must be 1, not less and not more.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-code; a new issue has been opened at https://github.com/laminas/laminas-code/issues/10.