From ab17af29035969c363f5f2ecaab8cd4c7f052ae5 Mon Sep 17 00:00:00 2001 From: Max Jacobson Date: Thu, 2 Jan 2025 18:42:36 -0500 Subject: [PATCH 1/2] Extend SpaceInHtmlTag to trim values In our code base, we merged in some code that looked like this: ``` ``` Which ultimately did not work. The browser did not understand this. We wondered if ERB Lint could have caught it. And now here we are. What do we think? --- lib/erb_lint/linters/space_in_html_tag.rb | 13 +++++++ .../linters/space_in_html_tag_spec.rb | 38 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/lib/erb_lint/linters/space_in_html_tag.rb b/lib/erb_lint/linters/space_in_html_tag.rb index 44360c9b..2698b41d 100644 --- a/lib/erb_lint/linters/space_in_html_tag.rb +++ b/lib/erb_lint/linters/space_in_html_tag.rb @@ -96,6 +96,15 @@ def process_attributes(processed_source, attributes) name, equal, value = *attribute no_space(processed_source, name.loc.end_pos...equal.loc.begin_pos) if name && equal no_space(processed_source, equal.loc.end_pos...value.loc.begin_pos) if equal && value + if value && quoted_value?(value) + open_quote, str, close_quote = *value + leading_spaces = str.chars.take_while { |char| char == " " }.count + trailing_spaces = str.chars.reverse.take_while { |char| char == " " }.count + first_non_space_pos = open_quote.loc.end_pos + leading_spaces + last_non_space_pos = close_quote.loc.begin_pos - trailing_spaces + no_space(processed_source, (open_quote.loc.end_pos)...first_non_space_pos) + no_space(processed_source, last_non_space_pos...close_quote.loc.begin_pos) + end next if index >= attributes.children.size - 1 @@ -107,6 +116,10 @@ def process_attributes(processed_source, attributes) ) end end + + def quoted_value?(value) + value.children.length == 3 + end end end end diff --git a/spec/erb_lint/linters/space_in_html_tag_spec.rb b/spec/erb_lint/linters/space_in_html_tag_spec.rb index 7028f5c4..f95a3153 100644 --- a/spec/erb_lint/linters/space_in_html_tag_spec.rb +++ b/spec/erb_lint/linters/space_in_html_tag_spec.rb @@ -52,6 +52,16 @@ it { expect(subject).to(eq([])) } end + context "self-closing tag with valueless attribute" do + let(:file) { "" } + it { expect(subject).to(eq([])) } + end + + context "self-closing tag with no quotes around attribute value" do + let(:file) { "" } + it { expect(subject).to(eq([])) } + end + context "between attributes" do let(:file) { '' } it { expect(subject).to(eq([])) } @@ -147,6 +157,24 @@ ])) end end + + context "at start of value" do + let(:file) { "
" } + it do + expect(subject).to(eq([ + build_offense(10..11, "Extra space detected where there should be no space."), + ])) + end + end + + context "at end of value" do + let(:file) { "
" } + it do + expect(subject).to(eq([ + build_offense(13..14, "Extra space detected where there should be no space."), + ])) + end + end end context "when space is missing" do @@ -431,6 +459,16 @@ let(:file) { "
" } it { expect(subject).to(eq("
")) } end + + context "at start of value" do + let(:file) { "
" } + it { expect(subject).to(eq("
")) } + end + + context "at end of value" do + let(:file) { "
" } + it { expect(subject).to(eq("
")) } + end end context "when space is missing" do From fe4c333a2cfc4848528d08b1b0400ed0a1df29a7 Mon Sep 17 00:00:00 2001 From: Max Jacobson Date: Mon, 17 Mar 2025 16:01:12 -0400 Subject: [PATCH 2/2] cover a few more cases --- lib/erb_lint/linters/space_in_html_tag.rb | 28 +++++++++++++++---- .../linters/space_in_html_tag_spec.rb | 25 +++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/erb_lint/linters/space_in_html_tag.rb b/lib/erb_lint/linters/space_in_html_tag.rb index 2698b41d..bd4620e2 100644 --- a/lib/erb_lint/linters/space_in_html_tag.rb +++ b/lib/erb_lint/linters/space_in_html_tag.rb @@ -97,11 +97,25 @@ def process_attributes(processed_source, attributes) no_space(processed_source, name.loc.end_pos...equal.loc.begin_pos) if name && equal no_space(processed_source, equal.loc.end_pos...value.loc.begin_pos) if equal && value if value && quoted_value?(value) - open_quote, str, close_quote = *value - leading_spaces = str.chars.take_while { |char| char == " " }.count - trailing_spaces = str.chars.reverse.take_while { |char| char == " " }.count - first_non_space_pos = open_quote.loc.end_pos + leading_spaces - last_non_space_pos = close_quote.loc.begin_pos - trailing_spaces + children = value.children.dup + open_quote = children.shift + close_quote = children.pop + + leading_spaces_count = if children.first.is_a?(String) + children.first.chars.take_while { |char| char == " " }.count + else + # value starts with erb tag + 0 + end + + trailing_spaces_count = if children.last.is_a?(String) + children.last.chars.reverse.take_while { |char| char == " " }.count + else + # value ends with erb tag + 0 + end + first_non_space_pos = open_quote.loc.end_pos + leading_spaces_count + last_non_space_pos = close_quote.loc.begin_pos - trailing_spaces_count no_space(processed_source, (open_quote.loc.end_pos)...first_non_space_pos) no_space(processed_source, last_non_space_pos...close_quote.loc.begin_pos) end @@ -118,7 +132,9 @@ def process_attributes(processed_source, attributes) end def quoted_value?(value) - value.children.length == 3 + value.children.length >= 3 && + value.children.first.type == :quote && + value.children.last.type == :quote end end end diff --git a/spec/erb_lint/linters/space_in_html_tag_spec.rb b/spec/erb_lint/linters/space_in_html_tag_spec.rb index f95a3153..58c1c1e9 100644 --- a/spec/erb_lint/linters/space_in_html_tag_spec.rb +++ b/spec/erb_lint/linters/space_in_html_tag_spec.rb @@ -83,6 +83,13 @@ it { expect(subject).to(eq([])) } end + context "tag with erb in attribute value" do + let(:file) { <<~HTML } + + HTML + it { expect(subject).to(eq([])) } + end + context "multi-line tag with erb" do let(:file) { <<~HTML } " } + it do + expect(subject).to(eq([ + build_offense(10..11, "Extra space detected where there should be no space."), + ])) + end + end + context "at end of value" do let(:file) { "
" } it do @@ -175,6 +191,15 @@ ])) end end + + context "at end of value with embedded ruby" do + let(:file) { "
" } + it do + expect(subject).to(eq([ + build_offense(31..32, "Extra space detected where there should be no space."), + ])) + end + end end context "when space is missing" do