Improve Coding Standard Checks

[gordonbanderson]

hi Sam,

This is a tool I originally wrote to tidy up some old (and new) SilverStripe modules so that strict coding standards are adhered to - https://github.com/gordonbanderson/php-travis-enhancer

Given this module was only a couple of files I decided to run the enhancer tool over this. It adds the following checks:

• Slevomat Coding Standards - a selection thereof, as some break PHP 7.1
• PHPStan - static analysis tool
• Psalm - ditto
• PHP Linting - check for syntax errors
• JSCPD - Code duplication check

This was actually the first project I've tried where duplication was picked up, and it didn't fail the build initially, due to a missing threshold parameter :(

One other tool that could be added to the mix is leasot, which one can configure to fail a build if FIXME or TODO comments are found.

I have tried updating a couple of large PHP projects to comply, it's a lot of effort. But for small modules or new modules started from scratch , I am finding it useful.

Cheers

Gordon

PS I 've updated the README badges, but the Scrutinizer badge is not working. I guess the project has not been submitted to Scrutinizer, as such either submit it or remove the badge.

[gordonbanderson]

And I forgot to squash the commits for the PR :(

[sminnee]

Thanks!

• Using both phpstan and psalm feels like overkill, can we remove psalm please?
• I'm not comfortable with the changes that JSCPD introduced especially to the tests. Copy/paste keeps tests clearer. I'd suggest pulling a it JSCPD for now, or at least limiting it to src

[gordonbanderson]

1) Fair enough. 2) Re jscpd your suggestion of running this tool avoiding the tests directory does in retrospect make sense.

[gordonbanderson]

Thanks for the feedback, I'm fine critical feedback, it's how one learns and moves forward. I will make requested fixes over the weekend.

[sminnee]

Great, thanks for the PR! :-)

[sminnee]

Hi @gordonbanderson I hope you don't mind, I force-pushed back to your branch the rebasing and changes needed to get it mergeable.

Notably:

• removed the some of the badges to keep it a more manageable list
• travis.yml uses 2 spaces for indentation
• travis uses the composer scripts for execution
• no psalm
• no duplication cleanup on the test
• one travis run for all 'checks'

Thanks for starting this off, it should make the codebase a lot more solid!

[codecov-commenter]

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@924b4f9). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff            @@
##             master       #3   +/-   ##
=========================================
  Coverage          ?   91.66%           
  Complexity        ?       11           
=========================================
  Files             ?        1           
  Lines             ?       24           
  Branches          ?        0           
=========================================
  Hits              ?       22           
  Misses            ?        2           
  Partials          ?        0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 924b4f9...3dcb52f. Read the comment docs.

[gordonbanderson]

@sminnee I did not get this PR correct but I can see from your comments that it has got you thinking. Zero intentions to upset applecarts, just a suggestion of moving code forward. I hope I got this right

[sminnee]

Not at all, I appreciate the work! The module is much more robust now :-)