zendframework / zend-code

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

Generate declare strict_types #102

Closed GaryJones closed 5 years ago

GaryJones commented 7 years ago

Is there currently a way to add declare(strict_types=1); to a generated file?

I couldn't see a suitable method in the FileGenerator, and wasn't sure where else something like this might be hiding.

Ocramius commented 7 years ago

Is there currently a way to add declare(strict_types=1); to a generated file?

No, but I'd expect a map of supported declares to be added. Want to give it a shot?

GaryJones commented 7 years ago

Completed untested, posting here for a sanity check of the approach:


    /**
     * @var array
     */
    protected $declares = [];

    // Add to fromReflection() ?

    /**
     * @return array
     */
    public function getDeclares()
    {
        return $this->declares;
    }

    /**
     * @param  array $declares
     * @return FileGenerator
     */
    public function setDeclares(array $declares)
    {
        foreach ($declares as $directive => $value) {
            $this->setDeclare($directive, $value);
        }
        return $this;
    }

    /**
     * @param  string $directive
     * @param  int|string $value
     * @return FileGenerator
     */
    public function setDeclare($directive, $value)
    {
        if (!in_array([$directive, $value], $this->declares)) {
            $this->declares[] = [$directive, $value];
        }
        return $this;
    }

    public function generate() // Already exists
    {
        // ...

        // Declares, if any
        $declares = $this->getDeclares();
        if ($declares) {
            foreach ($declares as $directive => $value) { // Replace with array_map()?
                $value = is_string($value) ? "'$value'" : $value;
                $declareStatements[] = sprintf('declare(%s=%s);%s', $directive, $value, self::LINE_FEED);
            }
            $declareStatements = implode('', $declareStatements);
            if (preg_match('#/\* Zend_Code_Generator_FileGenerator-DeclaresMarker \*/#m', $output)) {
                $output = preg_replace(
                    '#/\* Zend_Code_Generator_FileGenerator-DeclaresMarker \*/#m',
                    $declareStatements,
                    $output,
                    1
                );
            } else {
                $output .= $declareStatements;
            }
        }

        // ...

    }
Ocramius commented 7 years ago

@GaryJones something like that, yes, although I wouldn't allow unknown declares, and wouldn't allow just strings. This enforcement can be applied via a tiny Declare object.

I'd also simply remove setDeclare, getDeclares, and just allow setDeclares

GaryJones commented 7 years ago

@Ocramius How about this:

class FileGenerator // Already exists
{
    // ...

    /**
     * @var array
     */
    protected $declares = [];

    // Add to fromReflection() ?

    /**
     * @param  Declares[] $declares
     * @return FileGenerator
     */
    public function setDeclares(array $declares)
    {
        foreach ($declares as $declare) {
            if (!$declare instanceof Declare) {
                throw new Exception\InvalidArgumentException(sprintf(
                    '%s is expecting an array of %s\Declare objects',
                    __METHOD__,
                    __NAMESPACE__
                ));
            }
            if (!in_array($declare, $this->declares) && $declare->isValid()) {
                $this->declares[] = $declare;
            }
        }
        return $this;
    }

    public function generate() // Already exists
    {
        // ...

        // Declares, if any
        $declares = $this->declares;
        if ($declares) {
            foreach ($declares as $declare) {
                if ( $declare->isValid() ) {
                    $declareStatements[] = $declare->getStatement() . self::LINE_FEED;
                }
            }

            $declareStatements = implode('', $declareStatements);

            if (preg_match('#/\* Zend_Code_Generator_FileGenerator-DeclaresMarker \*/#m', $output)) {
                $output = preg_replace(
                    '#/\* Zend_Code_Generator_FileGenerator-DeclaresMarker \*/#m',
                    $declareStatements,
                    $output,
                    1
                );
            } else {
                $output .= $declareStatements;
            }
        }

        // ...

    }

    // ...
}

class Declare
{
    protected $directive = '';
    protected $value;

    public function __construct($directive, $value)
    {
        $this->directive = $directive;
        $this->value = $value;
    }

    public function isValid() {
        $allowed = [
            // Directive => type
            'ticks'        => 'integer',
            'strict_types' => 'integer',
            'encoding'     => 'string',
        ];

        if (!in_array($this->directive, array_keys($allowed), true)) {
            throw new InvalidArgumentException('Declare directive unknown: '.$this->directive);
        }

        if (gettype($this->value) !== $allowed[$this->directive]) {
            throw new InvalidArgumentException('Declare value is not the expected type: '.$this->value);
        }

        return true;
    }

    public function getStatement() {
        $value = is_string($this->value) ? "'$value'" : $this->value;

        return 'declare('.$this->$directive.'='.$value.');';
    }
}

This allows any Declare objected to be created, but only the valid ones are allowed to be queued up to be added to the file.

Ocramius commented 7 years ago

I suggest sending a PR first: inline reviews will show the progress in the design.

On 17 Feb 2017 7:42 p.m., "Gary Jones" notifications@github.com wrote:

@Ocramius https://github.com/Ocramius How about this:

class FileGenerator // Already exists{ // ... / @var array / protected $declares = []; // Add to fromReflection() ? / @param Declares[] $declares @return FileGenerator / public function setDeclares(array $declares) { foreach ($declares as $declare) { if (!$declare instanceof Declare) { throw new Exception\InvalidArgumentException(sprintf( '%s is expecting an array of %s\Declare objects', METHOD, NAMESPACE )); } if (!in_array($declare, $this->declares) && $declare->isValid()) { $this->declares[] = $declare; } } return $this; } public function generate() // Already exists { // ... // Declares, if any $declares = $this->declares; if ($declares) { foreach ($declares as $declare) { if ( $declare->isValid() ) { $declareStatements[] = $declare->getStatement() . self::LINE_FEED; } } $declareStatements = implode('', $declareStatements); if (preg_match('#/* Zend_Code_Generator_FileGenerator-DeclaresMarker */#m', $output)) { $output = preg_replace( '#/* Zend_Code_Generator_FileGenerator-DeclaresMarker \/#m', $declareStatements, $output, 1 ); } else { $output .= $declareStatements; } } // ... } // ...}class Declare{ protected $directive = ''; protected $value; public function __construct($directive, $value) { $this->directive = $directive; $this->value = $value; } public function isValid() { $allowed = [ // Directive => type 'ticks' => 'integer', 'strict_types' => 'integer', 'encoding' => 'string', ]; if (!in_array($this->directive, array_keys($allowed), true)) { throw new InvalidArgumentException('Declare directive unknown: '.$this->directive); } if (gettype($this->value) !== $allowed[$this->directive]) { throw new InvalidArgumentException('Declare value is not the expected type: '.$this->value); } return true; } public function getStatement() { $value = is_string($this->value) ? "'$value'" : $this->value; return 'declare('.$this->$directive.'='.$value.');'; }}

This allows any Declare objected to be created, but only the valid ones are allowed to be queued up to be added to the file.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zendframework/zend-code/issues/102#issuecomment-280732165, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakFHXnCVWQWVykpOCCaQS--kZtCBWks5rdeoHgaJpZM4MEUJB .

vlakarados commented 7 years ago

Hey, have I missed something or why doesn't this make it way to PRs? I may create one

GaryJones commented 7 years ago

@vlakarados I never got around to it, so, please, go ahead :-)

icanhazstring commented 5 years ago

Seems like no one will do it? Then i'll do it ;)

michalbundyra commented 5 years ago

Solved in #169 which is already merged, preparing 3.4.0 release.