Skip to content

Commit

Permalink
Fix tag attribute spacing (#487)
Browse files Browse the repository at this point in the history
* Add UPDATE_EXAMPLES=1 rspec
Helps deal with updating generated ruby when changing extractor stuff

* ChunkExtractor: Properly transfer the whitespace around tag attributes
This allows cleaner representation of tag attributes, and improved corrections.
Also moved the begin...ensure of tags's to be only for its children, not the tag placeholder, nor tag attributes nor tag scripts.
This may also help with performance a little by often not needing to have a begin...ensure at all.
  • Loading branch information
MaxLap authored Feb 4, 2024
1 parent 06b18d4 commit 25120e8
Show file tree
Hide file tree
Showing 18 changed files with 587 additions and 832 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# HAML-Lint Changelog

* Rubocop: Properly pass the whitespace around tag attributes to RuboCop. This allows some improved corrections and fixes bugs.

### 0.55.0

* Lazily open files to avoid file descriptor exhaustion
Expand Down
29 changes: 15 additions & 14 deletions lib/haml_lint/ruby_extraction/chunk_extractor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,31 +247,29 @@ def finish_visit_any_script(node, lines, raw_code: nil, must_start_chunk: false,
def visit_tag(node)
indent = @original_haml_lines[node.line - 1].index(/\S/)

@ruby_chunks << PlaceholderMarkerChunk.new(node, 'tag', indent: indent)

current_line_index = visit_tag_attributes(node, indent: indent)
visit_tag_script(node, line_index: current_line_index, indent: indent)

# We don't want to use a block because assignments in a block are local to that block,
# so the semantics of the extracted ruby would be different from the one generated by
# Haml. Those differences can make some cops, such as UselessAssignment, have false
# positives
code = 'begin'
@ruby_chunks << AdHocChunk.new(node,
[' ' * indent + code])
indent += 2

tag_chunk = PlaceholderMarkerChunk.new(node, 'tag', indent: indent)
@ruby_chunks << tag_chunk
begin_chunk = AdHocChunk.new(node, [' ' * indent + code])
@ruby_chunks << begin_chunk

current_line_index = visit_tag_attributes(node, indent: indent)
visit_tag_script(node, line_index: current_line_index, indent: indent)
indent += 2

yield

indent -= 2

if @ruby_chunks.last.equal?(tag_chunk)
if @ruby_chunks.last.equal?(begin_chunk)
# So there is nothing going "in" the tag, remove the wrapping "begin" and replace the PlaceholderMarkerChunk
# by one less indented
@ruby_chunks.pop
@ruby_chunks.pop
@ruby_chunks << PlaceholderMarkerChunk.new(node, 'tag', indent: indent)
else
@ruby_chunks << AdHocChunk.new(node,
[' ' * indent + 'ensure', ' ' * indent + ' HL.noop', ' ' * indent + 'end'],
Expand Down Expand Up @@ -600,9 +598,12 @@ def extract_raw_tag_attributes_ruby_lines(haml_processed_ruby_code, first_line_i
return [char_index, [haml_processed_ruby_code]]
end

min_non_white_chars_to_add = haml_processed_ruby_code.scan(/\S/).size
# The +1 is for the closing brace, which we need
min_non_white_chars_to_add = haml_processed_ruby_code.scan(/\S/).size + 1

regexp = HamlLint::Utils.regexp_for_parts(haml_processed_ruby_code.split(/\s+/), '\\s+')
regexp = HamlLint::Utils.regexp_for_parts(
haml_processed_ruby_code.split(/\s+/), '\\s+', prefix: '\s*', suffix: '\s*(?=[)}])'
)

joined_lines = first_line.rstrip
process_multiline!(joined_lines)
Expand All @@ -624,7 +625,7 @@ def extract_raw_tag_attributes_ruby_lines(haml_processed_ruby_code, first_line_i

first_line_offset = match.begin(0)
raw_ruby = match[0]
ruby_lines = raw_ruby.split("\n")
ruby_lines = raw_ruby.split("\n", -1)

[first_line_offset, ruby_lines]
end
Expand Down
36 changes: 33 additions & 3 deletions lib/haml_lint/spec/shared_rubocop_autocorrect_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def follows_steps # rubocop:disable Metrics

syntax_lints = subject.lints.select { |lint| lint.message =~ %r{Lint/Syntax} }

if start_ruby.strip != 'SKIP' && subject.last_extracted_source
if start_ruby.strip != 'SKIP' && subject.last_extracted_source && ENV['UPDATE_EXAMPLES'] != '1'
matcher = eq(start_ruby)
subject.last_extracted_source.source.should(
matcher,
Expand All @@ -125,7 +125,7 @@ def follows_steps # rubocop:disable Metrics

syntax_lints.should(be_empty, "Generated Ruby has Syntax Lints:\n#{format_lints(syntax_lints)}")

if end_ruby.strip != 'SKIP' && subject.last_new_ruby_source
if end_ruby.strip != 'SKIP' && subject.last_new_ruby_source && ENV['UPDATE_EXAMPLES'] != '1'
matcher = eq(end_ruby)
subject.last_new_ruby_source.should(
matcher,
Expand All @@ -141,10 +141,40 @@ def follows_steps # rubocop:disable Metrics
-> { "Final HAML is different from expected. #{matcher.failure_message}\n#{format_lints}" }
)

if subject.last_extracted_source && start_ruby.strip != 'SKIP'
if subject.last_extracted_source && start_ruby.strip != 'SKIP' && ENV['UPDATE_EXAMPLES'] != '1'
subject.last_extracted_source.source_map.should == source_map
end

if ENV['UPDATE_EXAMPLES'] == '1' && respond_to?(:example_first_line_no) &&
start_ruby.strip != 'SKIP' && end_ruby.strip != 'SKIP'
original_content = File.read(example_path)
old_steps_string = '---' + steps_string.partition('---').last.rpartition('---').first + '---'

if old_steps_string.scan(/\$\$\d+/).tally.values.any? { |v| v > 1 }
# If the $$3 was there twice, let's not update the example
return
end
original_content.scan(old_steps_string).count

new_generated_ruby_lines = subject.last_extracted_source.source.split("\n", -1)
last_seen_haml_line_no = 1
subject.last_extracted_source.source_map.each do |ruby_line_no, haml_line_no|
if haml_line_no != last_seen_haml_line_no
last_seen_haml_line_no = haml_line_no
new_generated_ruby_lines[ruby_line_no - 1] += " $$#{haml_line_no}"
end
end

new_steps_string = <<~NEW.chop # Chop to remove the added newlines
---
#{new_generated_ruby_lines.join("\n")}---
#{subject.last_new_ruby_source}---
NEW

new_content = original_content.gsub(old_steps_string, new_steps_string)
File.write(example_path, new_content)
end

haml_different = start_haml != end_haml
document.source_was_changed.should == haml_different
end
Expand Down
3 changes: 2 additions & 1 deletion lib/haml_lint/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,9 @@ def with_captured_streams(stdin_str, &_block)
$stdin = original_stdin
end

def regexp_for_parts(parts, join_regexp)
def regexp_for_parts(parts, join_regexp, prefix: nil, suffix: nil)
regexp_code = parts.map { |c| Regexp.quote(c) }.join(join_regexp)
regexp_code = "#{prefix}#{regexp_code}#{suffix}"
Regexp.new(regexp_code)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ haml_lint_tag_5
haml_lint_marker_1
foo.each do |bar|
next if bar $$2
$$3
$$3
HL.out = execute $$4
end
haml_lint_marker_7
Expand Down Expand Up @@ -132,7 +132,7 @@ haml_lint_marker_7
haml_lint_marker_1
foo.each do |bar|
next if bar $$2
$$3
$$3
haml_lint_marker_5
haml_lint_tag_6 $$4
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,28 +259,20 @@ haml_lint_marker_1
if a
if b $$2
haml_lint_marker_4
begin $$3
haml_lint_tag_6
haml_lint_marker_7
WW(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5 $$3
haml_lint_marker_6
WWWW(bar: 123, abc: '42')
haml_lint_marker_8
end
end
---
haml_lint_marker_1
if a && b
haml_lint_marker_4
begin
haml_lint_tag_6
haml_lint_marker_7
WW(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5
haml_lint_marker_6
WWWW(bar: 123, abc: '42')
haml_lint_marker_8
end
---
- if a && b
Expand All @@ -296,28 +288,20 @@ haml_lint_marker_1
if a
if b $$2
haml_lint_marker_4
begin $$3
haml_lint_tag_6
haml_lint_marker_7
WW(:bar => 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5 $$3
haml_lint_marker_6
WWWW(:bar => 123, abc: '42')
haml_lint_marker_8
end
end
---
haml_lint_marker_1
if a && b
haml_lint_marker_4
begin
haml_lint_tag_6
haml_lint_marker_7
WW(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5
haml_lint_marker_6
WWWW(bar: 123, abc: '42')
haml_lint_marker_8
end
---
- if a && b
Expand All @@ -333,28 +317,20 @@ haml_lint_marker_1
if a
if b $$2
haml_lint_marker_4
begin $$3
haml_lint_tag_6
haml_lint_marker_7
WW(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5 $$3
haml_lint_marker_6
WWWW(bar: 123, abc: '42')
haml_lint_marker_8
end
end
---
haml_lint_marker_1
if a && b
haml_lint_marker_4
begin
haml_lint_tag_6
haml_lint_marker_7
WW(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5
haml_lint_marker_6
WWWW(bar: 123, abc: '42')
haml_lint_marker_8
end
---
- if a && b
Expand Down Expand Up @@ -393,28 +369,20 @@ haml_lint_marker_1
if a
if b $$2
haml_lint_marker_4
begin $$3
haml_lint_tag_6
haml_lint_marker_7
HL.out = foo(:bar => 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5 $$3
haml_lint_marker_6
HL.out = foo(:bar => 123, abc: '42')
haml_lint_marker_8
end
end
---
haml_lint_marker_1
if a && b
haml_lint_marker_4
begin
haml_lint_tag_6
haml_lint_marker_7
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5
haml_lint_marker_6
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_8
end
---
- if a && b
Expand All @@ -430,28 +398,20 @@ haml_lint_marker_1
if a
if b $$2
haml_lint_marker_4
begin $$3
haml_lint_tag_6
haml_lint_marker_7
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5 $$3
haml_lint_marker_6
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_8
end
end
---
haml_lint_marker_1
if a && b
haml_lint_marker_4
begin
haml_lint_tag_6
haml_lint_marker_7
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_9
ensure
HL.noop
end
haml_lint_tag_5
haml_lint_marker_6
HL.out = foo(bar: 123, abc: '42')
haml_lint_marker_8
end
---
- if a && b
Expand Down
Loading

0 comments on commit 25120e8

Please sign in to comment.