Register an offence in the case of the string without zone #3951
Conversation
| def safe_to_time?(node) | ||
| receiver, _method_name, *args = *node | ||
| if receiver.str_type? | ||
| zone_regexp = /[\+-]{1}[\d:]+$/ |
There was a problem hiding this comment.
Matching exactly one is the default behaviour, so you shouldn't need the {1} quantifier. 😀 Also + does not need to be escaped in a list, so the whole thing could be simplified to:
[+-][\d:]+$
(My personal preference is for regular expressions to be stored in constants, but I don't think there's a convention for this in the project.)
There was a problem hiding this comment.
Also you can probably do \z instead of $. $ means end of line. \z means end of string.
irb(main):001:0> "John\nJacob\nJingleheimer\nSchmidt" =~ /^Jacob$/
=> 5
irb(main):002:0> "John\nJacob\nJingleheimer\nSchmidt" =~ /^Jacob\z/
=> nil| zone_regexp = /[\+-]{1}[\d:]+$/ | ||
| receiver.str_content.match(zone_regexp) | ||
| else | ||
| args.length == 1 |
There was a problem hiding this comment.
args.length == 1 and args.one? are not equal.
http://ruby-doc.org/core-1.9.3/Enumerable.html#method-i-one-3F
For example
[1] pry(main)> [].one?
=> false
[2] pry(main)> [1].one?
=> true
[3] pry(main)> [false].one?
=> false
[4] pry(main)> [1, nil, nil].one?
=> true
[5] pry(main)>
There was a problem hiding this comment.
That's even more robust in this case, as we only care about nodes. 😀
|
You should also add a changelog entry. |
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| * [#3915](https://github.com/bbatsov/rubocop/issues/3915): Make configurable whitelist for `Lint/SafeNavigationChain` cop. ([@pocke][]) | |||
| * [#3944](https://github.com/bbatsov/rubocop/issues/3944): Allow keyword arguments in `Style/RaiseArgs` cop. ([@mikegee][]) | |||
| * [#3951](https://github.com/bbatsov/rubocop/pull/3951): Make `Rails/Date` cop to register an offence for the string without zone. ([@sinsoku][]) | |||
There was a problem hiding this comment.
the string -> a string
zone -> timezone
There was a problem hiding this comment.
You forgot to add yourself to the end of this file (that's why the build is failing).
| @@ -49,6 +49,20 @@ | |||
| end | |||
| end | |||
|
|
|||
| context 'when the string literal with zone' do | |||
There was a problem hiding this comment.
a string literal with timezone
| end | ||
| end | ||
|
|
||
| context 'when the string literal without zone' do |
|
I fixed the grammar and CHANGELOG. |
|
👍 |
Using the string without zone is dangerous. It doesn't use Rails time zone, use the system timezone.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).