inpsyde / php-coding-standards

Style guide for writing consistent PHP for WordPress projects.
MIT License
98 stars 23 forks source link

NoAccessors.NoGetter - don't check for native/psr interfaces #7

Closed Chrico closed 6 years ago

Chrico commented 6 years ago

Sometimes it is required to implement get/set or whatevery methods which are maybe not allowed by coding standard but fixed by an interface.

Following error occurs in version 0.7.x - release:

Getters are discouraged. "Tell Don't Ask" principle should be applied if possible, and if getters are really needed consider naming methods after properties, e.g. name() instead of getName().(InpsydeCodingStandard.CodeQuality.NoAccessors.NoGetter)

As code example we've following:

<?php declare(strict_types=1); # -*- coding: utf-8 -*-

class Foo implements IteratorAggregate
{

    public function getIterator(): Traversable
    {
        // snip
    }
}

Link: http://php.net/manual/en/iteratoraggregate.getiterator.php


Secondly it could be also possible to e.G. implement PSR-standard interfaces like PSR container which are having a get($id) method.

gmazzap commented 6 years ago

The getIterator bug is already fixed in master, with https://github.com/inpsyde/php-coding-standards/commit/412336470cc51e645c6e9103da4bba611db2c9e9

The get() method is allowed because the regex that check for getters is ~(get|set)[_A-Z0-9]+~ which will not match for get (or set).

gmazzap commented 6 years ago

Releasee v0.8.0 which includes this fix.

Chrico commented 6 years ago

Nice! What i see so far is that following are missing:

gmazzap commented 6 years ago

@Chrico there's not need for getReturn. There's no way you can declare that method,because Generator is final, so can't be extended.

You can only use that method. And using a method does not trigger any problem with the style.

Thw same applies to all the Exception methods: getMessage(), getCode() and so on...

Chrico commented 6 years ago

arg, seems that i've missed that when browsing on phone...so i guess all get-methods are implement - for now. :)