diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PmdAnalysis.java b/pmd-core/src/main/java/net/sourceforge/pmd/PmdAnalysis.java index 1a32815a76..2234b0a660 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PmdAnalysis.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PmdAnalysis.java @@ -13,7 +13,9 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.logging.Logger; + +import org.slf4j.LoggerFactory; +import org.slf4j.event.Level; import net.sourceforge.pmd.Report.GlobalReportBuilderListener; import net.sourceforge.pmd.benchmark.TimeTracker; @@ -33,7 +35,6 @@ import net.sourceforge.pmd.util.ClasspathClassLoader; import net.sourceforge.pmd.util.IOUtil; import net.sourceforge.pmd.util.datasource.DataSource; import net.sourceforge.pmd.util.log.PmdLogger; -import net.sourceforge.pmd.util.log.PmdLogger.Level; import net.sourceforge.pmd.util.log.SimplePmdLogger; /** @@ -72,7 +73,7 @@ public final class PmdAnalysis implements AutoCloseable { private final List listeners = new ArrayList<>(); private final List ruleSets = new ArrayList<>(); private final PMDConfiguration configuration; - private final SimplePmdLogger logger = new SimplePmdLogger(Logger.getLogger("net.sourceforge.pmd")); + private final SimplePmdLogger logger = new SimplePmdLogger(LoggerFactory.getLogger(getClass())); /** * Constructs a new instance. The files paths (input files, filelist, @@ -199,6 +200,7 @@ public final class PmdAnalysis implements AutoCloseable { GlobalAnalysisListener listener; try { + @SuppressWarnings("PMD.CloseResource") AnalysisCacheListener cacheListener = new AnalysisCacheListener( configuration.getAnalysisCache(), rulesets, @@ -216,22 +218,14 @@ public final class PmdAnalysis implements AutoCloseable { } try (TimedOperation ignored = TimeTracker.startOperation(TimedOperationCategory.FILE_PROCESSING)) { - // todo Just like we throw for invalid properties, "broken rules" - // shouldn't be a "config error". This is the only instance of - // config errors... - - if (isEmpty(this.ruleSets)) { - if (!configuration.getRuleSetPaths().isEmpty()) { - logger.error("No rules found. Maybe you misspelled a rule name? ({0})", - String.join(",", configuration.getRuleSetPaths())); - - } else { - logger.error("No rules found."); - } + if (checkNoRulesRegistered()) { return; } for (final Rule rule : removeBrokenRules(rulesets)) { + // todo Just like we throw for invalid properties, "broken rules" + // shouldn't be a "config error". This is the only instance of + // config errors... listener.onConfigError(new Report.ConfigurationError(rule, rule.dysfunctionReason())); } @@ -250,6 +244,20 @@ public final class PmdAnalysis implements AutoCloseable { } } + private boolean checkNoRulesRegistered() { + if (isEmpty(this.ruleSets)) { + if (!configuration.getRuleSetPaths().isEmpty()) { + logger.error("No rules found. Maybe you misspelled a rule name? ({})", + String.join(",", configuration.getRuleSetPaths())); + + } else { + logger.error("No rules found."); + } + return true; + } + return false; + } + private static GlobalAnalysisListener createComposedRendererListener(List renderers) throws Exception { if (renderers.isEmpty()) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java index 12c2d2fd99..d2739cfc92 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java @@ -102,7 +102,7 @@ public class FileAnalysisCache extends AbstractAnalysisCache { } @Override - public void persist() throws IOException { + public void persist() { try (TimedOperation ignored = TimeTracker.startOperation(TimedOperationCategory.ANALYSIS_CACHE, "persist")) { if (cacheFile.isDirectory()) { LOG.error("Cannot persist the cache, the given path points to a directory."); @@ -145,6 +145,8 @@ public class FileAnalysisCache extends AbstractAnalysisCache { } else { LOG.info("Analysis cache updated"); } + } catch (final IOException e) { + LOG.error("Could not persist analysis cache to file: {}", e.getMessage()); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java index 65357dbb9a..3a3dee4470 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java @@ -5,7 +5,6 @@ package net.sourceforge.pmd.cache; import java.io.File; -import java.io.IOException; import java.util.Collections; import java.util.List; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/FileCollectionUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/FileCollectionUtil.java index 3235be1819..195eeb0d54 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/FileCollectionUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/FileCollectionUtil.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Set; import org.apache.commons.io.IOUtils; +import org.slf4j.event.Level; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.lang.Language; @@ -27,7 +28,6 @@ import net.sourceforge.pmd.util.database.DBURI; import net.sourceforge.pmd.util.database.SourceObject; import net.sourceforge.pmd.util.datasource.DataSource; import net.sourceforge.pmd.util.log.PmdLogger; -import net.sourceforge.pmd.util.log.PmdLogger.Level; import net.sourceforge.pmd.util.log.PmdLoggerScope; /** @@ -105,7 +105,7 @@ public final class FileCollectionUtil { public static void collectFileList(FileCollector collector, String fileListLocation) { Path path = Paths.get(fileListLocation); if (!Files.exists(path)) { - collector.getLog().error("No such file {0}", fileListLocation); + collector.getLog().error("No such file {}", fileListLocation); return; } @@ -113,7 +113,7 @@ public final class FileCollectionUtil { try { filePaths = FileUtil.readFilelist(path.toFile()); } catch (IOException e) { - collector.getLog().errorEx("Error reading {0}", new Object[] { fileListLocation }, e); + collector.getLog().errorEx("Error reading {}", new Object[] { fileListLocation }, e); return; } collectFiles(collector, filePaths); @@ -122,7 +122,7 @@ public final class FileCollectionUtil { private static void addRoot(FileCollector collector, String rootLocation) throws IOException { Path path = Paths.get(rootLocation); if (!Files.exists(path)) { - collector.getLog().error("No such file {0}", path); + collector.getLog().error("No such file {}", path); return; } @@ -140,27 +140,27 @@ public final class FileCollectionUtil { } else if (Files.isRegularFile(path)) { collector.addFile(path); } else { - collector.getLog().trace("Ignoring {0}: not a regular file or directory", path); + collector.getLog().trace("Ignoring {}: not a regular file or directory", path); } } public static void collectDB(FileCollector collector, String uriString) { try { - collector.getLog().trace("Connecting to {0}", uriString); + collector.getLog().trace("Connecting to {}", uriString); DBURI dbUri = new DBURI(uriString); DBMSMetadata dbmsMetadata = new DBMSMetadata(dbUri); collector.getLog().trace("DBMSMetadata retrieved"); List sourceObjectList = dbmsMetadata.getSourceObjectList(); - collector.getLog().trace("Located {0} database source objects", sourceObjectList.size()); + collector.getLog().trace("Located {} database source objects", sourceObjectList.size()); for (SourceObject sourceObject : sourceObjectList) { String falseFilePath = sourceObject.getPseudoFileName(); - collector.getLog().trace("Adding database source object {0}", falseFilePath); + collector.getLog().trace("Adding database source object {}", falseFilePath); try (Reader sourceCode = dbmsMetadata.getSourceCode(sourceObject)) { String source = IOUtils.toString(sourceCode); collector.addSourceFile(source, falseFilePath); } catch (SQLException ex) { - collector.getLog().warningEx("Cannot get SourceCode for {0} - skipping ...", + collector.getLog().warningEx("Cannot get SourceCode for {} - skipping ...", new Object[] { falseFilePath}, ex); } @@ -168,7 +168,7 @@ public final class FileCollectionUtil { } catch (ClassNotFoundException e) { collector.getLog().errorEx("Cannot get files from DB - probably missing database JDBC driver", e); } catch (Exception e) { - collector.getLog().errorEx("Cannot get files from DB - ''{0}''", new Object[] { uriString }, e); + collector.getLog().errorEx("Cannot get files from DB - ''{}''", new Object[] { uriString }, e); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/FileCollector.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/FileCollector.java index ab92d265ed..f45b98e669 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/FileCollector.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/FileCollector.java @@ -127,7 +127,7 @@ public final class FileCollector implements AutoCloseable { */ public boolean addFile(Path file) { if (!Files.isRegularFile(file)) { - log.error("Not a regular file {0}", file); + log.error("Not a regular file {}", file); return false; } LanguageVersion languageVersion = discoverLanguage(file.toString()); @@ -151,7 +151,7 @@ public final class FileCollector implements AutoCloseable { public boolean addFile(Path file, Language language) { AssertionUtil.requireParamNotNull("language", language); if (!Files.isRegularFile(file)) { - log.error("Not a regular file {0}", file); + log.error("Not a regular file {}", file); return false; } NioTextFile nioTextFile = new NioTextFile(file, charset, discoverer.getDefaultLanguageVersion(language), getDisplayName(file)); @@ -195,7 +195,7 @@ public final class FileCollector implements AutoCloseable { } private void addFileImpl(TextFile textFile) { - log.trace("Adding file {0} (lang: {1}) ", textFile.getPathId(), textFile.getLanguageVersion().getTerseName()); + log.trace("Adding file {} (lang: {}) ", textFile.getPathId(), textFile.getLanguageVersion().getTerseName()); allFilesToProcess.add(textFile); } @@ -206,12 +206,12 @@ public final class FileCollector implements AutoCloseable { List languages = discoverer.getLanguagesForFile(file); if (languages.isEmpty()) { - log.trace("File {0} matches no known language, ignoring", file); + log.trace("File {} matches no known language, ignoring", file); return null; } Language lang = languages.get(0); if (languages.size() > 1) { - log.trace("File {0} matches multiple languages ({1}), selecting {2}", file, languages, lang); + log.trace("File {} matches multiple languages ({}), selecting {}", file, languages, lang); } return discoverer.getDefaultLanguageVersion(lang); } @@ -227,10 +227,10 @@ public final class FileCollector implements AutoCloseable { LanguageVersion contextVersion = discoverer.getDefaultLanguageVersion(language); if (!fileVersion.equals(contextVersion)) { log.error( - "Cannot add file {0}: version ''{2}'' does not match ''{1}''", + "Cannot add file {}: version ''{}'' does not match ''{}''", textFile.getPathId(), - contextVersion, - fileVersion + fileVersion, + contextVersion ); return false; } @@ -270,7 +270,7 @@ public final class FileCollector implements AutoCloseable { */ public boolean addDirectory(Path dir) throws IOException { if (!Files.isDirectory(dir)) { - log.error("Not a directory {0}", dir); + log.error("Not a directory {}", dir); return false; } Files.walkFileTree(dir, new SimpleFileVisitor() { @@ -298,7 +298,7 @@ public final class FileCollector implements AutoCloseable { } else if (Files.isRegularFile(file)) { return addFile(file); } else { - log.error("Not a file or directory {0}", file); + log.error("Not a file or directory {}", file); return false; } } @@ -361,7 +361,7 @@ public final class FileCollector implements AutoCloseable { for (Iterator iterator = allFilesToProcess.iterator(); iterator.hasNext();) { TextFile file = iterator.next(); if (toExclude.contains(file)) { - log.trace("Excluding file {0}", file.getPathId()); + log.trace("Excluding file {}", file.getPathId()); iterator.remove(); } } @@ -376,7 +376,7 @@ public final class FileCollector implements AutoCloseable { TextFile file = iterator.next(); Language lang = file.getLanguageVersion().getLanguage(); if (!languages.contains(lang)) { - log.trace("Filtering out {0}, no rules for language {1}", file.getPathId(), lang); + log.trace("Filtering out {}, no rules for language {}", file.getPathId(), lang); iterator.remove(); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/NoopAnalysisListener.java b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/NoopAnalysisListener.java index 865a566142..fcd5da121c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/NoopAnalysisListener.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/NoopAnalysisListener.java @@ -9,7 +9,7 @@ import net.sourceforge.pmd.util.datasource.DataSource; /** * @author Clément Fournier */ -class NoopAnalysisListener implements GlobalAnalysisListener { +final class NoopAnalysisListener implements GlobalAnalysisListener { static final NoopAnalysisListener INSTANCE = new NoopAnalysisListener(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/NoopPmdLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/NoopPmdLogger.java index 19a0c3fbc9..4741856f1c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/NoopPmdLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/NoopPmdLogger.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.util.log; +import org.slf4j.event.Level; + import net.sourceforge.pmd.annotation.InternalApi; /** @@ -14,11 +16,7 @@ import net.sourceforge.pmd.annotation.InternalApi; @InternalApi public final class NoopPmdLogger extends PmdLoggerBase implements PmdLogger { - public static final NoopPmdLogger INSTANCE = new NoopPmdLogger(); - - private NoopPmdLogger() { - - } + // note: not singleton because PmdLogger accumulates error count. @Override protected boolean isLoggableImpl(Level level) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLogger.java index dc1c1977b9..8f37f9dbff 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLogger.java @@ -4,8 +4,9 @@ package net.sourceforge.pmd.util.log; +import org.slf4j.event.Level; + import net.sourceforge.pmd.annotation.InternalApi; -import net.sourceforge.pmd.internal.util.AssertionUtil; /** * Logger façade. Can probably be converted to just SLF4J logger in PMD 7. @@ -21,50 +22,43 @@ public interface PmdLogger { void logEx(Level level, String message, Object[] formatArgs, Throwable error); - void info(String message, Object... formatArgs); + default void info(String message, Object... formatArgs) { + log(Level.INFO, message, formatArgs); + } - void trace(String message, Object... formatArgs); + // todo trace and debug should be on SLF4J logger directly + default void trace(String message, Object... formatArgs) { + log(Level.TRACE, message, formatArgs); + } - void debug(String message, Object... formatArgs); + default void debug(String message, Object... formatArgs) { + log(Level.DEBUG, message, formatArgs); + } - void warning(String message, Object... formatArgs); + default void warning(String message, Object... formatArgs) { + log(Level.WARN, message, formatArgs); + } - void warningEx(String message, Throwable error); + default void warningEx(String message, Throwable error) { + logEx(Level.WARN, message, new Object[0], error); + } - void warningEx(String message, Object[] formatArgs, Throwable error); + default void warningEx(String message, Object[] formatArgs, Throwable error) { + logEx(Level.WARN, message, formatArgs, error); + } - void error(String message, Object... formatArgs); + default void error(String message, Object... formatArgs) { + log(Level.ERROR, message, formatArgs); + } - void errorEx(String message, Throwable error); + default void errorEx(String message, Throwable error) { + logEx(Level.ERROR, message, new Object[0], error); + } - void errorEx(String message, Object[] formatArgs, Throwable error); + default void errorEx(String message, Object[] formatArgs, Throwable error) { + logEx(Level.ERROR, message, formatArgs, error); + } int numErrors(); - // levels, in sync with SLF4J levels - enum Level { - TRACE, - DEBUG, - INFO, - WARN, - ERROR; - - java.util.logging.Level toJutilLevel() { - switch (this) { - case DEBUG: - return java.util.logging.Level.FINE; - case ERROR: - return java.util.logging.Level.SEVERE; - case INFO: - return java.util.logging.Level.INFO; - case TRACE: - return java.util.logging.Level.FINER; - case WARN: - return java.util.logging.Level.WARNING; - default: - throw AssertionUtil.shouldNotReachHere("exhaustive"); - } - } - } - } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerBase.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerBase.java index c1af32b07b..4370556543 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerBase.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerBase.java @@ -5,12 +5,12 @@ package net.sourceforge.pmd.util.log; import java.text.MessageFormat; -import java.util.logging.Logger; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.slf4j.event.Level; /** - * A logger based on a {@link Logger}. + * Base implementation. * * @author Clément Fournier */ @@ -63,50 +63,6 @@ abstract class PmdLoggerBase implements PmdLogger { */ protected abstract void logImpl(Level level, String message, Object[] formatArgs); - @Override - public void trace(String message, Object... formatArgs) { - log(Level.TRACE, message, formatArgs); - } - - @Override - public void debug(String message, Object... formatArgs) { - log(Level.DEBUG, message, formatArgs); - } - - @Override - public void info(String message, Object... formatArgs) { - log(Level.INFO, message, formatArgs); - } - - @Override - public void warning(String message, Object... formatArgs) { - log(Level.WARN, message, formatArgs); - } - - @Override - public final void warningEx(String message, Throwable error) { - warningEx(message, new Object[0], error); - } - - @Override - public void warningEx(String message, Object[] formatArgs, Throwable error) { - logEx(Level.WARN, message, formatArgs, error); - } - - @Override - public void error(String message, Object... formatArgs) { - log(Level.ERROR, message, formatArgs); - } - - @Override - public final void errorEx(String message, Throwable error) { - errorEx(message, new Object[0], error); - } - - @Override - public void errorEx(String message, Object[] formatArgs, Throwable error) { - logEx(Level.ERROR, message, formatArgs, error); - } @Override public int numErrors() { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerScope.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerScope.java index 34d414eee5..faead30e2b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerScope.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/PmdLoggerScope.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.util.log; +import org.slf4j.event.Level; + import net.sourceforge.pmd.annotation.InternalApi; /** diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/SimplePmdLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/SimplePmdLogger.java index 6f3e670b78..f2dde02d4d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/log/SimplePmdLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/log/SimplePmdLogger.java @@ -4,8 +4,8 @@ package net.sourceforge.pmd.util.log; -import java.text.MessageFormat; -import java.util.logging.Logger; +import org.slf4j.Logger; +import org.slf4j.event.Level; import net.sourceforge.pmd.annotation.InternalApi; @@ -25,11 +25,11 @@ public class SimplePmdLogger extends PmdLoggerBase implements PmdLogger { @Override protected boolean isLoggableImpl(Level level) { - return backend.isLoggable(level.toJutilLevel()); + return backend.isEnabledForLevel(level); } @Override protected void logImpl(Level level, String message, Object[] formatArgs) { - backend.log(level.toJutilLevel(), MessageFormat.format(message, formatArgs)); + backend.atLevel(level).log(message, formatArgs); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java index 37fc4e4b19..a81cbb55b5 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java @@ -89,7 +89,7 @@ public class FileAnalysisCacheTest { } @Test - public void testStoreOnUnwritableFileShouldntThrow() throws Exception { + public void testStoreOnUnwritableFileShouldntThrow() throws IOException { emptyCacheFile.setWritable(false); final FileAnalysisCache cache = new FileAnalysisCache(emptyCacheFile); cache.persist(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cli/PMDFilelistTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cli/PMDFilelistTest.java index 34dc393732..395f15b791 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cli/PMDFilelistTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cli/PMDFilelistTest.java @@ -24,7 +24,7 @@ import net.sourceforge.pmd.util.log.NoopPmdLogger; public class PMDFilelistTest { private static @NonNull FileCollector newCollector() { - return FileCollector.newCollector(new LanguageVersionDiscoverer(), NoopPmdLogger.INSTANCE); + return FileCollector.newCollector(new LanguageVersionDiscoverer(), new NoopPmdLogger()); } @Test diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/document/PmdTestLogger.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/document/PmdTestLogger.java index 6eb7568d18..a8a99fdd67 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/document/PmdTestLogger.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/document/PmdTestLogger.java @@ -4,7 +4,8 @@ package net.sourceforge.pmd.lang.document; -import java.util.logging.Logger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import net.sourceforge.pmd.util.log.SimplePmdLogger; @@ -13,7 +14,7 @@ import net.sourceforge.pmd.util.log.SimplePmdLogger; */ public class PmdTestLogger extends SimplePmdLogger { - private static final Logger LOG = Logger.getLogger("testlogger"); + private static final Logger LOG = LoggerFactory.getLogger(PmdTestLogger.class.getName()); public PmdTestLogger() { super(LOG);