Skip to content

Commit 09f0550

Browse files
craig-davissds
authored andcommitted
Add initial support for phpcs and php lint (sds#507)
* 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. * Rename phpcs hook to pass proper camelcase convention
1 parent 3de81fd commit 09f0550

File tree

6 files changed

+217
-0
lines changed

6 files changed

+217
-0
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ Gemfile.lock
22
coverage/
33
pkg/
44
.bundle
5+
.idea

config/default.yml

+15
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,21 @@ PreCommit:
448448
install_command: 'pip install pep8'
449449
include: '**/*.py'
450450

451+
PhpLint:
452+
enabled: false
453+
description: 'Testing with PHP lint'
454+
required_executable: 'php'
455+
command: 'php'
456+
flags: ['-l']
457+
include: '**/*.php'
458+
459+
PhpCs:
460+
enabled: false
461+
description: 'Analyze with PHP_CodeSniffer'
462+
command: 'vendor/bin/phpcs'
463+
flags: ['--standard=PSR2', '--report=csv']
464+
include: '**/*.php'
465+
451466
Pronto:
452467
enabled: false
453468
description: 'Analyzing with pronto'
+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
module Overcommit::Hook::PreCommit
2+
# Runs `phpcs` against any modified PHP files.
3+
class PhpCs < Base
4+
# Parse `phpcs` csv mode output
5+
MESSAGE_REGEX = /^\"(?<file>.+)\",(?<line>\d+),\d+,(?<type>.+),\"(?<msg>.+)\"/
6+
MESSAGE_TYPE_CATEGORIZER = lambda do |type|
7+
'error'.include?(type) ? :error : :warning
8+
end
9+
10+
def run
11+
messages = []
12+
13+
applicable_files.each do |file|
14+
result = execute(command, args: [file])
15+
if result.status
16+
rows = result.stdout.split("\n")
17+
18+
# Discard the csv header
19+
rows.shift
20+
21+
# Push each of the errors in the particular file into the array
22+
rows.map do |row|
23+
messages << row
24+
end
25+
end
26+
end
27+
28+
return :pass if messages.empty?
29+
30+
parse_messages(messages)
31+
end
32+
33+
# Transform the CSV output into a tidy human readable message
34+
def parse_messages(messages)
35+
output = []
36+
37+
messages.map do |message|
38+
message.scan(MESSAGE_REGEX).map do |file, line, type, msg|
39+
type = MESSAGE_TYPE_CATEGORIZER.call(type)
40+
text = " #{file}:#{line}\n #{msg}"
41+
output << Overcommit::Hook::Message.new(type, file, line.to_i, text)
42+
end
43+
end
44+
45+
output
46+
end
47+
end
48+
end
+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
module Overcommit::Hook::PreCommit
2+
# Runs `php -l` against any modified PHP files.
3+
class PhpLint < Base
4+
# Sample String
5+
# rubocop:disable Metrics/LineLength
6+
# PHP Parse error: syntax error, unexpected 'require_once' (T_REQUIRE_ONCE) in site/sumo.php on line 12
7+
# rubocop:enable Metrics/LineLength
8+
MESSAGE_REGEX = /^(?<type>.+)\:\s+(?<message>.+) in (?<file>.+) on line (?<line>\d+)/
9+
10+
def run
11+
# A list of error messages
12+
messages = []
13+
14+
# Exit status for all of the runs. Should be zero!
15+
exit_status_sum = 0
16+
17+
# Run for each of our applicable files
18+
applicable_files.each do |file|
19+
result = execute(command, args: [file])
20+
output = result.stdout.chomp
21+
exit_status_sum += result.status
22+
if result.status
23+
# `php -l` returns with a leading newline, and we only need the first
24+
# line, there is usually some redundancy
25+
messages << output.lstrip.split("\n").first
26+
end
27+
end
28+
29+
# If the sum of all lint status is zero, then none had exit status
30+
return :pass if exit_status_sum == 0
31+
32+
# No messages is great news for us
33+
return :pass if messages.empty?
34+
35+
# Return the list of message objects
36+
extract_messages(
37+
messages,
38+
MESSAGE_REGEX
39+
)
40+
end
41+
end
42+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
require 'spec_helper'
2+
3+
describe Overcommit::Hook::PreCommit::PhpLint do
4+
let(:config) { Overcommit::ConfigurationLoader.default_configuration }
5+
let(:context) { double('context') }
6+
subject { described_class.new(config, context) }
7+
8+
before do
9+
subject.stub(:applicable_files).and_return(['sample.php'])
10+
end
11+
12+
context 'when php lint exits successfully' do
13+
before do
14+
result = double('result')
15+
result.stub(:status).and_return(0)
16+
result.stub(:success?).and_return(true)
17+
result.stub(:stdout).and_return('No syntax errors detected in sample.php')
18+
subject.stub(:execute).and_return(result)
19+
end
20+
21+
it { should pass }
22+
end
23+
24+
context 'when php lint exits unsuccessfully' do
25+
before do
26+
# php -l prints the same to both stdout and stderr
27+
# rubocop:disable Metrics/LineLength
28+
sample_output = [
29+
'',
30+
"Parse error: syntax error, unexpected '0' (T_LNUMBER), expecting variable (T_VARIABLE) or '{' or '$' in sample.php on line 3 ",
31+
'Errors parsing invalid.php',
32+
].join("\n")
33+
# rubocop:enable Metrics/LineLength
34+
35+
result = double('result')
36+
result.stub(:status).and_return(255)
37+
result.stub(:success?).and_return(false)
38+
result.stub(:stdout).and_return(sample_output)
39+
result.stub(:stderr).and_return(sample_output)
40+
subject.stub(:execute).and_return(result)
41+
end
42+
43+
it { should fail_hook }
44+
end
45+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
require 'spec_helper'
2+
3+
describe Overcommit::Hook::PreCommit::PhpCs do
4+
let(:config) { Overcommit::ConfigurationLoader.default_configuration }
5+
let(:context) { double('context') }
6+
subject { described_class.new(config, context) }
7+
8+
before do
9+
subject.stub(:applicable_files).and_return(%w[sample.php])
10+
end
11+
12+
context 'when phpcs exits successfully' do
13+
before do
14+
sample_output = [
15+
'File,Line,Column,Type,Message,Source,Severity,Fixable',
16+
''
17+
].join("\n")
18+
19+
result = double('result')
20+
result.stub(:success?).and_return(true)
21+
result.stub(:stdout).and_return(sample_output)
22+
result.stub(:status).and_return(0)
23+
subject.stub(:execute).and_return(result)
24+
end
25+
26+
it { should pass }
27+
end
28+
29+
context 'when phpcs exits unsuccessfully' do
30+
let(:result) { double('result') }
31+
32+
before do
33+
result.stub(:success?).and_return(false)
34+
result.stub(:status).and_return(2)
35+
subject.stub(:execute).and_return(result)
36+
end
37+
38+
context 'and it reports a warning' do
39+
before do
40+
# rubocop:disable Metrics/LineLength
41+
sample_output = [
42+
'File,Line,Column,Type,Message,Source,Severity,Fixable',
43+
'"/Users/craig/HelpScout/overcommit-testing/invalid.php",5,1,warning,"Possible parse error: FOREACH has no AS statement",Squiz.ControlStructures.ForEachLoopDeclaration.MissingAs,5,0'
44+
].join("\n")
45+
# rubocop:enable Metrics/LineLength
46+
result.stub(:stdout).and_return(sample_output)
47+
end
48+
49+
it { should warn }
50+
end
51+
52+
context 'and it reports an error' do
53+
before do
54+
# rubocop:disable Metrics/LineLength
55+
sample_output = [
56+
'File,Line,Column,Type,Message,Source,Severity,Fixable',
57+
'"/Users/craig/HelpScout/overcommit-testing/invalid.php",5,1,error,"Inline control structures are not allowed",Generic.ControlStructures.InlineControlStructure.NotAllowed,5,1'
58+
].join("\n")
59+
# rubocop:enable Metrics/LineLength
60+
result.stub(:stdout).and_return(sample_output)
61+
end
62+
63+
it { should fail_hook }
64+
end
65+
end
66+
end

0 commit comments

Comments
 (0)