iframe-proxy

hammadxcm · 2026-03-23T16:12:55Z

Summary

Adds a DisallowedCops configuration option to Style/DisableCopsWithinSourceCodeDirective, the inverse of the existing AllowedCops.
When DisallowedCops is set, only directives for the listed cops (and rubocop:disable all) are flagged — all other cops can be freely disabled.
This is useful when you want to protect a small set of critical cops (e.g. Lint/Void) from being disabled rather than allowlisting hundreds of cops.

Context

This addresses a different use case from #14598. Where #14598 is about preventing Style/DisableCopsWithinSourceCodeDirective itself from being disabled, this feature is about selectively protecting a small set of critical cops without needing to allowlist every other cop. See the discussion in #14598 and the prior PR #14705 for background.

Example configuration

Style/DisableCopsWithinSourceCodeDirective:
  Enabled: true
  DisallowedCops:
    - Lint/Void
    - Security/Eval

With this config, # rubocop:disable Lint/Void is flagged, but # rubocop:disable Metrics/AbcSize is allowed.

Test plan

Added specs for DisallowedCops: single disallowed cop, non-disallowed cop, mixed directives, multiple disallowed cops, rubocop:enable all, and leading source comments
Verified all existing AllowedCops specs still pass (no regressions)
bundle exec rake passes (all specs + self-lint + doc syntax check)

Before submitting the PR make sure the following are checked:

The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

corsonknowles · 2026-04-19T02:04:35Z

hammadxcm · 2026-04-29T19:11:30Z

@bbatsov Please review when you are available. Some people need this one.

github-actions · 2026-05-30T02:21:24Z

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed soon if no further activity occurs. Thank you for your contribution and understanding!

hammadxcm · 2026-05-30T07:38:58Z

still waiting on its merge

bbatsov · 2026-05-30T08:50:27Z

I haven't forgotten about this and I'll take a closer look soon.

corsonknowles · 2026-05-30T18:07:19Z

Thank you so much!

corsonknowles · 2026-06-12T19:04:23Z

Suggest file rename to something like:changelog/new_add_disallowed_cops_option_20260323210952.md

Done — renamed to changelog/new_add_disallowed_cops_option_20260323210952.md.

corsonknowles · 2026-06-12T19:05:20Z

      #
+      # Alternatively, specific cops can be disallowed with the `DisallowedCops`
+      # configuration. When `DisallowedCops` is set, only directives for the listed
+      # cops (and `rubocop:disable all`) will be flagged. This is useful when you want


:enable all is also flagged, so let's shorten this accordiingly

Suggested change

# cops (and `rubocop:disable all`) will be flagged. This is useful when you want

# cops (and `all`) will be flagged. This is useful when you want

corsonknowles · 2026-06-12T19:07:23Z

+      end
+    end
+
+    context 'when enabling all cops' do


I suggest the corresponding disable spec as well, something like

context 'when disabling all cops' do it 'registers an offense and corrects' do expect_offense(<<~RUBY) foo # rubocop:disable all ^^^^^^^^^^^^^^^^^^^^^ RuboCop disable/enable directives for `all` are not permitted. RUBY expect_correction(<<~RUBY) foo#{trailing_whitespace} RUBY end end

Good catch — but the suggested spec actually fails: # rubocop:disable all disables Style/DisableCopsWithinSourceCodeDirective itself on that line, so the offense is suppressed by Cop::Base#enabled_line? before it is registered. This is pre-existing behavior on master (it also applies without DisallowedCops/AllowedCops — easy to verify with --force-default-config on a file containing foo # rubocop:disable all), and it is why the existing "disabling all cops" spec at the top of this file uses rubocop:enable all rather than disable all.

Making disable all flaggable would require bypassing directive suppression for this cop, which feels out of scope for this PR. I added a spec documenting the current behavior instead, with a comment explaining the self-suppression.

Makes a ton of sense, and there's a separate issue about addressing that. Thanks for explaining!

corsonknowles · 2026-06-12T19:08:28Z

+            end
+          else
+            directive_cops - allowed_cops
+          end


This logic is perfectly clear, but in the interests of making future development easier, we can add a test for it like so:

context 'when both AllowedCops and DisallowedCops are set' do let(:cop_config) do { 'AllowedCops' => ['Lint/Void'], 'DisallowedCops' => ['Security/Eval'] } end it 'gives DisallowedCops precedence and ignores AllowedCops' do expect_offense(<<~RUBY) foo # rubocop:disable Lint/Void, Security/Eval ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RuboCop disable/enable directives for `Security/Eval` are not permitted. RUBY expect_correction(<<~RUBY) foo # rubocop:disable Lint/Void RUBY end end

Done — added the precedence spec (with the offense carets adjusted to match the actual range length).

corsonknowles · 2026-06-12T19:11:04Z

@hammadxcm thanks so much for submitting this. I added some very minor comments in hope that they speed along acceptance. All non-blocking nits from my perspective.

…WithinSourceCodeDirective`

hammadxcm · 2026-06-15T19:25:14Z

@hammadxcm thanks so much for submitting this. I added some very minor comments in hope that they speed along acceptance. All non-blocking nits from my perspective.

Its done. @corsonknowles Please review.

corsonknowles · 2026-06-15T19:48:21Z

@hammadxcm thanks so much for submitting this. I added some very minor comments in hope that they speed along acceptance. All non-blocking nits from my perspective.

All looks good to me, and thanks for the explanation on the disable all part. I hope we're in good shape for when @bbatsov gets to take a deeper look

corsonknowles · 2026-07-09T20:30:15Z

Just dropping by to say I continue to be excited about this.

corsonknowles · 2026-07-29T01:37:05Z

In order to help validate this for general adoption, we've ported this PR into our Rails monorepo as a local cop to exercise DisallowedCops at scale

Here are the useful findings:

It works on a large, real set of configs
Our repo has ~1,265 registered cops (RuboCop plus nine plugins and an in-repo plugin) and currently uses AllowedCops: a 271-entry allowlist, so ~1,004 cops may not be disabled inline.
To exercise the new code path in detail, we inverted that allowlist into a 1,004-entry DisallowedCops denylist — The point was that inverting an allowlist works just fine. There were no bugs found, the behavior on runs matches identically.

Across all 6,794 files containing a rubocop:disable|enable|todo directive (~14,000 directive comments), the offense set is identical before and after the switch.

Handling of all, mixed allowed/disallowed autocorrection, and message selection all behaved as the specs describe, with no new edge cases found.

Repeating that he directive_cops.include?('all') guard is a nice catch. It keeps rubocop:disable all flagged in DisallowedCops mode, matching the documented AllowedCops semantics.

For what it's worth, the reason we want this option is exactly the one in your description: an allowlist has to be revisited on every RuboCop or extension upgrade, because newly added cops land outside it and start failing builds. A denylist of genuinely critical cops or already perfectly observed rules makes upgrades a non-event for the config for this Cop, which is very nice for keeping up to date.

The very tiniest of nits:

We had one good finding.
Array cost scales with the size of the denylist or the allowlist. A set is very slightly faster (both are very fast)
But that's separate from the scope of this change, could be its own minor PR.

In summary, "works great, validated at scale"

hammadxcm · 2026-07-29T08:39:19Z

hammadxcm mentioned this pull request Mar 23, 2026

Add DisallowedCops configuration option to Style/DisableCopsWithinSourceCodeDirective #15042

Closed

1 task done

hammadxcm force-pushed the add-disallowed-cops-option branch from 588a5ad to d19e9bf Compare April 19, 2026 06:46

github-actions Bot added the stale Issues that haven't been active in a while label May 30, 2026

github-actions Bot removed the stale Issues that haven't been active in a while label May 30, 2026

corsonknowles reviewed Jun 12, 2026

View reviewed changes

[Fix rubocop#15042] Add DisallowedCops option to `Style/DisableCops…

e5c4ee6

…WithinSourceCodeDirective`

hammadxcm force-pushed the add-disallowed-cops-option branch from d19e9bf to e5c4ee6 Compare June 12, 2026 23:24

bbatsov merged commit 061235d into rubocop:master Jul 29, 2026
23 checks passed

corsonknowles mentioned this pull request Jul 29, 2026

Improve Style/DisableCopsWithinSourceCodeDirective performance with memoized Set lookups #15513

Merged

	# cops (and `rubocop:disable all`) will be flagged. This is useful when you want
	# cops (and `all`) will be flagged. This is useful when you want

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

hammadxcm commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context

Example configuration

Test plan

Uh oh!

corsonknowles commented Apr 19, 2026

Uh oh!

hammadxcm commented Apr 29, 2026

Uh oh!

github-actions Bot commented May 30, 2026

Uh oh!

hammadxcm commented May 30, 2026

Uh oh!

bbatsov commented May 30, 2026

Uh oh!

corsonknowles commented May 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

corsonknowles commented Jun 12, 2026

Uh oh!

hammadxcm commented Jun 15, 2026

Uh oh!

corsonknowles commented Jun 15, 2026

Uh oh!

corsonknowles commented Jul 9, 2026

Uh oh!

corsonknowles commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hammadxcm commented Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hammadxcm commented Mar 23, 2026 •

edited

Loading

corsonknowles commented Jul 29, 2026 •

edited

Loading