Ensure custom linters are loaded at the same time as linters #165
Conversation
jen-taylor-shopify
changed the title
brendo
force-pushed
the
fix-custom-linters
branch
from
bd711e3 to
d14f8b7
Compare
| @@ -51,7 +51,11 @@ def run(_processed_source) | |||
| expect { subject }.to(output(/erblint \[options\] \[file1, file2, ...\]/).to_stderr) | |||
| end | |||
|
|
|||
| it 'shows all known linters in stderr' do | |||
| it 'runs load_custom_linters when showing all known linters in stderr' do | |||
| expect(ERBLint::LinterRegistry).to(receive(:load_custom_linters)) | |||
There was a problem hiding this comment.
This is understandably cheeky, but loading of linters is completely stubbed in this test 🙈
This feels like a quick & dirty way to assert that the loading of custom linters happens even though .linters will always return the stubbed linters.
brendo
changed the title
| @@ -169,6 +169,13 @@ def load_options(args) | |||
| option_parser.parse!(args) | |||
| end | |||
|
|
|||
| def load_linters | |||
| @load_linters ||= begin | |||
| ERBLint::LinterRegistry.load_custom_linters | |||
There was a problem hiding this comment.
Is there any reason why this is not being done at ERBLint::LinterRegistry.linters?
There was a problem hiding this comment.
linters is an attr_reader for an array with no logic at the moment. It's populated when files include the LinterRegistry class.
For linters that come with the project, this kinda all happens magically with autoload, but for linters defined outside of ERB Lint they need to be explicitly loaded by calling load_custom_linters:
I don't have any historical context as to why this is the case.
There was a problem hiding this comment.
It feels more logical have it there. Maybe the attr_reader should be called loaded_linters, and linters should call load_custom_linters before returning the value of the instance variable? That way we wouldn't have to change ERBLint::CLI at all.
There was a problem hiding this comment.
I like that approach! @jen-taylor-shopify and I will pair tomorrow and give it a whirl, thanks!
|
Closing this branch and implementing Etienne's suggestion in this new branch: #178 |
Closes #164
This PR updates the CLI to also load the custom linters when the options are parsed. This prevents
not a valid linter nameerrors when attempting to runerb-lintwith a specific custom linter:A lot of the "why" is detailed in #164, but this seemed like the shortest lift.