Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial support for phpcs and php lint #507

Merged
merged 2 commits into from
Aug 18, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -2,3 +2,4 @@ Gemfile.lock
coverage/
pkg/
.bundle
.idea
15 changes: 15 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
@@ -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'
48 changes: 48 additions & 0 deletions lib/overcommit/hook/pre_commit/php_cs.rb
Original file line number Diff line number Diff line change
@@ -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 = /^\"(?<file>.+)\",(?<line>\d+),\d+,(?<type>.+),\"(?<msg>.+)\"/
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
42 changes: 42 additions & 0 deletions lib/overcommit/hook/pre_commit/php_lint.rb
Original file line number Diff line number Diff line change
@@ -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 = /^(?<type>.+)\:\s+(?<message>.+) in (?<file>.+) on line (?<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
45 changes: 45 additions & 0 deletions spec/overcommit/hook/pre_commit/php_lint_spec.rb
Original file line number Diff line number Diff line change
@@ -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
66 changes: 66 additions & 0 deletions spec/overcommit/hook/pre_commit/phpcs_spec.rb
Original file line number Diff line number Diff line change
@@ -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