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.

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

Dates to Unix Timestamp #2860

Closed
ryanwjackson opened this issue Feb 18, 2016 · 5 comments
Closed

Dates to Unix Timestamp #2860

ryanwjackson opened this issue Feb 18, 2016 · 5 comments

Comments

@ryanwjackson
Copy link

I can't find a way to satisfy Rubocop and convert dates to unix timestamps. For time, you can do something like Time.current.to_i. For dates, I have Date.today.to_time.to_i. Rubocop complains about the to_time aspect as Date does not know anything about TimeZones.

Is there a way around this without disabling date checks?

@rrosenblum
Copy link
Contributor

What is your configuration for this cop?

From the documentation for this class

      # This cop checks for the correct use of Date methods,
      # such as Date.today, Date.current etc.
      #   
      # Using Date.today is dangerous, because it doesn't know anything about
      # Rails time zone. You must use Time.zone.today instead.
      #   
      # The cop also reports warnings when you are using 'to_time' method,
      # because it doesn't know about Rails time zone either.
      #   
      # Two styles are supported for this cop. When EnforcedStyle is 'strict'
      # then the Date methods (today, current, yesterday, tomorrow)
      # are prohibited and the usage of both 'to_time'
      # and 'to_time_in_current_zone' is reported as warning.
      #   
      # When EnforcedStyle is 'flexible' then only 'Date.today' is prohibited
      # and only 'to_time' is reported as warning.
      #   
      # @example
      #   # no offense
      #   Time.zone.today
      #   Time.zone.today - 1.day
      #   
      #   # acceptable
      #   Date.current
      #   Date.yesterday
      #   
      #   # always reports offense
      #   Date.today
      #   date.to_time
      #   
      #   # reports offense only when style is 'strict'
      #   date.to_time_in_current_zone

It looks like the cop is wanting you to use Time.zone.now.to_i and Time.zone.today. Based on the config, to_time_in_current_zone will work if not set to strict. I am not sure what the preferred way to convert Date to Time is.

@ryanwjackson
Copy link
Author

So the code it doesn't like is in a conditional:

val = send(sattr)
if val.is_a?(Date)
  val = val.to_time.to_i # This is the line it hates.
elsif val.is_a?(Time) || val.is_a?(DateTime)
  val = val.to_i
end

@rrosenblum
Copy link
Contributor

I need someone else to weigh in on this because I am unsure what the accepted way of converting Date to Time is. The way I interpret the documentation in the cop is, under strict style it is never OK to convert Date to Time. Meaning that all occurrences of Date would need to be refactored out. This would be impossible to do because the value could be coming from a gem. If your configuration is not set to strict, val.to_time_in_current_zone will not register an offense.

@palkan appears to have authored this cop, maybe he can shed some light on the issue.

@palkan
Copy link
Contributor

palkan commented Feb 18, 2016

The issue exists. PR attached.

@rrosenblum
Copy link
Contributor

Thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants