From 53601bbe4b6e6272cc403337a6977f7941a06f79 Mon Sep 17 00:00:00 2001
From: Zhao Song <zsong@openjdk.org>
Date: Mon, 3 Jun 2024 18:10:15 +0000
Subject: [PATCH] 2272: Improve issuestitleCheck

Reviewed-by: erikj
---
 .../org/openjdk/skara/bots/pr/CheckTests.java |  4 +--
 .../skara/jcheck/IssuesTitleCheck.java        | 28 +++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
index 1461c0e14..26e53c152 100644
--- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
+++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
@@ -3446,7 +3446,7 @@ void issuesTitleCheck(TestInfo testInfo) throws IOException {
             var issue2 = issues.createIssue("this is an issue2 etc.", List.of("Hello"), Map.of());
             pr.addComment("/issue add " + issue2.id());
             TestBotRunner.runPeriodicItems(prBot);
-            assertTrue(pr.store().body().contains("Found trailing period in issue title for `2: this is an issue2 etc.`"));
+            assertFalse(pr.store().body().contains("Found trailing period in issue title for `2: this is an issue2 etc.`"));
             assertTrue(pr.store().body().contains("Found leading lowercase letter in issue title for `2: this is an issue2 etc.`"));
 
             // Change the leading letter to upper case
@@ -3455,7 +3455,7 @@ void issuesTitleCheck(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(prBot);
             // The additional issue marker should be updated, so the warning of leading lowercase letter no longer exists
             assertFalse(pr.store().body().contains("Found leading lowercase letter in issue title for `2: this is an issue2 etc.`"));
-            assertTrue(pr.store().body().contains("Found trailing period in issue title for `2: This is an issue2 etc.`"));
+            assertFalse(pr.store().body().contains("Found trailing period in issue title for `2: This is an issue2 etc.`"));
 
             // Approve it as Reviewer, warnings shouldn't prevent adding ready label to the pr
             var reviewerPr = reviewer.pullRequest(pr.id());
diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesTitleCheck.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesTitleCheck.java
index 8a299d5ec..06b83b1be 100644
--- a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesTitleCheck.java
+++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesTitleCheck.java
@@ -28,10 +28,14 @@
 
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 
 public class IssuesTitleCheck extends CommitCheck {
     private final Logger log = Logger.getLogger("org.openjdk.skara.jcheck.issuesTitle");
+    private final static List<String> VALID_WORD_WITH_TRAILING_PERIOD = List.of("et al.", "etc.", "...");
+    private final static Pattern FILE_OR_FUNCTION_PATTERN = Pattern.compile(".*[()/._:].*");
 
     @Override
     Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration conf, Census census) {
@@ -48,10 +52,10 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
         var issuesWithLeadingLowerCaseLetter = new ArrayList<String>();
 
         for (var issue : message.issues()) {
-            if (issue.description().endsWith(".")) {
+            if (hasTrailingPeriod(issue.description())) {
                 issuesWithTrailingPeriod.add("`" + issue + "`");
             }
-            if (Character.isLowerCase(issue.description().charAt(0))) {
+            if (hasLeadingLowerCaseLetter(issue.description())) {
                 issuesWithLeadingLowerCaseLetter.add("`" + issue + "`");
             }
         }
@@ -71,4 +75,24 @@ public String description() {
         return "Issue's title should be properly formatted";
     }
 
+    private boolean hasTrailingPeriod(String description) {
+        if (!description.endsWith(".")) {
+            return false;
+        }
+        for (String phrase : VALID_WORD_WITH_TRAILING_PERIOD) {
+            if (description.endsWith(phrase)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private boolean hasLeadingLowerCaseLetter(String description) {
+        if (Character.isUpperCase(description.charAt(0))) {
+            return false;
+        }
+        var firstWord = description.split(" ")[0];
+        // If first word contains special character, it's very likely a reference to file or function, ignore it
+        return !FILE_OR_FUNCTION_PATTERN.matcher(firstWord).matches();
+    }
 }