Skip to content

Enable Style/FrozenStringLiteralComment - #91

Merged
dduugg merged 1 commit into
mainfrom
enable-frozen-string-literal-comment
Sep 26, 2026
Merged

dduugg merged 1 commit into
mainfrom
enable-frozen-string-literal-comment

Conversation

@dduugg

@dduugg dduugg commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Why

The # frozen_string_literal: true pragma turns accidental mutation of a string literal into an immediate FrozenError, so it can't quietly corrupt a shared value. It also saves allocations, because each literal becomes one shared frozen object. Nearly every file here already had the pragma, but the cop was disabled, so nothing enforced it on new files.

Config change

  • .rubocop.yml: removed the override below (plus its comment):
    # Disabling for now until it's clearer why we want this
    Style/FrozenStringLiteralComment:
      Enabled: false
    The cop now runs with the config inherited from rubocop-gusto, which is Enabled: true, EnforcedStyle: always_true. .rubocop_todo.yml had no entry for this cop and is unchanged.

Files that got the pragma (5)

  • packwerk-extensions.gemspec
  • test/fixtures/skeleton/components/timeline/app/models/graphql.rb
  • test/fixtures/skeleton/components/timeline/app/models/graphql/private_thing.rb
  • test/fixtures/skeleton/components/timeline/app/models/private_thing.rb
  • test/fixtures/skeleton/config/environment.rb

Everything under lib/, bin/, and test/ already had the pragma, and so did Rakefile and Gemfile. In the four fixtures the pragma goes after the existing # typed: sigil, matching every other file in the repo. None of those fixtures contains a string literal. No test depends on their line numbers, and none has a pack_public: true sigil that an extra line could push out of the privacy checker's 5-line scan window.

Exclusions: I added none.

  • test/fixtures/minimal/config/environment.rb is an empty file, already excluded from Lint/EmptyFile. The cop skips files with no tokens, so it needs no pragma.
  • The tapioca-generated sorbet/rbi/gems/*.rbi files are not in RuboCop's default AllCops/Include, so RuboCop never inspects them.

Runtime fixes

None were needed. The only shipped file whose runtime behavior changes is the gemspec. It passes its literals to Gem::Specification setters and Gem::Requirement/Gem::Version, and none of those mutate their arguments. The four fixtures contain no string literals.

These values are frozen but safe:

  • The VIOLATION_TYPE constants.
  • Privacy::Package#public_path's 'app/public/' fallback.
  • The permitted_keys elements.
  • The "\n---\n" separator.

All of them were already frozen on main, because lib/ had the pragma. packwerk only reads them: it uses them as hash keys, calls include?/lstrip/join on them, and interpolates them. The << calls in the validators and in packwerk's PackageTodo append to Arrays, not Strings.

Verification

  • Specs: bundle exec rake gave 88 runs, 142 assertions, 0 failures, 0 errors on both main and this branch.
    • Also 88/142/0/0 with RUBYOPT="--enable-frozen-string-literal --debug-frozen-string-literal", which freezes every literal in every loaded file, including gems.
    • Run locally on Ruby 4.0.5, and separately on Ruby 3.3.11 (the minimum supported version).
    • No "literal string will be frozen" warnings under -W:deprecated.
  • RuboCop: bundle exec rubocop inspected 67 files with no offenses.
  • Sorbet: bundle exec srb tc reported no errors.
  • Static sweep: I read every file in lib/, bin/, and test/support, plus the gemspec, Rakefile, and Gemfile, looking for mutating String calls, default-argument buffers, mutated constants, and literals passed to libraries that mutate their arguments. I also checked how packwerk 3.3.1 uses the values these checkers return. I found none.
  • Dynamic checks:
    • I ran packwerk's validate, check, and update-todo against scratch Rails apps, both under --debug-frozen-string-literal and with every literal frozen. The apps exercised every checker message, strict mode for all four checkers, both the layers and architecture_layers configs, cache/parallel mode, and every validator error branch, including the three that no spec covers. No FrozenError.
    • The gemspec loads, validates, and builds with gem build and rake build, and it installs cleanly with its literals frozen.
  • Fresh Eyes: a local pre-push review reported 0 blockers, 0 majors, 0 minors, and 0 info findings.

Remove the `Style/FrozenStringLiteralComment: Enabled: false` override from
.rubocop.yml so the cop runs with rubocop-gusto's `EnforcedStyle: always_true`.
.rubocop_todo.yml has no entry for this cop.

Add `# frozen_string_literal: true` to the five files that lacked it:
packwerk-extensions.gemspec and four test fixtures under
test/fixtures/skeleton. In the fixtures it goes after the existing
`# typed:` sigil, matching every other file in the repo. None of the
fixtures contain string literals, and no test depends on their line
numbers. Everything else in lib/, bin/, test/, Rakefile and Gemfile
already had the pragma.

No mutation fixes were needed. The gemspec only passes its literals to
Gem::Specification setters, which don't mutate them, and it loads and
builds cleanly. The suite passes under --enable-frozen-string-literal,
and the packwerk validate/check/update-todo commands run clean against
a scratch app that triggers every checker and validator.
@dduugg
dduugg requested a review from a team as a code owner September 26, 2026 16:46
@github-project-automation github-project-automation Bot moved this to Triage in Modularity Sep 26, 2026
@dduugg
dduugg merged commit fed784a into main Sep 26, 2026
10 checks passed
@dduugg
dduugg deleted the enable-frozen-string-literal-comment branch September 26, 2026 18:38
@github-project-automation github-project-automation Bot moved this from Triage to Done in Modularity Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant