mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
Régis HanolandSam Saffron
1390bcf33a
`bin/lint` had several ways to print "All lints passed" without a linter ever running. That is worse than failing: it tells you your code is clean when nothing looked at it. Crashes and no-ops: - `Set#exclude?` is an ActiveSupport method, and this script loads only optparse, open3, pathname and shellwords. Every path under `plugins/` raised `NoMethodError`, so no plugin file could be linted at all. - `lib/` contains a `plugin.rb`, so it was mistaken for an external plugin. `--recent` ran `bundle install` inside `lib/` and linted 135 files with the wrong configuration. - The script never moved to the repository root, so any invocation from a subdirectory resolved paths against the wrong base and linted nothing. - A mistyped path was dropped in silence. Bad paths are now reported, and the remaining valid paths are still linted. - An empty result set claimed success. It now says "Nothing was linted". - A directory argument built an argv larger than `ARG_MAX`. `system` returns nil rather than raising there, which was recorded as a lint failure with no explanation. The `--file` arguments are now batched. Wrong file sets: - `--recent` intersected the last 50 commits with tracked files, so an uncommitted change to a file nobody had committed recently was never linted. It now includes the working tree. - Directory expansion walked the filesystem, so build output that git ignores was linted and files deleted from the working tree were handed to the linters. It now asks git, and falls back to a walk for directories git ignores, such as unbundled plugins. - `lintable_file?` compared substrings, so the `.git` test excluded all of `.github/`, and any path containing `tmp` or `vendor` anywhere was skipped. It now compares whole path segments. Unwanted side effects: - Checking an external plugin ran `bundle install` and `pnpm i` even without `--fix`, which can rewrite `Gemfile.lock` and `pnpm-lock.yaml`. Both installs are now frozen unless `--fix` is given, so a read-only check stays read-only. - A failed dependency install called `abort`, discarding the results already collected for other plugins. It is now recorded as a failure. Finally, `lefthook.yml` routes everything under `bin/` to the Ruby linters, but the exclude list was missing `bin/dev` (node) and `bin/system_rspec` (bash), so `syntax_tree` tried to parse them as Ruby. --------- Co-authored-by: Sam Saffron <sam.saffron@gmail.com>