diff --git a/.ameba.yml b/.ameba.yml new file mode 100644 index 0000000..74885d6 --- /dev/null +++ b/.ameba.yml @@ -0,0 +1,30 @@ +# Ameba configuration for cjules. +# https://github.com/crystal-ameba/ameba +# +# Only rules that conflict with deliberate, documented project conventions are +# disabled here; everything else runs with ameba defaults and is expected to +# stay clean. + +# Locals and instance vars intentionally mirror the Jules REST API JSON field +# names verbatim (createTime, gitPatch, planGenerated, nextPageToken, ...) so the +# code maps 1:1 onto the upstream API. snake_case would obscure that mapping. +Naming/VariableNames: + Enabled: false + +# Short block parameter names (s, a, l, j, ...) are used idiomatically throughout +# the command and serialization layers. +Naming/BlockParameterName: + Enabled: false + +# The CLI dispatcher and per-command OptionParser flag handlers are inherently +# branch-heavy; high cyclomatic complexity is expected for argument parsing here +# and does not indicate a refactor is needed. +Metrics/CyclomaticComplexity: + Enabled: false + +# not_nil! is used deliberately: flag values are captured inside OptionParser +# blocks (closures), so Crystal's flow typing cannot narrow them and not_nil! is +# the idiomatic guard; specs also use it over decoded JSON fixtures whose shape is +# guaranteed by the fixture. +Lint/NotNil: + Enabled: false diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b74d266..fd2af30 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,8 +47,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: crystal-lang/install-crystal@v1 + with: + crystal: 1.20.0 + - name: Install dependencies + run: shards install + # Use the ameba version pinned in shard.lock (respects .ameba.yml) so local + # `just lint` and CI agree and an upstream rule change can't break CI silently. - name: Run Ameba linter - uses: crystal-ameba/github-action@master + run: bin/ameba build-snapcraft: if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest diff --git a/justfile b/justfile index ea25c0c..25aedc4 100644 --- a/justfile +++ b/justfile @@ -36,10 +36,11 @@ check: crystal tool format --check # Run ameba static linter (requires shards install first). +# Rule configuration lives in .ameba.yml so local and CI stay in sync. [group('development')] lint: - @[ -f lib/ameba/src/ameba.cr ] || shards install - crystal lib/ameba/src/ameba.cr src spec --except Metrics/CyclomaticComplexity --except Metrics/MethodLength + @[ -f bin/ameba ] || shards install + bin/ameba # Run all tests. [group('development')] diff --git a/shard.lock b/shard.lock index f612c4d..26eb0c5 100644 --- a/shard.lock +++ b/shard.lock @@ -4,3 +4,7 @@ shards: git: https://github.com/crystal-ameba/ameba.git version: 1.6.4 + webmock: + git: https://github.com/manastech/webmock.cr.git + version: 0.14.0 + diff --git a/shard.yml b/shard.yml index c3c41b4..d777146 100644 --- a/shard.yml +++ b/shard.yml @@ -20,3 +20,6 @@ development_dependencies: ameba: github: crystal-ameba/ameba version: "~> 1.6" + webmock: + github: manastech/webmock.cr + version: "~> 0.14" diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index e469081..19ebe76 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -2,18 +2,13 @@ require "spec" require "file_utils" require "../src/cjules" -# WebMock is vendored under lib/webmock (committed in tree) to enable hermetic -# HTTP tests for Client, API, and command layers without a production dependency. -require "../lib/webmock/src/webmock" - -def require_webmock! - # always available in this project -end +# WebMock is a development_dependency used to enable hermetic HTTP tests for the +# Client, API, and command layers without adding a production dependency. +require "webmock" # Run a block with WebMock enabled (real net disabled, stubs required). -# Automatically resets after the block. Skips/pends if webmock not vendored. +# Automatically resets after the block. def with_webmock(&) - require_webmock! WebMock.wrap do yield end @@ -22,7 +17,6 @@ end # Stub a successful Jules API response for common paths. # Usage: stub_jules(:get, "/v1alpha/sessions", body: json, status: 200) def stub_jules(method : Symbol, path : String, body : String = "", status : Int32 = 200, headers : HTTP::Headers? = nil) - require_webmock! url = "https://jules.googleapis.com#{path}" stub = WebMock.stub(method, url) stub.to_return(status: status, body: body, headers: headers || HTTP::Headers{"Content-Type" => "application/json"}) diff --git a/spec/unidiff_spec.cr b/spec/unidiff_spec.cr index cbb3799..e77f4a8 100644 --- a/spec/unidiff_spec.cr +++ b/spec/unidiff_spec.cr @@ -16,8 +16,8 @@ describe Cjules::Unidiff do chunks = Cjules::Unidiff.chunks(patch) chunks.size.should eq(2) chunks[0].lines.first.should start_with("diff --git a/foo.txt b/foo.txt") - chunks[0].lines.any? { |l| l.starts_with?("@@ -0,0 +1,2 @@") }.should be_true - chunks[1].lines.any? { |l| l.starts_with?("@@ -10,0 +12,1 @@") }.should be_true + chunks[0].lines.any?(&.starts_with?("@@ -0,0 +1,2 @@")).should be_true + chunks[1].lines.any?(&.starts_with?("@@ -10,0 +12,1 @@")).should be_true end it "treats no-@@ sections as one chunk" do @@ -28,7 +28,7 @@ describe Cjules::Unidiff do PATCH chunks = Cjules::Unidiff.chunks(patch) chunks.size.should eq(1) - chunks[0].lines.any? { |l| l.includes?("new mode") }.should be_true + chunks[0].lines.any?(&.includes?("new mode")).should be_true end end @@ -91,7 +91,7 @@ describe Cjules::Unidiff::Interactive do PATCH result, _, _ = run_select.call(patch, "d\n") result.selected_patch.empty?.should be_true - result.quit_early.should be_false + result.quit_early?.should be_false result.skipped_chunks.should eq(2) end @@ -106,6 +106,6 @@ describe Cjules::Unidiff::Interactive do PATCH result, _, _ = run_select.call(patch, "q\n") result.selected_patch.empty?.should be_true - result.quit_early.should be_true + result.quit_early?.should be_true end end diff --git a/src/cjules/client.cr b/src/cjules/client.cr index 64c6eb9..1304ce9 100644 --- a/src/cjules/client.cr +++ b/src/cjules/client.cr @@ -46,7 +46,7 @@ module Cjules end private def full_path(path : String, query : Hash(String, String)? = nil) : String - return path unless query && !query.empty? + return path if query.nil? || query.empty? params = URI::Params.build do |form| query.each { |k, v| form.add(k, v) } end diff --git a/src/cjules/commands/activity.cr b/src/cjules/commands/activity.cr index be7e39c..e134b45 100644 --- a/src/cjules/commands/activity.cr +++ b/src/cjules/commands/activity.cr @@ -64,7 +64,7 @@ module Cjules (plan.steps || [] of Models::PlanStep).each do |s| puts " #{(s.index || 0) + 1}. #{s.title || "(untitled)"}" if d = s.description - d.lines.each { |l| puts " #{l.chomp}" } unless d.empty? + d.each_line { |l| puts " #{l.chomp}" } unless d.empty? end end end @@ -114,7 +114,7 @@ module Cjules puts Output::Colors.bold("#{label} (bashOutput):") puts " $ #{bo.command}" if out = bo.output - out.lines.each { |l| puts " #{l.chomp}" } + out.each_line { |l| puts " #{l.chomp}" } end puts " exit: #{bo.exitCode}" elsif med = art.media diff --git a/src/cjules/commands/logs.cr b/src/cjules/commands/logs.cr index bba829e..a9956b1 100644 --- a/src/cjules/commands/logs.cr +++ b/src/cjules/commands/logs.cr @@ -83,7 +83,7 @@ module Cjules any = true puts "$ #{bo.command}" if out = bo.output - out.lines.each { |l| puts l.chomp } + out.each_line { |l| puts l.chomp } end puts "[exit #{bo.exitCode}]" puts "" @@ -103,7 +103,7 @@ module Cjules arts.each_with_index do |art, idx| if med = art.media data = med.data - next unless data && !data.empty? + next if data.nil? || data.empty? ext = ext_for(med.mimeType) fname = if aid = a.id @@ -260,14 +260,22 @@ module Cjules n = pg.plan.try(&.steps).try(&.size) || 0 return "plan generated (#{n} step(s))" end - return "plan #{a.planApproved.not_nil!.planId || "?"} approved" if a.planApproved - return "user> #{a.userMessaged.not_nil!.userMessage || ""}" if a.userMessaged - return "agent> #{a.agentMessaged.not_nil!.agentMessage || ""}" if a.agentMessaged + if pa = a.planApproved + return "plan #{pa.planId || "?"} approved" + end + if um = a.userMessaged + return "user> #{um.userMessage || ""}" + end + if am = a.agentMessaged + return "agent> #{am.agentMessage || ""}" + end if pu = a.progressUpdated parts = [pu.title, pu.description].compact.reject(&.empty?) return parts.join(" — ") end - return "failed: #{a.sessionFailed.not_nil!.reason || "(no reason)"}" if a.sessionFailed + if sf = a.sessionFailed + return "failed: #{sf.reason || "(no reason)"}" + end return "completed" if a.sessionCompleted "" end diff --git a/src/cjules/commands/new.cr b/src/cjules/commands/new.cr index 7c18c50..5fbb070 100644 --- a/src/cjules/commands/new.cr +++ b/src/cjules/commands/new.cr @@ -157,7 +157,7 @@ module Cjules else e.message || "unknown error" end - end.uniq + end.uniq! error_msgs.each { |msg| STDERR.puts " - #{msg}" } return 1 end @@ -239,7 +239,7 @@ module Cjules JSON.build do |j| j.object do j.field "prompt", prompt - j.field "title", title.not_nil! if title + j.field "title", title if title j.field "requirePlanApproval", true if require_approval j.field "automationMode", "AUTO_CREATE_PR" if auto_pr if source && starting_branch diff --git a/src/cjules/commands/patch.cr b/src/cjules/commands/patch.cr index aab3b6c..0a8c261 100644 --- a/src/cjules/commands/patch.cr +++ b/src/cjules/commands/patch.cr @@ -95,7 +95,7 @@ module Cjules end result = Unidiff::Interactive.select(text, input: STDIN, output: STDERR, display: STDOUT) if result.selected_patch.empty? - if result.quit_early + if result.quit_early? STDERR.puts "aborted" return 1 end diff --git a/src/cjules/commands/pick.cr b/src/cjules/commands/pick.cr index 7beb02a..9750be3 100644 --- a/src/cjules/commands/pick.cr +++ b/src/cjules/commands/pick.cr @@ -82,13 +82,11 @@ module Cjules end private def has_fzf? : Bool - begin - Process.run("which", ["fzf"], - output: Process::Redirect::Close, - error: Process::Redirect::Close).success? - rescue Exception - false - end + Process.run("which", ["fzf"], + output: Process::Redirect::Close, + error: Process::Redirect::Close).success? + rescue Exception + false end private def run_fzf(input : String) : String? diff --git a/src/cjules/commands/plan.cr b/src/cjules/commands/plan.cr index c01f845..2e8a3e5 100644 --- a/src/cjules/commands/plan.cr +++ b/src/cjules/commands/plan.cr @@ -76,7 +76,7 @@ module Cjules puts "" puts " #{(s.index || 0) + 1}. #{Output::Colors.bold(s.title || "(untitled)")}" if d = s.description - d.lines.each { |l| puts " #{l.chomp}" } unless d.empty? + d.each_line { |l| puts " #{l.chomp}" } unless d.empty? end end end diff --git a/src/cjules/commands/retry.cr b/src/cjules/commands/retry.cr index 10b6721..bd1b070 100644 --- a/src/cjules/commands/retry.cr +++ b/src/cjules/commands/retry.cr @@ -98,7 +98,7 @@ module Cjules source = sc.try(&.source) starting_branch = branch_override || sc.try(&.githubRepoContext).try(&.startingBranch) - if source && (starting_branch.nil? || starting_branch.not_nil!.empty?) + if source && (starting_branch.nil? || starting_branch.empty?) STDERR.puts "error: original session has no startingBranch; pass --branch BRANCH" return 2 end diff --git a/src/cjules/commands/templates.cr b/src/cjules/commands/templates.cr index b87ea4b..fbba72d 100644 --- a/src/cjules/commands/templates.cr +++ b/src/cjules/commands/templates.cr @@ -35,7 +35,7 @@ module Cjules f end end - names.uniq.sort + names.uniq.sort! end def run(args : Array(String)) : Int32 diff --git a/src/cjules/template_renderer.cr b/src/cjules/template_renderer.cr index bcdbdf4..1362e90 100644 --- a/src/cjules/template_renderer.cr +++ b/src/cjules/template_renderer.cr @@ -87,23 +87,21 @@ module Cjules end private def get_git_diff : String - begin - io = IO::Memory.new - status = Process.run("git", ["diff"], output: io, error: Process::Redirect::Close) - - unless status.success? - return "[git diff failed]" - end + io = IO::Memory.new + status = Process.run("git", ["diff"], output: io, error: Process::Redirect::Close) - diff = io.to_s - if diff.empty? - return "[no git changes]" - end + unless status.success? + return "[git diff failed]" + end - diff - rescue e : Exception - "[error running git diff: #{e.message}]" + diff = io.to_s + if diff.empty? + return "[no git changes]" end + + diff + rescue e : Exception + "[error running git diff: #{e.message}]" end end end diff --git a/src/cjules/unidiff.cr b/src/cjules/unidiff.cr index 5a06911..cd32de6 100644 --- a/src/cjules/unidiff.cr +++ b/src/cjules/unidiff.cr @@ -47,7 +47,7 @@ module Cjules getter selected_patch : String getter selected_chunks : Int32 getter skipped_chunks : Int32 - getter quit_early : Bool + getter? quit_early : Bool def initialize(@selected_patch : String, @selected_chunks : Int32, @skipped_chunks : Int32, @quit_early : Bool) end @@ -131,7 +131,8 @@ module Cjules private def show_chunk(chunk : Chunk, idx : Int32, total : Int32, display : IO) display.puts "\n#{chunk.label} (hunk #{idx}/#{total})" display.puts "-" * 72 - chunk.lines.each { |l| display.puts l } + # ameba:disable Performance/ExcessiveAllocations + chunk.lines.each { |l| display.puts l } # `lines` is an Array field, not a String display.puts "-" * 72 display.flush end @@ -280,17 +281,17 @@ module Cjules end private def section_label(section : Array(String)) : String - diff = section.find { |l| l.starts_with?("diff --git ") } + diff = section.find(&.starts_with?("diff --git ")) if diff if m = diff.match(/^diff --git a\/(\S+)\s+b\/(\S+)$/) return m[2] end end - plus = section.find { |l| l.starts_with?("+++ ") } + plus = section.find(&.starts_with?("+++ ")) if plus return plus.sub(/^\+\+\+\s+/, "") end - minus = section.find { |l| l.starts_with?("--- ") } + minus = section.find(&.starts_with?("--- ")) if minus return minus.sub(/^---\s+/, "") end diff --git a/src/cjules/util.cr b/src/cjules/util.cr index f308010..423d11a 100644 --- a/src/cjules/util.cr +++ b/src/cjules/util.cr @@ -107,17 +107,15 @@ module Cjules end private def run(cmd : String, *args) : String? - begin - io = IO::Memory.new - status = Process.run(cmd, args.to_a, - output: io, - error: Process::Redirect::Close) - return nil unless status.success? - out = io.to_s.strip - out.empty? ? nil : out - rescue Exception - nil - end + io = IO::Memory.new + status = Process.run(cmd, args.to_a, + output: io, + error: Process::Redirect::Close) + return nil unless status.success? + out = io.to_s.strip + out.empty? ? nil : out + rescue Exception + nil end end