From c98f26526b61f24291f9e937f3ed0dfbd8250d9f Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Fri, 17 Dec 2021 10:02:54 -0500 Subject: [PATCH] Add support for new comment syntax Fixes #179 --- README.md | 15 +- docs/checks/asset_size_javascript.md | 4 +- docs/checks/missing_enable_comment.md | 6 +- docs/checks/nested_snippet.md | 16 +- docs/checks/translation_key_exists.md | 8 +- docs/checks/valid_html_translation.md | 2 +- .../checks/missing_enable_comment.rb | 4 + lib/theme_check/disabled_checks.rb | 7 +- lib/theme_check/liquid_node.rb | 5 + lib/theme_check/liquid_visitor.rb | 2 +- test/checks/missing_enable_comment_test.rb | 143 ++++++---- test/disabled_checks_test.rb | 265 ++++++++++-------- test/liquid_node_test.rb | 15 + theme-check.gemspec | 2 +- 14 files changed, 287 insertions(+), 207 deletions(-) diff --git a/README.md b/README.md index 637df125..166b779f 100644 --- a/README.md +++ b/README.md @@ -115,32 +115,31 @@ See [config/default.yml](config/default.yml) for available options & defaults. Use Liquid comments to disable and re-enable all checks for a section of your template: ```liquid -{% comment %}theme-check-disable{% endcomment %} +{% # theme-check-disable %} {% assign x = 1 %} -{% comment %}theme-check-enable{% endcomment %} +{% # theme-check-enable %} ``` Disable a specific check by including it in the comment: ```liquid -{% comment %}theme-check-disable UnusedAssign{% endcomment %} +{% # theme-check-disable UnusedAssign %} {% assign x = 1 %} -{% comment %}theme-check-enable UnusedAssign{% endcomment %} +{% # theme-check-enable UnusedAssign %} ``` Disable multiple checks by including them as a comma-separated list: ```liquid -{% comment %}theme-check-disable UnusedAssign,SpaceInsideBraces{% endcomment %} +{% # theme-check-disable UnusedAssign,SpaceInsideBraces %} {%assign x = 1%} -{% comment %}theme-check-enable UnusedAssign,SpaceInsideBraces{% endcomment %} +{% # theme-check-enable UnusedAssign,SpaceInsideBraces %} ``` Disable checks for the _entire document_ by placing the comment on the first line: ```liquid -{% comment %}theme-check-disable SpaceInsideBraces{% endcomment %} - +{% # theme-check-disable SpaceInsideBraces %} {%assign x = 1%} ``` diff --git a/docs/checks/asset_size_javascript.md b/docs/checks/asset_size_javascript.md index 7ef075e7..ffcee239 100644 --- a/docs/checks/asset_size_javascript.md +++ b/docs/checks/asset_size_javascript.md @@ -57,9 +57,9 @@ This includes theme and remote scripts. When you can't do anything about it, it is preferable to disable this rule using the comment syntax: ``` -{% comment %}theme-check-disable AssetSizeJavaScript{% endcomment %} +{% # theme-check-disable AssetSizeJavaScript %} -{% comment %}theme-check-enable AssetSizeJavaScript{% endcomment %} +{% # theme-check-enable AssetSizeJavaScript %} ``` This makes disabling the rule an explicit affair and shows that the code is smelly. diff --git a/docs/checks/missing_enable_comment.md b/docs/checks/missing_enable_comment.md index ac65969b..4e393c69 100644 --- a/docs/checks/missing_enable_comment.md +++ b/docs/checks/missing_enable_comment.md @@ -12,7 +12,7 @@ This check aims at eliminating missing `theme-check-enable` comments. - {% comment %}theme-check-disable ParserBlockingJavaScript{% endcomment %} + {% # theme-check-disable ParserBlockingJavaScript %} @@ -27,9 +27,9 @@ This check aims at eliminating missing `theme-check-enable` comments. - {% comment %}theme-check-disable ParserBlockingJavaScript{% endcomment %} + {% # theme-check-disable ParserBlockingJavaScript %} - {% comment %}theme-check-enable ParserBlockingJavaScript{% endcomment %} + {% # theme-check-enable ParserBlockingJavaScript %} diff --git a/docs/checks/nested_snippet.md b/docs/checks/nested_snippet.md index 405dfe6d..50cbf5f8 100644 --- a/docs/checks/nested_snippet.md +++ b/docs/checks/nested_snippet.md @@ -9,32 +9,32 @@ This check is aimed at eliminating excessive nesting of snippets. :-1: Examples of **incorrect** code for this check: ```liquid -{% comment %}templates/index.liquid{% endcomment %} +{% # templates/index.liquid %} {% render 'one' %} -{% comment %}snippets/one.liquid{% endcomment %} +{% # snippets/one.liquid %} {% render 'two' %} -{% comment %}snippets/two.liquid{% endcomment %} +{% # snippets/two.liquid %} {% render 'three' %} -{% comment %}snippets/three.liquid{% endcomment %} +{% # snippets/three.liquid %} {% render 'four' %} -{% comment %}snippets/four.liquid{% endcomment %} +{% # snippets/four.liquid %} ok ``` :+1: Examples of **correct** code for this check: ```liquid -{% comment %}templates/index.liquid{% endcomment %} +{% # templates/index.liquid %} {% render 'one' %} -{% comment %}snippets/one.liquid{% endcomment %} +{% # snippets/one.liquid %} {% render 'two' %} -{% comment %}snippets/two.liquid{% endcomment %} +{% # snippets/two.liquid %} ok ``` diff --git a/docs/checks/translation_key_exists.md b/docs/checks/translation_key_exists.md index 5e46ef6f..813dd540 100644 --- a/docs/checks/translation_key_exists.md +++ b/docs/checks/translation_key_exists.md @@ -9,7 +9,7 @@ This check is aimed at eliminating the use of translations that do not exist. :-1: Examples of **incorrect** code for this check: ```liquid -{% comment %}locales/en.default.json{% endcomment %} +{% # locales/en.default.json %} { "greetings": "Hello, world!", "general": { @@ -17,14 +17,14 @@ This check is aimed at eliminating the use of translations that do not exist. } } -{% comment %}templates/index.liquid{% endcomment %} +{% # templates/index.liquid %} {{ "notfound" | t }} ``` :+1: Examples of **correct** code for this check: ```liquid -{% comment %}locales/en.default.json{% endcomment %} +{% # locales/en.default.json %} { "greetings": "Hello, world!", "general": { @@ -32,7 +32,7 @@ This check is aimed at eliminating the use of translations that do not exist. } } -{% comment %}templates/index.liquid{% endcomment %} +{% # templates/index.liquid %} {{ "greetings" | t }} {{ "general.close" | t }} ``` diff --git a/docs/checks/valid_html_translation.md b/docs/checks/valid_html_translation.md index 756e480b..5b56c131 100644 --- a/docs/checks/valid_html_translation.md +++ b/docs/checks/valid_html_translation.md @@ -18,7 +18,7 @@ This check is aimed at eliminating invalid HTML in translations. :+1: Examples of **correct** code for this check: ```liquid -{% comment %}locales/en.default.json{% endcomment %} +{% # locales/en.default.json %} { "hello_html": "

Hello, world

", "image_html": "", diff --git a/lib/theme_check/checks/missing_enable_comment.rb b/lib/theme_check/checks/missing_enable_comment.rb index a559a42c..50588728 100644 --- a/lib/theme_check/checks/missing_enable_comment.rb +++ b/lib/theme_check/checks/missing_enable_comment.rb @@ -16,6 +16,10 @@ def on_comment(node) @disabled_checks.update(node) end + def on_inline_comment(node) + @disabled_checks.update(node) + end + def after_document(node) checks_missing_end_index = @disabled_checks.checks_missing_end_index return if checks_missing_end_index.empty? diff --git a/lib/theme_check/disabled_checks.rb b/lib/theme_check/disabled_checks.rb index 0dfd4a1c..0cabea26 100644 --- a/lib/theme_check/disabled_checks.rb +++ b/lib/theme_check/disabled_checks.rb @@ -67,7 +67,12 @@ def remove_disabled_offenses(checks) private def comment_text(node) - node.value.nodelist.join + case node.type_name + when :comment + node.value.nodelist.join + when :inline_comment + node.markup.sub(/\s*#+\s*/, '') + end end def start_disabling?(text) diff --git a/lib/theme_check/liquid_node.rb b/lib/theme_check/liquid_node.rb index 40a45eee..d0dd4712 100644 --- a/lib/theme_check/liquid_node.rb +++ b/lib/theme_check/liquid_node.rb @@ -158,6 +158,11 @@ def comment? @value.is_a?(Liquid::Comment) end + # {% # comment %} + def inline_comment? + @value.is_a?(Liquid::InlineComment) + end + # Top level node of every liquid_file. def document? @value.is_a?(Liquid::Document) diff --git a/lib/theme_check/liquid_visitor.rb b/lib/theme_check/liquid_visitor.rb index e7fafa6d..e158e315 100644 --- a/lib/theme_check/liquid_visitor.rb +++ b/lib/theme_check/liquid_visitor.rb @@ -28,7 +28,7 @@ def visit(node) call_checks(:after_node, node) end - @disabled_checks.update(node) if node.comment? + @disabled_checks.update(node) if node.comment? || node.inline_comment? end def call_checks(method, *args) diff --git a/test/checks/missing_enable_comment_test.rb b/test/checks/missing_enable_comment_test.rb index 8e4da08c..eecc36c3 100644 --- a/test/checks/missing_enable_comment_test.rb +++ b/test/checks/missing_enable_comment_test.rb @@ -3,91 +3,110 @@ require "minitest/focus" class MissingEnableCommentTest < Minitest::Test + def comment_types + [ + -> (text) { "{% comment %}#{text}{% endcomment %}" }, + -> (text) { "{% # #{text} %}" }, + ] + end + def test_always_enabled_by_default refute(ThemeCheck::MissingEnableComment.new.can_disable?) end def test_no_default_noops - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - "templates/index.liquid" => <<~END, - {% comment %}theme-check-disable{% endcomment %} - {% assign x = 1 %} - {% comment %}theme-check-enable{% endcomment %} - END - ) - assert_offenses("", offenses) + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + "templates/index.liquid" => <<~END, + #{comment.call('theme-check-disable')} + {% assign x = 1 %} + #{comment.call('theme-check-enable')} + END + ) + assert_offenses("", offenses) + end end def test_first_line_comment_disables_entire_file - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - "templates/index.liquid" => <<~END, - {% comment %}theme-check-disable{% endcomment %} - {% assign x = 1 %} - END - ) - assert_offenses("", offenses) + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + "templates/index.liquid" => <<~END, + #{comment.call('theme-check-disable')} + {% assign x = 1 %} + END + ) + assert_offenses("", offenses) + end end def test_non_first_line_comment_triggers_offense - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - "templates/index.liquid" => <<~END, -

Hello, world

- {% comment %}theme-check-disable{% endcomment %} - {% assign x = 1 %} + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + "templates/index.liquid" => <<~END, +

Hello, world

+ #{comment.call('theme-check-disable')} + {% assign x = 1 %} + END + ) + assert_offenses(<<~END, offenses) + All checks were disabled but not re-enabled with theme-check-enable at templates/index.liquid END - ) - assert_offenses(<<~END, offenses) - All checks were disabled but not re-enabled with theme-check-enable at templates/index.liquid - END + end end def test_specific_checks_disabled - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - Minitest::Test::TracerCheck.new, - "templates/index.liquid" => <<~END, -

Hello, world

- {% comment %}theme-check-disable TracerCheck{% endcomment %} - {% assign x = 1 %} + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + Minitest::Test::TracerCheck.new, + "templates/index.liquid" => <<~END, +

Hello, world

+ #{comment.call('theme-check-disable TracerCheck')} + {% assign x = 1 %} + END + ) + assert_offenses(<<~END, offenses) + TracerCheck was disabled but not re-enabled with theme-check-enable at templates/index.liquid END - ) - assert_offenses(<<~END, offenses) - TracerCheck was disabled but not re-enabled with theme-check-enable at templates/index.liquid - END + end end def test_specific_checks_disabled_and_reenabled - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - Minitest::Test::TracerCheck.new, - "templates/index.liquid" => <<~END, -

Hello, world

- {% comment %}theme-check-disable TracerCheck, AnotherCheck{% endcomment %} - {% assign x = 1 %} + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + Minitest::Test::TracerCheck.new, + "templates/index.liquid" => <<~END, +

Hello, world

+ #{comment.call('theme-check-disable TracerCheck, AnotherCheck')} + {% assign x = 1 %} + END + ) + assert_offenses(<<~END, offenses) + TracerCheck, AnotherCheck were disabled but not re-enabled with theme-check-enable at templates/index.liquid END - ) - assert_offenses(<<~END, offenses) - TracerCheck, AnotherCheck were disabled but not re-enabled with theme-check-enable at templates/index.liquid - END + end end def test_specific_checks_disabled_and_reenabled_with_all_checks_disabled - offenses = analyze_theme( - ThemeCheck::MissingEnableComment.new, - Minitest::Test::TracerCheck.new, - "templates/index.liquid" => <<~END, -

Hello, world

- {% comment %}theme-check-disable TracerCheck, AnotherCheck{% endcomment %} - {% assign x = 1 %} - {% comment %}theme-check-disable{% endcomment %} - {% assign x = 1 %} + comment_types.each do |comment| + offenses = analyze_theme( + ThemeCheck::MissingEnableComment.new, + Minitest::Test::TracerCheck.new, + "templates/index.liquid" => <<~END, +

Hello, world

+ #{comment.call('theme-check-disable TracerCheck, AnotherCheck')} + {% assign x = 1 %} + #{comment.call('theme-check-disable')} + {% assign x = 1 %} + END + ) + assert_offenses(<<~END, offenses) + All checks were disabled but not re-enabled with theme-check-enable at templates/index.liquid END - ) - assert_offenses(<<~END, offenses) - All checks were disabled but not re-enabled with theme-check-enable at templates/index.liquid - END + end end end diff --git a/test/disabled_checks_test.rb b/test/disabled_checks_test.rb index d3a675ad..515a986d 100644 --- a/test/disabled_checks_test.rb +++ b/test/disabled_checks_test.rb @@ -53,18 +53,35 @@ def setup @visitor = LiquidVisitor.new(@checks, @disabled_checks) end + def block_comment(text) + "{% comment %}#{text}{% endcomment %}" + end + + def inline_comment(text) + "{% # #{text} %}" + end + + def comment_types + [ + -> (text) { block_comment(text) }, + -> (text) { inline_comment(text) }, + ] + end + def test_ignore_all_checks - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable{% endcomment %} - {% assign x = 'x' %} - RegexError 1 - {% comment %}theme-check-enable{% endcomment %} - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable')} + {% assign x = 'x' %} + RegexError 1 + #{comment.call('theme-check-enable')} + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) - assert_empty(@assign_check.offenses) - assert_empty(@regex_check.offenses) + assert_empty(@assign_check.offenses) + assert_empty(@regex_check.offenses) + end end def test_ignore_all_checks_issue_583 @@ -86,137 +103,153 @@ def test_ignore_all_checks_issue_583 end def test_ignore_all_checks_without_end - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable{% endcomment %} - {% assign x = 'x' %} - RegexError 1 - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable')} + {% assign x = 'x' %} + RegexError 1 + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) - assert_empty(@assign_check.offenses) - assert_empty(@regex_check.offenses) + assert_empty(@assign_check.offenses) + assert_empty(@regex_check.offenses) + end end def test_ignore_all_checks_between_bounds - liquid_file = parse_liquid(<<~END) - {% assign x = 'x' %} - RegexError 1 - {% comment %}theme-check-disable{% endcomment %} - {% assign y = 'y' %} - RegexError 2 - {% comment %}theme-check-enable{% endcomment %} - {% assign z = 'z' %} - RegexError 3 - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) - - assert_includes(@assign_check.offenses.map(&:markup), "assign x = 'x' ") - refute_includes(@assign_check.offenses.map(&:markup), "assign y = 'y' ") - assert_includes(@assign_check.offenses.map(&:markup), "assign z = 'z' ") - assert_includes(@regex_check.offenses.map(&:markup), "RegexError 1") - refute_includes(@regex_check.offenses.map(&:markup), "RegexError 2") - assert_includes(@regex_check.offenses.map(&:markup), "RegexError 3") + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + {% assign x = 'x' %} + RegexError 1 + #{comment.call('theme-check-disable')} + {% assign y = 'y' %} + RegexError 2 + #{comment.call('theme-check-enable')} + {% assign z = 'z' %} + RegexError 3 + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) + + assert_includes(@assign_check.offenses.map(&:markup), "assign x = 'x' ") + refute_includes(@assign_check.offenses.map(&:markup), "assign y = 'y' ") + assert_includes(@assign_check.offenses.map(&:markup), "assign z = 'z' ") + assert_includes(@regex_check.offenses.map(&:markup), "RegexError 1") + refute_includes(@regex_check.offenses.map(&:markup), "RegexError 2") + assert_includes(@regex_check.offenses.map(&:markup), "RegexError 3") + end end def test_ignore_specific_checks - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable AssignCheck{% endcomment %} - {% assign x = 'x' %} - RegexError 1 - {% comment %}theme-check-enable AssignCheck{% endcomment %} - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable AssignCheck')} + {% assign x = 'x' %} + RegexError 1 + #{comment.call('theme-check-enable AssignCheck')} + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) - assert_empty(@assign_check.offenses) - refute_empty(@regex_check.offenses) + assert_empty(@assign_check.offenses) + refute_empty(@regex_check.offenses) + end end def test_ignore_multiple_checks - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable AssignCheck, RegexCheck{% endcomment %} - {% assign x = 'x' %} - RegexError 1 - {% comment %}theme-check-enable AssignCheck, RegexCheck{% endcomment %} - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable AssignCheck, RegexCheck')} + {% assign x = 'x' %} + RegexError 1 + #{comment.call('theme-check-enable AssignCheck, RegexCheck')} + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) - assert_empty(@assign_check.offenses) - assert_empty(@regex_check.offenses) + assert_empty(@assign_check.offenses) + assert_empty(@regex_check.offenses) + end end def test_enable_specific_checks_individually - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable AssignCheck, RegexCheck{% endcomment %} - {% assign x = 'x' %} - RegexError 1 - {% comment %}theme-check-enable AssignCheck{% endcomment %} - {% assign y = 'y' %} - RegexError 2 - {% comment %}theme-check-enable RegexCheck{% endcomment %} - {% assign z = 'z' %} - RegexError 3 - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) - - refute_empty(@assign_check.offenses) - refute_includes(@assign_check.offenses.map(&:markup), "assign x = 'x' ") - assert_includes(@assign_check.offenses.map(&:markup), "assign y = 'y' ") - assert_includes(@assign_check.offenses.map(&:markup), "assign z = 'z' ") - - refute_empty(@regex_check.offenses) - refute_includes(@regex_check.offenses.map(&:markup), "RegexError 1") - refute_includes(@regex_check.offenses.map(&:markup), "RegexError 2") - assert_includes(@regex_check.offenses.map(&:markup), "RegexError 3") + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable AssignCheck, RegexCheck')} + {% assign x = 'x' %} + RegexError 1 + #{comment.call('theme-check-enable AssignCheck')} + {% assign y = 'y' %} + RegexError 2 + #{comment.call('theme-check-enable RegexCheck')} + {% assign z = 'z' %} + RegexError 3 + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) + + refute_empty(@assign_check.offenses) + refute_includes(@assign_check.offenses.map(&:markup), "assign x = 'x' ") + assert_includes(@assign_check.offenses.map(&:markup), "assign y = 'y' ") + assert_includes(@assign_check.offenses.map(&:markup), "assign z = 'z' ") + + refute_empty(@regex_check.offenses) + refute_includes(@regex_check.offenses.map(&:markup), "RegexError 1") + refute_includes(@regex_check.offenses.map(&:markup), "RegexError 2") + assert_includes(@regex_check.offenses.map(&:markup), "RegexError 3") + end end def test_comments_can_have_spaces - liquid_file = parse_liquid(<<~END) - {% comment %} theme-check-disable {% endcomment %} - {% assign x = 'x' %} - RegexError 1 - {% comment %} theme-check-enable {% endcomment %} - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call(' theme-check-disable ')} + {% assign x = 'x' %} + RegexError 1 + #{comment.call(' theme-check-enable ')} + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) - assert_empty(@assign_check.offenses) - assert_empty(@regex_check.offenses) + assert_empty(@assign_check.offenses) + assert_empty(@regex_check.offenses) + end end def test_ignore_disable_check_that_cant_be_disabled - RegexCheck.can_disable(false) - liquid_file = parse_liquid(<<~END) - {% comment %} theme-check-disable {% endcomment %} - RegexError 1 - {% comment %} theme-check-enable {% endcomment %} - {% comment %} theme-check-disable RegexCheck {% endcomment %} - RegexError 2 - {% comment %} theme-check-enable RegexCheck {% endcomment %} - END - @visitor.visit_liquid_file(liquid_file) - @disabled_checks.remove_disabled_offenses(@checks) - RegexCheck.can_disable(true) - - assert_empty(@assign_check.offenses) - assert_includes(@regex_check.offenses.map(&:markup), "RegexError 1") - assert_includes(@regex_check.offenses.map(&:markup), "RegexError 2") + comment_types.each do |comment| + RegexCheck.can_disable(false) + liquid_file = parse_liquid(<<~END) + #{comment.call(' theme-check-disable ')} + RegexError 1 + #{comment.call(' theme-check-enable ')} + #{comment.call(' theme-check-disable RegexCheck ')} + RegexError 2 + #{comment.call(' theme-check-enable RegexCheck ')} + END + @visitor.visit_liquid_file(liquid_file) + @disabled_checks.remove_disabled_offenses(@checks) + RegexCheck.can_disable(true) + + assert_empty(@assign_check.offenses) + assert_includes(@regex_check.offenses.map(&:markup), "RegexError 1") + assert_includes(@regex_check.offenses.map(&:markup), "RegexError 2") + end end def test_can_disable_check_that_run_on_end - liquid_file = parse_liquid(<<~END) - {% comment %}theme-check-disable OnEndCheck{% endcomment %} - Hello there - END - @visitor.visit_liquid_file(liquid_file) - @checks.call(:on_end) - @disabled_checks.remove_disabled_offenses(@checks) - - assert_empty(@on_end_check.offenses.map(&:theme_file)) + comment_types.each do |comment| + liquid_file = parse_liquid(<<~END) + #{comment.call('theme-check-disable OnEndCheck')} + Hello there + END + @visitor.visit_liquid_file(liquid_file) + @checks.call(:on_end) + @disabled_checks.remove_disabled_offenses(@checks) + + assert_empty(@on_end_check.offenses.map(&:theme_file)) + end end def test_can_ignore_check_using_pattern diff --git a/test/liquid_node_test.rb b/test/liquid_node_test.rb index c8c1ff03..d44f160c 100644 --- a/test/liquid_node_test.rb +++ b/test/liquid_node_test.rb @@ -26,6 +26,10 @@ def test_markup {{ 'x' }} + {% + # inline comment + # block + %} END assert_can_find_node_with_markup(root, "if true ") @@ -39,6 +43,7 @@ def test_markup assert_can_find_node_with_markup(root, "if\n true") assert_can_find_node_with_markup(root, " 'x' ") assert_can_find_node_with_markup(root, "\n 'x'\n ") + assert_can_find_node_with_markup(root, "# inline comment\n # block\n ") end def test_inside_liquid_tag? @@ -547,6 +552,16 @@ def test_outer_markup_repeated_comments COMMENT end + def test_outer_markup_repeated_comments_inline_comment + liquid = <<~LIQUID + {% # theme-check-disable foo %} + {% # theme-check-disable bar %} + {%# theme-check-disable baz%} + LIQUID + assert_can_find_node_with_outer_markup('{% # theme-check-disable foo %}', liquid) + assert_can_find_node_with_outer_markup('{%# theme-check-disable baz%}', liquid) + end + def test_inner_markup assert_node_inner_markup_equals('', '{{x}}') assert_node_inner_markup_equals('', '{{ x }}') diff --git a/theme-check.gemspec b/theme-check.gemspec index 315a6c29..fa50b331 100644 --- a/theme-check.gemspec +++ b/theme-check.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - spec.add_dependency('liquid', '>= 5.2.0') + spec.add_dependency('liquid', '>= 5.4.0') spec.add_dependency('nokogiri', '>= 1.12') spec.add_dependency('parser', '~> 3') end