FactoryBot/ConsistentParenthesesStyle in disagree with Style/HashSyntax when enforcing omit_parentheses in before blocks
#135
Comments
|
Do I read it correctly that Style/HashSyntax‘s autocorrection is causing syntax errors? Can you open an issue with them? I’m not too familiar with the syntax, it looks like it expects the value for the keyword argument on the next line? I am not convinced that this “shorthand” syntax is for the good, as it makes it less obvious that many keyword arguments are fine, when they usually are a sign of a design that can be improved. To me, I’d say that it’s Style/HashSyntax‘s problem with autocorrecting, and that it shouldn’t even bother RSpec DSL, and would recommend disabling it in spec/.rubocop.yml |
|
Another reason I suggested this rule is disabled by default in #132 - these are just method calls, let rubocops for general method calls handle them. create/build/build_stubbed do not need their own special rules for most projects. And I understand it is a bit controversial, but I love the shorthand syntax, @pirj - but yes, it will not work with a newline after the colon, must be within parens or curly brackets. |
|
Good point. It’s not for me to judge the Ruby syntax. Do you think it will do the job if both cops are smart enough not to remove parentheses when the shorthand hash syntax is used? |
|
Just my two cents... Ideally, this cop would allow parentheses when there the shorthand syntax is used (AND that is allowed by |
|
Exactly my thinking, @thejonroberts 👍 I believe, both this cop and that other one from rubocop will need a check for shorthand syntax. Do you want to send a PR? |
|
I will take a crack at it! But anyone else interested, go for it - This would be my first time in this or any rubocop/linting/AST project's codebase. It will also be in my spare time. So I have no idea how long it will take. |
|
Investigation note: it seems that the hash syntax / hash pinning consideration was taken care of in #86 , but That is, |
When ConsistentParenthesesStyle is :omit_parentheses, we ignore factory calls that have an omitted hash value (ruby 3.1 shorthand syntax). Parentheses are required if the arguments end with the key of the omitted value. (e.g. `create(:user, name:)`) However, if a trait was used, (e.g. `create(:user, :trait, name:)`) omit_hash_value? matcher did not match it, and thus would remove the parentheses, potentially causing a syntax error. This change adds a wildcard to the omit_hash_value matcher to allow any number of symbols at the start of the factory call (before the hash values). see rubocop#135
When ConsistentParenthesesStyle is :omit_parentheses, we ignore factory calls that have an omitted hash value (ruby 3.1 shorthand syntax). Parentheses are required if the arguments end with the key of the omitted value. (e.g. `create(:user, name:)`) However, if a trait was used, (e.g. `create(:user, :trait, name:)`) omit_hash_value? matcher did not match it, and thus would remove the parentheses, potentially causing a syntax error. This change adds a wildcard to the omit_hash_value? matcher to allow any number of symbols/traits at the start of the pattern (before the hash values). see rubocop#135
When ConsistentParenthesesStyle is :omit_parentheses, we ignore factory calls that have an omitted hash value (ruby 3.1 shorthand syntax). Parentheses are required if the arguments end with the key of the omitted value. (e.g. `create(:user, name:)`) However, if a trait was used, (e.g. `create(:user, :trait, name:)`) omit_hash_value? matcher did not match it, and thus would remove the parentheses, potentially causing a syntax error. This change adds a wildcard to the omit_hash_value? matcher to allow any number of symbols/traits at the start of the pattern (before the hash values). see rubocop#135
When ConsistentParenthesesStyle is :omit_parentheses, we ignore factory calls that have an omitted hash value (ruby 3.1 shorthand syntax). Parentheses are required if the arguments end with the key of the omitted value. (e.g. `create(:user, name:)`) However, if a trait was used, (e.g. `create(:user, :trait, name:)`) omit_hash_value? matcher did not match it, and thus would remove the parentheses, potentially causing a syntax error. This change adds a wildcard to the omit_hash_value? matcher to allow any number of symbols/traits at the start of the pattern (before the hash values). see rubocop#135
It's been a while that sometimes I'm getting this. I always have a "workaround" but it's not pretty, but I think that this could be addressed properly.
When you have a before block, and inside there is more than a factory, and within that creates/builds there are hashes with the same name in key/value, there is a crossroad (I don't know if that's a good word). I'll explain:
The code above raises the flag of Style/HashSyntax, so we have to do this:
But the code above does not run because of Ruby sintax error, so we have to do this:
But if you have the omit_parenthesis EnforcedStyle, it raises the flag of FactoryBot/ConsistentParenthesesStyle.
So, there you have it. Sometime when there are other attributes to use, I put the last one that does not have the same name in key/value such as this:
Or even get some more lines in it like this:
But that's very ugly.
So, do you guys think this is a ruby problem, a rubocop problem or a rubocop-factory_bot problem?
The text was updated successfully, but these errors were encountered: