Issue #5057: fixed xml violations/exceptions with file started/finished

8 files changed, 42 insertions, 23 deletions

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/XMLLogger.java b/src/main/java/com/puppycrawl/tools/checkstyle/XMLLogger.java
index 86587bf6e..10037c524 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/XMLLogger.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/XMLLogger.java
@@ -121,8 +121,6 @@ public class XMLLogger
 
     @Override
     public void auditFinished(AuditEvent event) {
-        fileMessages.forEach(this::writeFileMessages);
-
         writer.println("</checkstyle>");
         if (closeStream) {
             writer.close();
@@ -186,14 +184,13 @@ public class XMLLogger
     public void addError(AuditEvent event) {
         if (event.getSeverityLevel() != SeverityLevel.IGNORE) {
             final String fileName = event.getFileName();
-            if (fileName == null) {
+            if (fileName == null || !fileMessages.containsKey(fileName)) {
                 synchronized (writerLock) {
                     writeFileError(event);
                 }
             }
             else {
-                final FileMessages messages = fileMessages.computeIfAbsent(
-                        fileName, name -> new FileMessages());
+                final FileMessages messages = fileMessages.get(fileName);
                 messages.addError(event);
             }
         }
@@ -227,14 +224,13 @@ public class XMLLogger
     @Override
     public void addException(AuditEvent event, Throwable throwable) {
         final String fileName = event.getFileName();
-        if (fileName == null) {
+        if (fileName == null || !fileMessages.containsKey(fileName)) {
             synchronized (writerLock) {
                 writeException(throwable);
             }
         }
         else {
-            final FileMessages messages = fileMessages.computeIfAbsent(
-                    fileName, name -> new FileMessages());
+            final FileMessages messages = fileMessages.get(fileName);
             messages.addException(throwable);
         }
     }
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java
index c52d109e4..76601aeae 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java
@@ -47,6 +47,7 @@ import com.puppycrawl.tools.checkstyle.Definitions;
 import com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck;
 import com.puppycrawl.tools.checkstyle.api.FileText;
 import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
+import com.puppycrawl.tools.checkstyle.api.MessageDispatcher;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
 
 /**
@@ -325,8 +326,11 @@ public class TranslationCheck extends AbstractFileSetCheck {
      * @param fileName file name.
      */
     private void logMissingTranslation(String filePath, String fileName) {
+        final MessageDispatcher dispatcher = getMessageDispatcher();
+        dispatcher.fireFileStarted(filePath);
         log(0, MSG_KEY_MISSING_TRANSLATION_FILE, fileName);
         fireErrors(filePath);
+        dispatcher.fireFileFinished(filePath);
     }
 
     /**
@@ -453,6 +457,9 @@ public class TranslationCheck extends AbstractFileSetCheck {
     private void checkFilesForConsistencyRegardingTheirKeys(SetMultimap<File, String> fileKeys,
                                                             Set<String> keysThatMustExist) {
         for (File currentFile : fileKeys.keySet()) {
+            final MessageDispatcher dispatcher = getMessageDispatcher();
+            final String path = currentFile.getPath();
+            dispatcher.fireFileStarted(path);
             final Set<String> currentFileKeys = fileKeys.get(currentFile);
             final Set<String> missingKeys = keysThatMustExist.stream()
                 .filter(e -> !currentFileKeys.contains(e)).collect(Collectors.toSet());
@@ -461,9 +468,8 @@ public class TranslationCheck extends AbstractFileSetCheck {
                     log(0, MSG_KEY, key);
                 }
             }
-            final String path = currentFile.getPath();
             fireErrors(path);
-            getMessageDispatcher().fireFileFinished(path);
+            dispatcher.fireFileFinished(path);
         }
     }
 
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/AbstractXmlTestSupport.java b/src/test/java/com/puppycrawl/tools/checkstyle/AbstractXmlTestSupport.java
index 75973d21c..9387ec28b 100644
--- a/src/test/java/com/puppycrawl/tools/checkstyle/AbstractXmlTestSupport.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/AbstractXmlTestSupport.java
@@ -117,17 +117,19 @@ public abstract class AbstractXmlTestSupport extends AbstractModuleTestSupport {
     private static void verifyXmlNode(Node expected, Node actual, String path,
             BiPredicate<Node, Node> ordered) {
         if (expected == null) {
-            Assert.assertNull("no node should exist: " + path, actual);
+            if (actual != null) {
+                Assert.fail("no node should exist: " + path + actual.getNodeName() + "/");
+            }
         }
         else {
-            Assert.assertNotNull("node should exist: " + path, actual);
-            Assert.assertEquals("node should have same name: " + path, expected.getNodeName(),
+            final String newPath = path + expected.getNodeName() + "/";
+
+            Assert.assertNotNull("node should exist: " + newPath, actual);
+            Assert.assertEquals("node should have same name: " + newPath, expected.getNodeName(),
                     actual.getNodeName());
-            Assert.assertEquals("node should have same type: " + path, expected.getNodeType(),
+            Assert.assertEquals("node should have same type: " + newPath, expected.getNodeType(),
                     actual.getNodeType());
 
-            final String newPath = path + expected.getNodeName() + "/";
-
             verifyXmlAttributes(expected.getAttributes(), actual.getAttributes(), newPath);
 
             verifyXmlNodes(expected, actual, newPath, ordered);
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java
index 5d3a7ffd2..cfb14357d 100644
--- a/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java
@@ -146,6 +146,7 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
         logger.auditStarted(null);
         final AuditEvent ev = new AuditEvent(this, "Test.java");
         logger.fileStarted(ev);
+        logger.fileFinished(ev);
         logger.auditFinished(null);
         verifyXml(getPath("ExpectedXMLLogger.xml"), outStream);
     }
@@ -170,7 +171,9 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
                 "messages.properties", "key", null, SeverityLevel.ERROR, null,
                     getClass(), null);
         final AuditEvent ev = new AuditEvent(this, "Test.java", message);
+        logger.fileStarted(ev);
         logger.addError(ev);
+        logger.fileFinished(ev);
         logger.auditFinished(null);
         verifyXml(getPath("ExpectedXMLLoggerError.xml"), outStream, message.getMessage());
     }
@@ -213,7 +216,9 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
                         "messages.properties", "key", null, SeverityLevel.ERROR, null,
                         getClass(), null);
         final AuditEvent ev = new AuditEvent(this, "Test.java", message);
+        logger.fileStarted(ev);
         logger.addError(ev);
+        logger.fileFinished(ev);
         logger.auditFinished(null);
         verifyXml(getPath("ExpectedXMLLoggerErrorZeroColumn.xml"), outStream,
                 message.getMessage());
@@ -278,8 +283,9 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
         final AuditEvent ev = new AuditEvent(this, "Test.java", message);
         logger.addException(ev, new TestException("msg", new RuntimeException("msg")));
 
+        logger.fileFinished(ev);
         logger.auditFinished(null);
-        verifyXml(getPath("ExpectedXMLLoggerException.xml"), outStream);
+        verifyXml(getPath("ExpectedXMLLoggerException2.xml"), outStream);
         assertEquals("Invalid close count", 1, outStream.getCloseCount());
     }
 
@@ -296,7 +302,7 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
         final AuditEvent fileFinishedEvent = new AuditEvent(this, "Test.java");
         logger.fileFinished(fileFinishedEvent);
         logger.auditFinished(null);
-        verifyXml(getPath("ExpectedXMLLoggerException.xml"), outStream);
+        verifyXml(getPath("ExpectedXMLLoggerException3.xml"), outStream);
         assertEquals("Invalid close count", 1, outStream.getCloseCount());
     }
 
@@ -315,7 +321,7 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
         final AuditEvent fileFinishedEvent = new AuditEvent(this, "Test.java");
         logger.fileFinished(fileFinishedEvent);
         logger.auditFinished(null);
-        verifyXml(getPath("ExpectedXMLLoggerException.xml"), outStream);
+        verifyXml(getPath("ExpectedXMLLoggerException2.xml"), outStream);
         assertEquals("Invalid close count", 1, outStream.getCloseCount());
     }
 
@@ -333,6 +339,7 @@ public class XMLLoggerTest extends AbstractXmlTestSupport {
         final AuditEvent errorEvent = new AuditEvent(this, "Test.java", message);
         logger.addError(errorEvent);
 
+        logger.fileFinished(errorEvent);
         logger.auditFinished(null);
         verifyXml(getPath("ExpectedXMLLoggerError.xml"), outStream, message.getMessage());
     }
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerErrorModuleId.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerErrorModuleId.xml
index dc78a6df5..cd3bec8ed 100644
--- a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerErrorModuleId.xml
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerErrorModuleId.xml
@@ -1,6 +1,4 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <checkstyle version="">
-<file name="Test.java">
 <error line="1" column="1" severity="error" message="$0" source="module"/>
-</file>
 </checkstyle>
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException.xml
index 9f125a0da..0a5b431d7 100644
--- a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException.xml
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException.xml
@@ -1,6 +1,4 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <checkstyle version="">
-<file name="Test.java">
 &lt;exception&gt;&#10;&lt;![CDATA[&#10;stackTrace&#10;example]]&gt;&#10;&lt;/exception&gt;&#10;
-</file>
 </checkstyle>
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException2.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException2.xml
new file mode 100644
index 000000000..9f125a0da
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException2.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<checkstyle version="">
+<file name="Test.java">
+&lt;exception&gt;&#10;&lt;![CDATA[&#10;stackTrace&#10;example]]&gt;&#10;&lt;/exception&gt;&#10;
+</file>
+</checkstyle>
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException3.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException3.xml
new file mode 100644
index 000000000..a28d5156f
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException3.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<checkstyle version="">
+&lt;exception&gt;&#10;&lt;![CDATA[&#10;stackTrace&#10;example]]&gt;&#10;&lt;/exception&gt;&#10;
+<file name="Test.java">
+</file>
+</checkstyle>