RuboCop upgrade

[natesholland]

I am trying to get more involved in open source code maintenance. In order to do this I figured I would try upgrading RuboCop for a few gems I use frequently to help them stay up to date. RuboCop on this project was very out of date so I started by upgrading it by ten minor versions to 0.59.2 but I can go further if the maintainers want.

I broke this PR into different commits based on what work I was doing. The first commit is the update and auto-fixes. The commits after that are the commits where I made judgement calls on what to do.

Please let me know if there's anything I can do to help or improve.

---

This change is 

[natesholland]

Thanks for the pointer @mikegee, I've made the recommended changes.

[mikegee]

Awesome!

I didn't comment on spec/support/dummy_server/servlet.rb earlier, but I think the same recommendation applies there too.

[natesholland]

@mikegee addressed.

As a side note, I would like to get #546 merged before mine if possible. I really want that change in place and don't want my RuboCop changes to conflict with it.

[ixti]

Reviewable status: 0 of 24 files reviewed, 4 unresolved discussions (waiting on @ixti and @natesholland)

---

.rubocop.yml, line 65 at r2 (raw file):

Performance/UnfreezeString:
  Enabled: false

What it will take to keep this one enabled?

---

lib/http/client.rb, line 96 at r2 (raw file):

      res
    rescue StandardError

Please change rubocop config to enforce implicit style:

Style/RescueStandardError:
  EnforcedStyle: implicit

---

lib/http/options.rb, line 21 at r2 (raw file):

      def new(options = {})
        return options if options.is_a?(self)

We can make this a ternary:

options.is_a?(self) ? options : super

---

lib/http/mime_type/adapter.rb, line 18 at r2 (raw file):

      %w[encode decode].each do |operation|
        class_eval <<-RUBY, __FILE__, __LINE__ + 1

What cop is this enforced by? I think we should disable it. I find it looking ugly this __LINE__ + 1.

[natesholland]

Reviewable status: 0 of 24 files reviewed, 4 unresolved discussions (waiting on @ixti and @natesholland)

---

.rubocop.yml, line 65 at r2 (raw file):

Previously, ixti (Alexey Zapparov) wrote…

What it will take to keep this one enabled?

The spec for this one is here: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Performance/UnfreezeString

It has a note that states: Note: String.new (without operator) is not exactly the same as +''. These differ in encoding. String.new.encoding is always ASCII-8BIT. However, (+'').encoding is the same as script encoding(e.g. UTF-8). So, if you expect ASCII-8BIT encoding, disable this cop.

Since we explicitly care about encoding in many of these specs enabling this cop causes those specs to break. I don't think it's worth it since most of these are only in the specs and not in lib.

---

lib/http/client.rb, line 96 at r2 (raw file):

Previously, ixti (Alexey Zapparov) wrote…

Please change rubocop config to enforce implicit style:

Style/RescueStandardError:
  EnforcedStyle: implicit

will do

---

lib/http/options.rb, line 21 at r2 (raw file):

Previously, ixti (Alexey Zapparov) wrote…

We can make this a ternary:

options.is_a?(self) ? options : super

yes, will do.

---

lib/http/mime_type/adapter.rb, line 18 at r2 (raw file):

Previously, ixti (Alexey Zapparov) wrote…

What cop is this enforced by? I think we should disable it. I find it looking ugly this __LINE__ + 1.

This comes from this cop: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/EvalWithLocation

The reason that rubocop suggests we do this is that if you add the __LINE__ + 1 then the formatting of the backtrace is more usable and accurate.

If you don't think the benefit of the better backtraces is useful this can easily be changed as it is all wrapped up in commit fb6d66e

[natesholland]

@ixti I am new to the Reviewable tool but I believe I have addressed all of your feedback. If you want me to remove the Style/EvalWithLocation changes in fb6d66e please let me know and I'll do that.

[ixti]

@natesholland Honestly I just wanted to try our reviewable ;))

Re Style/EvalWithLocation - I'm totally fine with the requirement of file/line. I don't like + 1 part only. Although I see where does it comes from, so I guess let's keep it the way you have it now.

[ixti]

As a side note, I would like to get #546 merged before mine if possible. I really want that change in place and don't want my RuboCop changes to conflict with it.

That PR needs more work to be done. I will get to it as soon as I will have time for that. Thus merging this prior that change.

[natesholland]

@ixti if I have time, would you like me to upgrade RuboCop again in the future? The version I bumped to now is much more recent, but still a ways behind the latest version.

[ixti]

@natesholland Sure, feel free to do this. I'm all for keeping rubocop to the latest.