(CAT-1698) Slim .rubocop.yml to the NewCops: enable deep-merge model and validate clean scaffolds

[david22swan]

Summary

This PR replaces the pdk-templates RuboCop template with a slim deep-merge model
and makes a freshly-scaffolded module validate clean under it.

The generated .rubocop.yml now uses AllCops.NewCops: enable to govern which
cops run, instead of an enumerated per-cop Enabled: true/false list. Only
intentionally-styled cops (EnforcedStyle and option tunings) are written out. The
result is a shorter, self-maintaining file that stays current as RuboCop adds new
cops.

On top of the slim model, this PR tunes the genuine PDK / rspec-puppet convention
cops and fixes the object templates so a module scaffolded from these templates
passes pdk validate with zero offences under NewCops: enable — taking the
empirical offence count on a fresh full-object-type scaffold from 44 down to 0,
with CI green end-to-end.

---

What changed

• Slim the model — rewrite config_defaults.yml to a two-profile model
  ('on' = NewCops: enable, 'off' = DisabledByDefault/NewCops: disable,
  with cleanups_only/strict/hardcore as backward-compatible aliases of
  'on'), drop the per-cop enumeration and the cleanup_cops anchor, and add
  cop_overrides as the authoritative per-module override key.
• Remove obsolete machinery — delete rubocop/defaults-1.50.0.yml,
  cleanup_cops.yml, profile_builder.rb, and rubocop/README.md.
• Rewrite the render template — moduleroot/.rubocop.yml.erb now layers
  config via deep_merge (defaults -> profile configs -> cop_overrides), guards
  the selected_profile lookup against invalid/boolean-coerced values, and uses a
  single --- knockout prefix consistent with the docs.
• Tune convention cops — add PDK / rspec-puppet convention tunings to the
  default_configs anchor (trailing-comma style, rspec-puppet describe/context/
  double idioms, etc.), each with an inline Description: rationale.
• Fix the object templates — emit clean, idiomatic Ruby from the object
  templates (squiggly heredocs and aligned hashes in the resource-API type
  templates, idiomatic param name and single-quoted symbol in the v4 function
  template, a single-resource SimpleProvider get with a matching spec, and the
  corrected Naming/ClassAndModuleCamelCase namespace in the transport device
  disable directive), with unchanged runtime behaviour.
• Docs + CI — update README.md and CLAUDE.md for the deep-merge model and
  the convention tunings, and update .ci/test_script.sh (and its fixture) to
  assert the slim model.

---

Verification

Fresh-scaffold gate (PDK 3.7.0, Ruby 3.2.11, Puppet 8.18.0). A module
scaffolded from these templates covering every CI object type (class, defined
type, fact, native + v4 function, provider, task, transport):

• pdk validate — exit 0, 0 RuboCop offences (down from 44), no
  .rubocop_todo.yml generated or relied on.
• pdk test unit — 51 examples, 0 failures.
• the full .ci/test_script.sh (new-module flow + both convert flows) runs
  green end-to-end, with the CI script and fixture left unchanged.

The PR's CI pipeline is green, confirming the above on CI infrastructure.

Migration impact on existing modules was verified against two live puppetlabs
modules:

• puppetlabs-apache (main): pdk validate exit 2 (one pre-existing puppet-lint
  warning, 0 RuboCop offences; apache's up-to-date .rubocop_todo.yml suppresses
  all current findings).
• puppetlabs-motd (main): pdk validate exit 1 (11 RuboCop CONVENTION offences
  surfaced by NewCops: enable across RSpec/DescribeClass,
  RSpec/MultipleExpectations, Style/MixinUsage, and
  Style/TrailingCommaInHashLiteral).

Both modules converted and validated without a hard process failure.

---

Migration / breaking-class change

Three concrete migration points for module maintainers:

1. selected_profile: 'off' mechanism changed

Previously, 'off' emitted an enumerated list of per-cop Enabled: false entries
sourced from rubocop/defaults-1.50.0.yml. That file has been removed.

The new 'off' profile uses RuboCop's native switch:

AllCops:
  DisabledByDefault: true
  NewCops: disable

This achieves the same effect (all cops disabled) through a single idiomatic
mechanism. Modules using selected_profile: 'off' will get this new form on
pdk update — no action required unless you relied on the enumerated list for
non-PDK tooling.

2. NewCops: enable surfaces more offences in modules with older .rubocop_todo.yml files

Switching from the old curated enumeration to NewCops: enable activates cops that
were either not in the previous strict enable-list or not covered by an older
.rubocop_todo.yml. This can surface new CONVENTION-level RuboCop offences on
pdk validate for modules whose .rubocop_todo.yml was generated against an older
RuboCop version (for example, motd's .rubocop_todo.yml from RuboCop 1.48.1 in 2023
did not suppress the 11 offences that appeared in the verification run).

To triage new offences, module maintainers should:

• Set include_todos: true in .sync.yml, then run pdk update to ensure
  .rubocop.yml inherits from .rubocop_todo.yml.
• Regenerate .rubocop_todo.yml using bundle exec rubocop --auto-gen-config or
  pdk validate --auto-correct to baseline the new findings.
• Alternatively, address each offence directly or disable individual cops via
  cop_overrides in .sync.yml.

3. cleanups_only, strict, and hardcore are backward-compatible aliases for 'on'

All three legacy profile names resolve to the same output as selected_profile: 'on'
(i.e. AllCops.NewCops: enable with the base default_configs EnforcedStyle tunings).
Existing .sync.yml files that set any of these names continue to work with zero
migration required.

---

Durable reference

See the updated README.md "Manage Rubocop rules" section and the .rubocop.yml
config-key table for the authoritative key reference (selected_profile, profiles,
cop_overrides, include_todos, TargetRubyVersion, default_configs) and usage
examples under the new model. CLAUDE.md documents the deep-merge model, the
convention tunings (and their inline Description: rationale), and the ---
knockout prefix.

---

Out of scope

Release cut, version bump, and tag order are not part of this PR.

🤖 Generated with Claude Code

[gavindidrichsen]

/pedev:pr-review surfaced five implementation edge cases worth addressing before or shortly after merge. Comments below are on specific lines.

[gavindidrichsen]

cop_overrides type guard is incomplete — should confirm it is a Hash.

The guard @configs['cop_overrides'] && !@configs['cop_overrides'].empty? checks presence but not type. A misconfigured .sync.yml like cop_overrides: "not a hash" passes the guard and causes DeepMerge::deep_merge! to raise a cryptic library error with no hint that the problem is in the module's .sync.yml.

Suggestion:

if @configs['cop_overrides'].is_a?(Hash) && !@configs['cop_overrides'].empty?

Or raise explicitly before the guard:

raise "cop_overrides must be a Hash, got #{@configs['cop_overrides'].class}" unless @configs['cop_overrides'].nil? || @configs['cop_overrides'].is_a?(Hash)

[gavindidrichsen]

profile['configs'] is checked for truthiness but not confirmed to be a Hash.

The raise guard unless profile.is_a?(Hash) && profile['configs'] checks that profile['configs'] is truthy, but not that it is itself a Hash. A custom profile with configs: "not a hash" would pass this guard and cause DeepMerge::deep_merge! to fail opaquely on the next line.

Suggestion:

raise "..." unless profile.is_a?(Hash) && profile['configs'].is_a?(Hash)

[gavindidrichsen]

defaults.dup produces a shallow copy — nested hashes are shared.

Hash#dup copies the top-level hash but not nested objects. After dup, configs['AllCops'] and defaults['AllCops'] point to the same object. DeepMerge::deep_merge! mutates the nested hash in place, so defaults is also mutated alongside configs. No current bug (the template renders once per Ruby process invocation), but this is a latent hazard if the template is ever invoked more than once in one process.

Suggestion: use Marshal.load(Marshal.dump(defaults)) for a deep copy, or add a comment documenting the single-render assumption so future maintainers know why dup is safe here.

[gavindidrichsen]

Pre-update CI assertion is weaker than it looks.

grep -A 5 "AllCops" ./.rubocop.yml | grep -q "NewCops" would pass even if the profile deep-merge were completely broken, because NewCops is hardcoded in the defaults hash unconditionally — it appears in the rendered file regardless of whether profile['configs'] contributed anything.

The old assertion (grep -A 1 "Performance/CaseWhenSplat" | grep -q "true") at least verified that the profile merge contributed a specific cop setting. Consider adding an assertion that checks a tuning from default_configs actually appears — e.g. checking that Style/TrailingCommaInHashLiteral or RSpec/DescribeClass appears in the rendered file with the expected value.

[gavindidrichsen]

RSpec/MessageSpies EnforcedStyle: receive was silently dropped when strict was collapsed into an alias for on.

The old strict profile explicitly set:

RSpec/MessageSpies:
  EnforcedStyle: receive

The new default_configs block scopes RSpec/MessageSpies to an Exclude list for provider/transport spec paths but does not carry forward the EnforcedStyle: receive setting for all other spec files. RuboCop's default for this cop is have_received (spy style), which differs from the receive (expectation style) that modules were previously validated against under strict.

Existing modules using expectation-style doubles outside the excluded paths may start failing pdk validate after updating. Worth verifying against a real module before merge, and documenting the behaviour change in the PR description if the drop is intentional.