Skip to content

Make tagged_with take keywords and reject unknown options - #80

Merged
igor-alexandrov merged 1 commit into
masterfrom
indep/tagged-with-options
Aug 18, 2026
Merged

Make tagged_with take keywords and reject unknown options#80
igor-alexandrov merged 1 commit into
masterfrom
indep/tagged-with-options

Conversation

@igor-alexandrov

@igor-alexandrov igor-alexandrov commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Independent PR off master (#66 has landed, so the diff is just this change). Mergeable in any order relative to its siblings #75#81.

The scope declared no keyword parameters, so every option arrived in a catch-all hash and a typo was silently swallowed:

Post.tagged_with(%w[ruby], anyy: true)   # ran as if nothing was passed

It now validates against the four options it accepts:

ArgumentError: Unknown tagged_with options [:anyy], expected [:any, :exclude, :join_operator, :on]

That also removes three lines of hand-rolled defaulting and the options.delete(:on) that made caller-hash mutation possible.

Backwards compatible

Ruby 3 does not convert a positional hash into keywords, so pure keywords would have broken any caller building an options hash:

options = { any: true }
Post.tagged_with(%w[ruby], options)   # would raise under pure keywords

The scope takes that hash explicitly and merges it, so both call styles work — and typos raise whichever style they arrive by. Tested, including that the caller's hash comes back unmutated.

What I did not fix, and why

The return if base.respond_to?(:tagged_with) guard silently skips defining the scope when the host model already has one, while the four generated column scopes call tagged_with unconditionally — so a host with an incompatible implementation gets four quietly broken scopes.

Both obvious fixes are worse than the problem: raising leaves a model that defines its own tagged_with unable to use the gem at all; dropping the guard silently overwrites the host's method. That is a judgement call about the gem's contract, so it belongs to the maintainers rather than being slipped in here.

103 runs, 247 assertions, 0 failures, 0 errors, 0 skips

🤖 Generated with Claude Code

The scope declared no keyword parameters, so every option arrived in a
catch-all hash and a typo was silently swallowed:

    Post.tagged_with(%w[ruby], anyy: true)   # ran as if nothing was passed

It now validates against the four options it accepts and raises
ArgumentError naming the unknown key and the expected ones. It also drops
three lines of hand-rolled defaulting and the options.delete(:on) that
made the caller-hash mutation possible in the first place.

Backwards compatible. Ruby 3 will not convert a positional hash into
keywords, so `tagged_with(tags, options)` would have broken; the scope
takes that hash explicitly and merges it. Both call styles are tested,
including that the caller's hash comes back unmutated.

Deliberately not addressed: the `return if base.respond_to?(:tagged_with)`
guard. It silently skips defining the scope when the host model already
has one, while the four generated column scopes call tagged_with
unconditionally — so a host with an incompatible implementation gets four
quietly broken scopes. Raising there would leave a model that defines its
own tagged_with no way to use the gem at all, so the fix is a judgement
call for the maintainers rather than something to slip into this commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@igor-alexandrov
igor-alexandrov force-pushed the indep/tagged-with-options branch from cbfeade to deb2386 Compare August 18, 2026 13:13
@igor-alexandrov
igor-alexandrov merged commit afffb37 into master Aug 18, 2026
6 checks passed
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