🚨

We have a contradicting PHPCS ruleset 😿

 746 | ERROR | [x] Expected 1 space(s) after cast statement; 0 found
     |       |     (Generic.Formatting.SpaceAfterCast.NoSpace)

 746 | ERROR | [x] No space before opening casting parenthesis is
     |       |     prohibited
     |       |     (WordPress.WhiteSpace.CastStructureSpacing.NoSpaceBeforeOpenParenthesis)

 746 | ERROR | [x] No space after closing casting parenthesis is prohibited
     |       |     (WordPress.WhiteSpace.CastStructureSpacing.NoSpaceAfterCloseParenthesis)

szepeviktor commented 3 years ago

@acobster Please consider publishing a well-made SiteCrafting Coding Standard.

szepeviktor commented 3 years ago

You may see my useful frankenstein here https://github.com/szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset

acobster commented 3 years ago

@szepeviktor

Please consider publishing a well-made SiteCrafting Coding Standard.

Sure thing, wanna help pitch my time on this to my boss? :laughing:

I will take a look at the Frankenstein monster before 1.0 and get it figured out. Thanks, that's super helpful!

szepeviktor commented 3 years ago

wanna help pitch my time on this to my boss?

Without a Coding Standard you cannot run CS check thus you manufacture a mess.

acobster commented 3 years ago

Hold on - I want to make sure I understand the big picture here. My understanding is that PHPStan would replace PHPCS as the "code sniffing" tool of choice. Why, then, do we care about PHPCS rulesets at all? Is it because PHPStan can reuse PHPCS ruleset definitions? Or does it actually solve a different problem than PHPCS, and it's useful to have both?

We have a contradicting PHPCS ruleset

By that do you just mean that the standards set out in phpcs.xml contradict your Frankenstein rules? If a contributor on a coding standards tool has developed their own ruleset for working on WP code, I'm all for adopting those. I'm sure our Frankenstein is even more monstrous than yours. :stuck_out_tongue:

acobster commented 3 years ago

@szepeviktor another quick question - for the bootstrap file for discovering symbols is it OK to reuse the same bootstrap file as we use for tests, or do you recommend using something more like what would run in production?

szepeviktor commented 3 years ago

My understanding is that PHPStan would replace PHPCS as the "code sniffing" tool of choice.

These tools have some overlap but

PHPCS' main feature is to check the indentation, spacing, line breaks of your code (PHPStan is blind in this regard)
PHPStan analysis your whole code base concentrating on types (PHPCS does no/very little type analysis and does it file-by-file)

Please read the docs.

By that do you just mean that the standards set out in phpcs.xml contradict your Frankenstein rules?

Your PHPCS rules contradicts internally, one with another. See the above output: "Use!" and "Don't use!" at the same time.

for the bootstrap file for discovering symbols

Bootstrap file for executed tests are functional, you really need those inside it. For static analysis you only need empty constant and function stubs in the bootstrap file.

acobster commented 3 years ago

@szepeviktor FYI I tried your ruleset but there appears to be some kind of incompatibility between the Neutron definitions and PHPCS:

$ ./vendor/bin/phpcs --standard=PSR12NeutronRuleset test lib
PHP Fatal error:  Declaration of PSR12NeutronRuleset\Sniffs\Strings\ConcatenationUsageSniff::process(PHP_CodeSniffer\Files\File $phpcsFile, int $stackPtr) must be compatible with PHP_CodeSniffer\Sniffs\Sniff::process(PHP_CodeSniffer\Files\File $phpcsFile, $stackPtr) in /home/ctamayo/workspace/conifer/vendor/szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset/PSR12NeutronRuleset/Sniffs/Strings/ConcatenationUsageSniff.php on line 38

In case you feel inclined to look into it, here's my composer.json when I got this error:

{
  "name": "sitecrafting/conifer",
  "description": "Powerful abstractions for serious WordPress theme development",
  "repositories": [
    {
      "type": "composer",
      "url": "https://wpackagist.org"
    },
    {
      "type": "vcs",
      "url": "https://github.com/sitecrafting/wp_mock"
    }
  ],
  "license": "MIT",
  "authors": [
    {
      "name": "Coby Tamayo",
      "email": "ctamayo@sitecrafting.com"
    }
  ],
  "minimum-stability": "dev",
  "require": {
    "php": "^7.4"
  },
  "require-dev": {
    "10up/wp_mock": "dev-dev",
    "phpunit/phpunit": "^6.0",
    "mikey179/vfsstream": "~1",
    "behat/behat": "^3.4",
    "johnpbloch/wordpress-core-installer": "^1.0",
    "johnpbloch/wordpress-core": "5.5.1",
    "mnsami/composer-custom-directory-installer": "^1.1",
    "sitecrafting/groot": "dev-timber-2.x",
    "squizlabs/php_codesniffer": "^3.5",
    "timber/timber": "dev-2.x-factories",
    "acobster/wp-cli-yaml-fixtures": "^0.5.0",
    "victorjonsson/markdowndocs": "dev-master",
    "szepeviktor/phpstan-wordpress": "^0.6.5",
    "paulthewalton/acf-stubs": "^5.8.7",
    "szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset": "^0.4"
  },
  "config": {
    "platform": {
      "php": "7.4"
    }
  },
  "extra": {
    "wordpress-install-dir": {
      "johnpbloch/wordpress-core": "wp"
    },
    "installer-paths": {
      "wp/wp-content/themes/groot": [
        "sitecrafting/groot"
      ]
    }
  },
  "autoload": {
    "psr-4": {
      "Conifer\\": "lib/Conifer/"
    }
  },
  "scripts": {
    "unit": [
      "./vendor/bin/phpunit"
    ],
    "phpstan": [
      "./vendor/bin/phpstan analyse"
    ]
  }
}

I need to be done with the yak-shaving for now. I will circle back to coding standards compliance stuff as I get closer to 1.0.

szepeviktor commented 3 years ago

FYI I tried your ruleset but there appears to be some kind of incompatibility between the Neutron definitions and PHPCS

Thank you! Fixed and released.

ConcatenationUsageSniff was never actually in action :)

sitecrafting / conifer

Fix Coding Standards compliance #138

🚨