From ab4beba508171b9b5cbc2bb39808b3f2477aa1b8 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Fri, 20 Jun 2025 12:04:22 -0400 Subject: [PATCH 1/4] Add solargraph pre-commit hook for typechecking --- config/default.yml | 9 ++ lib/overcommit/hook/pre_commit/solargraph.rb | 48 ++++++++ .../hook/pre_commit/solargraph_spec.rb | 109 ++++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 lib/overcommit/hook/pre_commit/solargraph.rb create mode 100644 spec/overcommit/hook/pre_commit/solargraph_spec.rb diff --git a/config/default.yml b/config/default.yml index ba8af533..83c61e98 100644 --- a/config/default.yml +++ b/config/default.yml @@ -799,6 +799,15 @@ 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' + command: ['solargraph', 'typecheck', '--level', 'strong'] + include: '**/*.rb' + Sorbet: enabled: false description: 'Analyze with Sorbet' diff --git a/lib/overcommit/hook/pre_commit/solargraph.rb b/lib/overcommit/hook/pre_commit/solargraph.rb new file mode 100644 index 00000000..c928bfba --- /dev/null +++ b/lib/overcommit/hook/pre_commit/solargraph.rb @@ -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*(?(?:\w:)?[^:]+):(?\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] + 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 diff --git a/spec/overcommit/hook/pre_commit/solargraph_spec.rb b/spec/overcommit/hook/pre_commit/solargraph_spec.rb new file mode 100644 index 00000000..d7effa1d --- /dev/null +++ b/spec/overcommit/hook/pre_commit/solargraph_spec.rb @@ -0,0 +1,109 @@ +# 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 + expect(messages.first.content).to eq '/home/username/src/solargraph-rails/file1.rb:36 - Unresolved constant Solargraph::Parser::Legacy::NodeChainer' + 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 From a3b376c3396be45759fb15d0482c83e8f0b95365 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 29 Jun 2025 07:50:03 -0400 Subject: [PATCH 2/4] Fix RuboCop issues --- spec/overcommit/hook/pre_commit/solargraph_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/overcommit/hook/pre_commit/solargraph_spec.rb b/spec/overcommit/hook/pre_commit/solargraph_spec.rb index d7effa1d..320081f1 100644 --- a/spec/overcommit/hook/pre_commit/solargraph_spec.rb +++ b/spec/overcommit/hook/pre_commit/solargraph_spec.rb @@ -77,7 +77,8 @@ expect(messages.first.line).to eq 36 end it 'parses and returns error message content' do - expect(messages.first.content).to eq '/home/username/src/solargraph-rails/file1.rb:36 - Unresolved constant Solargraph::Parser::Legacy::NodeChainer' + 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 @@ -86,9 +87,9 @@ 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 + Typecheck finished in 8.095239999704063 seconds. + 0 problems found in 0 of 16 files. + MSG end context 'with no stderr output' do @@ -101,7 +102,7 @@ context 'with stderr output' do let(:stderr) { 'something' } it 'should raise' do - expect { messages }.to raise_error(Overcommit::Exceptions::MessageProcessingError) + expect { messages }.to raise_error(Overcommit::Exceptions::MessageProcessingError) end end end From fdeeb63d2204b154845a1acc131981225e8da8de Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 29 Jun 2025 08:11:48 -0400 Subject: [PATCH 3/4] Use flags config --- config/default.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/default.yml b/config/default.yml index 83c61e98..b121e762 100644 --- a/config/default.yml +++ b/config/default.yml @@ -805,7 +805,7 @@ PreCommit: requires_files: true required_executable: 'solargraph' install_command: 'gem install solargraph' - command: ['solargraph', 'typecheck', '--level', 'strong'] + flags: ['typecheck', '--level', 'strong'] include: '**/*.rb' Sorbet: From 4bc8db73d212f6c132887bf7c1353f85bed50836 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 29 Jun 2025 09:00:12 -0400 Subject: [PATCH 4/4] Match exclusions from Solargraph default config --- config/default.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/default.yml b/config/default.yml index b121e762..c13055fd 100644 --- a/config/default.yml +++ b/config/default.yml @@ -807,6 +807,11 @@ PreCommit: install_command: 'gem install solargraph' flags: ['typecheck', '--level', 'strong'] include: '**/*.rb' + exclude: + - 'spec/**/*.rb' + - 'test/**/*.rb' + - 'vendor/**/*.rb' + - '.bundle/**/*.rb' Sorbet: enabled: false