Merge branch 'extract-template-parser-module' into 'master'

Extract changelog template parser to new module

See merge request gitlab-org/gitlab!63864

lib/gitlab/changelog/config.rb

@@ -52,7 +52,12 @@ module Gitlab
         end
 
         if (template = hash['template'])
-          config.template = Parser.new.parse_and_transform(template)
+          config.template =
+            begin
+              TemplateParser::Parser.new.parse_and_transform(template)
+            rescue TemplateParser::Error => e
+              raise Error, e.message
+            end
         end
 
         if (categories = hash['categories'])
@@ -73,7 +78,12 @@ module Gitlab
       def initialize(project)
         @project = project
         @date_format = DEFAULT_DATE_FORMAT
-        @template = Parser.new.parse_and_transform(DEFAULT_TEMPLATE)
+        @template =
+          begin
+            TemplateParser::Parser.new.parse_and_transform(DEFAULT_TEMPLATE)
+          rescue TemplateParser::Error => e
+            raise Error, e.message
+          end
         @categories = {}
         @tag_regex = DEFAULT_TAG_REGEX
       end

lib/gitlab/changelog/release.rb

@@ -54,7 +54,7 @@ module Gitlab
       end
 
       def to_markdown
-        state = EvalState.new
+        state = TemplateParser::EvalState.new
         data = { 'categories' => entries_for_template }
 
         # While not critical, we would like release sections to be separated by
@@ -63,7 +63,12 @@ module Gitlab
         #
         # Since it can be a bit tricky to get this right in a template, we
         # enforce an empty line separator ourselves.
-        markdown = @config.template.evaluate(state, data).strip
+        markdown =
+          begin
+            @config.template.evaluate(state, data).strip
+          rescue TemplateParser::ParseError => e
+            raise Error, e.message
+          end
 
         # The release header can't be changed using the Liquid template, as we
         # need this to be in a known format. Without this restriction, we won't

lib/gitlab/changelog/ast.rb → lib/gitlab/template_parser/ast.rb

 # frozen_string_literal: true
 
 module Gitlab
-  module Changelog
+  module TemplateParser
     # AST nodes to evaluate when rendering a template.
     #
     # Evaluating an AST is done by walking over the nodes and calling

lib/gitlab/template_parser/error.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module TemplateParser
+    # An error raised when a template couldn't be rendered.
+    Error = Class.new(StandardError)
+  end
+end

lib/gitlab/changelog/eval_state.rb → lib/gitlab/template_parser/eval_state.rb

 # frozen_string_literal: true
 
 module Gitlab
-  module Changelog
+  module TemplateParser
     # A class for tracking state when evaluating a template
     class EvalState
       MAX_LOOPS = 4

lib/gitlab/changelog/parser.rb → lib/gitlab/template_parser/parser.rb

 # frozen_string_literal: true
 
 module Gitlab
-  module Changelog
-    # A parser for the template syntax used for generating changelogs.
+  module TemplateParser
+    # A parser for a simple template syntax, used for example to generate changelogs.
     #
     # As a quick primer on the template syntax, a basic template looks like
     # this:
@@ -166,7 +166,7 @@ module Gitlab
       def parse_and_transform(input)
         AST::Transformer.new.apply(parse(input))
       rescue Parslet::ParseFailed => ex
-        # We raise a custom error so it's easier to catch different changelog
+        # We raise a custom error so it's easier to catch different parser
         # related errors. In addition, this ensures the caller of this method
         # doesn't depend on a Parslet specific error class.
         raise Error, "Failed to parse the template: #{ex.message}"

spec/lib/gitlab/changelog/config_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Gitlab::Changelog::Config do
 
       expect(config.date_format).to eq('foo')
       expect(config.template)
-        .to be_instance_of(Gitlab::Changelog::AST::Expressions)
+        .to be_instance_of(Gitlab::TemplateParser::AST::Expressions)
 
       expect(config.categories).to eq({ 'foo' => 'bar' })
       expect(config.tag_regex).to eq('foo')
@@ -53,6 +53,16 @@ RSpec.describe Gitlab::Changelog::Config do
       expect { described_class.from_hash(project, 'categories' => 10) }
         .to raise_error(Gitlab::Changelog::Error)
     end
+
+    it 'raises a Gitlab::Changelog::Error when the template is invalid' do
+      invalid_template = <<~TPL
+        {% each {{foo}} %}
+        {% end %}
+      TPL
+
+      expect { described_class.from_hash(project, 'template' => invalid_template) }
+        .to raise_error(Gitlab::Changelog::Error)
+    end
   end
 
   describe '#contributor?' do

spec/lib/gitlab/changelog/ast_spec.rb → spec/lib/gitlab/template_parser/ast_spec.rb

@@ -2,8 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Changelog::AST::Identifier do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Identifier do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'evaluates a selector' do
@@ -26,8 +26,8 @@ RSpec.describe Gitlab::Changelog::AST::Identifier do
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Integer do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Integer do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'evaluates a selector' do
@@ -44,33 +44,33 @@ RSpec.describe Gitlab::Changelog::AST::Integer do
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Selector do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Selector do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
   let(:data) { { 'numbers' => [10] } }
 
   describe '#evaluate' do
     it 'evaluates a selector' do
-      ident = Gitlab::Changelog::AST::Identifier.new('numbers')
-      int = Gitlab::Changelog::AST::Integer.new(0)
+      ident = Gitlab::TemplateParser::AST::Identifier.new('numbers')
+      int = Gitlab::TemplateParser::AST::Integer.new(0)
 
       expect(described_class.new([ident, int]).evaluate(state, data)).to eq(10)
     end
 
     it 'evaluates a selector that returns nil' do
-      int = Gitlab::Changelog::AST::Integer.new(0)
+      int = Gitlab::TemplateParser::AST::Integer.new(0)
 
       expect(described_class.new([int]).evaluate(state, data)).to be_nil
     end
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Variable do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Variable do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
   let(:data) { { 'numbers' => [10] } }
 
   describe '#evaluate' do
     it 'evaluates a variable' do
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{{numbers.0}}')
         .nodes[0]
@@ -80,26 +80,26 @@ RSpec.describe Gitlab::Changelog::AST::Variable do
 
     it 'evaluates an undefined variable' do
       node =
-        Gitlab::Changelog::Parser.new.parse_and_transform('{{foobar}}').nodes[0]
+        Gitlab::TemplateParser::Parser.new.parse_and_transform('{{foobar}}').nodes[0]
 
       expect(node.evaluate(state, data)).to eq('')
     end
 
     it 'evaluates the special variable "it"' do
       node =
-        Gitlab::Changelog::Parser.new.parse_and_transform('{{it}}').nodes[0]
+        Gitlab::TemplateParser::Parser.new.parse_and_transform('{{it}}').nodes[0]
 
       expect(node.evaluate(state, data)).to eq(data.to_s)
     end
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Expressions do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Expressions do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'evaluates all expressions' do
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{{number}}foo')
 
@@ -108,8 +108,8 @@ RSpec.describe Gitlab::Changelog::AST::Expressions do
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Text do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Text do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'returns the text' do
@@ -118,12 +118,12 @@ RSpec.describe Gitlab::Changelog::AST::Text do
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::If do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::If do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'evaluates a truthy if expression without an else clause' do
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{% if thing %}foo{% end %}')
         .nodes[0]
@@ -132,7 +132,7 @@ RSpec.describe Gitlab::Changelog::AST::If do
     end
 
     it 'evaluates a falsy if expression without an else clause' do
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{% if thing %}foo{% end %}')
         .nodes[0]
@@ -141,7 +141,7 @@ RSpec.describe Gitlab::Changelog::AST::If do
     end
 
     it 'evaluates a falsy if expression with an else clause' do
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{% if thing %}foo{% else %}bar{% end %}')
         .nodes[0]
@@ -177,13 +177,13 @@ RSpec.describe Gitlab::Changelog::AST::If do
   end
 end
 
-RSpec.describe Gitlab::Changelog::AST::Each do
-  let(:state) { Gitlab::Changelog::EvalState.new }
+RSpec.describe Gitlab::TemplateParser::AST::Each do
+  let(:state) { Gitlab::TemplateParser::EvalState.new }
 
   describe '#evaluate' do
     it 'evaluates the expression' do
       data = { 'animals' => [{ 'name' => 'Cat' }, { 'name' => 'Dog' }] }
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{% each animals %}{{name}}{% end %}')
         .nodes[0]
@@ -193,7 +193,7 @@ RSpec.describe Gitlab::Changelog::AST::Each do
 
     it 'returns an empty string when the input is not a collection' do
       data = { 'animals' => 10 }
-      node = Gitlab::Changelog::Parser
+      node = Gitlab::TemplateParser::Parser
         .new
         .parse_and_transform('{% each animals %}{{name}}{% end %}')
         .nodes[0]
@@ -237,10 +237,10 @@ RSpec.describe Gitlab::Changelog::AST::Each do
       TPL
 
       node =
-        Gitlab::Changelog::Parser.new.parse_and_transform(template).nodes[0]
+        Gitlab::TemplateParser::Parser.new.parse_and_transform(template).nodes[0]
 
       expect { node.evaluate(state, data) }
-        .to raise_error(Gitlab::Changelog::Error)
+        .to raise_error(Gitlab::TemplateParser::Error)
     end
   end
 end

spec/lib/gitlab/changelog/parser_spec.rb → spec/lib/gitlab/template_parser/parser_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Changelog::Parser do
+RSpec.describe Gitlab::TemplateParser::Parser do
   let(:parser) { described_class.new }
 
   describe '#root' do
@@ -67,12 +67,12 @@ RSpec.describe Gitlab::Changelog::Parser do
     it 'parses and transforms a template' do
       node = parser.parse_and_transform('foo')
 
-      expect(node).to be_instance_of(Gitlab::Changelog::AST::Expressions)
+      expect(node).to be_instance_of(Gitlab::TemplateParser::AST::Expressions)
     end
 
     it 'raises parsing errors using a custom error class' do
       expect { parser.parse_and_transform('{% each') }
-        .to raise_error(Gitlab::Changelog::Error)
+        .to raise_error(Gitlab::TemplateParser::Error)
     end
   end
 end