From 9c45852e7c9bcea5dddb84d5ce3d677bdce2456c Mon Sep 17 00:00:00 2001 From: Craig Davis Date: Sun, 6 Aug 2017 20:22:37 -0700 Subject: [PATCH 1/2] Add initial support for phpcs and php lint Add two new linters: * PHP Lint via the 'php -l' command to check for valid php syntax and confirm that the file will compile * PHP Code Sniffer via 'phpcs' to check for style problems This can later be followed by other php tooling, but these are the most common and are a good starting point. --- .gitignore | 1 + config/default.yml | 15 +++++ lib/overcommit/hook/pre_commit/php_lint.rb | 42 ++++++++++++ lib/overcommit/hook/pre_commit/phpcs.rb | 48 ++++++++++++++ .../hook/pre_commit/php_lint_spec.rb | 45 +++++++++++++ spec/overcommit/hook/pre_commit/phpcs_spec.rb | 66 +++++++++++++++++++ 6 files changed, 217 insertions(+) create mode 100644 lib/overcommit/hook/pre_commit/php_lint.rb create mode 100644 lib/overcommit/hook/pre_commit/phpcs.rb create mode 100644 spec/overcommit/hook/pre_commit/php_lint_spec.rb create mode 100644 spec/overcommit/hook/pre_commit/phpcs_spec.rb diff --git a/.gitignore b/.gitignore index 410188f0..2bb7f841 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ Gemfile.lock coverage/ pkg/ .bundle +.idea diff --git a/config/default.yml b/config/default.yml index 01d27645..2f73647d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -448,6 +448,21 @@ PreCommit: install_command: 'pip install pep8' include: '**/*.py' + PhpLint: + enabled: false + description: 'Testing with PHP lint' + required_executable: 'php' + command: 'php' + flags: ['-l'] + include: '**/*.php' + + PhpCs: + enabled: false + description: 'Analyze with PHP_CodeSniffer' + command: 'vendor/bin/phpcs' + flags: ['--standard=PSR2', '--report=csv'] + include: '**/*.php' + Pronto: enabled: false description: 'Analyzing with pronto' diff --git a/lib/overcommit/hook/pre_commit/php_lint.rb b/lib/overcommit/hook/pre_commit/php_lint.rb new file mode 100644 index 00000000..f34fc433 --- /dev/null +++ b/lib/overcommit/hook/pre_commit/php_lint.rb @@ -0,0 +1,42 @@ +module Overcommit::Hook::PreCommit + # Runs `php -l` against any modified PHP files. + class PhpLint < Base + # Sample String + # rubocop:disable Metrics/LineLength + # PHP Parse error: syntax error, unexpected 'require_once' (T_REQUIRE_ONCE) in site/sumo.php on line 12 + # rubocop:enable Metrics/LineLength + MESSAGE_REGEX = /^(?.+)\:\s+(?.+) in (?.+) on line (?\d+)/ + + def run + # A list of error messages + messages = [] + + # Exit status for all of the runs. Should be zero! + exit_status_sum = 0 + + # Run for each of our applicable files + applicable_files.each do |file| + result = execute(command, args: [file]) + output = result.stdout.chomp + exit_status_sum += result.status + if result.status + # `php -l` returns with a leading newline, and we only need the first + # line, there is usually some redundancy + messages << output.lstrip.split("\n").first + end + end + + # If the sum of all lint status is zero, then none had exit status + return :pass if exit_status_sum == 0 + + # No messages is great news for us + return :pass if messages.empty? + + # Return the list of message objects + extract_messages( + messages, + MESSAGE_REGEX + ) + end + end +end diff --git a/lib/overcommit/hook/pre_commit/phpcs.rb b/lib/overcommit/hook/pre_commit/phpcs.rb new file mode 100644 index 00000000..86c400d7 --- /dev/null +++ b/lib/overcommit/hook/pre_commit/phpcs.rb @@ -0,0 +1,48 @@ +module Overcommit::Hook::PreCommit + # Runs `phpcs` against any modified PHP files. + class PhpCs < Base + # Parse `phpcs` csv mode output + MESSAGE_REGEX = /^\"(?.+)\",(?\d+),\d+,(?.+),\"(?.+)\"/ + MESSAGE_TYPE_CATEGORIZER = lambda do |type| + 'error'.include?(type) ? :error : :warning + end + + def run + messages = [] + + applicable_files.each do |file| + result = execute(command, args: [file]) + if result.status + rows = result.stdout.split("\n") + + # Discard the csv header + rows.shift + + # Push each of the errors in the particular file into the array + rows.map do |row| + messages << row + end + end + end + + return :pass if messages.empty? + + parse_messages(messages) + end + + # Transform the CSV output into a tidy human readable message + def parse_messages(messages) + output = [] + + messages.map do |message| + message.scan(MESSAGE_REGEX).map do |file, line, type, msg| + type = MESSAGE_TYPE_CATEGORIZER.call(type) + text = " #{file}:#{line}\n #{msg}" + output << Overcommit::Hook::Message.new(type, file, line.to_i, text) + end + end + + output + end + end +end diff --git a/spec/overcommit/hook/pre_commit/php_lint_spec.rb b/spec/overcommit/hook/pre_commit/php_lint_spec.rb new file mode 100644 index 00000000..7f2203dd --- /dev/null +++ b/spec/overcommit/hook/pre_commit/php_lint_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Overcommit::Hook::PreCommit::PhpLint do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(['sample.php']) + end + + context 'when php lint exits successfully' do + before do + result = double('result') + result.stub(:status).and_return(0) + result.stub(:success?).and_return(true) + result.stub(:stdout).and_return('No syntax errors detected in sample.php') + subject.stub(:execute).and_return(result) + end + + it { should pass } + end + + context 'when php lint exits unsuccessfully' do + before do + # php -l prints the same to both stdout and stderr + # rubocop:disable Metrics/LineLength + sample_output = [ + '', + "Parse error: syntax error, unexpected '0' (T_LNUMBER), expecting variable (T_VARIABLE) or '{' or '$' in sample.php on line 3 ", + 'Errors parsing invalid.php', + ].join("\n") + # rubocop:enable Metrics/LineLength + + result = double('result') + result.stub(:status).and_return(255) + result.stub(:success?).and_return(false) + result.stub(:stdout).and_return(sample_output) + result.stub(:stderr).and_return(sample_output) + subject.stub(:execute).and_return(result) + end + + it { should fail_hook } + end +end diff --git a/spec/overcommit/hook/pre_commit/phpcs_spec.rb b/spec/overcommit/hook/pre_commit/phpcs_spec.rb new file mode 100644 index 00000000..f480e071 --- /dev/null +++ b/spec/overcommit/hook/pre_commit/phpcs_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Overcommit::Hook::PreCommit::PhpCs do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(%w[sample.php]) + end + + context 'when phpcs exits successfully' do + before do + sample_output = [ + 'File,Line,Column,Type,Message,Source,Severity,Fixable', + '' + ].join("\n") + + result = double('result') + result.stub(:success?).and_return(true) + result.stub(:stdout).and_return(sample_output) + result.stub(:status).and_return(0) + subject.stub(:execute).and_return(result) + end + + it { should pass } + end + + context 'when phpcs exits unsuccessfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(false) + result.stub(:status).and_return(2) + subject.stub(:execute).and_return(result) + end + + context 'and it reports a warning' do + before do + # rubocop:disable Metrics/LineLength + sample_output = [ + 'File,Line,Column,Type,Message,Source,Severity,Fixable', + '"/Users/craig/HelpScout/overcommit-testing/invalid.php",5,1,warning,"Possible parse error: FOREACH has no AS statement",Squiz.ControlStructures.ForEachLoopDeclaration.MissingAs,5,0' + ].join("\n") + # rubocop:enable Metrics/LineLength + result.stub(:stdout).and_return(sample_output) + end + + it { should warn } + end + + context 'and it reports an error' do + before do + # rubocop:disable Metrics/LineLength + sample_output = [ + 'File,Line,Column,Type,Message,Source,Severity,Fixable', + '"/Users/craig/HelpScout/overcommit-testing/invalid.php",5,1,error,"Inline control structures are not allowed",Generic.ControlStructures.InlineControlStructure.NotAllowed,5,1' + ].join("\n") + # rubocop:enable Metrics/LineLength + result.stub(:stdout).and_return(sample_output) + end + + it { should fail_hook } + end + end +end From 5c5003930456cca5f18518da11339a37823d052e Mon Sep 17 00:00:00 2001 From: Craig Davis Date: Wed, 16 Aug 2017 19:55:18 -0700 Subject: [PATCH 2/2] Rename phpcs hook to pass proper camelcase convention --- lib/overcommit/hook/pre_commit/{phpcs.rb => php_cs.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/overcommit/hook/pre_commit/{phpcs.rb => php_cs.rb} (100%) diff --git a/lib/overcommit/hook/pre_commit/phpcs.rb b/lib/overcommit/hook/pre_commit/php_cs.rb similarity index 100% rename from lib/overcommit/hook/pre_commit/phpcs.rb rename to lib/overcommit/hook/pre_commit/php_cs.rb