Add new Rails/I18nLazyLookup cop
#326
Conversation
fatkodima
force-pushed
the
i18n_lazy_lookup-cop
branch
2 times, most recently
from
c0b9a10
to
216afa2
Compare
fatkodima
force-pushed
the
i18n_lazy_lookup-cop
branch
from
216afa2
to
8b84e60
Compare
| "#{path}.#{action_name}.#{key}" | ||
| end | ||
|
|
||
| def controller_path(controller) |
There was a problem hiding this comment.
Why is this only for controllers? How about mailers, models, serializers etc?
There was a problem hiding this comment.
If I remember correctly, "Lazy Lookup" works only inside views and controllers? https://guides.rubyonrails.org/i18n.html#lazy-lookup
There was a problem hiding this comment.
Fair enough. Sadly, the guide confusingly tells "Rails implements a convenient way to look up the locale inside views". - edit they also tell "can also be used in controllers", so it's fine.
Views are unlikely something we can parse with RuboCop, can we?
So it's all good here 👍
There was a problem hiding this comment.
I haven't seen that rubocop has any view checking code in any cops, so do not know.
| expect_no_offenses(<<~RUBY) | ||
| class FooController | ||
| def action | ||
| t ['foo.action.key'] |
There was a problem hiding this comment.
Rails' t helpers delegate to I18n.translate and this is used for bulk translates.
There was a problem hiding this comment.
Tested this and found a couple of notable things:
-
t ['.key']won't work as doesn't Rails allow lazy lookup for bulk lookup. So this example in question is perfectly fine. -
What troubled me is the implementation of
translatein controller code:
> ['.a', '.b'].start_with?('.')
NoMethodError: undefined method `start_with?' for [".a", ".b"]:ArrayIt would work before Rails 7, because it was implemented as:
if key.to_s.first == "."where key is ['.a', '.b'], but I suspect it would fail in Rails 7.
AbstractController::Translationis not included toActionController::API, only toActionController::Base](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/base.rb). It doesn't make the cop unsafe, since it's not us who suggest usingt, it's already there in the code.
There was a problem hiding this comment.
Thanks for investigation 👍
Yes, I think no one actually uses this bulk feature, since, as you mentioned, it doesn't work.
This test actually tests that no offenses are generated for this case. Only when the keys is a string or a symbol.
So, wdyt, should I remove this test case or left it as is?
There was a problem hiding this comment.
It's fine to keep it, just to make sure we don't add to the existing mess 😄
fatkodima
force-pushed
the
i18n_lazy_lookup-cop
branch
from
8b84e60
to
4bfbea5
Compare
|
Updated. |
fatkodima
force-pushed
the
i18n_lazy_lookup-cop
branch
from
4bfbea5
to
7092c04
Compare
fatkodima
force-pushed
the
i18n_lazy_lookup-cop
branch
from
7092c04
to
eed8c42
Compare
|
Updated. |
|
I will merge this new cop after the next bug fix release. Thank you. |
…of multiline Follow up to #326 (comment). For now this repository can be set to `SafeMultiline: false`.
|
Thanks! |
Implementation of this https://rails.rubystyle.guide/#lazy-lookup