Refactor commit_messages#lint_commit

This introduces additional method for linting,
to reduce the complexity of `lint_commits`.

1 files changed, 128 insertions, 118 deletions

diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile
index 241462234c5..d9ba71a73cd 100644
--- a/danger/commit_messages/Dangerfile
+++ b/danger/commit_messages/Dangerfile
@@ -64,11 +64,12 @@ def too_many_changed_lines?(commit)
     lines_changed_in_commit(commit) >= 30
 end
 
-def lint_commits(commits)
-  failures = false
-  emoji_checker = EmojiChecker.new
+def emoji_checker
+  @emoji_checker ||= EmojiChecker.new
+end
 
-  unicode_emoji_regex = %r((
+def unicode_emoji_regex
+  @unicode_emoji_regex ||= %r((
     [\u{1F300}-\u{1F5FF}] |
     [\u{1F1E6}-\u{1F1FF}] |
     [\u{2700}-\u{27BF}] |
@@ -77,123 +78,132 @@ def lint_commits(commits)
     [\u{1F680}-\u{1F6FF}] |
     [\u{2600}-\u{26FF}]
   ))x
+end
+
+def lint_commit(commit)
+  # For now we'll ignore merge commits, as getting rid of those is a problem
+  # separate from enforcing good commit messages.
+  return false if commit.message.start_with?('Merge branch')
+
+  # We ignore revert commits as they are well structured by Git already
+  return false if commit.message.start_with?('Revert "')
+
+  failures = false
+  subject, separator, details = commit.message.split("\n", 3)
+
+  if subject.split.length < 3
+    fail_commit(
+      commit,
+      'The commit subject must contain at least three words'
+    )
+
+    failures = true
+  end
+
+  if subject.length > 72
+    fail_commit(
+      commit,
+      'The commit subject may not be longer than 72 characters'
+    )
+
+    failures = true
+  elsif subject.length > 50
+    warn_commit(
+      commit,
+      "This commit's subject line is acceptable, but please try to [reduce it to 50 characters](#{URL_LIMIT_SUBJECT})."
+    )
+  end
+
+  unless subject_starts_with_capital?(subject)
+    fail_commit(commit, 'The commit subject must start with a capital letter')
+    failures = true
+  end
+
+  if subject.end_with?('.')
+    fail_commit(commit, 'The commit subject must not end with a period')
+    failures = true
+  end
+
+  if separator && !separator.empty?
+    fail_commit(
+      commit,
+      'The commit subject and body must be separated by a blank line'
+    )
+
+    failures = true
+  end
+
+  details&.each_line do |line|
+    line = line.strip
 
-  commits.each do |commit|
-    # For now we'll ignore merge commits, as getting rid of those is a problem
-    # separate from enforcing good commit messages.
-    next if commit.message.start_with?('Merge branch')
-
-    # We ignore revert commits as they are well structured by Git already
-    next if commit.message.start_with?('Revert "')
-
-    subject, separator, details = commit.message.split("\n", 3)
-
-    if subject.split.length < 3
-      fail_commit(
-        commit,
-        'The commit subject must contain at least three words'
-      )
-
-      failures = true
-    end
-
-    if subject.length > 72
-      fail_commit(
-        commit,
-        'The commit subject may not be longer than 72 characters'
-      )
-
-      failures = true
-    elsif subject.length > 50
-      warn_commit(
-        commit,
-        "This commit's subject line is acceptable, but please try to [reduce it to 50 characters](#{URL_LIMIT_SUBJECT})."
-      )
-    end
-
-    unless subject_starts_with_capital?(subject)
-      fail_commit(commit, 'The commit subject must start with a capital letter')
-      failures = true
-    end
-
-    if subject.end_with?('.')
-      fail_commit(commit, 'The commit subject must not end with a period')
-      failures = true
-    end
-
-    if separator && !separator.empty?
-      fail_commit(
-        commit,
-        'The commit subject and body must be separated by a blank line'
-      )
-
-      failures = true
-    end
-
-    details&.each_line do |line|
-      line = line.strip
-
-      next if line.length <= 72
-
-      url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length }
-
-      # If the line includes a URL, we'll allow it to exceed 72 characters, but
-      # only if the line _without_ the URL does not exceed this limit.
-      next if line.length - url_size <= 72
-
-      fail_commit(
-        commit,
-        'The commit body should not contain more than 72 characters per line'
-      )
-
-      failures = true
-    end
-
-    if !details && too_many_changed_lines?(commit)
-      fail_commit(
-        commit,
-        'Commits that change 30 or more lines across at least three files ' \
-          'must describe these changes in the commit body'
-      )
-
-      failures = true
-    end
-
-    if emoji_checker.includes_emoji?(commit.message)
-      fail_commit(
-        commit,
-        'Avoid the use of Markdown Emoji such as `:+1:`. ' \
-          'These add no value to the commit message, ' \
-          'and are displayed as plain text outside of GitLab'
-      )
-
-      failures = true
-    end
-
-    if commit.message.match?(unicode_emoji_regex)
-      fail_commit(
-        commit,
-        'Avoid the use of Unicode Emoji. ' \
-          'These add no value to the commit message, ' \
-          'and may not be displayed properly everywhere'
-      )
-
-      failures = true
-    end
-
-    if commit.message.match?(%r(([\w\-\/]+)?(#|!|&|%)\d+\b))
-      fail_commit(
-        commit,
-        'Use full URLs instead of short references ' \
-          '(`gitlab-org/gitlab-ce#123` or `!123`), as short references are ' \
-          'displayed as plain text outside of GitLab'
-      )
-
-      failures = true
-    end
+    next if line.length <= 72
+
+    url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length }
+
+    # If the line includes a URL, we'll allow it to exceed 72 characters, but
+    # only if the line _without_ the URL does not exceed this limit.
+    next if line.length - url_size <= 72
+
+    fail_commit(
+      commit,
+      'The commit body should not contain more than 72 characters per line'
+    )
+
+    failures = true
+  end
+
+  if !details && too_many_changed_lines?(commit)
+    fail_commit(
+      commit,
+      'Commits that change 30 or more lines across at least three files ' \
+        'must describe these changes in the commit body'
+    )
+
+    failures = true
+  end
+
+  if emoji_checker.includes_emoji?(commit.message)
+    fail_commit(
+      commit,
+      'Avoid the use of Markdown Emoji such as `:+1:`. ' \
+        'These add no value to the commit message, ' \
+        'and are displayed as plain text outside of GitLab'
+    )
+
+    failures = true
+  end
+
+  if commit.message.match?(unicode_emoji_regex)
+    fail_commit(
+      commit,
+      'Avoid the use of Unicode Emoji. ' \
+        'These add no value to the commit message, ' \
+        'and may not be displayed properly everywhere'
+    )
+
+    failures = true
+  end
+
+  if commit.message.match?(%r(([\w\-\/]+)?(#|!|&|%)\d+\b))
+    fail_commit(
+      commit,
+      'Use full URLs instead of short references ' \
+        '(`gitlab-org/gitlab-ce#123` or `!123`), as short references are ' \
+        'displayed as plain text outside of GitLab'
+    )
+
+    failures = true
+  end
+
+  failures
+end
+
+def lint_commits(commits)
+  failed = commits.reject do |commit|
+    lint_commit(commit)
   end
 
-  if failures
+  if failed.any?
     markdown(<<~MARKDOWN)
       ## Commit message standards