Introduce Ruby linting #1699
Merged
Introduce Ruby linting #1699
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schneems
force-pushed
the
schneems/standardrb
branch
from
22dbe86 to
f736286
Compare
schneems
force-pushed
the
schneems/standardrb
branch
from
f736286 to
5a8b33d
Compare
edmorley
approved these changes
Jan 10, 2026
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.
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.
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.
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.
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.
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 <<.
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.
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.
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.
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.
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).
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.
Lint failure: Layout/IndentationConsistency - Inconsistent indentation detected. Fixed extra indentation on the bundle_command assignment line.
schneems
force-pushed
the
schneems/standardrb
branch
from
c6a37fb to
0192c29
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Standardize on style and catch errors with rubocop (via the
standardrbgem). This caught a logic bug