Issue checkstyle#11584: WriteTag reports violation with confusing mes…

…sage when there is no javadoc

config/jsoref-spellchecker/whitelist.words

@@ -840,6 +840,7 @@ mockit
 mockito
 modifiedcontrolvariable
 modifierorder
+Mohanad
 Mohnen
 mojohaus
 moks

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());
         }
     }

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">

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."),
         };

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.'
             /**

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()
     {

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()
+    {
+    }
+}
+

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

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

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

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

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>

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>