Rubocop: Prefer `require_relative`

[marcandre]

To load dependencies withing a gem/app, require_relative should always be preferred to require

We should check for any instance of (send nil? require `{:__dir__ :__FILE__}) and raise an offense.

I can't event think of a single instance where require should be called with an interpolated string tbh

[utkarsh2102]

Hi @marcandre 👋🏻

I know this is a bit untraditional (& you might be aware of this already) but there's this cop Packaging/RequireRelativeHardcodingLib which wants people to use require over require_relative only inside tests.
Otherwise, we always want to use require_relative (just not from tests to lib/).

Rationale at https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#require-relative-hardcoding-lib-rationale
This will not only help Debian but a lot of other downstreams, too!

Please let me know what you think?

[tas50]

We've been converting our codebase to use require_relative for some time. It works around some pretty terrible dir walking behavior in rubygems that is particularly slow on Windows where disk IO performance in Ruby is less than ideal. It's been on my todo list for a while to write a rule to do this, but we cranked through it with this script. It also uses if defined?(CLASS) to further avoid rubygems performance issues, so just ignore that bit.

https://gist.github.com/tas50/65682084613887ac7e4a4900cfd1e495

[coding-bunny]

What about ruby's autoload ?

[marcandre]

What about ruby's autoload ?

Matz has not been persuaded yet to add a autoload_relative feature... https://bugs.ruby-lang.org/issues/15330
Or was that not the question?

[coding-bunny]

I didn't mean the autoload_relative feature, simply using autoload for loading stuff relevant to your project and sticking to require purely for gems.

[marcandre]

sticking to require purely for gems

Yes

using autoload for loading stuff relevant to your project

Exclusively? Especially without a autoload_relative it doesn't seem like a good idea.

[coding-bunny]

I don't see why not?
You define what's being loaded as a symbol and provide the path to it.
This is done once, usually in the module defining your gem, or an initializer and everything is done, Ruby loads everything else.

https://medium.com/rubycademy/the-autoload-method-in-ruby-11fd079562ef writes it our cleanly with examples.
It's how I've been handling loading in pretty much every project and avoids all kind of issues with require vs require_relative, checking paths and what not.

[marcandre]

I don't see why not?

• makes loading order less clear
• autoload without an absolute path is slow (like require)
• autoload with an absolute path (e.g. using __dir__) is ugly
• autoloading a feature that is known to be needed immediately is an extra burden that provides no benefit.
• in some cases, features are not referenced directly, only indirectly because they register themselves (like cops in Rubocop).

https://medium.com/rubycademy/the-autoload-method-in-ruby-11fd079562ef writes it our cleanly with examples.

...with bad examples using paths like './b.rb'. If we don't already, we should consider a cop that check for require './anything' or require '../anything', it's basically never what should be done.

Please note that the decision of require vs autoload is orthogonal to require vs require_relative.

[coding-bunny]

I can definitely support the idea that require should not be used for relative paths or interpolated paths. A Cop for that is something nice to have for sure.

[utkarsh2102]

FWIW, there's already a Packaging/RequireHardcodingLib cop which "almost" checks the same thing and autocorrects the offenses.

Why I say almost is because it only checks for those calls which are mapped to the lib/ directory.

That said, I am sure it can be extended to a more generic cop with a few tweaks. I can put together a PR (or take a stab at it) if you'd like so? 😄

[bbatsov]

Yeah, that'd be great.

[terceiro]

FWIW, for downstream distributors, and for other use cases, require_relative from the test files to "[...]/../lib" is a problem because for example you can't run the tests against the code that is installed system-wide (this is an actual use case in Debian for integration testing of the distribution). This what @utkarsh2102 has been working on, starting with GSOC this year, and now continuing beyond the internship.

In general, I think that require_relative is OK within lib/, but I'm a little dubious about the value of saying that everyone should do it. My personal taste is that I prefer requires to be more explicit, because that makes it more clear what exactly they are referring to.

[mvz]

@terceiro what do you mean by 'more explicit'? Can you give an example?

[coding-bunny]

I'd like to see an example as well, as I have a hard time understanding what is being implied.
Especially for "running tests against code that's installed system-wide", I'd like to see a use-case for this actually, unless you are referring to installed gems/packages in Ruby, in which case they would fall under require

[utkarsh2102]

Hi @coding-bunny,

I'd like to see an example as well, as I have a hard time understanding what is being implied.
Especially for "running tests against code that's installed system-wide", I'd like to see a use-case for this actually, unless you are referring to installed gems/packages in Ruby, in which case they would fall under require

Here's the rationale that might suffice: https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#require-relative-hardcoding-lib-rationale. Please feel free to ask if something's not clear!

[coding-bunny]

That doesn't really explain it.
When you use require "foo/bar.rb" (explicitly with the .rb) then for me you are doing something wrong. You're requiring files explicitly to something that should be existing on the $LOAD_PATH which either means you're building something very specific, or you have something messing with the load-path for you such as Bundler or Rubygems, on which you should never count as you don't know what is being used for loading the code.

• Loading specific files should be done with require_relative : e.g require_relative "#{Rails.root.join("lib/foo/bar.rb")
• Loading gems and frameworks should be done with require: e.g require "myTool/myClass"

[mvz]

@coding-bunny the example with .rb is a bit of a red herring. The 'Rationale' section explains the use case you asked for.

[coding-bunny]

I'm totally fine with the rubocop-packaging checking that constraint when you're using that for the rationale written out there, but that shouldn't be the standard in normal RuboCop, as Debian is/should not be the standard go to for this or example.

For normal Ruby projects, require_relative is the correct case IMO to load things like library files or loading your code inside the test-suite without mangling the $LOAD_PATH, which is a responsibility of Bundler/Rubygems, not your project/code.

[deivid-rodriguez]

Just to throw my 2 cents here. I think mostly everyone here agrees that require_relative should be used for requiring "internal code" whereas require should be used for requiring "external code". The problem is where to draw the line of what's internal code and what's external code.

I'd like to make the case that anything outside of the current gem "section" (lib/, test/, gemspec, exe/, and so on) should be considered external code and thus require should be used in that case. One of the reasons for this is to be friendly to OS packagers (not only Debian, but in general), but there is another reason I can think of also due to "gem sections" been potentially "split" upon installation. It's the following: If you're coding a default gem, requiring relatively from exe into lib will not work, because when a default gem is installed, the "folder relativity" between your executable and your lib code is different from the one in the source code repository (lib code of default gems lives in RbConfig::CONFIG["rubylibdir"] whereas executables live in a standard gem structure).

Also, since prettiness has been mentioned, I personally consider things like require_relative "../../lib/foo/bar" pretty ugly.

I think it'd be nice that a require_relative cop would appease the different sensibilities and have this choice of what to consider as "external code" made configurable.

[utkarsh2102]

Hi @coding-bunny,

When you use require "foo/bar.rb" (explicitly with the .rb) then for me you are doing something wrong. You're requiring files explicitly to something that should be existing on the $LOAD_PATH which either means you're building something very specific, or you have something messing with the load-path for you such as Bundler or Rubygems, on which you should never count as you don't know what is being used for loading the code.

* Loading specific files should be done with `require_relative`
    : e.g `require_relative "#{Rails.root.join("lib/foo/bar.rb")`

* Loading gems and frameworks should be done with `require`
    : e.g `require "myTool/myClass"`

Eeeks! That's certainly not what's being implied here. The whole idea of providing that as an example was -
Some people, I've seen, use require and require_relative that way (that is, using .rb). And the point of the Packaging extension is not to flag such things but to just say that "don't use require_relative path from tests to lib/". That's all!
Of course, requiring with .rb is not something someone would want to do but people do and it's not Packaging extension's job to tell them that is wrong or whatever :)
I suppose that's another potential cop for rubocop instead?

But yes, I think it's better I just trim off the .rb right away so it doesn't carry the wrong message forward when it's not even intended to! 😄

[rrosenblum]

It's been a while since I looked into require vs require_relative. There used to be an issue with mixing require and require_relative for paths that lived under a symlink. The issue was that the path to the file could be resolved in more than one way which would cause Ruby to require the same file twice. It looks like this issue has already been fixed so there is nothing to worry about.

This may help to provide some historical context on decisions to use require vs require_relative. https://bugs.ruby-lang.org/issues/13695