From 56cc11416ebdf52dd299d11424fb04fda42fa63f Mon Sep 17 00:00:00 2001 From: rnveach Date: Thu, 6 Oct 2016 19:54:00 -0400 Subject: [PATCH] Issue #3488: save files into cache with no un-suppressed violations --- .../puppycrawl/tools/checkstyle/Checker.java | 11 ++-- .../tools/checkstyle/PropertyCacheFile.java | 8 +++ .../tools/checkstyle/CheckerTest.java | 51 +++++++++++++++++++ .../tools/checkstyle/suppress_all.xml | 9 ++++ 4 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/suppress_all.xml diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java b/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java index 396d91e14..7b50f2a55 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java @@ -271,13 +271,13 @@ public class Checker extends AutomaticBean implements MessageDispatcher, RootMod || !acceptFileStarted(fileName)) { continue; } + if (cache != null) { + cache.put(fileName, timestamp); + } fireFileStarted(fileName); final SortedSet fileMessages = processFile(file); fireErrors(fileName, fileMessages); fireFileFinished(fileName); - if (cache != null && fileMessages.isEmpty()) { - cache.put(fileName, timestamp); - } } // -@cs[IllegalCatch] There is no other way to deliver filename that was under // processing. See https://github.com/checkstyle/checkstyle/issues/2285 @@ -352,14 +352,19 @@ public class Checker extends AutomaticBean implements MessageDispatcher, RootMod @Override public void fireErrors(String fileName, SortedSet errors) { final String stripped = CommonUtils.relativizeAndNormalizePath(basedir, fileName); + boolean hasNonFilteredViolations = false; for (final LocalizedMessage element : errors) { final AuditEvent event = new AuditEvent(this, stripped, element); if (filters.accept(event)) { + hasNonFilteredViolations = true; for (final AuditListener listener : listeners) { listener.addError(event); } } } + if (hasNonFilteredViolations && cache != null) { + cache.remove(fileName); + } } /** diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java index 4d998f308..9fcd55053 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java @@ -200,6 +200,14 @@ final class PropertyCacheFile { return details.getProperty(name); } + /** + * Removed a specific file from the cache. + * @param checkedFileName The name of the file to remove. + */ + public void remove(String checkedFileName) { + details.remove(checkedFileName); + } + /** * Calculates the hashcode for the serializable object based on its content. * @param object serializable object. diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java index 607a3b3a0..b2a29cc42 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java @@ -22,12 +22,14 @@ package com.puppycrawl.tools.checkstyle; import static org.hamcrest.CoreMatchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.when; import java.io.File; +import java.io.FileInputStream; import java.io.IOError; import java.io.IOException; import java.io.UnsupportedEncodingException; @@ -36,6 +38,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Properties; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -740,6 +743,38 @@ public class CheckerTest extends BaseCheckTestSupport { verify(checker, filePath, filePath, expected); } + @Test + public void testCacheOnViolationSuppression() throws Exception { + final File cacheFile = temporaryFolder.newFile(); + final DefaultConfiguration violationCheck = + createCheckConfig(DummyFileSetViolationCheck.class); + final DefaultConfiguration defaultConfig = new DefaultConfiguration("defaultConfiguration"); + defaultConfig.addAttribute("cacheFile", cacheFile.getPath()); + defaultConfig.addChild(violationCheck); + + final DefaultConfiguration filterConfig = createCheckConfig(SuppressionFilter.class); + filterConfig.addAttribute("file", getPath("suppress_all.xml")); + defaultConfig.addChild(filterConfig); + + final Checker checker = new Checker(); + checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader()); + checker.addListener(new BriefUtLogger(stream)); + checker.configure(defaultConfig); + + final String fileViolationPath = temporaryFolder.newFile("ViolationFile.java").getPath(); + final String[] expected = CommonUtils.EMPTY_STRING_ARRAY; + + verify(checker, fileViolationPath, expected); + + try (FileInputStream input = new FileInputStream(cacheFile)) { + final Properties details = new Properties(); + details.load(input); + + assertNotNull("suppressed violation file saved in cache", + details.getProperty(fileViolationPath)); + } + } + private Checker createMockCheckerWithCacheForModule( Class mockClass) throws IOException, CheckstyleException { @@ -778,6 +813,22 @@ public class CheckerTest extends BaseCheckTestSupport { } } + private static class DummyFileSetViolationCheck extends AbstractFileSetCheck + implements ExternalResourceHolder { + + @Override + protected void processFiltered(File file, List lines) throws CheckstyleException { + log(0, "test"); + } + + @Override + public Set getExternalResourceLocations() { + final Set externalResourceLocation = new HashSet<>(1); + externalResourceLocation.add("non_existing_external_resource.xml"); + return externalResourceLocation; + } + } + private static class MockMissingExternalResourcesFileSetCheck extends AbstractFileSetCheck implements ExternalResourceHolder { diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/suppress_all.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/suppress_all.xml new file mode 100644 index 000000000..8e75cc951 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/suppress_all.xml @@ -0,0 +1,9 @@ + + + + + + +