Skip to content

Conversation

@khiga8
Copy link
Collaborator

@khiga8 khiga8 commented Oct 29, 2021

This PR:

  • adds missing test coverage for the logic we have to allow linters to be disabled in a file
  • adds missing test coverage for the custom helpers we use.
  • updates the logic that allows linter rules to be a bit more conflict proof.

@file = <<~HTML
<%# erblint:disable GitHub::Accessibility::NoRedundantImageAlt %>
<img alt='image of an octopus'></img>
HTML
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our linters have logic that allows a linter to be disabled/ignored in a file. This comes from: rule_disabled?

Right now, we only check the demodulized name so:

<%# erblint:disable NoRedundantImageAlt %>

and a totally invalid name like:

<%# erblint:disable BadModule::NoRedundantImageAlt %>

can be used to disable a linter rule in a file.

I updated this line, to require the fuller linter name (not including ERBLint::Linters bcuz that is redundant).

Our linter names are namespaced and this seems more conflict proof!

@khiga8 khiga8 marked this pull request as ready for review October 30, 2021 00:09
@khiga8 khiga8 requested a review from joelhawksley October 30, 2021 00:10
Copy link
Collaborator

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be time to add a CHANGELOG 😄

@khiga8 khiga8 merged commit b7a2fe2 into main Nov 2, 2021
@joelhawksley joelhawksley deleted the kh-add_disable_rule_test branch November 2, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants