Skip to content

Commit afa136f

Browse files
authored
Introduce Ruby linting (#1699)
* Add standardrb gem for Ruby linting * Add standardrb to CI lint job * Auto-fix standardrb violations * Add .standard.yml to exclude test fixture repos from linting The repos/ directory contains external Rails application fixtures used for integration testing. These should not be linted as they represent external code, not the buildpack's own code. * Fix Style/StringLiterals in Gemfile Lint failure: Style/StringLiterals - Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. The fix uses double quotes consistently for all gem declarations, matching the style used by the rest of the Gemfile. * Disable lint rules for intentional patterns in bin/support scripts Lint failures disabled: - Lint/RescueException: Rescuing Exception is intentional in buildpack entry points to ensure all errors (including SignalException, SystemExit) are properly reported to users rather than causing cryptic failures. - Style/MixinUsage: Including modules at top level is intentional for these standalone scripts, which are not library code and benefit from direct access to shell helper methods. * Fix lint violations in heroku_build_report.rb Lint failures: - Lint/NonLocalExitFromIterator: Changed 'return' to 'next' in the each block. Using 'return' would exit the entire capture method, while 'next' correctly skips to the next iteration. - Style/RedundantInterpolation: Changed "\#{key}" to key.to_s since the key is already a string after strip. This is more explicit and avoids unnecessary string interpolation. * Fix lint violations in language_pack.rb Lint failures: - Lint/AssignmentInCondition: Wrapped 'pack = LanguagePack.detect(...)' in parentheses to clarify that the assignment is intentional. - Style/SafeNavigation: Changed 'if pack_klass; pack_klass.new(...); end' to 'pack_klass&.new(...)' using safe navigation operator. This is more idiomatic Ruby and clearly expresses the intent to call new only if pack_klass is not nil. * Fix Performance/UnfreezeString violations Lint failure: Performance/UnfreezeString - Use unary plus to get an unfrozen string literal. Changed String.new("") and String.new("...") to +"" and +"..." respectively. The unary plus operator on a frozen string literal returns an unfrozen copy, which is more idiomatic and performant than String.new. These mutable strings are necessary because we append to them with <<. * Fix Lint/AssignmentInCondition and Lint/MixedRegexpCaptureTypes Lint failures fixed: - Lint/AssignmentInCondition: Wrapped 'if var = expr' in parentheses to clarify that the assignment is intentional, not a typo for '=='. - Lint/MixedRegexpCaptureTypes: Changed numbered capture groups (\r?\n) to non-capturing groups (?:\r?\n) in regex patterns. Mixing named and numbered captures causes confusion because numbered captures are renumbered when named captures are present, making the regex behavior unpredictable. * Fix Style/RedundantSort in outdated_ruby_version.rb Lint failure: Style/RedundantSort - Use max_by instead of sort_by...last. Changed 'sort_by { |v| Gem::Version.new(v) }.last' to 'max_by { |v| Gem::Version.new(v) }'. Using max_by is more efficient as it finds the maximum in O(n) time without sorting the entire array, and clearly expresses the intent to find the maximum value. * Fix Lint/DuplicateMethods in rake_runner.rb Lint failure: Lint/DuplicateMethods - Method RakeTask#status is defined at both line 11 (via attr_accessor) and line 37 (explicit def). The explicit status method includes validation logic that ensures the status is set to an allowed value before returning it. Changed 'attr_accessor :status' to 'attr_writer :status' so we only generate the setter, keeping the custom getter with its validation logic. * Fix lint violations in ruby.rb Lint failures fixed: - Performance/UnfreezeString: Changed String.new("") to +"" for creating a mutable string to append to. - Lint/BinaryOperatorWithIdenticalOperands: Fixed 'version != version' which always evaluates to false. Changed to 'old_version != version' to correctly detect when the default Node.js/Yarn version has changed. This was a bug where the version change warning would never be displayed. - Lint/IneffectiveAccessModifier: Added file-level ignore in .standard.yml. The class has public class methods interspersed after a 'private' declaration that only affects instance methods. The class methods are intentionally public and called from lib/language_pack.rb. * Fix lint violations in ruby_version.rb and shell_helpers.rb Lint failures fixed: - Style/RedundantInterpolation: Changed "\#{engine_version}" to engine_version.to_s and "\#{path}" to path.to_s. String interpolation on a single variable is redundant when the variable is already a string or should be explicitly converted. - Removed unused RUBY_VERSION_REGEX constant (dead code). * Fix lint violations in spec files Lint failures fixed: - Lint/RescueException (ruby_spec.rb): Added inline disable. The test rescues Exception intentionally to provide debugging output for any failure. - Style/RedundantInterpolation (outdated_ruby_version_spec.rb): Removed redundant string interpolation. - Lint/ShadowedArgument (ruby_version_spec.rb): Renamed block arg to _dir since it's immediately shadowed by a local variable assignment. - Lint/ConstantDefinitionInBlock (shell_spec.rb): Added inline disable. Defining test helper classes inside describe blocks is a common RSpec pattern. - Lint/DuplicateRequire (spec_helper.rb): Removed duplicate require 'hatchet'. - Lint/MixedRegexpCaptureTypes (spec_helper.rb): Changed numbered capture group to non-capturing group. - Lint/DuplicateMethods (spec_helper.rb): Removed duplicate hatchet_path method definition. * Fix Layout/IndentationConsistency in ruby.rb Lint failure: Layout/IndentationConsistency - Inconsistent indentation detected. Fixed extra indentation on the bundle_command assignment line. * Changelog * Remove unused include * Apply linting fixes
1 parent 88d5afd commit afa136f

File tree

84 files changed

+940
-913
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

84 files changed

+940
-913
lines changed

.github/workflows/ci.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ jobs:
1616
steps:
1717
- name: Checkout
1818
uses: actions/checkout@v6
19+
- name: Install Ruby
20+
uses: ruby/setup-ruby@v1
21+
with:
22+
bundler-cache: true
23+
ruby-version: "3.3.9"
24+
- name: Run StandardRB
25+
run: bundle exec standardrb
1926
- name: Run ShellCheck bin top level
2027
run: |
2128
# All bash files that don't end in '.rb'

.standard.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# StandardRB configuration
2+
# https://github.com/standardrb/standard
3+
4+
ignore:
5+
- "repos/**/*" # Test fixture repos (external Rails apps for integration tests)
6+
- "lib/language_pack/ruby.rb":
7+
- Lint/IneffectiveAccessModifier # Class methods after `private` are intentionally public; private only affects instance methods

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- Set `WEB_CONCURRENCY_SET_BY=heroku/ruby` if `WEB_CONCURRENCY` gets set in (deprecated) `SENSIBLE_DEFAULTS` mode (https://github.com/heroku/heroku-buildpack-ruby/pull/1700)
66
- Default Node.js version now 24.13.0 (https://github.com/heroku/heroku-buildpack-ruby/pull/1704)
7+
- Fix yarn and nodejs version change warnings (https://github.com/heroku/heroku-buildpack-ruby/pull/1699)
78

89
## [v344] - 2026-01-14
910

@@ -14,7 +15,6 @@
1415

1516
- Ruby 4.0.1 is now available
1617

17-
1818
## [v342] - 2026-01-09
1919

2020
- Update bundler version warning output (https://github.com/heroku/heroku-buildpack-ruby/pull/1697)

Gemfile

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ group :development, :test do
1111
gem "excon"
1212
gem "rake"
1313
gem "parallel_split_test"
14-
gem 'rspec-retry'
15-
gem 'json'
16-
gem 'redis'
14+
gem "rspec-retry"
15+
gem "json"
16+
gem "redis"
17+
gem "standard"
1718
end

Gemfile.lock

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
GEM
22
remote: https://rubygems.org/
33
specs:
4+
ast (2.4.3)
45
base64 (0.3.0)
56
citrus (3.0.2)
67
connection_pool (2.5.3)
@@ -22,24 +23,32 @@ GEM
2223
thor (~> 1)
2324
threaded (~> 0)
2425
json (2.18.0)
26+
language_server-protocol (3.17.0.5)
27+
lint_roller (1.1.0)
2528
logger (1.7.0)
2629
moneta (1.0.0)
2730
multi_json (1.17.0)
2831
parallel (1.27.0)
2932
parallel_split_test (0.10.0)
3033
parallel (>= 0.5.13)
3134
rspec-core (>= 3.9.0)
35+
parser (3.3.10.0)
36+
ast (~> 2.4.1)
37+
racc
3238
platform-api (3.8.0)
3339
heroics (~> 0.1.1)
3440
moneta (~> 1.0.0)
3541
rate_throttle_client (~> 0.1.0)
42+
prism (1.7.0)
3643
racc (1.8.1)
44+
rainbow (3.1.1)
3745
rake (13.3.1)
3846
rate_throttle_client (0.1.2)
3947
redis (5.4.1)
4048
redis-client (>= 0.22.0)
4149
redis-client (0.25.1)
4250
connection_pool
51+
regexp_parser (2.11.3)
4352
rrrretry (1.0.0)
4453
rspec-core (3.13.6)
4554
rspec-support (~> 3.13.0)
@@ -49,11 +58,45 @@ GEM
4958
rspec-retry (0.6.2)
5059
rspec-core (> 3.3)
5160
rspec-support (3.13.6)
61+
rubocop (1.81.7)
62+
json (~> 2.3)
63+
language_server-protocol (~> 3.17.0.2)
64+
lint_roller (~> 1.1.0)
65+
parallel (~> 1.10)
66+
parser (>= 3.3.0.2)
67+
rainbow (>= 2.2.2, < 4.0)
68+
regexp_parser (>= 2.9.3, < 3.0)
69+
rubocop-ast (>= 1.47.1, < 2.0)
70+
ruby-progressbar (~> 1.7)
71+
unicode-display_width (>= 2.4.0, < 4.0)
72+
rubocop-ast (1.49.0)
73+
parser (>= 3.3.7.2)
74+
prism (~> 1.7)
75+
rubocop-performance (1.26.1)
76+
lint_roller (~> 1.1)
77+
rubocop (>= 1.75.0, < 2.0)
78+
rubocop-ast (>= 1.47.1, < 2.0)
79+
ruby-progressbar (1.13.0)
80+
standard (1.52.0)
81+
language_server-protocol (~> 3.17.0.2)
82+
lint_roller (~> 1.0)
83+
rubocop (~> 1.81.7)
84+
standard-custom (~> 1.0.0)
85+
standard-performance (~> 1.8)
86+
standard-custom (1.0.2)
87+
lint_roller (~> 1.0)
88+
rubocop (~> 1.50)
89+
standard-performance (1.9.0)
90+
lint_roller (~> 1.1)
91+
rubocop-performance (~> 1.26.0)
5292
thor (1.4.0)
5393
threaded (0.0.4)
5494
toml-rb (4.1.0)
5595
citrus (~> 3.0, > 3.0)
5696
racc (~> 1.7)
97+
unicode-display_width (3.2.0)
98+
unicode-emoji (~> 4.1)
99+
unicode-emoji (4.2.0)
57100
webrick (1.9.1)
58101

59102
PLATFORMS
@@ -69,6 +112,7 @@ DEPENDENCIES
69112
rspec-core
70113
rspec-expectations
71114
rspec-retry
115+
standard
72116
toml-rb
73117

74118
RUBY VERSION

Rakefile

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
#
66
require "fileutils"
77
require "tmpdir"
8-
require 'hatchet/tasks'
9-
require_relative 'lib/rake/deploy_check'
8+
require "hatchet/tasks"
9+
require_relative "lib/rake/deploy_check"
1010

1111
namespace :buildpack do
1212
desc "prepares the next version of the buildpack for release"
@@ -44,13 +44,13 @@ namespace :buildpack do
4444
end
4545

4646
begin
47-
require 'rspec/core/rake_task'
47+
require "rspec/core/rake_task"
4848

4949
desc "Run specs"
5050
RSpec::Core::RakeTask.new(:spec) do |t|
51-
t.rspec_opts = %w(-fd --color)
52-
#t.ruby_opts = %w(-w)
51+
t.rspec_opts = %w[-fd --color]
52+
# t.ruby_opts = %w(-w)
5353
end
54-
task :default => :spec
55-
rescue LoadError => e
54+
task default: :spec
55+
rescue LoadError
5656
end

bin/support/ruby_compile.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
app_path: app_path,
2626
cache_path: cache_path,
2727
gemfile_lock: gemfile_lock,
28-
bundle_default_without: "development:test",
28+
bundle_default_without: "development:test"
2929
)
30-
rescue Exception => e
30+
rescue Exception => e # standard:disable Lint/RescueException
3131
LanguagePack::ShellHelpers.display_error_and_exit(e)
3232
end

bin/support/ruby_test-compile.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
require "language_pack"
1717
require "language_pack/shell_helpers"
1818
require "language_pack/test"
19-
include LanguagePack::ShellHelpers
2019

2120
begin
2221
app_path = Pathname(ARGV[0])
@@ -30,8 +29,8 @@
3029
cache_path: cache_path,
3130
gemfile_lock: gemfile_lock,
3231
bundle_default_without: "development",
33-
environment_name: "test",
32+
environment_name: "test"
3433
)
35-
rescue Exception => e
34+
rescue Exception => e # standard:disable Lint/RescueException
3635
LanguagePack::ShellHelpers.display_error_and_exit(e)
3736
end

bin/support/ruby_test.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
require "language_pack/test"
1313
require "language_pack/ruby"
1414

15-
include LanguagePack::ShellHelpers
15+
include LanguagePack::ShellHelpers # standard:disable Style/MixinUsage
1616

1717
def execute_test(command)
1818
topic("Running test: #{command}")
@@ -26,7 +26,7 @@ def execute_command(command)
2626
# having no whitespace before output. To avoid adding whitespace
2727
# for the original Kernel.puts to be used by passing in the
2828
# Kernel object.
29-
pipe(command, :user_env => true, :output_object => Kernel)
29+
pipe(command, user_env: true, output_object: Kernel)
3030
end
3131

3232
# $ bin/test app_path ENV_DIR ARTIFACT_DIR
@@ -35,7 +35,7 @@ def execute_command(command)
3535
Dir.chdir(app_path)
3636

3737
gems_list = LanguagePack::Helpers::BundleList::HumanCommand.new(
38-
stream_to_user: false,
38+
stream_to_user: false
3939
).call
4040

4141
execute_test(
@@ -53,4 +53,3 @@ def execute_command(command)
5353
"rake test"
5454
end
5555
)
56-

lib/heroku_build_report.rb

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
require 'yaml'
2-
require 'json'
3-
require 'pathname'
1+
require "yaml"
2+
require "json"
3+
require "pathname"
44

55
# Observability reporting for builds
66
#
@@ -17,7 +17,7 @@ class JsonReport
1717
MALFORMED_JSON_KEY = "build_report_file_malformed"
1818
attr_reader :data
1919

20-
def initialize(path: , io: $stdout)
20+
def initialize(path:, io: $stdout)
2121
@io = io
2222
@path = Pathname(path).expand_path
2323
@path.dirname.mkpath
@@ -35,9 +35,9 @@ def safe_load(contents)
3535
else
3636
JSON.parse(contents)
3737
end
38-
rescue => e
38+
rescue
3939
@io.puts "Internal Warning: Expected build report to be JSON, but it is malformed: #{contents.inspect}"
40-
{ MALFORMED_JSON_KEY => true }
40+
{MALFORMED_JSON_KEY => true}
4141
end
4242

4343
def complex_object?(value)
@@ -46,7 +46,7 @@ def complex_object?(value)
4646

4747
def capture(metrics = {})
4848
metrics.each do |(key, value)|
49-
return if key.nil? || key.to_s.strip.empty?
49+
next if key.nil? || key.to_s.strip.empty?
5050

5151
key = key&.strip
5252
raise "Key cannot be empty (#{key.inspect} => #{value})" if key.nil? || key.empty?
@@ -56,15 +56,15 @@ def capture(metrics = {})
5656
value = value.to_s
5757
end
5858

59-
@data["#{key}"] = value
59+
@data[key.to_s] = value
6060
end
6161

6262
@path.write(@data.to_json)
6363
end
6464
end
6565

6666
# Current load order of the various "language packs"
67-
def self.set_global(path: )
67+
def self.set_global(path:)
6868
JsonReport.new(path: path).tap { |report|
6969
# Silence warning about setting a constant
7070
begin
@@ -82,5 +82,5 @@ def self.dev_null
8282
JsonReport.new(path: "/dev/null")
8383
end
8484

85-
GLOBAL = self.dev_null # Changed via `set_global`
85+
GLOBAL = dev_null # Changed via `set_global`
8686
end

0 commit comments

Comments
 (0)