Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

Add support for # inline comment tag #533

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Add support for # inline comment tag #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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%}
```

Expand Down
4 changes: 2 additions & 2 deletions docs/checks/asset_size_javascript.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
<script src="https://code.jquery.com/jquery-3.6.0.min.js" defer></script>
{% 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.
Expand Down
6 changes: 3 additions & 3 deletions docs/checks/missing_enable_comment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ This check aims at eliminating missing `theme-check-enable` comments.
<!doctype html>
<html>
<head>
{% comment %}theme-check-disable ParserBlockingJavaScript{% endcomment %}
{% # theme-check-disable ParserBlockingJavaScript %}
<script src="https://cdnjs.com/jquery.min.js"></script>
</head>
<body>
Expand All @@ -27,9 +27,9 @@ This check aims at eliminating missing `theme-check-enable` comments.
<!doctype html>
<html>
<head>
{% comment %}theme-check-disable ParserBlockingJavaScript{% endcomment %}
{% # theme-check-disable ParserBlockingJavaScript %}
<script src="https://cdnjs.com/jquery.min.js"></script>
{% comment %}theme-check-enable ParserBlockingJavaScript{% endcomment %}
{% # theme-check-enable ParserBlockingJavaScript %}
</head>
<body>
<!-- ... -->
Expand Down
16 changes: 8 additions & 8 deletions docs/checks/nested_snippet.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down
8 changes: 4 additions & 4 deletions docs/checks/translation_key_exists.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,30 @@ 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": {
"close": "Close"
}
}

{% 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": {
"close": "Close"
}
}

{% comment %}templates/index.liquid{% endcomment %}
{% # templates/index.liquid %}
{{ "greetings" | t }}
{{ "general.close" | t }}
```
Expand Down
2 changes: 1 addition & 1 deletion docs/checks/valid_html_translation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "<h1>Hello, world</h1>",
"image_html": "<img src='spongebob.png'>",
Expand Down
4 changes: 4 additions & 0 deletions lib/theme_check/checks/missing_enable_comment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
7 changes: 6 additions & 1 deletion lib/theme_check/disabled_checks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions lib/theme_check/liquid_node.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/theme_check/liquid_visitor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
143 changes: 81 additions & 62 deletions test/checks/missing_enable_comment_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
<p>Hello, world</p>
{% comment %}theme-check-disable{% endcomment %}
{% assign x = 1 %}
comment_types.each do |comment|
offenses = analyze_theme(
ThemeCheck::MissingEnableComment.new,
"templates/index.liquid" => <<~END,
<p>Hello, world</p>
#{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,
<p>Hello, world</p>
{% 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,
<p>Hello, world</p>
#{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,
<p>Hello, world</p>
{% 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,
<p>Hello, world</p>
#{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,
<p>Hello, world</p>
{% 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,
<p>Hello, world</p>
#{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
Loading