DEV: fix linter and linting config (#36815)

This amends lefthook to use the more reliable doublestar globber:
https://github.com/evilmartians/lefthook/blob/master/docs/mdbook/configuration/glob_matcher.md
it also cleans up the config so we can properly run and filter.

Notably ember linter explodes if you pass it files your are not meant to
lint.

The current fix has bin/lint read the config from lefthook and filter
out the files that need no linting. `--file` overrides the glob so we
can not pass in files unfiltered.

This also simplifies some of the linters internals for easier
maintenance
This commit is contained in:
Sam
2025-12-22 07:23:19 +11:00
committed by GitHub
parent 9cc884d289
commit bb8ed7d6f9
7 changed files with 251 additions and 191 deletions
+4 -2
View File
@@ -16,8 +16,10 @@ if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
abort(
"Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.",
)
end
end
+89 -87
View File
@@ -10,105 +10,107 @@
require "rubygems"
m = Module.new do
module_function
m =
Module.new do
module_function
def invoked_as_script?
File.expand_path($0) == File.expand_path(__FILE__)
end
def invoked_as_script?
File.expand_path($0) == File.expand_path(__FILE__)
end
def env_var_version
ENV["BUNDLER_VERSION"]
end
def env_var_version
ENV["BUNDLER_VERSION"]
end
def cli_arg_version
return unless invoked_as_script? # don't want to hijack other binstubs
return unless "update".start_with?(ARGV.first || " ") # must be running `bundle update`
bundler_version = nil
update_index = nil
ARGV.each_with_index do |a, i|
if update_index && update_index.succ == i && a =~ Gem::Version::ANCHORED_VERSION_PATTERN
bundler_version = a
def cli_arg_version
return unless invoked_as_script? # don't want to hijack other binstubs
return unless "update".start_with?(ARGV.first || " ") # must be running `bundle update`
bundler_version = nil
update_index = nil
ARGV.each_with_index do |a, i|
if update_index && update_index.succ == i && a =~ Gem::Version::ANCHORED_VERSION_PATTERN
bundler_version = a
end
next unless a =~ /\A--bundler(?:[= ](#{Gem::Version::VERSION_PATTERN}))?\z/
bundler_version = $1
update_index = i
end
next unless a =~ /\A--bundler(?:[= ](#{Gem::Version::VERSION_PATTERN}))?\z/
bundler_version = $1
update_index = i
bundler_version
end
bundler_version
end
def gemfile
gemfile = ENV["BUNDLE_GEMFILE"]
return gemfile if gemfile && !gemfile.empty?
def gemfile
gemfile = ENV["BUNDLE_GEMFILE"]
return gemfile if gemfile && !gemfile.empty?
File.expand_path("../../Gemfile", __FILE__)
end
File.expand_path("../../Gemfile", __FILE__)
end
def lockfile
lockfile =
case File.basename(gemfile)
when "gems.rb" then gemfile.sub(/\.rb$/, gemfile)
else "#{gemfile}.lock"
def lockfile
lockfile =
case File.basename(gemfile)
when "gems.rb"
gemfile.sub(/\.rb$/, gemfile)
else
"#{gemfile}.lock"
end
File.expand_path(lockfile)
end
def lockfile_version
return unless File.file?(lockfile)
lockfile_contents = File.read(lockfile)
unless lockfile_contents =~ /\n\nBUNDLED WITH\n\s{2,}(#{Gem::Version::VERSION_PATTERN})\n/
return
end
File.expand_path(lockfile)
end
def lockfile_version
return unless File.file?(lockfile)
lockfile_contents = File.read(lockfile)
return unless lockfile_contents =~ /\n\nBUNDLED WITH\n\s{2,}(#{Gem::Version::VERSION_PATTERN})\n/
Regexp.last_match(1)
end
def bundler_version
@bundler_version ||=
env_var_version || cli_arg_version ||
lockfile_version
end
def bundler_requirement
return "#{Gem::Requirement.default}.a" unless bundler_version
bundler_gem_version = Gem::Version.new(bundler_version)
requirement = bundler_gem_version.approximate_recommendation
return requirement if Gem::Version.new(Gem::VERSION) >= Gem::Version.new("2.7.0")
requirement += ".a" if bundler_gem_version.prerelease?
requirement
end
def load_bundler!
ENV["BUNDLE_GEMFILE"] ||= gemfile
activate_bundler
end
def activate_bundler
gem_error = activation_error_handling do
gem "bundler", bundler_requirement
Regexp.last_match(1)
end
return if gem_error.nil?
require_error = activation_error_handling do
require "bundler/version"
end
return if require_error.nil? && Gem::Requirement.new(bundler_requirement).satisfied_by?(Gem::Version.new(Bundler::VERSION))
warn "Activating bundler (#{bundler_requirement}) failed:\n#{gem_error.message}\n\nTo install the version of bundler this project requires, run `gem install bundler -v '#{bundler_requirement}'`"
exit 42
end
def activation_error_handling
yield
nil
rescue StandardError, LoadError => e
e
def bundler_version
@bundler_version ||= env_var_version || cli_arg_version || lockfile_version
end
def bundler_requirement
return "#{Gem::Requirement.default}.a" unless bundler_version
bundler_gem_version = Gem::Version.new(bundler_version)
requirement = bundler_gem_version.approximate_recommendation
return requirement if Gem::Version.new(Gem::VERSION) >= Gem::Version.new("2.7.0")
requirement += ".a" if bundler_gem_version.prerelease?
requirement
end
def load_bundler!
ENV["BUNDLE_GEMFILE"] ||= gemfile
activate_bundler
end
def activate_bundler
gem_error = activation_error_handling { gem "bundler", bundler_requirement }
return if gem_error.nil?
require_error = activation_error_handling { require "bundler/version" }
if require_error.nil? &&
Gem::Requirement.new(bundler_requirement).satisfied_by?(
Gem::Version.new(Bundler::VERSION),
)
return
end
warn "Activating bundler (#{bundler_requirement}) failed:\n#{gem_error.message}\n\nTo install the version of bundler this project requires, run `gem install bundler -v '#{bundler_requirement}'`"
exit 42
end
def activation_error_handling
yield
nil
rescue StandardError, LoadError => e
e
end
end
end
m.load_bundler!
if m.invoked_as_script?
load Gem.bin_path("bundler", "bundle")
end
load Gem.bin_path("bundler", "bundle") if m.invoked_as_script?
+9 -4
View File
@@ -117,12 +117,17 @@ if ARGV.include?("-u") || ARGV.include?("--unicorn")
end
Thread.new do
Open3.popen2e(pnpm_env, "pnpm", "ember-tsc", "-b", "--watch", "--preserveWatchOutput") do |i, oe, t|
Open3.popen2e(
pnpm_env,
"pnpm",
"ember-tsc",
"-b",
"--watch",
"--preserveWatchOutput",
) do |i, oe, t|
tsc_pid = t.pid
puts "Ember TSC running on PID: #{tsc_pid}"
oe.each do |line|
puts "[ember-tsc] #{line}"
end
oe.each { |line| puts "[ember-tsc] #{line}" }
end
if process_running?(unicorn_pid)
puts "[bin/ember-cli] ember-tsc process stopped. Terminating unicorn."
+85 -49
View File
@@ -4,6 +4,8 @@
require "optparse"
require "open3"
require "shellwords"
require "yaml"
require "pathname"
class LefthookLinter
def initialize(options = {})
@@ -19,25 +21,37 @@ class LefthookLinter
EVERYTHING = ["ALL FILES"]
def run
files_to_lint = determine_files
files = determine_files
if @fix
run_fix_mode(files_to_lint)
run_fix_mode(files)
else
run_check_mode(files_to_lint)
run_check_mode(files)
end
end
private
def determine_files
return recent_files if @recent
return staged_files if @staged
return unstaged_files if @unstaged
return wip_files if @wip
return @files if !@files.empty?
files =
if @recent
recent_files
elsif @staged
staged_files
elsif @unstaged
unstaged_files
elsif @wip
wip_files
elsif !@files.empty?
@files
else
return EVERYTHING
end
EVERYTHING
files
.map { |f| normalize_relative_path(f) }
.select { |f| File.file?(f) && lintable_file?(f) }
.uniq
end
def recent_files
@@ -46,58 +60,40 @@ class LefthookLinter
log_files = log_output.lines.map(&:strip).reject(&:empty?)
tracked_out, tracked_status = Open3.capture2("git", "ls-files")
untracked_out, untracked_status =
Open3.capture2("git", "ls-files", "--others", "--exclude-standard")
return [] unless tracked_status.success? && untracked_status.success?
tracked_out, _ = Open3.capture2("git", "ls-files")
untracked_out, _ = Open3.capture2("git", "ls-files", "--others", "--exclude-standard")
tracked = Set.new(tracked_out.lines.map(&:strip))
untracked = Set.new(untracked_out.lines.map(&:strip))
# Only keep log files that are still tracked, then add all untracked
candidates = Set.new
candidates = []
log_files.each { |f| candidates << f if tracked.include?(f) }
untracked.each { |f| candidates << f }
candidates.select { |f| File.file?(f) && lintable_file?(f) }
candidates + untracked.to_a
end
def staged_files
git_output, status = Open3.capture2("git", "diff", "--cached", "--name-only")
return [] unless status.success?
git_output.lines.map(&:strip).reject(&:empty?).select { |f| File.file?(f) && lintable_file?(f) }
git_output.lines.map(&:strip).reject(&:empty?)
end
def unstaged_files
git_output, status = Open3.capture2("git", "diff", "--name-only")
return [] unless status.success?
git_output.lines.map(&:strip).reject(&:empty?).select { |f| File.file?(f) && lintable_file?(f) }
git_output.lines.map(&:strip).reject(&:empty?)
end
def wip_files
# Get files changed since main branch
main_diff_output, main_status = Open3.capture2("git", "diff", "main...HEAD", "--name-only")
main_files = main_status.success? ? main_diff_output.lines.map(&:strip).reject(&:empty?) : []
main_diff_output, _ = Open3.capture2("git", "diff", "main...HEAD", "--name-only")
main_files = main_diff_output.lines.map(&:strip).reject(&:empty?)
# Get staged files
staged_output, staged_status = Open3.capture2("git", "diff", "--cached", "--name-only")
staged = staged_status.success? ? staged_output.lines.map(&:strip).reject(&:empty?) : []
# Get unstaged files
unstaged_output, unstaged_status = Open3.capture2("git", "diff", "--name-only")
unstaged = unstaged_status.success? ? unstaged_output.lines.map(&:strip).reject(&:empty?) : []
# Combine all files and remove duplicates
all_files = Set.new(main_files + staged + unstaged)
all_files.select { |f| File.file?(f) && lintable_file?(f) }
main_files + staged_files + unstaged_files
end
def lintable_file?(file)
# Skip certain directories and files
if file.include?("node_modules") || file.include?("vendor") || file.include?("tmp") ||
file.include?(".git") || file == "database.yml"
file.include?(".git") || file == "config/database.yml"
return false
end
@@ -157,27 +153,67 @@ class LefthookLinter
end
end
def run_lefthook_command(hook_name, files)
cmd = ["pnpm", "lefthook", "run", hook_name]
GLOB_FLAGS = File::FNM_PATHNAME | File::FNM_EXTGLOB | File::FNM_CASEFOLD | File::FNM_DOTMATCH
LEFTHOOK_CONFIG_PATH = File.expand_path("../lefthook.yml", __dir__)
PROJECT_ROOT = File.expand_path("..", __dir__)
# Only add file arguments if we have specific files
# For lints and fix-staged without files, let lefthook handle all files
files.each { |file| cmd << "--file" << file } unless files.empty?
def run_lefthook_command(hook, files)
commands = lefthook_config.dig(hook, "commands")
# Add verbose flag if requested
if !files.empty? && commands
normalized = files.map { |f| normalize_relative_path(f) }
any = false
commands.each do |name, config|
filtered = filter_files_for_command(normalized, config)
next if filtered.empty?
any = true
exec_lefthook(hook, name, filtered)
end
puts "No matching linters for provided files." if !any && @verbose
else
exec_lefthook(hook, nil, files)
end
end
def exec_lefthook(hook, command, files)
cmd = ["pnpm", "lefthook", "run", hook]
cmd << "--command" << command if command
files.each { |f| cmd << "--file" << f }
cmd << "--verbose" if @verbose
puts "Running: #{cmd.shelljoin}" if @verbose
exit 1 unless system(*cmd)
end
# Use system to preserve colored output and proper exit codes
success = system(*cmd)
def lefthook_config
@lefthook_config ||= YAML.load_file(LEFTHOOK_CONFIG_PATH)
end
unless success
puts "❌ Linting failed"
exit 1
def filter_files_for_command(files, config)
globs = Array(config["glob"]).compact
excludes = Array(config["exclude"]).compact
files.select do |file|
matches_includes =
globs.empty? || globs.any? { |pattern| File.fnmatch?(pattern, file, GLOB_FLAGS) }
matches_excludes = excludes.any? { |pattern| File.fnmatch?(pattern, file, GLOB_FLAGS) }
matches_includes && !matches_excludes
end
end
puts "✅ All linting checks passed" if @verbose
def normalize_relative_path(file)
cleaned = file.start_with?("./") ? file[2..] : file
path = Pathname.new(cleaned)
if path.absolute?
begin
path = path.relative_path_from(Pathname.new(PROJECT_ROOT))
rescue ArgumentError
# leave as is if it's outside the repo
end
end
path.to_s
end
end
+5 -5
View File
@@ -1,13 +1,13 @@
#!/usr/bin/env ruby
# frozen_string_literal: true
if ENV['RAILS_ENV'] == 'test' && ENV['LOAD_PLUGINS'].nil?
if ARGV.include?('db:migrate') || ARGV.include?('parallel:migrate')
if ENV["RAILS_ENV"] == "test" && ENV["LOAD_PLUGINS"].nil?
if ARGV.include?("db:migrate") || ARGV.include?("parallel:migrate")
STDERR.puts "You are attempting to run migrations in your test environment and are not loading plugins, setting LOAD_PLUGINS to 1"
ENV['LOAD_PLUGINS'] = '1'
ENV["LOAD_PLUGINS"] = "1"
end
end
require_relative '../config/boot'
require 'rake'
require_relative "../config/boot"
require "rake"
Rake.application.run
+5 -4
View File
@@ -9,8 +9,7 @@
#
require "pathname"
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
Pathname.new(__FILE__).realpath)
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", Pathname.new(__FILE__).realpath)
bundle_binstub = File.expand_path("../bundle", __FILE__)
@@ -18,8 +17,10 @@ if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300) =~ /This file was generated by Bundler/
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
abort(
"Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.",
)
end
end
+54 -40
View File
@@ -7,6 +7,8 @@ output:
- execution_info
- skips
glob_matcher: doublestar
pre-commit:
parallel: true
skip:
@@ -14,49 +16,55 @@ pre-commit:
- rebase
commands:
rubocop:
exclude:
- "bin/notify_file_change"
- "bin/docker/**/*"
glob:
- "*.{rb,rake,thor}"
- "bin/*"
- "**/*.{rb,rake,thor}"
- "bin/**/*"
- "Gemfile"
run: bundle exec rubocop --force-exclusion {staged_files}
syntax_tree:
exclude:
- "bin/notify_file_change"
- "bin/docker/**/*"
glob:
- "*.{rb,rake,thor}"
- "bin/*"
- "**/*.{rb,rake,thor}"
- "bin/**/*"
- "Gemfile"
run: bundle exec stree check Gemfile {staged_files}
prettier:
glob:
- "app/assets/stylesheets/*.{css,scss}"
- "frontend/*.{js,gjs,scss,css,cjs,mjs}"
- "plugins/*.{js,gjs,scss,css,cjs,mjs}"
- "themes/*.{js,gjs,scss,css,cjs,mjs}"
- "app/assets/stylesheets/**/*.{css,scss}"
- "frontend/**/*.{js,gjs,scss,css,cjs,mjs}"
- "plugins/**/*.{js,gjs,scss,css,cjs,mjs}"
- "themes/**/*.{js,gjs,scss,css,cjs,mjs}"
run: pnpm pprettier --list-different {staged_files}
eslint:
glob:
- "frontend/*.{js,gjs}"
- "plugins/*.{js,gjs}"
- "themes/*.{js,gjs}"
- "frontend/**/*.{js,gjs}"
- "plugins/**/*.{js,gjs}"
- "themes/**/*.{js,gjs}"
run: pnpm eslint --quiet {staged_files}
ember-template-lint:
glob:
- "frontend/*.gjs"
- "plugins/*.gjs"
- "themes/*.gjs"
- "frontend/**/*.gjs"
- "plugins/**/*.gjs"
- "themes/**/*.gjs"
run: pnpm ember-template-lint {staged_files}
yaml-syntax:
glob: "*.{yaml,yml}"
glob: "**/*.{yaml,yml}"
# database.yml is an erb file not a yaml file
exclude: "database.yml"
exclude: "config/database.yml"
run: bundle exec yaml-lint {staged_files}
i18n-lint:
glob: "**/{client,server}.en.yml"
run: bundle exec ruby script/i18n_lint.rb {staged_files}
stylelint:
glob:
- "app/assets/stylesheets/*.scss"
- "plugins/*/assets/stylesheets/*.scss"
- "themes/*.scss"
- "app/assets/stylesheets/**/*.scss"
- "plugins/**/assets/stylesheets/**/*.scss"
- "themes/**/*.scss"
run: pnpm stylelint {staged_files}
fix-staged:
@@ -64,39 +72,45 @@ fix-staged:
commands:
prettier:
glob:
- "app/assets/stylesheets/*.{css,scss}"
- "frontend/*.{js,gjs,scss,css,cjs,mjs}"
- "plugins/*.{js,gjs,scss,css,cjs,mjs}"
- "themes/*.{js,gjs,scss,css,cjs,mjs}"
- "app/assets/stylesheets/**/*.{css,scss}"
- "frontend/**/*.{js,gjs,scss,css,cjs,mjs}"
- "plugins/**/*.{js,gjs,scss,css,cjs,mjs}"
- "themes/**/*.{js,gjs,scss,css,cjs,mjs}"
run: pnpm pprettier --write {staged_files}
eslint:
glob:
- "frontend/*.{js,gjs}"
- "plugins/*.{js,gjs}"
- "themes/*.{js,gjs}"
- "frontend/**/*.{js,gjs}"
- "plugins/**/*.{js,gjs}"
- "themes/**/*.{js,gjs}"
run: pnpm eslint --fix {staged_files}
ember-template-lint:
glob:
- "frontend/*.gjs"
- "plugins/*.gjs"
- "themes/*.gjs"
- "frontend/**/*.gjs"
- "plugins/**/*.gjs"
- "themes/**/*.gjs"
run: pnpm ember-template-lint --fix {staged_files}
stylelint:
glob:
- "app/assets/stylesheets/*.scss"
- "plugins/*/assets/stylesheets/*.scss"
- "themes/*.scss"
- "app/assets/stylesheets/**/*.scss"
- "plugins/**/assets/stylesheets/**/*.scss"
- "themes/**/*.scss"
run: pnpm stylelint --fix {staged_files}
rubocop:
exclude:
- "bin/notify_file_change"
- "bin/docker/**/*"
glob:
- "*.{rb,rake,thor}"
- "bin/*"
- "**/*.{rb,rake,thor}"
- "bin/**/*"
- "Gemfile"
run: bundle exec rubocop --force-exclusion -A {staged_files}
syntax_tree:
exclude:
- "bin/notify_file_change"
- "bin/docker/**/*"
glob:
- "*.{rb,rake,thor}"
- "bin/*"
- "**/*.{rb,rake,thor}"
- "bin/**/*"
- "Gemfile"
run: bundle exec stree write Gemfile {staged_files}
@@ -119,8 +133,8 @@ lints:
commands:
rubocop:
glob:
- "*.{rb,rake,thor}"
- "bin/*"
- "**/*.{rb,rake,thor}"
- "bin/**/*"
- "Gemfile"
run: bundle exec rubocop
prettier:
@@ -132,9 +146,9 @@ lints:
stylelint:
run: pnpm lint:css
yaml-syntax:
glob: "*.{yaml,yml}"
glob: "**/*.{yaml,yml}"
# database.yml is an erb file not a yaml file
exclude: "database.yml"
exclude: "config/database.yml"
run: bundle exec yaml-lint {all_files}
i18n-lint:
glob: "**/{client,server}.en.yml"