From bfc88aa7c1bd61d47b2f494376b5ce77c91fd0ff Mon Sep 17 00:00:00 2001 From: Vincent Rolea <3525369+virolea@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:21:57 +0000 Subject: [PATCH 1/2] Show progress while checking, drop rule severity A full run gave no sign of life until the report printed. The runner now yields after each file and the CLI draws "Checking 12/42 files" on stderr, redrawn in place on a terminal and erased when done; off a terminal it prints one line announcing the run so CI logs and piped JSON stay clean. An offense is an offense, so `severity` and `--fail-on` are gone: every offense fails the run and is a GitHub error annotation. A config that still sets `severity` is rejected as an unknown key. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UhtJDjXrgmwwet27LAiY5f --- .lintus.yml | 2 -- CHANGELOG.md | 4 +++ README.md | 14 +++++------ action.yml | 2 +- lib/lintus/cli.rb | 19 ++++++++------ lib/lintus/formatter/github.rb | 2 +- lib/lintus/formatter/json.rb | 2 -- lib/lintus/formatter/text.rb | 2 +- lib/lintus/offense.rb | 4 +-- lib/lintus/progress.rb | 45 ++++++++++++++++++++++++++++++++++ lib/lintus/report.rb | 14 +++-------- lib/lintus/rule.rb | 18 ++------------ lib/lintus/runner.rb | 6 ++++- lib/lintus/template.rb | 3 --- test/test_cli.rb | 7 +++--- test/test_formatters.rb | 11 ++++----- test/test_helper.rb | 1 - test/test_progress.rb | 45 ++++++++++++++++++++++++++++++++++ test/test_report.rb | 9 ++----- test/test_rule.rb | 8 +++--- test/test_runner.rb | 13 ++++++++-- 21 files changed, 150 insertions(+), 81 deletions(-) create mode 100644 lib/lintus/progress.rb create mode 100644 test/test_progress.rb diff --git a/.lintus.yml b/.lintus.yml index 496a560..8b30252 100644 --- a/.lintus.yml +++ b/.lintus.yml @@ -21,12 +21,10 @@ rules: criteria: "true": At least one raised message is vague, such as "invalid" or "failed", with no detail. "false": Every raised message names the problem, or no errors are raised. - severity: warning classes_are_documented: description: Each class and module should open with a comment describing its responsibility. question: Does every top-level class or module in this file have a comment describing what it is for? offense_when: false - severity: warning exclude: - "exe/*" diff --git a/CHANGELOG.md b/CHANGELOG.md index 02e83f4..cfef76b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## [Unreleased] +- A progress counter on stderr while files are being checked. +- Removed rule `severity` and the `--fail-on` flag: an offense is an offense. A config that still + sets `severity` is rejected with a message naming the key. + ## [0.1.0] - 2026-09-21 - Initial release. diff --git a/README.md b/README.md index 1a0ad32..c2a49a6 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ rules: ``` $ lintus -app/jobs/retry_job.rb: [no_sleep_in_jobs] Background jobs must never block on sleep. (error, noul 0.94) +app/jobs/retry_job.rb: [no_sleep_in_jobs] Background jobs must never block on sleep. (noul 0.94) 42 files inspected, 1 offense detected ``` @@ -81,13 +81,11 @@ rules: - "app/**/*.rb" exclude: - "app/models/legacy/**" - severity: error service_objects_are_documented: description: Service objects carry a comment explaining what they do. question: Does every class in this file have a comment describing its responsibility? offense_when: false - severity: warning paths: - "app/services/**/*.rb" ``` @@ -102,7 +100,6 @@ rules: | `threshold` | no | Probability above which the answer counts as `true`. Defaults to 0.5. Raise it for rules where a false positive is costly. | | `paths` | no | Globs the rule applies to. Defaults to the top-level `paths`; with neither, every file. | | `exclude` | no | Globs the rule never applies to, on top of the top-level `exclude`. | -| `severity` | no | `error` (default) or `warning`. Only errors fail the run, unless `--fail-on` says otherwise. | | `offense_when` | no | `true` (default) or `false`. Set to `false` for rules phrased positively, such as "Does every class have a comment?". | Rule ids are snake_case. They become the question identifiers in the Jev request and the @@ -142,9 +139,11 @@ Only files that at least one rule applies to are sent. Deleted files are never s each offense becomes an annotation on the file in the pull request; it is the default when `GITHUB_ACTIONS` is set. `--format json` is for other tools. -Exit status is `0` when clean, `1` when there are offenses at or above `--fail-on` -(`error` by default, or `warning`, or `never`), and `2` when a request failed or the -invocation was wrong. +Exit status is `0` when clean, `1` when there are offenses, and `2` when a request failed or +the invocation was wrong. + +While a run is in flight, a counter on stderr shows how many files are done. Off a terminal +(CI logs, pipes) it is a single line announcing the run, so captured output stays clean. ## GitHub Actions @@ -204,7 +203,6 @@ Either way `JEV_API_KEY` must be in the environment of the shell running the com -c, --config PATH Config file to use instead of searching for one -j, --jobs N Concurrent requests to the Jev API (default 4) --api-key KEY Jev API key (default: $JEV_API_KEY) - --fail-on LEVEL error (default), warning, or never -l, --list Show what would be checked without calling the API ``` diff --git a/action.yml b/action.yml index 4322e40..378cdac 100644 --- a/action.yml +++ b/action.yml @@ -17,7 +17,7 @@ inputs: required: false default: "" args: - description: Extra arguments for the lintus command, for example "--fail-on warning" or "--jobs 8". + description: Extra arguments for the lintus command, for example "--jobs 8". required: false default: "" version: diff --git a/lib/lintus/cli.rb b/lib/lintus/cli.rb index af6b39c..8b5c5d1 100644 --- a/lib/lintus/cli.rb +++ b/lib/lintus/cli.rb @@ -15,7 +15,7 @@ class CLI # `command` is what to do; `selection` is which files, with `ref` for --diff # and `files` for paths given on the command line. Options = Struct.new( - :command, :selection, :ref, :files, :config, :format, :jobs, :api_key, :list, :fail_on, + :command, :selection, :ref, :files, :config, :format, :jobs, :api_key, :list, keyword_init: true ) @@ -50,9 +50,9 @@ def lint(options) return list(tasks) if options.list configure_api_key!(options) - report = runner.run(tasks) + report = with_progress(tasks.size) { |progress| runner.run(tasks) { |done| progress.update(done) } } Formatter.for(options.format).new(stdout).render(report) - report.exit_status(fail_on: options.fail_on) + report.exit_status end def parse(argv) @@ -79,7 +79,7 @@ def parse(argv) def default_options Options.new( command: :lint, selection: :all, format: @env["GITHUB_ACTIONS"] == "true" ? "github" : "text", - jobs: 4, api_key: @env["JEV_API_KEY"], list: false, fail_on: "error" + jobs: 4, api_key: @env["JEV_API_KEY"], list: false ) end @@ -109,9 +109,6 @@ def build_parser(options) # rubocop:disable Metrics/MethodLength options.jobs = jobs end parser.on("--api-key KEY", "Jev API key (default: $JEV_API_KEY)") { |key| options.api_key = key } - parser.on("--fail-on LEVEL", %w[error warning never], "Exit non-zero on: error (default), warning, never") do |level| - options.fail_on = level - end parser.on("-l", "--list", "List the files and rules that would be checked, without calling the API") do options.list = true end @@ -138,6 +135,14 @@ def find_files(config, options) end end + def with_progress(total) + progress = Progress.new(stderr, total) + progress.start + yield progress + ensure + progress.finish + end + def configure_api_key!(options) raise Error, "no API key: set JEV_API_KEY or pass --api-key" if options.api_key.nil? || options.api_key.empty? diff --git a/lib/lintus/formatter/github.rb b/lib/lintus/formatter/github.rb index ed96fcb..a9a2d69 100644 --- a/lib/lintus/formatter/github.rb +++ b/lib/lintus/formatter/github.rb @@ -8,7 +8,7 @@ class Github < Formatter private def offense_line(offense) - command(offense.severity, offense.message, file: offense.path, title: "lintus: #{offense.rule.id}") + command("error", offense.message, file: offense.path, title: "lintus: #{offense.rule.id}") end def skipped_line(skipped) diff --git a/lib/lintus/formatter/json.rb b/lib/lintus/formatter/json.rb index 8f81736..780979a 100644 --- a/lib/lintus/formatter/json.rb +++ b/lib/lintus/formatter/json.rb @@ -11,8 +11,6 @@ def render(report) summary: { files_inspected: report.checked.size, offenses: report.offenses.size, - errors: report.errors.size, - warnings: report.warnings.size, skipped: report.skipped.size, failures: report.failures.size }, diff --git a/lib/lintus/formatter/text.rb b/lib/lintus/formatter/text.rb index 4d92263..d8437ef 100644 --- a/lib/lintus/formatter/text.rb +++ b/lib/lintus/formatter/text.rb @@ -7,7 +7,7 @@ class Text < Formatter private def offense_line(offense) - "#{offense.path}: [#{offense.rule.id}] #{offense.message} (#{offense.severity}, noul #{offense.noul.round(2)})" + "#{offense.path}: [#{offense.rule.id}] #{offense.message} (noul #{offense.noul.round(2)})" end def skipped_line(skipped) = "#{skipped.path}: skipped, #{skipped.reason}" diff --git a/lib/lintus/offense.rb b/lib/lintus/offense.rb index c78765f..bdf7c0d 100644 --- a/lib/lintus/offense.rb +++ b/lib/lintus/offense.rb @@ -3,12 +3,10 @@ module Lintus # A rule the model flagged on a file, with the probability it gave. Offense = Struct.new(:path, :rule, :noul, keyword_init: true) do - def severity = rule.severity - def error? = rule.error? def message = rule.description def to_h - { path: path, rule: rule.id, severity: severity, message: message, noul: noul.round(3) } + { path: path, rule: rule.id, message: message, noul: noul.round(3) } end end end diff --git a/lib/lintus/progress.rb b/lib/lintus/progress.rb new file mode 100644 index 0000000..3981d80 --- /dev/null +++ b/lib/lintus/progress.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Lintus + # Tells the person waiting that something is happening. On a terminal it is a + # counter updated in place and erased when done; elsewhere (CI logs, pipes) it + # is a single line announcing the run, so nothing garbles captured output. + class Progress + def initialize(io, total) + @io = io + @total = total + @tty = io.respond_to?(:tty?) && io.tty? + @width = 0 + end + + def start + return if @total.zero? + + if @tty + draw(0) + else + @io.puts("Checking #{@total} #{@total == 1 ? "file" : "files"}...") + end + end + + def update(done) + draw(done) if @tty + end + + def finish + return unless @tty && @width.positive? + + @io.print "\r#{" " * @width}\r" + @io.flush + end + + private + + def draw(done) + line = "Checking #{done}/#{@total} files" + @width = [@width, line.size].max + @io.print "\r#{line.ljust(@width)}" + @io.flush + end + end +end diff --git a/lib/lintus/report.rb b/lib/lintus/report.rb index 7ea2cfc..5b02ca6 100644 --- a/lib/lintus/report.rb +++ b/lib/lintus/report.rb @@ -31,21 +31,13 @@ def record_failure(path, error) synchronize { failures << Failure.new(path: path, error: error) } end - def errors = offenses.select(&:error?) - def warnings = offenses.reject(&:error?) - def sorted_offenses = offenses.sort_by { |offense| [offense.path, offense.rule.id] } - # 2 when a file could not be checked, 1 when offenses reach `fail_on`, else 0. - def exit_status(fail_on: "error") + # 2 when a file could not be checked, 1 when there are offenses, else 0. + def exit_status return 2 if failures.any? - failing = case fail_on.to_s - when "warning" then offenses - when "never" then [] - else errors - end - failing.any? ? 1 : 0 + offenses.any? ? 1 : 0 end private diff --git a/lib/lintus/rule.rb b/lib/lintus/rule.rb index 291bfbd..5581dba 100644 --- a/lib/lintus/rule.rb +++ b/lib/lintus/rule.rb @@ -8,11 +8,10 @@ module Lintus # `offense_when` (true by default, so questions are phrased to describe the # offense: "Does this file call sleep?"). class Rule - KEYS = %w[question description criteria threshold paths exclude severity offense_when].freeze - SEVERITIES = %w[error warning].freeze + KEYS = %w[question description criteria threshold paths exclude offense_when].freeze ID_FORMAT = /\A[a-z][a-z0-9_]*\z/ - attr_reader :id, :description, :question, :criteria, :threshold, :paths, :exclude, :severity, :offense_when + attr_reader :id, :description, :question, :criteria, :threshold, :paths, :exclude, :offense_when def initialize(id, attrs, default_paths: [], default_exclude: []) @id = validate_id(id) @@ -24,7 +23,6 @@ def initialize(id, attrs, default_paths: [], default_exclude: []) @threshold = build_threshold(attrs["threshold"]) @paths = Schema.string_list(attrs.fetch("paths", default_paths)) @exclude = default_exclude + Schema.string_list(attrs["exclude"]) - @severity = build_severity(attrs.fetch("severity", "error")) @offense_when = build_offense_when(attrs.fetch("offense_when", true)) end @@ -41,8 +39,6 @@ def add_to(query) def offense?(answer) = answer.result == offense_when - def error? = severity == "error" - private def validate_id(id) @@ -86,16 +82,6 @@ def build_threshold(threshold) threshold.to_f end - def build_severity(severity) - severity = severity.to_s - unless SEVERITIES.include?(severity) - raise ConfigError, - "rule #{id}: `severity` must be one of #{SEVERITIES.join(", ")}" - end - - severity - end - def build_offense_when(value) raise ConfigError, "rule #{id}: `offense_when` must be true or false" unless [true, false].include?(value) diff --git a/lib/lintus/runner.rb b/lib/lintus/runner.rb index 1372545..6134de2 100644 --- a/lib/lintus/runner.rb +++ b/lib/lintus/runner.rb @@ -23,17 +23,21 @@ def plan(files) end end - # Checks the [file, rules] pairs from #plan concurrently. + # Checks the [file, rules] pairs from #plan concurrently. The block, if + # given, is called with the number of files done after each one finishes. def run(tasks) report = Report.new queue = Queue.new tasks.each { |task| queue << task } queue.close + done = 0 + counter = Mutex.new workers = Array.new([jobs, tasks.size].min) do Thread.new do while (task = queue.pop) check(*task, report) + counter.synchronize { yield(done += 1) } if block_given? end end end diff --git a/lib/lintus/template.rb b/lib/lintus/template.rb index e2e1c09..5955c6e 100644 --- a/lib/lintus/template.rb +++ b/lib/lintus/template.rb @@ -28,7 +28,6 @@ module Template criteria: "true": A debugger breakpoint or a throwaway print statement is present. "false": Any output is deliberate program behaviour, or there is none. - severity: error no_hardcoded_secrets: description: Secrets must come from the environment or a credentials store, never source code. @@ -37,14 +36,12 @@ module Template "true": A literal credential value is written in the source. "false": Credentials are read from configuration, or there are none. threshold: 0.7 - severity: error methods_are_documented: description: Public classes should carry a short comment explaining their purpose. question: Does every class or module defined in this file have a comment describing its responsibility? # This rule is phrased positively, so the offense is a "false" answer. offense_when: false - severity: warning YAML end end diff --git a/test/test_cli.rb b/test/test_cli.rb index 4aa5a84..4512bda 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -15,7 +15,6 @@ class TestCLI < Minitest::Test documented: question: Is every class documented? offense_when: false - severity: warning YAML def test_init_writes_a_loadable_config_and_refuses_to_overwrite @@ -71,12 +70,12 @@ def test_full_run_reports_offenses_and_exit_status assert_equal 1, run_cli([], dir: dir) assert_equal <<~TEXT, @stdout.string - app/jobs/a_job.rb: [no_sleep] Jobs must not sleep. (error, noul 0.95) + app/jobs/a_job.rb: [no_sleep] Jobs must not sleep. (noul 0.95) 1 file inspected, 1 offense detected TEXT + assert_equal "Checking 1 file...\n", @stderr.string - assert_equal 0, run_cli(%w[--fail-on never], dir: dir) assert_equal 1, run_cli(%w[--format json --api-key from-flag], dir: dir, env: {}) assert_equal 1, JSON.parse(@stdout.string)["summary"]["offenses"] end @@ -130,7 +129,7 @@ def test_github_format_is_the_default_under_actions run_cli([], dir: dir, env: { "JEV_API_KEY" => "k", "GITHUB_ACTIONS" => "true" }) - assert_includes @stdout.string, "::warning file=lib/b.rb,title=lintus%3A documented::Is every class documented?" + assert_includes @stdout.string, "::error file=lib/b.rb,title=lintus%3A documented::Is every class documented?" end end diff --git a/test/test_formatters.rb b/test/test_formatters.rb index 0ea0269..9232370 100644 --- a/test/test_formatters.rb +++ b/test/test_formatters.rb @@ -21,8 +21,8 @@ def test_text output = render("text") assert_equal <<~TEXT, output - app/jobs/a_job.rb: [documented] Classes carry a comment. (warning, noul 0.3) - app/jobs/a_job.rb: [no_sleep] Jobs must not sleep. (error, noul 0.91) + app/jobs/a_job.rb: [documented] Classes carry a comment. (noul 0.3) + app/jobs/a_job.rb: [no_sleep] Jobs must not sleep. (noul 0.91) lib/blob.rb: skipped, binary file lib/bad.rb: failed, Jev API error 500: boom @@ -37,7 +37,7 @@ def test_text_when_clean def test_github_emits_workflow_commands lines = render("github").lines(chomp: true) - assert_equal "::warning file=app/jobs/a_job.rb,title=lintus%3A documented::Classes carry a comment.", lines[0] + assert_equal "::error file=app/jobs/a_job.rb,title=lintus%3A documented::Classes carry a comment.", lines[0] assert_equal "::error file=app/jobs/a_job.rb,title=lintus%3A no_sleep::Jobs must not sleep.", lines[1] assert_equal "::notice file=lib/blob.rb,title=lintus::Skipped: binary file", lines[2] assert_equal "::error file=lib/bad.rb,title=lintus%3A request failed::Jev API error 500: boom", lines[3] @@ -56,9 +56,8 @@ def test_github_escapes_newlines_and_percent_in_messages def test_json data = JSON.parse(render("json")) - summary = { "files_inspected" => 2, "offenses" => 2, "errors" => 1, "warnings" => 1, "skipped" => 1, "failures" => 1 } - offense = { "path" => "app/jobs/a_job.rb", "rule" => "no_sleep", "severity" => "error", - "message" => "Jobs must not sleep.", "noul" => 0.912 } + summary = { "files_inspected" => 2, "offenses" => 2, "skipped" => 1, "failures" => 1 } + offense = { "path" => "app/jobs/a_job.rb", "rule" => "no_sleep", "message" => "Jobs must not sleep.", "noul" => 0.912 } assert_equal summary, data["summary"] assert_equal offense, data["offenses"].last diff --git a/test/test_helper.rb b/test/test_helper.rb index 08018d8..48231e0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -23,7 +23,6 @@ module LintusTestHelpers "description" => "Classes carry a comment.", "question" => "Is every class documented?", "offense_when" => false, - "severity" => "warning", "threshold" => 0.7 } }.freeze diff --git a/test/test_progress.rb b/test/test_progress.rb new file mode 100644 index 0000000..698f18e --- /dev/null +++ b/test/test_progress.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestProgress < Minitest::Test + class Terminal < StringIO + def tty? = true + end + + def test_on_a_terminal_redraws_in_place_and_clears_when_done + io = Terminal.new + progress = Lintus::Progress.new(io, 12) + + progress.start + progress.update(1) + progress.update(12) + progress.finish + + assert_equal ["Checking 0/12 files", "Checking 1/12 files", "Checking 12/12 files", " " * 20], + io.string.split("\r").reject(&:empty?) + assert io.string.end_with?("\r") + end + + def test_off_a_terminal_prints_one_line_and_never_redraws + io = StringIO.new + progress = Lintus::Progress.new(io, 3) + + progress.start + progress.update(1) + progress.update(3) + progress.finish + + assert_equal "Checking 3 files...\n", io.string + end + + def test_says_nothing_when_there_is_nothing_to_check + io = Terminal.new + progress = Lintus::Progress.new(io, 0) + + progress.start + progress.finish + + assert_empty io.string + end +end diff --git a/test/test_report.rb b/test/test_report.rb index fd9e35b..889c564 100644 --- a/test/test_report.rb +++ b/test/test_report.rb @@ -10,19 +10,14 @@ def setup @report = Lintus::Report.new end - def test_exit_status_by_fail_level + def test_exit_status assert_equal 0, @report.exit_status @report.record_checked("a.rb", [Lintus::Offense.new(path: "a.rb", rule: @config.rule(:documented), noul: 0.1)]) - assert_equal 0, @report.exit_status - assert_equal 1, @report.exit_status(fail_on: "warning") - - @report.record_checked("a.rb", [Lintus::Offense.new(path: "a.rb", rule: @config.rule(:no_sleep), noul: 0.9)]) assert_equal 1, @report.exit_status - assert_equal 0, @report.exit_status(fail_on: "never") @report.record_failure("b.rb", Jev::APIError.new(500, "boom")) - assert_equal 2, @report.exit_status(fail_on: "never") + assert_equal 2, @report.exit_status end def test_sorted_offenses_are_stable_by_path_then_rule diff --git a/test/test_rule.rb b/test/test_rule.rb index c9dd81c..57af109 100644 --- a/test/test_rule.rb +++ b/test/test_rule.rb @@ -10,11 +10,9 @@ def test_defaults assert_equal "no_sleep", rule.id assert_equal "Does this file call sleep?", rule.description - assert_equal "error", rule.severity assert_equal true, rule.offense_when assert_nil rule.threshold assert_nil rule.criteria - assert rule.error? assert rule.applies_to?("anything/at/all.txt") end @@ -40,9 +38,9 @@ def test_exclude_wins_over_paths end def test_symbol_keys_are_accepted - rule = Lintus::Rule.new(:r, { question: "q", severity: :warning, criteria: { "true" => "yes", "false" => "no" } }) + rule = Lintus::Rule.new(:r, { question: "q", offense_when: false, criteria: { "true" => "yes", "false" => "no" } }) - assert_equal "warning", rule.severity + assert_equal false, rule.offense_when assert_equal({ "true" => "yes", "false" => "no" }, rule.criteria) end @@ -71,7 +69,7 @@ def test_validation_errors assert_config_error("invalid rule id") { Lintus::Rule.new("No-Sleep", { "question" => "q" }) } assert_config_error("`question` is required") { Lintus::Rule.new("r", { "description" => "d" }) } assert_config_error("expected a map") { Lintus::Rule.new("r", "just a string") } - assert_config_error("`severity` must be one of") { Lintus::Rule.new("r", { "question" => "q", "severity" => "fatal" }) } + assert_config_error("unknown key(s) severity") { Lintus::Rule.new("r", { "question" => "q", "severity" => "error" }) } assert_config_error("`threshold` must be") { Lintus::Rule.new("r", { "question" => "q", "threshold" => 2 }) } assert_config_error("`criteria` must be a map") { Lintus::Rule.new("r", { "question" => "q", "criteria" => "x" }) } assert_config_error("`offense_when` must be") { Lintus::Rule.new("r", { "question" => "q", "offense_when" => "yes" }) } diff --git a/test/test_runner.rb b/test/test_runner.rb index 8ecfae9..1f0cec2 100644 --- a/test/test_runner.rb +++ b/test/test_runner.rb @@ -30,8 +30,7 @@ def test_sends_one_query_per_file_with_every_applicable_rule end assert_equal ["app/jobs/a_job.rb"], report.checked - assert_equal([["no_sleep", 0.9, "error"], ["documented", 0.2, "warning"]], - report.offenses.map { |o| [o.rule.id, o.noul, o.severity] }) + assert_equal([["no_sleep", 0.9], ["documented", 0.2]], report.offenses.map { |o| [o.rule.id, o.noul] }) end def test_thresholds_and_offense_when_are_honoured @@ -111,6 +110,16 @@ def test_runs_files_concurrently assert_operator Array.new(threads.size) { threads.pop }.uniq.size, :>, 1 end + def test_reports_progress_after_each_file + stub_jev({ "documented" => noul(0.1) }) + seen = [] + runner = Lintus::Runner.new(@config, jobs: 2) + + runner.run(runner.plan(Array.new(4) { |i| source("lib/f#{i}.rb") })) { |done| seen << done } + + assert_equal [1, 2, 3, 4], seen + end + def test_unreadable_files_are_recorded_as_failures file = Lintus::SourceFile.new("lib/gone.rb") { File.binread("/nonexistent/gone.rb") } From 8172b829f4b3ba74ab1e11f8a47e9893590fcfee Mon Sep 17 00:00:00 2001 From: Vincent Rolea <3525369+virolea@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:30:46 +0000 Subject: [PATCH 2/2] Tune the documentation rule from a real run, expose all probabilities The first real run flagged every lib file on "does every class or module have a comment?", with probabilities of 0.09 to 0.41: the model read "every" literally and counted the undocumented `module Lintus` wrapper. The rule now asks about the offense ("is there a class without a comment?") and its criteria say which classes to ignore. The starter template gets the same treatment, and the README explains the two lessons: ask about the offense, and name the unit and its exclusions. The JSON output now lists, per file, the probability every rule gave it, so a run can be read for confidence and not only for verdicts. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UhtJDjXrgmwwet27LAiY5f --- .lintus.yml | 11 +++++++++-- CHANGELOG.md | 3 +++ README.md | 20 +++++++++++++++++++- lib/lintus/formatter/json.rb | 8 ++++++++ lib/lintus/report.rb | 7 +++++-- lib/lintus/runner.rb | 3 ++- lib/lintus/template.rb | 13 ++++++++----- test/test_formatters.rb | 4 +++- test/test_runner.rb | 5 +++-- 9 files changed, 60 insertions(+), 14 deletions(-) diff --git a/.lintus.yml b/.lintus.yml index 8b30252..78402da 100644 --- a/.lintus.yml +++ b/.lintus.yml @@ -24,7 +24,14 @@ rules: classes_are_documented: description: Each class and module should open with a comment describing its responsibility. - question: Does every top-level class or module in this file have a comment describing what it is for? - offense_when: false + question: Is there a class or module defined in this file whose definition is not preceded by a comment describing what it is for? + criteria: + "true": >- + A class or module with a body of its own is introduced without a descriptive comment on the + lines right above its `class` or `module` line. + "false": >- + Every class or module that carries behaviour has such a comment, or the file defines none. + Ignore namespace wrappers whose body only nests other definitions, classes reopened only to + nest another definition, and one-line subclasses such as `class Error < StandardError; end`. exclude: - "exe/*" diff --git a/CHANGELOG.md b/CHANGELOG.md index cfef76b..7c03ff7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## [Unreleased] - A progress counter on stderr while files are being checked. +- The JSON output lists every file with the probability each rule gave it, not only offenses. +- The starter config's documentation rule asks about the offense and scopes it with criteria, + so namespace wrappers no longer trip it. - Removed rule `severity` and the `--fail-on` flag: an offense is an offense. A config that still sets `severity` is rejected with a message naming the key. diff --git a/README.md b/README.md index c2a49a6..8fa02f1 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,22 @@ model. So prefer several narrow questions over one broad one: "Does this file ca and "Does this file rescue `Exception`?" as two rules beat "Does this file do anything a job should not?". +**Ask about the offense, not about compliance.** "Does every class have a comment?" turns +false on a single edge case, and there is always one: the namespace wrapper, the reopened +class, the one-line error subclass. "Is there a class without a comment?" is the same +question, but its `criteria` can now spell out which classes count. Keep `offense_when: false` +for questions that are genuinely easier to phrase positively. + +**Name the unit and list what to ignore.** The model reads a question literally. When Lintus +first linted itself with "does every class or module have a comment?", every file was flagged +with a probability around 0.15: each one opens with an undocumented `module Lintus`. The fix +was not the threshold but the criteria, which now exclude wrappers whose body only nests other +definitions. + +**Read the probabilities before touching the threshold.** Scores clustered far from 0.5 mean +the model is sure of its reading of the question. If that reading is not yours, reword. Move +the threshold only when the scores of clean and offending files overlap around it. + Lintus sends the model the file's path and its full content. A question can therefore refer to the file name ("Is this a controller?") as well as the code. @@ -137,7 +153,9 @@ Only files that at least one rule applies to are sent. Deleted files are never s `--format text` is the default. `--format github` prints GitHub Actions workflow commands, so each offense becomes an annotation on the file in the pull request; it is the default when -`GITHUB_ACTIONS` is set. `--format json` is for other tools. +`GITHUB_ACTIONS` is set. `--format json` is for other tools, and it also lists the probability +every rule gave every file under `files`, offense or not, which is what you want when tuning +a rule's wording or threshold. Exit status is `0` when clean, `1` when there are offenses, and `2` when a request failed or the invocation was wrong. diff --git a/lib/lintus/formatter/json.rb b/lib/lintus/formatter/json.rb index 780979a..0e94f20 100644 --- a/lib/lintus/formatter/json.rb +++ b/lib/lintus/formatter/json.rb @@ -15,10 +15,18 @@ def render(report) failures: report.failures.size }, offenses: report.sorted_offenses.map(&:to_h), + files: report.checked.sort_by(&:path).map { |checked| file_entry(checked) }, skipped: report.skipped.map { |skipped| { path: skipped.path, reason: skipped.reason } }, failures: report.failures.map { |failure| { path: failure.path, error: failure.error.message } } ) end + + private + + # Every rule asked about the file and the probability it got, offense or not. + def file_entry(checked) + { path: checked.path, answers: checked.answers.transform_values { |noul| noul.round(3) } } + end end end end diff --git a/lib/lintus/report.rb b/lib/lintus/report.rb index 5b02ca6..3f27f9f 100644 --- a/lib/lintus/report.rb +++ b/lib/lintus/report.rb @@ -3,6 +3,7 @@ module Lintus # Everything a run produced: what was checked, what was flagged, what was skipped, what broke. class Report + Checked = Struct.new(:path, :answers, keyword_init: true) Skipped = Struct.new(:path, :reason, keyword_init: true) Failure = Struct.new(:path, :error, keyword_init: true) @@ -16,9 +17,11 @@ def initialize @mutex = Mutex.new end - def record_checked(path, offenses) + # `answers` maps every rule asked about the file to the probability it got, + # offense or not, so a run can be read for confidence and not just verdicts. + def record_checked(path, offenses, answers = {}) synchronize do - checked << path + checked << Checked.new(path: path, answers: answers) self.offenses.concat(offenses) end end diff --git a/lib/lintus/runner.rb b/lib/lintus/runner.rb index 6134de2..ad12473 100644 --- a/lib/lintus/runner.rb +++ b/lib/lintus/runner.rb @@ -54,7 +54,8 @@ def check(file, rules, report) end response = with_retries { perform(file, rules, content) } - report.record_checked(file.path, offenses_for(file, rules, response.answers)) + answers = rules.to_h { |rule| [rule.id, response.answers[rule.id].noul] } + report.record_checked(file.path, offenses_for(file, rules, response.answers), answers) rescue Jev::Error, Error => e report.record_failure(file.path, e) end diff --git a/lib/lintus/template.rb b/lib/lintus/template.rb index 5955c6e..7efba7e 100644 --- a/lib/lintus/template.rb +++ b/lib/lintus/template.rb @@ -37,11 +37,14 @@ module Template "false": Credentials are read from configuration, or there are none. threshold: 0.7 - methods_are_documented: - description: Public classes should carry a short comment explaining their purpose. - question: Does every class or module defined in this file have a comment describing its responsibility? - # This rule is phrased positively, so the offense is a "false" answer. - offense_when: false + classes_are_documented: + description: Classes should open with a short comment explaining their purpose. + question: Is there a class or module defined in this file whose definition is not preceded by a comment describing what it is for? + criteria: + "true": A class or module with a body of its own has no descriptive comment right above its definition. + "false": >- + Every class or module that carries behaviour has such a comment, or the file defines none. + Ignore namespace wrappers that only nest other definitions and one-line subclasses. YAML end end diff --git a/test/test_formatters.rb b/test/test_formatters.rb index 9232370..bbff7fb 100644 --- a/test/test_formatters.rb +++ b/test/test_formatters.rb @@ -8,7 +8,7 @@ class TestFormatters < Minitest::Test def setup config = build_config @report = Lintus::Report.new - @report.record_checked("lib/ok.rb", []) + @report.record_checked("lib/ok.rb", [], { "no_sleep" => 0.0123, "documented" => 0.9 }) @report.record_checked("app/jobs/a_job.rb", [ Lintus::Offense.new(path: "app/jobs/a_job.rb", rule: config.rule(:no_sleep), noul: 0.912), Lintus::Offense.new(path: "app/jobs/a_job.rb", rule: config.rule(:documented), noul: 0.3) @@ -61,6 +61,8 @@ def test_json assert_equal summary, data["summary"] assert_equal offense, data["offenses"].last + assert_equal [{ "path" => "app/jobs/a_job.rb", "answers" => {} }, + { "path" => "lib/ok.rb", "answers" => { "no_sleep" => 0.012, "documented" => 0.9 } }], data["files"] assert_equal [{ "path" => "lib/blob.rb", "reason" => "binary file" }], data["skipped"] assert_equal "Jev API error 500: boom", data["failures"].first["error"] end diff --git a/test/test_runner.rb b/test/test_runner.rb index 1f0cec2..27a257e 100644 --- a/test/test_runner.rb +++ b/test/test_runner.rb @@ -29,7 +29,8 @@ def test_sends_one_query_per_file_with_every_applicable_rule assert_equal({ "type" => "noul", "instructions" => "Does this file call sleep?" }, body["questions"]["no_sleep"]) end - assert_equal ["app/jobs/a_job.rb"], report.checked + assert_equal ["app/jobs/a_job.rb"], report.checked.map(&:path) + assert_equal({ "no_sleep" => 0.9, "documented" => 0.2 }, report.checked.first.answers) assert_equal([["no_sleep", 0.9], ["documented", 0.2]], report.offenses.map { |o| [o.rule.id, o.noul] }) end @@ -59,7 +60,7 @@ def test_skips_binary_and_oversized_files_without_calling_the_api assert_requested(:post, API_URL, times: 1) { |request| JSON.parse(request.body)["state"].include?("lib/ok.rb") } assert_equal ["lib/bin.rb", "lib/big.rb"], report.skipped.map(&:path) assert_includes report.skipped.last.reason, "max_file_size" - assert_equal ["lib/ok.rb"], report.checked + assert_equal ["lib/ok.rb"], report.checked.map(&:path) end def test_retries_rate_limits_with_backoff_then_succeeds