Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Rails/TimeZone should not report offense on String#to_time with timezone specifier #1367

Merged
merged 1 commit into from
Oct 14, 2024
+10 −0
Merged

Fix Rails/TimeZone should not report offense on String#to_time with timezone specifier #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_rails_time_zone_string_to_time_with_specifier.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1367](https://github.com/rubocop/rubocop-rails/pull/1367): Fix `Rails/TimeZone` should not report offense on `String#to_time` with timezone specifier. ([@armandmgt][])
3 changes: 3 additions & 0 deletions lib/rubocop/cop/rails/time_zone.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ module Rails
# Time.zone.now
# Time.zone.parse('2015-03-02T19:05:37')
# Time.zone.parse('2015-03-02T19:05:37Z') # Respect ISO 8601 format with timezone specifier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this an intentional removal? If so, what was the intention behind it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I wanted to explain the change in the PR body and forgot to do it

This example was added in PR #441 which specifically allows Time.parse when a timezone specifier is present so Time.parse(...) seems like the good syntax in that case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this deleted example (Time.zone.parse('2015-03-02T19:05:37Z')) still seems like a good use case. Can you restore it instead of removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point 👍 Done in 737721c

# Time.parse('2015-03-02T19:05:37Z') # Also respects ISO 8601
# '2015-03-02T19:05:37Z'.to_time # Also respects ISO 8601
#
# @example EnforcedStyle: flexible (default)
# # `flexible` allows usage of `in_time_zone` instead of `zone`.
Expand Down Expand Up @@ -67,6 +69,7 @@ def on_const(node)

def on_send(node)
return if !node.receiver&.str_type? || !node.method?(:to_time)
return if attach_timezone_specifier?(node.receiver)

add_offense(node.loc.selector, message: MSG_STRING_TO_TIME) do |corrector|
corrector.replace(node, "Time.zone.parse(#{node.receiver.source})") unless node.csend_type?
Expand Down
6 changes: 6 additions & 0 deletions spec/rubocop/cop/rails/time_zone_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,12 @@
expect_no_corrections
end

it 'does not register an offense for `to_time` when attaching timezone specifier `Z`' do
expect_no_offenses(<<~RUBY)
"2012-03-02T16:05:37Z".to_time
RUBY
end

it 'does not register an offense for `to_time` without receiver' do
expect_no_offenses(<<~RUBY)
to_time
Expand Down
Loading