Giter Club home page Giter Club logo

code-sniffer's Introduction

Spryker Code Sniffer

CI Latest Stable Version Minimum PHP Version PHPStan License Total Downloads

This sniffer package follows PSR-2 completely and ships with a lot of additional fixers on top (incl. PSR-12). Please see the Spryker Coding conventions for details.

List of included sniffs.

Documentation

See docs.

Upstream docs: squizlabs/PHP_CodeSniffer/wiki

Usage

How to use in Spryker projects

Make sure you include the sniffer as require-dev dependency:

composer require --dev spryker/code-sniffer

The Development module provides a convenience command:

console code:sniff:style

(or console c:s:s as shortcut)

To automatically fix fixable errors, use

console code:sniff:style -f

-v is useful for more info output. To run only a specific sniff, use the -s option. See -h for help.

You can also sniff a specific project level module or path:

console code:sniff:style [-m ModuleName] [optional-sub-path] -v

How to use in any project

You can also manually invoke the phpcs/phpcbf commands:

vendor/bin/phpcs --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml ./
vendor/bin/phpcbf --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml ./

The command phpcs just sniffs, phpcbf fixes.

You probably want to ignore some folders, e.g. --ignore=vendor/ or some of your test fixture folders.

Standards

You can always switch the standard to the stricter one named SprykerStrict. It is an extension of the Spryker standard with its own (strict) sniffs added on top.

How to include in your IDE

E.g. for PHPStorm:

  • Open Settings -> Tools -> External Tools
  • Add a new tool named "cs-sniffer" and set Program to $ProjectFileDir$/vendor/bin/phpcs, Parameters to --standard=$ProjectFileDir$/vendor/spryker/code-sniffer/Spryker/ruleset.xml -p $FilePath$ and Working directory to $ProjectFileDir$.
  • Add a new tool named "cs-fixer" and set Program to $ProjectFileDir$/vendor/bin/phpcbf, Parameters to --standard=$ProjectFileDir$/vendor/spryker/code-sniffer/Spryker/ruleset.xml -v $FilePath$ and Working directory to $ProjectFileDir$.
  • Remove the "Open console" if you don't want to see any output here for the fixer.
  • Now set up your hotkeys under Settings -> Keymap (search for cs-sniffer and cs-fixer). E.g. Control + Comma for sniffing, and Control + Dot for fixing.

You can also set up file watchers, but here you should better only whitelist certain sniffs that only add things and don't remove anything.

How to configure the default rule set

In order to simplify command line interface, phpcs allows to specify default rule set in and standards path the following way.

Assuming the following directory structure:

vendor/spryker/code-sniffer/                          # Base directory
                           |_ Spryker/                # Rule set name
                                      |_ ruleset.xml  # Rule set

The base directory and rule set can be used in configuration now.

vendor/bin/phpcs --config-set installed_paths vendor/spryker/code-sniffer/
vendor/bin/phpcs --config-set default_standard Spryker

You might need to specify full directory path. Now the tools can be used without --standard switch.

Using own project standard

You can exchange or extend the Spryker coding standard by providing your own ruleset.xml. This can be configured in the Development module config:

// DevelopmentConfig.php

    /**
     * Either a relative or full path to the ruleset.xml or a name of an installed
     * standard (see `phpcs -i` for a list of available ones).
     *
     * @return string
     */
    public function getCodingStandard()
    {
        return '/path/to/your/ruleset.xml';
    }

If you use it for custom projects, just use --standard to point to your ruleset file.

Make sure that you include the Spryker core standard ruleset in your custom one, e.g.:

<?xml version="1.0"?>
<ruleset name="SprykerProject">
    <description>
        Spryker Coding Standard for Project.
        Extends main Spryker Coding Standard.
        All sniffs in ./Sniffs/ will be auto loaded
    </description>

    <rule ref="vendor/spryker/code-sniffer/Spryker/ruleset.xml"/>

    <exclude-pattern>*/src/Generated/*</exclude-pattern>
    <exclude-pattern>*/src/Orm/*</exclude-pattern>
    <exclude-pattern>*/tests/_support/_generated/*</exclude-pattern>
    <exclude-pattern>*/tests/_helpers/*</exclude-pattern>
    <exclude-pattern>*/tests/_output/*</exclude-pattern>
    <exclude-pattern>./data/DE/*</exclude-pattern>

    <!-- Define your own sniffs here -->
</ruleset>

If you want to use the SprykerStrict standard in your project, you should replace the string:

<rule ref="vendor/spryker/code-sniffer/Spryker/ruleset.xml"/>

with this one:

<rule ref="vendor/spryker/code-sniffer/SprykerStrict/ruleset.xml"/>

code-sniffer's People

Contributors

a-sabaa avatar asaulenko avatar asmarovydlo avatar asonunique avatar darius-telycenas avatar demkos avatar dereuromark avatar dmytroklymanspryker avatar ehsanmx avatar fabianwesner avatar garyjones avatar gechetspr avatar geega avatar gerner-spryker avatar hhebbo avatar levacic avatar limeeugenia avatar lmanzke avatar localheinz avatar olhalivitchuk avatar pushokwhite avatar serhiisikachov avatar stereomon avatar sylvain-spryker avatar szepeviktor avatar vlunov-spryker avatar voku avatar vol4onok avatar zyuzka avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

code-sniffer's Issues

PHP 7 syntax support

Code sniffer breaks PHP7 return type hints

Steps to Reproduce:

(applicable to PHP v. 7)

  1. Create an interface which is specifying objects or interfaces as return type hint.
  2. Execute code sniffer (default demoshop's ruleset) to analyse the file;
  3. Execute code sniffer with -f option to fix found issues automatically

Current Behavior:
Code sniffer removes "use" statements for the objects, mentioned in the return type hints, as "not used". please see the attached screenshot as an example: errors and warnings reported at the bottom are not relevant.

Expected Behavior:
Code sniffer should be aware of PHP7 return type hints and not remove connected "use" statements.

screenshot from 2017-02-22 21 32 26

Bug with empty class and single member var

41 | ERROR | [x] Expected 1 blank line after member var; 0 found
51 | ERROR | [x] Closing brace of a class must have one extra new
| | line between itself and the last content.

class Coupon extends Entity
{

    /**
     * Fields that can be mass assigned using newEntity() or patchEntity().
     *
     * Note that when '*' is set to true, this allows all unspecified fields to
     * be mass assigned. For security purposes, it is advised to set '*' to false
     * (or remove it), and explicitly make individual fields accessible as needed.
     *
     * @var array
     */
    protected $_accessible = [
        '*' => true,
        'id' => false
    ];
}

try catch

try {
// somthing
}
catch (\Exception $e) {
// something;
}

catch should be on same line as curclybrace

Transfer and Entities suffix variables

Would be nice to have fixer for variable names for transfer and propel entities. Very often we miss/forget to add it. It could correct those variable based on method type hint.

Concat spacing

Currently we allow $foo.$bar this should be $foo . $bar

Remove leading \ in use statements

use Propel\Runtime\Connection\ConnectionInterface;
use \Closure;

The \ should be removed.

Maybe a

NormalizedUseStatementSniff

?
It should do nothing else, as the order sniff then picks it up and fixes the order afterwards.

License file doc blocks must be corrected

The license doc blocks are not valid as per specs.
They must be

/*
 * Copyright ...
 */

instead of

/**
 * Copyright ...
 */

The latter is only valid for methods, attributes, classes/interfaces directly, not for the top of files.
See symfony etc

FQCN Docblock/Body fixer influence each other

Consider this Factory class:

<?php

namespace Pyz\Zed\Example\Business;

use Pyz\Zed\Example\ExampleConfig;
use Spryker\Zed\Kernel\Business\AbstractBusinessFactory;

/**
 * @method \Pyz\Zed\Example\ExampleConfig getConfig()
 * @method \Pyz\Zed\Example\Persistence\ExampleQueryContainerInterface getQueryContainer()
 */
class ExampleBusinessFactory extends AbstractBusinessFactory
{

    /**
     * @return ExampleConfig
     */
    public function createMyTestClass()
    {
        return new \Pyz\Zed\Example\ExampleConfig();
    }

}

While architecture-wise, this one function does not make sense, it reveals a bug by the fixers:

If you run the standard fixer-set over this code, you'll get:

<?php

namespace Pyz\Zed\Example\Business;

use Spryker\Zed\Kernel\Business\AbstractBusinessFactory;

/**
 * @method \Pyz\Zed\Example\ExampleConfig getConfig()
 * @method \Pyz\Zed\Example\Persistence\ExampleQueryContainerInterface getQueryContainer()
 */
class ExampleBusinessFactory extends AbstractBusinessFactory
{

    /**
     * @return \Pyz\Zed\Example\ExampleConfig
     */
    public function createMyTestClass()
    {
        return new ExampleConfig();
    }

}

This is almost correct, but it won't work. The import for the example config has been wrongly removed and thus is unknown inside of createMyTestClass. The problem is that one fixer removes the FQCN in the method block (which is correct), adding the import statement. The next fixer, however, adds the FQCN to the docblock (which is also correct), but in turn removes the use statement. The problem occurs because, during fixing, there is a point where the fixer thinks the import is unused and removes it (and that is because the class is still fully qualified in the method block). That should be changed.

File Doc block issue

<?php

namespace Unit\Spryker\Zed\Gui\Communication\Fixture;

Gets the namespace keyword killed when running fixer,
and replaced by copyright text

/**
 * Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
 * Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
 */
Unit\Spryker\Zed\Gui\Communication\Fixture;

Use self in PHP7 typehinting

ArgumentInterface.php:

public function setCallbacks(array $callbacks): ArgumentInterface;

should be

public function setCallbacks(array $callbacks): self;

New sniff: Throwing exceptions with empty messages

Exceptions with empty messages can be frustrating because they lack valuable information.
Often, this information is present when throwing the exception, but is not passed in (example: Invalid order because an order could not be find by reference - here, the reference would be a useful info)

There is no reason why an empty exception message makes sense and a sniffer should be relatively straightforward - with good benefits for later debugging.

Sniff for inline doc blocks needed

First of all

/* @var $stateMachineItem StateMachineItemTransfer */

should be

/* @var StateMachineItemTransfer $stateMachineItem */

And then it should be FQCN.

It also should fix inline /** ... to /* ....
Lastly whitespace around it should be trimmed to a single one maybe.

Problem with multiple lines in the end of file

Code sniff fails if after any class we add 2 empty lines (or more)

............................................................ 1380 / 2092 (66%)
..........................................PHP Notice:  Undefined offset: 537 in /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php on line 826
PHP Stack trace:
PHP   1. {main}() /data/shop/development/current/vendor/squizlabs/php_codesniffer/scripts/phpcs:0
PHP   2. PHP_CodeSniffer_CLI->runphpcs() /data/shop/development/current/vendor/squizlabs/php_codesniffer/scripts/phpcs:25
PHP   3. PHP_CodeSniffer_CLI->process() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:113
PHP   4. PHP_CodeSniffer->processFiles() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:998
PHP   5. PHP_CodeSniffer->processFile() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:653
PHP   6. PHP_CodeSniffer->_processFile() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1772
PHP   7. PHP_CodeSniffer_File->start() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1894
PHP   8. Spryker\Sniffs\WhiteSpace\EmptyLinesSniff->process() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:576
PHP   9. Spryker\Sniffs\WhiteSpace\EmptyLinesSniff->assertMaximumOneEmptyLineBetweenContent() /data/shop/development/current/vendor/spryker/code-sniffer/Spryker/Sniffs/Whitespace/EmptyLinesSniff.php:40
PHP  10. PHP_CodeSniffer_File->addFixableError() /data/shop/development/current/vendor/spryker/code-sniffer/Spryker/Sniffs/Whitespace/EmptyLinesSniff.php:60
PHP  11. PHP_CodeSniffer_File->addError() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:938
PHP Notice:  Undefined offset: 537 in /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php on line 827
PHP Stack trace:
PHP   1. {main}() /data/shop/development/current/vendor/squizlabs/php_codesniffer/scripts/phpcs:0
PHP   2. PHP_CodeSniffer_CLI->runphpcs() /data/shop/development/current/vendor/squizlabs/php_codesniffer/scripts/phpcs:25
PHP   3. PHP_CodeSniffer_CLI->process() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:113
PHP   4. PHP_CodeSniffer->processFiles() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php:998
PHP   5. PHP_CodeSniffer->processFile() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:653
PHP   6. PHP_CodeSniffer->_processFile() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1772
PHP   7. PHP_CodeSniffer_File->start() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1894
PHP   8. Spryker\Sniffs\WhiteSpace\EmptyLinesSniff->process() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:576
PHP   9. Spryker\Sniffs\WhiteSpace\EmptyLinesSniff->assertMaximumOneEmptyLineBetweenContent() /data/shop/development/current/vendor/spryker/code-sniffer/Spryker/Sniffs/Whitespace/EmptyLinesSniff.php:40
PHP  10. PHP_CodeSniffer_File->addFixableError() /data/shop/development/current/vendor/spryker/code-sniffer/Spryker/Sniffs/Whitespace/EmptyLinesSniff.php:60
PHP  11. PHP_CodeSniffer_File->addError() /data/shop/development/current/vendor/squizlabs/php_codesniffer/CodeSniffer/File.php:938
E................. 1440 / 2092 (69%)
............................................................ 1500 / 2092 (72%)
............................................................ 1560 / 2092 (75%)

Add interface vs class doc block description sniff

We need a sniffer to assert that the

/**
     * Specification:
     *  - ...
     *
     * @api
     *

part is consistent for class and the implemented interface!

Only assert, no auto-fix to avoid killing the wrong version. Must be resolved manually.

Sniffer crashes on single-line dockblocks

Steps to reproduce:

  • Add new class into project and add a single-line dockblock to it's method:
<?php

class Test
{
    /** @return bool */
    public function test()
    {
        return true;
    }
}
  • Run code sniffer with vendor/bin/console code:sniff

Actual behaviour
Sniffer crashes with error

PHP Fatal error:  Allowed memory size of 2147483648 bytes exhausted (tried to allocate 4096 bytes) in /data/shop/development/current/vendor/spryker/code-sniffer/Spryker/Sniffs/Commenting/DocBlockTagGroupingSniff.php on line 245

Expected behaviour
Sniffer should handle (and fix automatically if -f option is selected) given dockblock

Comments with class-hints

Original issue: https://github.com/spryker/spryker/issues/1468

We need specific class-hints in dock-blocks. E.g. we need this in any controller of the discount-bundle:

/**
 * @method \Spryker\Zed\Discount\Communication\DiscountCommunicationFactory getFactory()
 * @method \Spryker\Zed\Discount\Business\DiscountFacade getFacade()
 * @method \Spryker\Zed\Discount\Persistence\DiscountQueryContainer getQueryContainer()
 */

Please a sniffer which adds missing @methods. It does not change existing ones! Of course the sniffer only adds if there is a related factory, facade or query container. In project code it prefers the ones from project, but falls back to the ones from core (not needed then as core already got them).

This is needed for:

  • Controllers
  • QueryContainer
  • Client
  • (Where else?)

Api tag should come after inheritdoc.

Just got a comment from @tamasnyulas and realized that

/**
     * @api
     *
     * {@inheritdoc}
     *
     * @param \Generated\Shared\Transfer\OrderTransfer $orderTransfer
     *
     * @return \Generated\Shared\Transfer\OrderTransfer
     */

should be fixed to

/**
     * {@inheritdoc}
     *
     * @api
     *
     * @param \Generated\Shared\Transfer\OrderTransfer $orderTransfer
     *
     * @return \Generated\Shared\Transfer\OrderTransfer
     */

I think a fixer for that would be a good idea.

Allow project specific licenses

Currently, as soon as the composer name is not spryker/demoshop, it will just skip the license part completely.

The idea could be that we have some meta file (.license) inside the project root, and if present it will use that one instead of the MIT one.

Trailing newline in files

Make sure

\ No newline at end of file

issue is not happening for PHP classes at least.

EDIT: Seems to be not an issue after all

Multi-byte string operations

All string operations must be multibyte compatible. That's why we need a code sniffer to detect cases where we don't use multi-byte string operations.

Please allow private members in CS for project scope

When we run on project, which uses private:
vendor/bin/phpcs /data/shop/development/current/src/Pyz --standard=/data/shop/development/current/vendor/spryker/code-sniffer/Spryker/ruleset.xml --ignore=vendor/

We receive:
15 | ERROR | [x] Pyz\Yves\Cart\CartFactory::test is private.

We should allow privates for the project scope.

PS: the same happens, when run through our wrapper.

Move default config to project

We need for spryker demoshop implementation to move the default config to ROOT/config etc.
We also need to change the -m to project
For this we need a new -c (--core) bool flag.

So

-m Acl -c

scans core module

while

-m Acl

would scan project dirs for all Acl application layers (Client, Shared, Yves, Zed, Service).

FQCN in closures

$container[static::SERVICE_UTIL_VALIDATE] = function (\Spryker\Yves\Kernel\Container $container)

does not fix the FQCN into use statement.

Array typehint cleanup

array|\Foo[]

should be auto-fxed to

\Foo[]

The typehint with [] includes array. We only need typehint extra if object \ArrayObject etc.

SprykerFacadeSniff does not work correctly in PhpStorm IDE

SprykerFacadeSniff fails when using in IDE (PhpStorm becomes annoying):

screen shot 2017-02-14 at 22 38 34

The reason is that PhpStorm uses tmp folders for caching (and local changes history), and SprykerFacadeSniff uses file_exists to check if XyzFacade has XyzFacadeInterface at the same location (which is not the case with tmp folder structure of IDE, and additionally file_get_contents fails for the same reason:

screen shot 2017-02-14 at 22 39 06

API tag for client methods

We already have some auto fixers for missing @api tags, but for Client methods this feature is missing.

license

MIT?
Some of those might be valuable for others or project level and should be able to be used freely.

Ordering wrong for aliasing

use Silex\Application\TranslationTrait;
use Silex\Application\TwigTrait;
use Silex\Application\UrlGeneratorTrait;
use Silex\Application as SilexApplication;
use Symfony\Cmf\Component\Routing\ChainRouter;
use Symfony\Component\Routing\RouterInterface;

should be

use Silex\Application as SilexApplication;
use Silex\Application\TranslationTrait;
use Silex\Application\TwigTrait;
use Silex\Application\UrlGeneratorTrait;
use Symfony\Cmf\Component\Routing\ChainRouter;
use Symfony\Component\Routing\RouterInterface;

note the " as "

`UseInAlphabeticalOrderSniff` sorting order

Sorting order in UseInAlphabeticalOrderSniff is different from PhpStorm implementation where Code > Reformat Code menu item and Before Commit > Optimize imports checkbox in Commit dialog re-sort imports differently:

// PhpStorm sorting
use Foo\Bar\Baz;
use Foo\BarBaz;
use FooBar\Baz;
use FooBarBaz;

// Spryker code-sniffer sorting
use FooBarBaz;
use FooBar\Baz;
use Foo\BarBaz;
use Foo\Bar\Baz;

A nice-to-have feature is to have sorting normalized. Just consider that Foo bundle is usually before FooBar bundle, but code-sniffer requires classes from Foo to be after classes from FooBar in imports section.

Bug in constructor args

Missing 1st param (utilDataReaderService) is not detected:

    /**
     * @param array|\Pyz\Zed\Importer\Business\Importer\ImporterInterface[] $importerCollection
     * @param string $dataDirectory
     * @param \Spryker\Zed\ProductSearch\Business\ProductSearchFacadeInterface $productSearchFacade
     */
    public function __construct(UtilDataReaderServiceInterface $utilDataReaderService, array $importerCollection, $dataDirectory, ProductSearchFacadeInterface $productSearchFacade)
    {
    }

Catch FQCN

} catch (\OutOfBoundsException $e) {

Should be FQCN resolved to use statement.

Add DocBlockTagGroupingSniff

so we can remove the fixers from Development bundle

=> see DocBlockTagGroupingSniff::checkAnnotationTagGrouping()

Always use normalized doc block

/** @var string */
protected $x;

to

/**
 * @var string 
 */
protected $x;

etc

Only inline doc blocks inside code is allowed.

Ternary operator spacing

$this->something()? $this->something() : []
should be
$this->something() ? $this->something() : []

Spacing is not correct here, missing whitespace before ?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.