Add Rails/ToSWithArgument cop

[r7kamura]

to_s(format) is deprecated in favor of to_formatted_s(format) at Rails 7.0.

• Deprecate to_s(format) in favor of to_formatted_s(format) rails/rails#43772

To ease the migration from Rails 6.1 to 7.0, I added Rails/ToSWithArgument cop with autocorrect support in this pull request.

Yes, I know to_fs(format) is the default replacement for to_fs(format) from Rails 7.0,
but to_fs is not implemented at Rails 6.1, and it will be taken care of by Rails/ToFormattedS cop.

• Make #to_fs the default replacement for #to_s(:format) rails/rails#44354

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• 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.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
• If this is a new cop, consider making a corresponding update to the Rails Style Guide.
  • It's not a style issue, it's code that won't work in the new Rails, so I don't think it needs to.

[koic]

Unfortunately, I think the gem cannot accept this cop because the risk of false positives is too high. For example, Rails doesn't support to_formatted_s for Integer.

42.to_s(16) # => "2a"
42.to_formatted_s(16) # => undefined method `to_formatted_s' for 42:Integer (NoMethodError)

I also thought about this cop when implementing #691, but that's why I gave up.

[r7kamura]

hmm, it seems to be present in Rails 7.0 or later, but Rails 6.1 does not.

$ rails c
Loading development environment (Rails 7.0.3.1)
irb(main):001:0> 42.to_formatted_s(16)
=> "2a"

If I specify minimum_target_rails_version 7.0, is that acceptable?

[koic]

If I specify minimum_target_rails_version 7.0, is that acceptable?

Ah, OK 👍 It seems that there is no problem when Rails 7.0 or higher. I leave some review comments.

[koic]

Suggested change

	VersionAdded: '2.16'
	VersionAdded: '<<next>>'

[koic]

Suggested change

	# to_s(:delimited)
	# obj.to_s(:delimited)

[koic]

Suggested change

	# to_formatted_s(:delimited)
	# obj.to_formatted_s(:delimited)

[koic]

That's my two cents. Can you tweak it?

Suggested change

	add_offense(node) do |rewriter|
	rewriter.replace(
	node.loc.selector,
	'to_formatted_s'
	)
	end
	add_offense(node) do |corrector|
	corrector.replace(node.loc.selector, 'to_formatted_s')
	end

[koic]

Seems to show a concrete value.

Suggested change

	^^^^^^^^^^^^^^^^ Use `to_formatted_s(...)` instead of `to_s(...)`.
	^^^^^^^^^^^^^^^^ Use `to_formatted_s(:delimited)` instead of `to_s(:delimited)`.

[r7kamura]

OK, this time I've decided to change it to the same format as Rails/ToFormattedS.

1.to_s(:delimited)
  ^^^^ Use `to_formatted_s` instead.

[koic]

Can you add tests for :rails61?

[koic]

Can you add @safety to explain why this cop is unsafe? e.g. #575

[koic]

This looks good to me. Can you squash your commits into one?

[r7kamura]

Thanks for the thoughtful review!

---

By the way, I feel like if you set up the GitHub repository to always squash merge, we wouldn't have to squash every time on the contributor's side, and it would be easier to review, but don't you do that or something?
I'm not saying you should, but if there's a reason for this, I'd like to know 👀

[koic]

I prefer "Create a merge commit" where commit (hash) is maintained. Development is transient, but maintenance is permanent as long as it lasts.