From 12e165963bacf1afdc3b981ba842314fb81f53a8 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:04:19 +0200 Subject: [PATCH] Don't use rexml for the JUnit formatter Closes https://github.com/Shopify/erb_lint/issues/371 Follows https://github.com/rubocop/rubocop/pull/13165 This leaves the output mostly the same, except that text is no longer wrapped in CDATA. Some things possibly need to be escaped now, and CDATA has different escaping needs. So, just use the same escaping everywhere --- Gemfile.lock | 16 ++- lib/erb_lint/reporters/junit_reporter.rb | 131 +++++++---------------- spec/erb_lint/cli_spec.rb | 1 - spec/erb_lint/fixtures/junit.xml | 4 +- 4 files changed, 49 insertions(+), 103 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ccebcda5..648c9a91 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -60,7 +60,7 @@ GEM mini_portile2 (~> 2.8.2) racc (~> 1.4) parallel (1.24.0) - parser (3.3.0.5) + parser (3.3.5.0) ast (~> 2.4.1) racc racc (1.7.3) @@ -74,8 +74,6 @@ GEM rainbow (3.1.1) rake (13.0.6) regexp_parser (2.9.0) - rexml (3.3.6) - strscan rspec (3.11.0) rspec-core (~> 3.11.0) rspec-expectations (~> 3.11.0) @@ -89,24 +87,22 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.11.0) rspec-support (3.11.0) - rubocop (1.62.1) + rubocop (1.66.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.31.1, < 2.0) + regexp_parser (>= 2.4, < 3.0) + rubocop-ast (>= 1.32.2, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.31.2) - parser (>= 3.3.0.4) + rubocop-ast (1.32.3) + parser (>= 3.3.1.0) rubocop-shopify (2.15.1) rubocop (~> 1.51) ruby-progressbar (1.13.0) smart_properties (1.17.0) - strscan (3.1.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (2.5.0) diff --git a/lib/erb_lint/reporters/junit_reporter.rb b/lib/erb_lint/reporters/junit_reporter.rb index 862559b4..e209714b 100644 --- a/lib/erb_lint/reporters/junit_reporter.rb +++ b/lib/erb_lint/reporters/junit_reporter.rb @@ -1,111 +1,62 @@ # frozen_string_literal: true -require "rexml/document" -require "rexml/formatters/pretty" - module ERBLint module Reporters class JunitReporter < Reporter + ESCAPE_MAP = { + '"' => """, + "'" => "'", + "<" => "<", + ">" => ">", + "&" => "&", + }.freeze + + PROPERTIES = [ + ["erb_lint_version", ERBLint::VERSION], + ["ruby_engine", RUBY_ENGINE], + ["ruby_version", RUBY_VERSION], + ["ruby_patchlevel", RUBY_PATCHLEVEL.to_s], + ["ruby_platform", RUBY_PLATFORM], + ].freeze + def preview; end def show - xml = create_junit_xml - formatted_xml_string = StringIO.new - REXML::Formatters::Pretty.new.write(xml, formatted_xml_string) - puts formatted_xml_string.string - end - - private - - CONTEXT = { - prologue_quote: :quote, - attribute_quote: :quote, - } - - def create_junit_xml - # create prologue - xml = REXML::Document.new(nil, CONTEXT) - xml << REXML::XMLDecl.new("1.0", "UTF-8") - - xml.add_element(create_testsuite_element) + puts %() + puts %() - xml - end - - def create_testsuite_element - tests = stats.processed_files.size - failures = stats.found - testsuite_element = REXML::Element.new("testsuite", nil, CONTEXT) - testsuite_element.add_attribute("name", "erblint") - testsuite_element.add_attribute("tests", tests.to_s) - testsuite_element.add_attribute("failures", failures.to_s) - - testsuite_element.add_element(create_properties) + puts %( ) + PROPERTIES.each do |key, value| + puts %( ) + end + puts %( ) processed_files.each do |filename, offenses| + filename_escaped = xml_escape(filename) if offenses.empty? - testcase_element = REXML::Element.new("testcase", nil, CONTEXT) - testcase_element.add_attribute("name", filename.to_s) - testcase_element.add_attribute("file", filename.to_s) - - testsuite_element.add_element(testcase_element) - end - - offenses.each do |offense| - testsuite_element.add_element(create_testcase(filename, offense)) + puts %( ) + else + offenses.each do |offense| + type = offense.simple_name + message = "#{type}: #{offense.message}" + body = "#{message} at #{filename}:#{offense.line_number}:#{offense.column}" + + puts %( ) + puts %( ) + puts %( #{xml_escape(body)}) + puts %( ) + puts %( ) + end end end - testsuite_element + puts %() end - def create_properties - properties_element = REXML::Element.new("properties", nil, CONTEXT) - - [ - ["erb_lint_version", ERBLint::VERSION], - ["ruby_engine", RUBY_ENGINE], - ["ruby_version", RUBY_VERSION], - ["ruby_patchlevel", RUBY_PATCHLEVEL.to_s], - ["ruby_platform", RUBY_PLATFORM], - ].each do |property_attribute| - properties_element.add_element(create_property(*property_attribute)) - end - - properties_element - end - - def create_property(name, value) - property_element = REXML::Element.new("property") - property_element.add_attribute("name", name) - property_element.add_attribute("value", value) - - property_element - end - - def create_testcase(filename, offense) - testcase_element = REXML::Element.new("testcase", nil, CONTEXT) - testcase_element.add_attribute("name", filename.to_s) - testcase_element.add_attribute("file", filename.to_s) - testcase_element.add_attribute("lineno", offense.line_number.to_s) - - testcase_element.add_element(create_failure(filename, offense)) - - testcase_element - end - - def create_failure(filename, offense) - message = offense.message - type = offense.simple_name - - failure_element = REXML::Element.new("failure", nil, CONTEXT) - failure_element.add_attribute("message", "#{type}: #{message}") - failure_element.add_attribute("type", type.to_s) - - cdata_element = REXML::CData.new("#{type}: #{message} at #{filename}:#{offense.line_number}:#{offense.column}") - failure_element.add_text(cdata_element) + private - failure_element + def xml_escape(string) + string.gsub(Regexp.union(ESCAPE_MAP.keys), ESCAPE_MAP) end end end diff --git a/spec/erb_lint/cli_spec.rb b/spec/erb_lint/cli_spec.rb index 40e1678a..1f60fa73 100644 --- a/spec/erb_lint/cli_spec.rb +++ b/spec/erb_lint/cli_spec.rb @@ -4,7 +4,6 @@ require "spec_utils" require "erb_lint/cli" require "erb_lint/cache" -require "pp" require "fakefs" require "fakefs/spec_helpers" diff --git a/spec/erb_lint/fixtures/junit.xml b/spec/erb_lint/fixtures/junit.xml index 61647e4e..61fbeb9f 100644 --- a/spec/erb_lint/fixtures/junit.xml +++ b/spec/erb_lint/fixtures/junit.xml @@ -9,12 +9,12 @@ - + SpaceInHtmlTag: Extra space detected where there should be no space. at app/views/subscriptions/_loader.html.erb:1:7 - ` to match start of tag. at app/views/subscriptions/_loader.html.erb:52:10]]> + ClosingErbTagIndent: Remove newline before `%%>` to match start of tag. at app/views/subscriptions/_loader.html.erb:52:10