`factory`キーとブロックの同時指定を `method_missing` 経由でも正しくエラーにする
DefinitionProxy#method_missing がアソシエーション定義時にブロックを #association へ転送しておらず、factoryキーとブロックを同時に渡してもエラーが発生しないという問題が修正されました。1文字の追加で既存のバリデーションが正しく機能するようになります。
背景
method_missing を介したアソシエーション定義でブロックが黙って無視される問題が #1503 で報告されていました。たとえば author(factory: :user) { :some_block } のように記述しても、ブロックは捨てられ、エラーも発生しないままアソシエーションが定義されます。ユーザーはブロックが有効だと思い込み、意図しない動作に気づきにくい状況でした。
この問題への対応として #1518 が作成されましたが、クローズされたため、本PR #1579 がその代替として提出されました。
技術的な変更
DefinitionProxy#method_missing の association 呼び出しに &block を追加するだけの1行変更です。
変更前:
association(name, association_options)
変更後:
association(name, association_options, &block)
#association メソッドにはすでにブロック付き呼び出しを検出して AssociationDefinitionError を発生させる実装が存在します。method_missing 側でブロックを転送していなかったため、そのバリデーションがバイパスされていました。転送を追加することで既存のエラーチェックが正しく機能するようになります。
スペックでは以下のシナリオが追加されています:
it "raises an AssociationDefinitionError when called with a `:factory`-key and providing a block" do
definition = FactoryBot::Definition.new(:user)
proxy = FactoryBot::DefinitionProxy.new(definition)
invalid_call = lambda do
proxy.author(factory: :user) { :this_should_raise_an_error }
end
expect(invalid_call).to raise_error(
FactoryBot::AssociationDefinitionError,
"Unexpected block passed to 'author' association in 'user' factory"
)
end
このテストにより、method_missing 経由でも #association 直接呼び出しと同等のバリデーションが行われることが保証されます。
設計判断
ブロックを無視してサイレントに処理を続けるのではなく、エラーを明示的に発生させる方針が選択されています。PR本文にも「このふるまいはユーザーが予期しない可能性があるため、渡されたブロックが無視される旨のエラーを発生させるべき」と明記されています。
また、PR #1518 のコメントでは破壊的変更の可能性も議論されていましたが、#association 側のバリデーション実装はすでに存在しており、本PRは「バリデーションのバイパスを塞ぐ」という位置づけです。新たな制約を加えるのではなく、意図したバリデーションが正しく適用されるよう経路を修正した変更といえます。
まとめ
1文字の追加(&block)によって、method_missing 経由のアソシエーション定義と #association 直接呼び出しの動作が統一されました。factoryキーとブロックを同時に指定した際の意図せぬサイレント無視が解消され、明示的なエラーでユーザーに誤りを通知できるようになります。