Packaging/RequireHardcodingLib: Warning when absolute path was used

[olleolleolle]

Offenses:

spec/generators/paper_trail/install_generator_spec.rb:5:1: C: Packaging/RequireHardcodingLib: Avoid using require with relative path to lib/. Use require with absolute path instead.
require File.expand_path("../../../lib/generators/paper_trail/install/install_generator", __dir__)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This warning happened.

But, File.expand_path was used, so it is an absolute path.

Perhaps the check can include knowing what File.expand_path does to a path?

[utkarsh2102]

Hi @olleolleolle 👋🏻

So the issue is using such calls from the spec/ to lib/ - which is across components. So when we perform the system wide testing, with lib/ code installed in its usual way, we get a LoadError. See here: https://packaging.rubystyle.guide/#using-require-relative-from-test-to-lib-rationale/

Does this make sense? Should I/we elaborate more? Or should we improve the documentation and make it more verbose?

[deivid-rodriguez]

Hi! What I'm noticing is that this "cop" is not really about require but about any code assuming specific relativeness between spec/ and lib/, isn't it? So this cop could potentially detect usages of File.expand_path("../../../lib/generators/paper_trail/install/install_generator", __dir__) anywhere.

[utkarsh2102]

Hi @deivid-rodriguez,

What I'm noticing is that this "cop" is not really about require but about any code assuming specific relativeness between spec/ and lib/, isn't it?

I am not sure what that means? This cop only checks the require calls, here's its AST: https://github.com/utkarsh2102/rubocop-packaging/blob/9eeb5a872c1ade7cec63a3c4c7279875352deb84/lib/rubocop/cop/packaging/require_hardcoding_lib.rb#L46-L52

[deivid-rodriguez]

Yeah, I know! I'm just saying that it doesn't need to be!

If I have this code in my spec/generators/paper_trail/install_generator_spec.rb file:

foo = File.expand_path("../../../lib/generators/paper_trail/install/install_generator", __dir__)

Then it'd be useful for you debian packagers that a cop alerts me that this is not friendly for you. Right?

[olleolleolle]

I want to take a moment to thank you both, @deivid-rodriguez and @utkarsh2102 for taking my short description describing it so well.

[utkarsh2102]

Yeah, I know! I'm just saying that it doesn't need to be!

Oooh, that's a great suggestion, indeed!

If I have this code in my spec/generators/paper_trail/install_generator_spec.rb file:

foo = File.expand_path("../../../lib/generators/paper_trail/install/install_generator", __dir__)

AFAIR, this is what happened recently, no? The issues I reported with rubygems and it boiled down to such lines of code? Or was it something different?

Then it'd be useful for you debian packagers that a cop alerts me that this is not friendly for you. Right?

Indeed, I think this is a great idea! We need to:

• Tweak the AST.
• Add more tests.
• Change the cop name.

[utkarsh2102]

Hey, @samyak-jn, do you want to give this a shot? (since you wanted to do something here? :P)

[samyak-jn]

Hey @utkarsh2102, Thanks, I am happy to give it a try. 🌮 I have taken pointers from your last comment, will ping you if stuck! :P

[deivid-rodriguez]

AFAIR, this is what happened recently, no? The issues I reported with rubygems and it boiled down to such lines of code? Or was it something different?

Correct, that's exactly how rubygems tests broke in debian last time :+1:.