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

Remove the undefined TimeZone#strftime from Rails/TimeZone #5402

Merged

Conversation

koic
Copy link
Member

@koic koic commented Jan 5, 2018

Follow up of #5381 (comment).

It seems that there is a mistake in solving the Issue below.
#1773

From Rails 4.2 to Rails 5.2 (beta), The following code will result in an error.

> str = '2015-03-02 19:05:37'
> Time.zone.strftime(str, "%Y-%m-%d %H:%M%Z")
NoMethodError: undefined method `strftime' for #<ActiveSupport::TimeZone:0x00007fdaed438c88>

AFAIK, ActiveSupport::TimeZone#strftime does not exist in Rails 4.2 at least.

Previously, #1773 was thought to mistype Time.zone.strftime to Time.zone.strptime. However, I think that it was false positives because ActiveSupport::TimeZone#strptime introduced in Rails 5.0 does not exist in Rails 4.2.
rails/rails@a5e507f

In this PR, ActiveSupport::TimeZone#strftime which does not exist is removed from detection targets.

I'm judging that this change has no effect on codes that works as expected.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

Follow up of rubocop#5381 (comment).

It seems that there is a mistake in solving the Issue below.

rubocop#1773

From Rails 4.2 to Rails 5.2 (beta), The following code will result in an error.

```console
> str = '2015-03-02 19:05:37'
> Time.zone.strftime(str, "%Y-%m-%d %H:%M%Z")
NoMethodError: undefined method `strftime' for #<ActiveSupport::TimeZone:0x00007fdaed438c88>
```

AFIAK, `ActiveSupport::TimeZone#strftime` does not exist in Rails 4.2 at least.

Previously, rubocop#1773 was thought to mistype `Time.zone.strftime` to
`Time.zone.strptime`. However, I think that it was false positives
because `ActiveSupport::TimeZone#strptime` introduced in Rails 5.0
does not exist in Rails 4.2.

rails/rails@a5e507f

In this PR, `ActiveSupport::TimeZone#strftime` which does not exist is
removed from detection targets.

I'm judging that this change has no effect on codes that works as expected.
@koic koic force-pushed the remove_a_dangerous_method_from_rails_time_zone branch from 614a7ad to 860f226 Compare January 5, 2018 15:10
@bbatsov bbatsov merged commit ebaf9db into rubocop:master Jan 5, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented Jan 5, 2018

👍

@koic koic deleted the remove_a_dangerous_method_from_rails_time_zone branch January 5, 2018 16:40
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

Successfully merging this pull request may close these issues.

2 participants