Skip to content

Commit 30ee192

Browse files
authored
Merge pull request #75 from github/kh-remove-counter-code
chore: remove counter-related code.
2 parents 88d250b + 34f632e commit 30ee192

29 files changed

+3
-349
lines changed

README.md

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -73,34 +73,6 @@ linters:
7373
- [GitHub::Accessibility::NoTitleAttribute](./docs/rules/accessibility/no-title-attribute.md)
7474
- [GitHub::Accessibility::SvgHasAccessibleText](./docs/rules/accessibility/svg-has-accessible-text.md)
7575
76-
## Disabling a rule (Deprecated)
77-
78-
_This is a soon-to-be deprecated feature. Do not use. See [migration guide](./docs/counter-migration-guide.md)_
79-
80-
`erblint` does not natively support rule disables. At GitHub, we've implemented these rules in a way to allow rules to be disabled at an offense-level via counters or disabled at a file-level because often times, we want to enable a rule but aren't able to address all offenses at once. We achieve this in one of two ways.
81-
82-
Rules that are marked as `Counter` can be disabled by adding a comment with the offense count that matches the number of offenses within the file like:
83-
84-
```.html.erb
85-
<%# erblint:counter GitHub::Accessibility::LinkHasHrefCounter 1 %>
86-
```
87-
88-
In this comment example, when a new `LinkHasHref` offense has been added, the counter will need to be bumped up to 2. More recent rules use a `Counter` format.
89-
90-
If you are enabling a rule for the first time and your codebase has a lot of offenses, you can use the `-a` command to automatically add these counter comments in the appropriate places.
91-
92-
```
93-
bundle exec erblint app/views app/components -a
94-
```
95-
96-
Rules that are not marked as `Counter` like `NoRedundantImageAlt` are considered to be legacy format. We are in the process of migrating these to counters. These rules can still be disabled at the file-level by adding this comment at the top of the file:
97-
98-
```.html.erb
99-
<%# erblint:disable GitHub::Accessibility::NoRedundantImageAlt %>
100-
```
101-
102-
However, unlike a counter, any subsequent offenses introduced to the file will not raise.
103-
10476
## Testing
10577
10678
```

docs/counter-migration-guide.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Counter migration guide
22

3+
This is relevant for `erblint-github` versions <= 0.3.2.
4+
35
[ERBLint v0.4.0](https://github.com/Shopify/erb-lint/releases/tag/v0.4.0) introduces support for inline lint rule disables.
46

57
Since an inline disable feature is now natively available, it is time to move away from the in-house (hacky) counter system we've used internally over the years and in this library. 🎉

lib/erblint-github/linters/custom_helpers.rb

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,39 +8,6 @@ module Linters
88
module CustomHelpers
99
INTERACTIVE_ELEMENTS = %w[button summary input select textarea a].freeze
1010

11-
def counter_correct?(processed_source)
12-
comment_node = nil
13-
expected_count = 0
14-
rule_name = simple_class_name
15-
offenses_count = @offenses.length
16-
17-
processed_source.parser.ast.descendants(:erb).each do |node|
18-
indicator_node, _, code_node, = *node
19-
indicator = indicator_node&.loc&.source
20-
comment = code_node&.loc&.source&.strip
21-
22-
if indicator == "#" && comment.start_with?("erblint:counter") && comment.match(rule_name)
23-
comment_node = node
24-
expected_count = comment.match(/\s(\d+)\s?$/)[1].to_i
25-
end
26-
end
27-
28-
if offenses_count.zero?
29-
# have to adjust to get `\n` so we delete the whole line
30-
add_offense(processed_source.to_source_range(comment_node.loc.adjust(end_pos: 1)), "Unused erblint:counter comment for #{rule_name}", "") if comment_node
31-
return
32-
end
33-
34-
first_offense = @offenses[0]
35-
36-
if comment_node.nil?
37-
add_offense(processed_source.to_source_range(first_offense.source_range), "#{rule_name}: If you must, add <%# erblint:disable #{rule_name} %> at the end of the offending line to bypass this check. See https://github.com/shopify/erb-lint#disable-rule-at-offense-level", "<%# erblint:disable #{rule_name} %>")
38-
else
39-
clear_offenses
40-
add_offense(processed_source.to_source_range(comment_node.loc), "Incorrect erblint:counter number for #{rule_name}. Expected: #{expected_count}, actual: #{offenses_count}.", "<%# erblint:counter #{rule_name}Counter #{offenses_count} %>") if expected_count != offenses_count
41-
end
42-
end
43-
4411
def generate_offense(klass, processed_source, tag, message = nil, replacement = nil)
4512
message ||= klass::MESSAGE
4613
message += "\nLearn more at https://github.com/github/erblint-github#rules.\n"

lib/erblint-github/linters/github/accessibility/avoid_both_disabled_and_aria_disabled.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@ class AvoidBothDisabledAndAriaDisabled < Linter
1313
ELEMENTS_WITH_NATIVE_DISABLED_ATTRIBUTE_SUPPORT = %w[button fieldset input optgroup option select textarea].freeze
1414
MESSAGE = "[aria-disabled] may be used in place of native HTML [disabled] to allow tab-focus on an otherwise ignored element. Setting both attributes is contradictory."
1515

16-
class ConfigSchema < LinterConfig
17-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
18-
end
19-
self.config_schema = ConfigSchema
20-
2116
def run(processed_source)
2217
tags(processed_source).each do |tag|
2318
next if tag.closing?
@@ -26,10 +21,6 @@ def run(processed_source)
2621

2722
generate_offense(self.class, processed_source, tag)
2823
end
29-
30-
if @config.counter_enabled?
31-
counter_correct?(processed_source)
32-
end
3324
end
3425
end
3526
end

lib/erblint-github/linters/github/accessibility/avoid_generic_link_text.rb

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ class AvoidGenericLinkText < Linter
2222

2323
MESSAGE = "Avoid using generic link text such as #{BANNED_GENERIC_TEXT.join(', ')} which do not make sense in isolation."
2424

25-
class ConfigSchema < LinterConfig
26-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
27-
end
28-
self.config_schema = ConfigSchema
29-
3025
def run(processed_source)
3126
processed_source.ast.children.each_with_index do |node, index|
3227
next unless node.methods.include?(:type) && node.type == :text
@@ -98,9 +93,6 @@ def run(processed_source)
9893
banned_text = nil
9994
end
10095
end
101-
if @config.counter_enabled?
102-
counter_correct?(processed_source)
103-
end
10496
end
10597

10698
private

lib/erblint-github/linters/github/accessibility/disabled_attribute.rb

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,6 @@ class DisabledAttribute < Linter
1313
VALID_DISABLED_TAGS = %w[button input textarea option select fieldset optgroup task-lists].freeze
1414
MESSAGE = "`disabled` is only valid on #{VALID_DISABLED_TAGS.join(', ')}."
1515

16-
class ConfigSchema < LinterConfig
17-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
18-
end
19-
self.config_schema = ConfigSchema
20-
21-
class ConfigSchema < LinterConfig
22-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
23-
end
24-
self.config_schema = ConfigSchema
25-
2616
def run(processed_source)
2717
tags(processed_source).each do |tag|
2818
next if tag.closing?
@@ -31,10 +21,6 @@ def run(processed_source)
3121

3222
generate_offense(self.class, processed_source, tag)
3323
end
34-
35-
if @config.counter_enabled?
36-
counter_correct?(processed_source)
37-
end
3824
end
3925
end
4026
end

lib/erblint-github/linters/github/accessibility/iframe_has_title.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@ class IframeHasTitle < Linter
1313
MESSAGE = "`<iframe>` with meaningful content should have a title attribute that identifies the content."\
1414
" If `<iframe>` has no meaningful content, hide it from assistive technology with `aria-hidden='true'`."\
1515

16-
class ConfigSchema < LinterConfig
17-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
18-
end
19-
self.config_schema = ConfigSchema
20-
2116
def run(processed_source)
2217
tags(processed_source).each do |tag|
2318
next if tag.name != "iframe"
@@ -27,10 +22,6 @@ def run(processed_source)
2722

2823
generate_offense(self.class, processed_source, tag) if title.empty? && !aria_hidden?(tag)
2924
end
30-
31-
if @config.counter_enabled?
32-
counter_correct?(processed_source)
33-
end
3425
end
3526

3627
private

lib/erblint-github/linters/github/accessibility/image_has_alt.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ class ImageHasAlt < Linter
1212

1313
MESSAGE = "<img> should have an alt prop with meaningful text or an empty string for decorative images"
1414

15-
class ConfigSchema < LinterConfig
16-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
17-
end
18-
self.config_schema = ConfigSchema
19-
2015
def run(processed_source)
2116
tags(processed_source).each do |tag|
2217
next if tag.name != "img"
@@ -26,10 +21,6 @@ def run(processed_source)
2621

2722
generate_offense(self.class, processed_source, tag) if alt.empty?
2823
end
29-
30-
if @config.counter_enabled?
31-
counter_correct?(processed_source)
32-
end
3324
end
3425
end
3526
end

lib/erblint-github/linters/github/accessibility/link_has_href.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ class LinkHasHref < Linter
1212

1313
MESSAGE = "Links should go somewhere, you probably want to use a `<button>` instead."
1414

15-
class ConfigSchema < LinterConfig
16-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
17-
end
18-
self.config_schema = ConfigSchema
19-
2015
def run(processed_source)
2116
tags(processed_source).each do |tag|
2217
next if tag.name != "a"
@@ -26,10 +21,6 @@ def run(processed_source)
2621
name = tag.attributes["name"]
2722
generate_offense(self.class, processed_source, tag) if (!name && href.empty?) || href.include?("#")
2823
end
29-
30-
if @config.counter_enabled?
31-
counter_correct?(processed_source)
32-
end
3324
end
3425
end
3526
end

lib/erblint-github/linters/github/accessibility/nested_interactive_elements.rb

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ class NestedInteractiveElements < Linter
1212

1313
MESSAGE = "Nesting interactive elements produces invalid HTML, and ssistive technologies, such as screen readers, might ignore or respond unexpectedly to such nested controls."
1414

15-
class ConfigSchema < LinterConfig
16-
property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
17-
end
18-
self.config_schema = ConfigSchema
1915
def run(processed_source)
2016
last_interactive_element = nil
2117
tags(processed_source).each do |tag|
@@ -34,10 +30,6 @@ def run(processed_source)
3430

3531
last_interactive_element = tag unless tag&.name == "input"
3632
end
37-
38-
if @config.counter_enabled?
39-
counter_correct?(processed_source)
40-
end
4133
end
4234
end
4335
end

0 commit comments

Comments
 (0)