Skip to content

Use RuboCop to clean up poms (9.4)#8974

Merged
headius merged 2 commits intojruby:jruby-9.4from
headius:rubocop_poms_9.4
Aug 25, 2025
Merged

Use RuboCop to clean up poms (9.4)#8974
headius merged 2 commits intojruby:jruby-9.4from
headius:rubocop_poms_9.4

Conversation

@headius
Copy link
Member

@headius headius commented Aug 23, 2025

Clean up all polyglot pom files using automated tools. These changes are highly unlikely to merge across branches, so we'll run this process against 9.4 and 10 separately.

@headius
Copy link
Member Author

headius commented Aug 23, 2025

I have provided .rubocop-pom.yml for future cleanup or auditing. Latest commit disables it in a couple of key places and makes a couple other small tweaks to get mvn deploy -Psnapshots to run (which should touch just about all pom.rb files). Tests are green.

I have only enabled one additional cop: Style/StringHashKeys to clean up hashes and keyword argument calls that still used strings. Some of these strings were not naturally "symbolable" (e.g. "Automatic-Module-Name") but the consistent use of key: value format looks much cleaner.

The other tweaks involved two changes:

  • Modify a [] call to a now-symbol-based Hash to use a symbol key instead of a string key (RuboCop does not catch this).
  • Reference the top-level Gem constant using ::Gem when reopening it to access extension builder logic. I'm not sure why this was necessary since it doesn't seem like any of the RuboCop changes should have broken it. cc @enebo for any thoughts since we've been playing with top-level constant resolution fixes a bit over the last few months.

* core/pom.rb: environmentVariables must be strings or we convert
  the underscore casing to camelCase and they don't work (I'm not
  sure why we do this). Disable StringHashKeys cop to avoid this.
* core/pom.rb: default_compiler_configuration can be symbol keys
  but must be accessed using same.
* lib/pom.rb: Revert to path-based reopening of ext builder class.
  Disable ClassAndModuleChildren cop as it is not safe here. See
  rubocop/rubocop#14476.
* maven/pom.rb: map of submodules must not use symbol keys, because
  we do not autocoerce to String when calling the Maven `profile`
  method. Disable StringHashKeys here as well.
@headius
Copy link
Member Author

headius commented Aug 25, 2025

Reference the top-level Gem constant using ::Gem

This turned out to be an unsafe correction by RuboCop, due to the difference in behavior from path-based module reopening versus nested module reopening. I've filed rubocop/rubocop#14476 and disabled that cop for the line in question (leaving it as a path).

headius added a commit to headius/jruby that referenced this pull request Aug 25, 2025
See jruby#8974 for the original exploration against the 9.4
branch and reasons for enabled and disabled cops and other changes.
@headius headius changed the title Use RuboCop to clean up poms Use RuboCop to clean up poms (9.4) Aug 25, 2025
@headius headius merged commit a179df9 into jruby:jruby-9.4 Aug 25, 2025
193 of 194 checks passed
@headius headius deleted the rubocop_poms_9.4 branch August 25, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant