[Fix #1071] Fix Rails/FilePath cop to correctly handle File.join with variables and ignore leading and multiple slashes in string literal arguments for Rails.root.join and File.join

[ydakuka]

Fix #1071

---

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• 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.

[koic]

String literals starting with / should also be ignored when the argument is a string literal. Can you update it and add tests?

[koic]

Cases where the string contains two or more // should also be ignored.

rails8-app(dev)> default_url = "default_images/ofn-logo.png"
rails8-app(dev)> Rails.root.join('public//', default_url).to_s
=> "/tmp/example/public/default_images/ofn-logo.png"
rails8-app(dev)> File.join(Rails.root, 'public//', default_url)
=> "/tmp/example/public//default_images/ofn-logo.png"

And, please document that cases potentially containing these / should be ignored.

[ydakuka]

I've done it.

[koic]

Can you refactor to the following?

Suggested change

	def valid_arguments_for_file_join_with_rails_root?(arguments)
	arguments.any? { |e| rails_root_nodes?(e) } &&
	arguments.none?(&:variable?) &&
	arguments.none? { |arg| string_contains_multiple_slashes?(arg) }
	end
	def valid_string_arguments_for_rails_root_join?(arguments)
	arguments.size > 1 &&
	arguments.all?(&:str_type?) &&
	arguments.none? { |arg| string_with_leading_slash?(arg) } &&
	arguments.none? { |arg| string_contains_multiple_slashes?(arg) }
	end
	def valid_slash_separated_path_for_rails_root_join?(arguments)
	arguments.any? { |arg| string_contains_slash?(arg) } &&
	arguments.none? { |arg| string_with_leading_slash?(arg) } &&
	arguments.none? { |arg| string_contains_multiple_slashes?(arg) }
	end
	def valid_arguments_for_file_join_with_rails_root?(arguments)
	return false unless arguments.any? { |arg| rails_root_nodes?(arg) }
	arguments.none? { |arg| arg.variable? || string_contains_multiple_slashes?(arg) }
	end
	def valid_string_arguments_for_rails_root_join?(arguments)
	return false unless arguments.size > 1
	return false unless arguments.all?(&:str_type?)
	arguments.none? { |arg| string_with_leading_slash?(arg) || string_contains_multiple_slashes?(arg) }
	end
	def valid_slash_separated_path_for_rails_root_join?(arguments)
	return false unless arguments.any? { |arg| string_contains_slash?(arg) }
	arguments.none? { |arg| string_with_leading_slash?(arg) || string_contains_multiple_slashes?(arg) }
	end

[ydakuka]

I've applied it.

[koic]

I'm not sure if this cop should be permanently disabled with rubocop:disable Metrics/ClassLength. Can you add Include entry to .rubocop_todo.yml instead?

Metrics/ClassLength:
  Max: 163
  Exclude:
    - 'lib/rubocop/cop/rails/file_path.rb'

[ydakuka]

Okay, I've fixed it, but I had seen the same kind of exception for Rails/RootPathnameMethods.

https://github.com/rubocop/rubocop-rails/blob/master/lib/rubocop/cop/rails/root_pathname_methods.rb#L38

[koic]

That case involves many multi-line arrays, so the context may be slightly different from this one. In any case, the existing directive doesn't require changes for now. Let's leave it as is -)

[koic]

Thanks!