From b167797ef03e9ef5bbf0de24b4337d7e808dc77f Mon Sep 17 00:00:00 2001
From: Mohanad <mohanadkhaled87@gmail.com>
Date: Sat, 2 Nov 2024 01:57:12 +0200
Subject: [PATCH] Issue #11584: WriteTag reports violation with confusing
 message when there is no javadoc

---
 config/jsoref-spellchecker/whitelist.words    |  1 +
 .../checks/javadoc/WriteTagCheck.java         |  6 +-
 .../meta/checks/javadoc/WriteTagCheck.xml     |  1 +
 .../checks/javadoc/WriteTagCheckTest.java     | 99 +++++++++++++++++--
 .../InputWriteTagRecordsAndCompactCtors.java  | 10 +-
 .../writetag/InputWriteTagNoJavadoc.java      |  2 +-
 .../writetag/InputWriteTagResetSeverity.java  | 32 ++++++
 .../checks/javadoc/writetag/Example1.txt      |  2 +
 .../checks/javadoc/writetag/Example2.txt      |  2 +
 .../checks/javadoc/writetag/Example3.txt      |  2 +
 .../checks/javadoc/writetag/Example4.txt      |  2 +
 src/xdocs/checks/javadoc/writetag.xml         |  9 ++
 .../checks/javadoc/writetag.xml.template      |  1 +
 13 files changed, 151 insertions(+), 18 deletions(-)
 create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagResetSeverity.java

diff --git a/config/jsoref-spellchecker/whitelist.words b/config/jsoref-spellchecker/whitelist.words
index 61869c514be..b4635841172 100644
--- a/config/jsoref-spellchecker/whitelist.words
+++ b/config/jsoref-spellchecker/whitelist.words
@@ -840,6 +840,7 @@ mockit
 mockito
 modifiedcontrolvariable
 modifierorder
+Mohanad
 Mohnen
 mojohaus
 moks
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java
index 7e520888580..735ffc211ef 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java
@@ -36,6 +36,7 @@
  * Requires user defined Javadoc tag to be present in Javadoc comment with defined format.
  * To define the format for a tag, set property tagFormat to a regular expression.
  * Property tagSeverity is used for severity of events when the tag exists.
+ * No violation reported in case there is no javadoc.
  * </div>
  *
  * <ul>
@@ -197,10 +198,7 @@ public void visitToken(DetailAST ast) {
         final int lineNo = ast.getLineNo();
         final TextBlock cmt =
             contents.getJavadocBefore(lineNo);
-        if (cmt == null) {
-            log(lineNo, MSG_MISSING_TAG, tag);
-        }
-        else {
+        if (cmt != null) {
             checkTag(lineNo, cmt.getText());
         }
     }
diff --git a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/WriteTagCheck.xml b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/WriteTagCheck.xml
index 03ba6e22070..dc233585ed6 100644
--- a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/WriteTagCheck.xml
+++ b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/WriteTagCheck.xml
@@ -8,6 +8,7 @@
  Requires user defined Javadoc tag to be present in Javadoc comment with defined format.
  To define the format for a tag, set property tagFormat to a regular expression.
  Property tagSeverity is used for severity of events when the tag exists.
+ No violation reported in case there is no javadoc.
  &lt;/div&gt;</description>
          <properties>
             <property name="tag" type="java.lang.String">
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheckTest.java
index 54f1db054cb..4f218df8eda 100644
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheckTest.java
@@ -25,19 +25,31 @@
 import static com.puppycrawl.tools.checkstyle.checks.javadoc.WriteTagCheck.MSG_WRITE_TAG;
 
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.InputStreamReader;
 import java.io.LineNumberReader;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.junit.jupiter.api.Test;
 
+import com.puppycrawl.tools.checkstyle.AbstractAutomaticBean;
 import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
 import com.puppycrawl.tools.checkstyle.Checker;
+import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+import com.puppycrawl.tools.checkstyle.DefaultLogger;
+import com.puppycrawl.tools.checkstyle.PackageObjectFactory;
+import com.puppycrawl.tools.checkstyle.TreeWalker;
+import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
+import de.thetaphi.forbiddenapis.SuppressForbidden;
 
 /**
  * Unit test for WriteTagCheck.
@@ -130,6 +142,83 @@ public void testSeverity() throws Exception {
                 getPath("InputWriteTagSeverity.java"), expected);
     }
 
+    /**
+     * Reason for low level testing:
+     * There is no direct way to fetch severity level directly.
+     * This is an exceptional case in which the logs are fetched indirectly using default
+     * logger listener in order to check for the severity level being reset by logging twice.
+     * First log should be the tag's severity level then it should reset the severity level back to
+     * error which is then checked upon using the log's severity level.
+     * This test needs to use a forbidden api {@code ByteArrayOutputStream#toString()}
+     * to get the logs as a string from the output stream
+     */
+    @Test
+    @SuppressForbidden
+    public void testResetSeverityLevel() throws Exception {
+
+        final Checker checker = new Checker();
+
+        final TreeWalker treeWalker = new TreeWalker();
+        final PackageObjectFactory factory = new PackageObjectFactory(
+                new HashSet<>(), Thread.currentThread().getContextClassLoader());
+
+        treeWalker.setModuleFactory(factory);
+        treeWalker.finishLocalSetup();
+
+        final DefaultConfiguration writeTagConfig = createModuleConfig(WriteTagCheck.class);
+        writeTagConfig.addProperty("tag", "@author");
+        writeTagConfig.addProperty("tagFormat", "Mohanad");
+        writeTagConfig.addProperty("tagSeverity", "warning");
+
+        treeWalker.setupChild(writeTagConfig);
+
+        checker.addFileSetCheck(treeWalker);
+
+        final ByteArrayOutputStream out = TestUtil.getInternalState(this, "stream");
+        final DefaultLogger logger = new DefaultLogger(out,
+                AbstractAutomaticBean.OutputStreamOptions.CLOSE);
+        checker.addListener(logger);
+
+        execute(checker, getPath("InputWriteTagResetSeverity.java"));
+
+        final String output = out.toString();
+
+        // logs severity levels are between square brackets []
+        final Pattern severityPattern = Pattern.compile("\\[(ERROR|WARN|INFO|IGNORE)]");
+
+        final Matcher matcher = severityPattern.matcher(output);
+
+        // First log is just the normal tag one
+        final boolean firstMatchFound = matcher.find();
+        assertWithMessage("Severity level should be wrapped in a square bracket []")
+                .that(firstMatchFound)
+                .isTrue();
+
+        final String tagExpectedSeverityLevel = "warn";
+        final String firstSeverityLevel = matcher.group(1).toLowerCase(Locale.ENGLISH);
+
+        assertWithMessage("First log should have an error severity level")
+                .that(firstSeverityLevel)
+                .isEqualTo(tagExpectedSeverityLevel);
+
+        // Now we check for the second log which should log error  if
+        // the previous log did not have an issue while resetting the original severity level
+        final boolean secondMatchFound = matcher.find();
+        assertWithMessage("Severity level should be wrapped in a square bracket []")
+                .that(secondMatchFound)
+                .isTrue();
+
+        final String expectedSeverityLevelAfterReset = "error";
+
+        final String secondSeverityLevel = matcher.group(1).toLowerCase(Locale.ENGLISH);
+
+        assertWithMessage("Second violation's severity level"
+                + " should have been reset back to default (error)")
+                .that(secondSeverityLevel)
+                .isEqualTo(expectedSeverityLevelAfterReset);
+
+    }
+
     @Test
     public void testIgnoreMissing() throws Exception {
         final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
@@ -170,27 +259,21 @@ public void testEnumsAndAnnotations() throws Exception {
 
     @Test
     public void testNoJavadocs() throws Exception {
-        final String[] expected = {
-            "13: " + getCheckMessage(MSG_MISSING_TAG, "null"),
-        };
+        final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
+
         verifyWithInlineConfigParserTwice(getPath("InputWriteTagNoJavadoc.java"), expected);
     }
 
     @Test
     public void testWriteTagRecordsAndCompactCtors() throws Exception {
         final String[] expected = {
-            "15: " + getCheckMessage(MSG_MISSING_TAG, "@incomplete"),
             "19: " + getCheckMessage(MSG_TAG_FORMAT, "@incomplete", "\\S"),
             "26: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
                     "Failed to recognize 'record' introduced in Java 14."),
-            "33: " + getCheckMessage(MSG_MISSING_TAG, "@incomplete"),
             "37: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
                     "Failed to recognize 'record' introduced in Java 14."),
-            "44: " + getCheckMessage(MSG_MISSING_TAG, "@incomplete"),
             "48: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
                     "Failed to recognize 'record' introduced in Java 14."),
-            "56: " + getCheckMessage(MSG_MISSING_TAG, "@incomplete"),
-            "58: " + getCheckMessage(MSG_MISSING_TAG, "@incomplete"),
             "62: " + getCheckMessage(MSG_WRITE_TAG, "@incomplete",
                     "Failed to recognize 'record' introduced in Java 14."),
         };
diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagRecordsAndCompactCtors.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagRecordsAndCompactCtors.java
index 4bdf8d1b28c..23ca41de870 100644
--- a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagRecordsAndCompactCtors.java
+++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagRecordsAndCompactCtors.java
@@ -12,7 +12,7 @@
 package com.puppycrawl.tools.checkstyle.checks.javadoc.writetag;
 
 
-public class InputWriteTagRecordsAndCompactCtors { // violation 'missing @incomplete tag.'
+public class InputWriteTagRecordsAndCompactCtors {
 
     // violation 2 lines below 'Type Javadoc tag @incomplete must match pattern '\\S''
     /**
@@ -30,7 +30,7 @@ record MyRecord1() {
     }
 
 
-    record MyRecord2(String myString) { // violation 'missing @incomplete tag.'
+    record MyRecord2(String myString) {
 
         // violation 2 lines below 'Failed to recognize 'record' introduced in Java 14.'
         /**
@@ -41,7 +41,7 @@ record MyRecord2(String myString) { // violation 'missing @incomplete tag.'
     }
 
 
-    record MyRecord3(int x) { // violation 'Type Javadoc comment is missing @incomplete tag.*'
+    record MyRecord3(int x) {
 
         // violation 2 lines below 'Failed to recognize 'record' introduced in Java 14.'
         /**
@@ -53,9 +53,9 @@ record MyRecord3(int x) { // violation 'Type Javadoc comment is missing @incompl
     }
 
 
-    record MyRecord4(int y) { // violation 'Type Javadoc comment is missing @incomplete tag.*'
+    record MyRecord4(int y) {
 
-        private record MyRecord5(int z) { // violation 'missing @incomplete tag.'
+        private record MyRecord5(int z) {
 
             // violation 2 lines below 'Failed to recognize 'record' introduced in Java 14.'
             /**
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagNoJavadoc.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagNoJavadoc.java
index 9af2bfcb647..2dc6ab615b5 100644
--- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagNoJavadoc.java
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagNoJavadoc.java
@@ -10,7 +10,7 @@
 
 package com.puppycrawl.tools.checkstyle.checks.javadoc.writetag;
 
-class InputWriteTagNoJavadoc // violation 'Type Javadoc comment is missing null tag.'
+class InputWriteTagNoJavadoc
 {
     public void method()
     {
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagResetSeverity.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagResetSeverity.java
new file mode 100644
index 00000000000..d76aa206c91
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/InputWriteTagResetSeverity.java
@@ -0,0 +1,32 @@
+package com.puppycrawl.tools.checkstyle.checks.javadoc.writetag;
+
+/**
+ * Testing tag writing
+ * @author Mohanad
+ * @author Another Author
+ * @incomplete This class needs more code...
+ * @doubletag first text
+ * @doubletag second text
+ * @emptytag
+ */
+class InputWriteTagResetSeverity
+{
+    /**
+     * @todo Add a constructor comment
+     */
+    public InputWriteTagResetSeverity()
+    {
+    }
+
+    public void method()
+    {
+    }
+
+    /**
+     * @todo Add a comment
+     */
+    public void anotherMethod()
+    {
+    }
+}
+
diff --git a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example1.txt b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example1.txt
index 52eb80f11a3..8ecbbcb4479 100644
--- a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example1.txt
+++ b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example1.txt
@@ -13,5 +13,7 @@
 public class Test {
   /** some doc */
   void foo() {}
+
+  public void method() {}
 }
 // xdoc section -- end
diff --git a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example2.txt b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example2.txt
index 93295b686bb..9afcb0e2354 100644
--- a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example2.txt
+++ b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example2.txt
@@ -15,5 +15,7 @@
 public class Test { // violation as required tag is missed
   /** some doc */
   void foo() {} // OK, as methods are not checked by default
+
+  public void method() {}
 }
 // xdoc section -- end
diff --git a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example3.txt b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example3.txt
index 596903afe42..8a3c05fa62e 100644
--- a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example3.txt
+++ b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example3.txt
@@ -17,5 +17,7 @@
 public class Test { // violation as required tag is missed
   /** some doc */
   void foo() {} // violation as required tag is missed
+
+  public void method() {}
 }
 // xdoc section -- end
diff --git a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example4.txt b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example4.txt
index e46c127acc3..8ece7f87b7f 100644
--- a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example4.txt
+++ b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/writetag/Example4.txt
@@ -22,5 +22,7 @@ public class Test {
   * @since violation
   */
   void foo() {}
+
+  public void method() {}
 }
 // xdoc section -- end
diff --git a/src/xdocs/checks/javadoc/writetag.xml b/src/xdocs/checks/javadoc/writetag.xml
index ad9c9da263b..8be7fe99650 100644
--- a/src/xdocs/checks/javadoc/writetag.xml
+++ b/src/xdocs/checks/javadoc/writetag.xml
@@ -13,6 +13,7 @@
          Requires user defined Javadoc tag to be present in Javadoc comment with
          defined format. To define the format for a tag, set property tagFormat to a regular
          expression. Property tagSeverity is used for severity of events when the tag exists.
+         No violation reported in case there is no javadoc.
         </div>
       </subsection>
 
@@ -113,6 +114,8 @@
 public class Test {
   /** some doc */
   void foo() {}
+
+  public void method() {}
 }
         </source>
         <p id="Example2-config">
@@ -138,6 +141,8 @@ public class Test {
 public class Test { // violation as required tag is missed
   /** some doc */
   void foo() {} // OK, as methods are not checked by default
+
+  public void method() {}
 }
         </source>
         <p id="Example3-config">
@@ -165,6 +170,8 @@ public class Test { // violation as required tag is missed
 public class Test { // violation as required tag is missed
   /** some doc */
   void foo() {} // violation as required tag is missed
+
+  public void method() {}
 }
         </source>
         <p id="Example4-config">
@@ -199,6 +206,8 @@ public class Test {
   * @since violation
   */
   void foo() {}
+
+  public void method() {}
 }
         </source>
       </subsection>
diff --git a/src/xdocs/checks/javadoc/writetag.xml.template b/src/xdocs/checks/javadoc/writetag.xml.template
index 6d9dc2af351..251d7073445 100644
--- a/src/xdocs/checks/javadoc/writetag.xml.template
+++ b/src/xdocs/checks/javadoc/writetag.xml.template
@@ -13,6 +13,7 @@
          Requires user defined Javadoc tag to be present in Javadoc comment with
          defined format. To define the format for a tag, set property tagFormat to a regular
          expression. Property tagSeverity is used for severity of events when the tag exists.
+         No violation reported in case there is no javadoc.
         </div>
       </subsection>