Skip to content

Commit 74be2e3

Browse files
committed
Add PksWrapper to run pks check with JSON output
Shells out to `pks check --output-format json <files>`, parses the response, and converts violations to PksOffense objects. Handles pks binary not found gracefully by raising PksBinaryNotFoundError.
1 parent d11ea6d commit 74be2e3

3 files changed

Lines changed: 168 additions & 0 deletions

File tree

lib/danger-packwerk.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@ module DangerPackwerk
1616
require 'danger-packwerk/update/offenses_formatter'
1717
require 'danger-packwerk/update/default_formatter'
1818
require 'danger-packwerk/pks_offense'
19+
require 'danger-packwerk/pks_wrapper'
1920
end

lib/danger-packwerk/pks_wrapper.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# typed: strict
2+
3+
require 'open3'
4+
require 'json'
5+
6+
module DangerPackwerk
7+
class PksWrapper
8+
extend T::Sig
9+
10+
class PksBinaryNotFoundError < StandardError; end
11+
12+
sig { params(files: T::Array[String]).returns(T::Array[PksOffense]) }
13+
def self.get_offenses_for_files(files)
14+
return [] if files.empty?
15+
16+
stdout, stderr, _status = run_pks_check(files)
17+
18+
if stderr.include?('command not found') || stderr.include?('No such file or directory')
19+
raise PksBinaryNotFoundError, 'pks binary not found. Please install pks to use this feature.'
20+
end
21+
22+
PksOffense.from_json(stdout)
23+
end
24+
25+
sig { params(files: T::Array[String]).returns([String, String, Process::Status]) }
26+
def self.run_pks_check(files)
27+
require 'shellwords'
28+
escaped_files = files.map { |f| Shellwords.escape(f) }.join(' ')
29+
command = "pks check --output-format json #{escaped_files}"
30+
Open3.capture3(command)
31+
end
32+
33+
sig { returns(T::Boolean) }
34+
def self.pks_available?
35+
_, _, status = Open3.capture3('which', 'pks')
36+
!!status.success?
37+
end
38+
end
39+
end
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
require 'spec_helper'
2+
3+
module DangerPackwerk
4+
RSpec.describe PksWrapper do
5+
let(:plugin) { dangerfile.packwerk }
6+
7+
let(:valid_json_output) do
8+
{
9+
'offenses' => [
10+
{
11+
'violation_type' => 'privacy',
12+
'file' => 'packs/my_pack/app/models/user.rb',
13+
'line' => 42,
14+
'column' => 10,
15+
'constant_name' => '::OtherPack::PrivateClass',
16+
'referencing_pack_name' => 'packs/my_pack',
17+
'defining_pack_name' => 'packs/other_pack',
18+
'strict' => false,
19+
'message' => 'Privacy violation'
20+
}
21+
]
22+
}.to_json
23+
end
24+
25+
let(:empty_json_output) do
26+
{ 'offenses' => [] }.to_json
27+
end
28+
29+
# Use real Process::Status from a successful command for Sorbet compatibility
30+
let(:success_status) { `true`; $? }
31+
let(:failure_status) { `false`; $? }
32+
33+
describe '.get_offenses_for_files' do
34+
it 'returns empty array when files is empty' do
35+
expect(described_class.get_offenses_for_files([])).to eq([])
36+
end
37+
38+
it 'calls pks check with JSON output format' do
39+
allow(described_class).to receive(:run_pks_check)
40+
.with(['file1.rb', 'file2.rb'])
41+
.and_return([valid_json_output, '', success_status])
42+
43+
described_class.get_offenses_for_files(['file1.rb', 'file2.rb'])
44+
45+
expect(described_class).to have_received(:run_pks_check).with(['file1.rb', 'file2.rb'])
46+
end
47+
48+
it 'returns PksOffense objects from JSON output' do
49+
allow(described_class).to receive(:run_pks_check)
50+
.and_return([valid_json_output, '', success_status])
51+
52+
offenses = described_class.get_offenses_for_files(['file1.rb'])
53+
54+
expect(offenses.length).to eq(1)
55+
expect(offenses.first).to be_a(PksOffense)
56+
expect(offenses.first.violation_type).to eq('privacy')
57+
expect(offenses.first.file).to eq('packs/my_pack/app/models/user.rb')
58+
end
59+
60+
it 'returns empty array when no violations found' do
61+
allow(described_class).to receive(:run_pks_check)
62+
.and_return([empty_json_output, '', success_status])
63+
64+
offenses = described_class.get_offenses_for_files(['file1.rb'])
65+
66+
expect(offenses).to eq([])
67+
end
68+
69+
it 'raises PksBinaryNotFoundError when pks command not found' do
70+
allow(described_class).to receive(:run_pks_check)
71+
.and_return(['', 'command not found: pks', failure_status])
72+
73+
expect do
74+
described_class.get_offenses_for_files(['file1.rb'])
75+
end.to raise_error(PksWrapper::PksBinaryNotFoundError, /pks binary not found/)
76+
end
77+
78+
it 'raises PksBinaryNotFoundError for No such file error' do
79+
allow(described_class).to receive(:run_pks_check)
80+
.and_return(['', 'No such file or directory', failure_status])
81+
82+
expect do
83+
described_class.get_offenses_for_files(['file1.rb'])
84+
end.to raise_error(PksWrapper::PksBinaryNotFoundError, /pks binary not found/)
85+
end
86+
end
87+
88+
describe '.run_pks_check' do
89+
it 'executes pks command with proper escaping' do
90+
allow(Open3).to receive(:capture3)
91+
.with('pks check --output-format json file1.rb file2.rb')
92+
.and_return([empty_json_output, '', success_status])
93+
94+
described_class.run_pks_check(['file1.rb', 'file2.rb'])
95+
96+
expect(Open3).to have_received(:capture3)
97+
.with('pks check --output-format json file1.rb file2.rb')
98+
end
99+
100+
it 'escapes file paths with special characters' do
101+
allow(Open3).to receive(:capture3).and_return([empty_json_output, '', success_status])
102+
103+
described_class.run_pks_check(['file with spaces.rb'])
104+
105+
expect(Open3).to have_received(:capture3)
106+
.with('pks check --output-format json file\\ with\\ spaces.rb')
107+
end
108+
end
109+
110+
describe '.pks_available?' do
111+
it 'returns true when pks is in PATH' do
112+
allow(Open3).to receive(:capture3)
113+
.with('which', 'pks')
114+
.and_return(['/usr/local/bin/pks', '', success_status])
115+
116+
expect(described_class.pks_available?).to eq(true)
117+
end
118+
119+
it 'returns false when pks is not in PATH' do
120+
allow(Open3).to receive(:capture3)
121+
.with('which', 'pks')
122+
.and_return(['', '', failure_status])
123+
124+
expect(described_class.pks_available?).to eq(false)
125+
end
126+
end
127+
end
128+
end

0 commit comments

Comments
 (0)