Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading Custom Linters is broken #164

Closed
brendo opened this issue May 5, 2020 · 2 comments · Fixed by #178
Closed

Loading Custom Linters is broken #164

brendo opened this issue May 5, 2020 · 2 comments · Fixed by #178

Comments

@brendo
Copy link

brendo commented May 5, 2020

It appears the loading custom linters from CLI doesn't work correctly. Following the README instructions is met with:

$ bundle exec erblint --enable-linters custom_linter --lint-all
custom_linter: not a valid linter name (rubocop, rubocop_text, parser_errors, space_around_erb_tag, space_in_html_tag, extra_newline, right_trim, erb_safety, trailing_whitespace, closing_erb_tag_indent, space_indentation, hard_coded_string, no_javascript_tag_helper, allowed_script_type, self_closing_tag, final_newline, deprecated_classes)

Taking a dive into this, it appears that options are loaded and validated before the custom linters are loaded.

https://github.com/Shopify/erb-lint/blob/90702271d5811cc9a677c69e2ae4f332ca53d248/lib/erb_lint/cli.rb#L35-L37

Here the args are validated against known_linter_names:

https://github.com/Shopify/erb-lint/blob/90702271d5811cc9a677c69e2ae4f332ca53d248/lib/erb_lint/cli.rb#L269-L277

known_linter_names expects that all the linters have already been loaded:

https://github.com/Shopify/erb-lint/blob/90702271d5811cc9a677c69e2ae4f332ca53d248/lib/erb_lint/cli.rb#L213-L217

But in the LinterRegistry we can see that custom linters are only explicitly loaded when load_custom_linters is called:

https://github.com/Shopify/erb-lint/blob/90702271d5811cc9a677c69e2ae4f332ca53d248/lib/erb_lint/linter_registry.rb#L20-L23

load_custom_linters is only called when the Runner is initialised, but that happens some 30 lines after the options have been validated:

https://github.com/Shopify/erb-lint/blob/90702271d5811cc9a677c69e2ae4f332ca53d248/lib/erb_lint/cli.rb#L58

Changing the known_linter_names to be the following appears to fix the issue:

def known_linter_names
  @known_linter_names ||= begin
    ERBLint::LinterRegistry.load_custom_linters
    ERBLint::LinterRegistry.linters
      .map(&:simple_name)
      .map(&:underscore)
  end
end

(Note that Custom Linters still don't work at this point as the README is out of date #123)

@thegedge
Copy link

thegedge commented May 6, 2020

This makes sense to me. We don't have cycles to work on anything, so fire off a PR and add me as a reviewer and I'll help get it in. I should really look over existing PRs too...

@Edouard-chin
Copy link

The fix make sense to me as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants