Skip to content

Add solargraph pre-commit hook for typechecking #871

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
14 changes: 14 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,20 @@ PreCommit:
install_command: 'gem install slim_lint'
include: '**/*.slim'

Solargraph:
enabled: false
description: 'Typecheck with Solargraph'
requires_files: true
required_executable: 'solargraph'
install_command: 'gem install solargraph'
flags: ['typecheck', '--level', 'strong']
include: '**/*.rb'
exclude:
- 'spec/**/*.rb'
- 'test/**/*.rb'
- 'vendor/**/*.rb'
- '.bundle/**/*.rb'

Sorbet:
enabled: false
description: 'Analyze with Sorbet'
Expand Down
48 changes: 48 additions & 0 deletions lib/overcommit/hook/pre_commit/solargraph.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

require 'overcommit'
require 'overcommit/hook/pre_commit/base'

module Overcommit
module Hook
module PreCommit
# Runs `solargraph typecheck` against any modified Ruby files.
#
# @see https://github.com/castwide/solargraph
class Solargraph < Base
MESSAGE_REGEX = /^\s*(?<file>(?:\w:)?[^:]+):(?<line>\d+) - /.freeze

def run
result = execute(command, args: applicable_files)
return :pass if result.success?

stderr_lines = remove_harmless_glitches(result.stderr)
violation_lines = result.stdout.split("\n").grep(MESSAGE_REGEX)
if violation_lines.empty?
if stderr_lines.empty?
[:fail, 'Solargraph failed to run']
else
# let's feed it stderr so users see the errors
extract_messages(stderr_lines, MESSAGE_REGEX)
end
else
extract_messages(violation_lines, MESSAGE_REGEX)
end
end

private

# @param stderr [String]
#
# @return [Array<String>]
def remove_harmless_glitches(stderr)
stderr.split("\n").reject do |line|
line.include?('[WARN]') ||
line.include?('warning: parser/current is loading') ||
line.include?('Please see https://github.com/whitequark')
end
end
end
end
end
end
110 changes: 110 additions & 0 deletions spec/overcommit/hook/pre_commit/solargraph_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# frozen_string_literal: true

require 'spec_helper'

describe Overcommit::Hook::PreCommit::Solargraph do
let(:config) do
Overcommit::ConfigurationLoader.default_configuration.merge(
Overcommit::Configuration.new(
'PreCommit' => {
'Solargraph' => {
'problem_on_unmodified_line' => problem_on_unmodified_line
}
}
)
)
end
let(:problem_on_unmodified_line) { 'ignore' }
let(:context) { double('context') }
let(:messages) { subject.run }
let(:result) { double('result') }
subject { described_class.new(config, context) }

before do
subject.stub(:applicable_files).and_return(%w[file1.rb file2.rb])
result.stub(:stderr).and_return(stderr)
result.stub(:stdout).and_return(stdout)
end

context 'when Solargraph exits successfully' do
before do
result.stub(:success?).and_return(true)
subject.stub(:execute).and_return(result)
end

context 'and it printed a message to stderr' do
let(:stderr) { 'stderr unexpected message that must be fine since command successful' }
let(:stdout) { '' }
it { should pass }
end

context 'and it printed a message to stdout' do
let(:stderr) { '' }
let(:stdout) { 'stdout message that must be fine since command successful' }
it { should pass }
end
end

context 'when Solargraph exits unsucessfully' do
before do
result.stub(:success?).and_return(false)
subject.stub(:execute).and_return(result)
end

context 'and it reports typechecking issues' do
let(:stdout) do
normalize_indent(<<-MSG)
/home/username/src/solargraph-rails/file1.rb:36 - Unresolved constant Solargraph::Parser::Legacy::NodeChainer
/home/username/src/solargraph-rails/file2.rb:44 - Unresolved call to []
/home/username/src/solargraph-rails/file2.rb:99 - Unresolved call to []
Typecheck finished in 8.921023999806494 seconds.
189 problems found in 14 of 16 files.
MSG
end

['', 'unexpected output'].each do |stderr_string|
context "with stderr output of #{stderr_string.inspect}" do
let(:stderr) { stderr_string }

it { should fail_hook }
it 'reports only three errors and assumes stderr is harmless' do
expect(messages.size).to eq 3
end
it 'parses filename' do
expect(messages.first.file).to eq '/home/username/src/solargraph-rails/file1.rb'
end
it 'parses line number of messages' do
expect(messages.first.line).to eq 36
end
it 'parses and returns error message content' do
msg = '/home/username/src/solargraph-rails/file1.rb:36 - Unresolved constant Solargraph::Parser::Legacy::NodeChainer'
expect(messages.first.content).to eq msg
end
end
end
end

context 'but it reports no typechecking issues' do
let(:stdout) do
normalize_indent(<<-MSG)
Typecheck finished in 8.095239999704063 seconds.
0 problems found in 0 of 16 files.
MSG
end

context 'with no stderr output' do
let(:stderr) { '' }
it 'should return no messages' do
expect(messages).to eq([:fail, 'Solargraph failed to run'])
end
end

context 'with stderr output' do
let(:stderr) { 'something' }
it 'should raise' do
expect { messages }.to raise_error(Overcommit::Exceptions::MessageProcessingError)
end
end
end
end
end