Issue checkstyle#4908: Remove thread-unsafe context from AbstractCheck

soon · Aug 8, 2017 · 0eefdc6 · 0eefdc6
1 parent 70761aa
commit 0eefdc6
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 19 deletions.
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractCheck.java
@@ -42,11 +42,8 @@ public abstract class AbstractCheck extends AbstractViolationReporter {
     /** The tokens the check is interested in. */
     private final Set<String> tokens = new HashSet<>();
 
-    /** The sorted set for collecting messages. */
-    private final SortedSet<LocalizedMessage> messages = new TreeSet<>();
-
-    /** The current file contents. */
-    private FileContents fileContents;
+    /** The check context. */
+    private static final ThreadLocal<Context> CONTEXT = ThreadLocal.withInitial(Context::new);
 
     /** The tab width for column reporting. */
     private int tabWidth = DEFAULT_TAB_WIDTH;
@@ -111,14 +108,14 @@ public final Set<String> getTokenNames() {
      * @return the sorted set of {@link LocalizedMessage}.
      */
     public SortedSet<LocalizedMessage> getMessages() {
-        return new TreeSet<>(messages);
+        return new TreeSet<>(CONTEXT.get().messages);
     }
 
     /**
      * Clears the sorted set of {@link LocalizedMessage} of the check.
      */
     public final void clearMessages() {
-        messages.clear();
+        CONTEXT.get().messages.clear();
     }
 
     /**
@@ -175,7 +172,7 @@ public void leaveToken(DetailAST ast) {
      * @return the file contents
      */
     public final String[] getLines() {
-        return fileContents.getLines();
+        return CONTEXT.get().fileContents.getLines();
     }
 
     /**
@@ -184,23 +181,23 @@ public final String[] getLines() {
      * @return the line from the file contents
      */
     public final String getLine(int index) {
-        return fileContents.getLine(index);
+        return CONTEXT.get().fileContents.getLine(index);
     }
 
     /**
      * Set the file contents associated with the tree.
      * @param contents the manager
      */
     public final void setFileContents(FileContents contents) {
-        fileContents = contents;
+        CONTEXT.get().fileContents = contents;
     }
 
     /**
      * Returns the file contents associated with the tree.
      * @return the file contents
      */
     public final FileContents getFileContents() {
-        return fileContents;
+        return CONTEXT.get().fileContents;
     }
 
     /**
@@ -249,7 +246,7 @@ public final void log(DetailAST ast, String key, Object... args) {
 
     @Override
     public final void log(int line, String key, Object... args) {
-        messages.add(
+        CONTEXT.get().messages.add(
             new LocalizedMessage(
                 line,
                 getMessageBundle(),
@@ -266,7 +263,7 @@ public final void log(int lineNo, int colNo, String key,
             Object... args) {
         final int col = 1 + CommonUtils.lengthExpandedTabs(
             getLines()[lineNo - 1], colNo, tabWidth);
-        messages.add(
+        CONTEXT.get().messages.add(
             new LocalizedMessage(
                 lineNo,
                 col,
@@ -278,4 +275,15 @@ public final void log(int lineNo, int colNo, String key,
                 getClass(),
                 getCustomMessages().get(key)));
     }
+
+    /**
+     * The actual context holder.
+     */
+    private static class Context {
+        /** The sorted set for collecting messages. */
+        private final SortedSet<LocalizedMessage> messages = new TreeSet<>();
+
+        /** The current file contents. */
+        private FileContents fileContents;
+    }
 }
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java
@@ -26,13 +26,11 @@
 import java.io.File;
 import java.nio.charset.Charset;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.mockito.internal.util.reflection.Whitebox;
 
 import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
 
@@ -220,11 +218,9 @@ public void testClearMessages() {
         final AbstractCheck check = new DummyAbstractCheck();
 
         check.log(0, "key", "args");
-        final Collection<LocalizedMessage> messages =
-                (Collection<LocalizedMessage>) Whitebox.getInternalState(check, "messages");
-        Assert.assertEquals("Invalid message size", 1, messages.size());
+        Assert.assertEquals("Invalid message size", 1, check.getMessages().size());
         check.clearMessages();
-        Assert.assertEquals("Invalid message size", 0, messages.size());
+        Assert.assertEquals("Invalid message size", 0, check.getMessages().size());
     }
 
     private static final class DummyAbstractCheck extends AbstractCheck {