diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java index 15508e1c0a..a84975595a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd; +import static java.util.Collections.synchronizedList; + import java.io.File; import java.io.IOException; import java.io.PrintWriter; @@ -12,6 +14,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import net.sourceforge.pmd.lang.ast.FileAnalysisException; import net.sourceforge.pmd.renderers.AbstractAccumulatingRenderer; /** @@ -23,10 +26,10 @@ public class Report { private final List listeners = new ArrayList<>(); - private final List violations = new ArrayList<>(); - private final List suppressedRuleViolations = new ArrayList<>(); - private final List errors = new ArrayList<>(); - private final List configErrors = new ArrayList<>(); + private final List violations = synchronizedList(new ArrayList<>()); + private final List suppressedRuleViolations = synchronizedList(new ArrayList<>()); + private final List errors = synchronizedList(new ArrayList<>()); + private final List configErrors = synchronizedList(new ArrayList<>()); /** * Creates a new, initialized, empty report for the given file name. @@ -52,6 +55,7 @@ public class Report { * Represents a configuration error. */ public static class ConfigurationError { + private final Rule rule; private final String issue; @@ -91,6 +95,7 @@ public class Report { * Represents a processing error, such as a parse error. */ public static class ProcessingError { + private final Throwable error; private final String file; @@ -113,7 +118,7 @@ public class Report { public String getDetail() { try (StringWriter stringWriter = new StringWriter(); - PrintWriter writer = new PrintWriter(stringWriter)) { + PrintWriter writer = new PrintWriter(stringWriter)) { error.printStackTrace(writer); return stringWriter.toString(); } catch (IOException e) { @@ -135,6 +140,7 @@ public class Report { * Represents a violation, that has been suppressed. */ public static class SuppressedViolation { + private final RuleViolation rv; private final String userMessage; private final ViolationSuppressor suppressor; @@ -277,10 +283,48 @@ public class Report { /** * Adds all given listeners to this report * - * @param allListeners - * the report listeners + * @param allListeners the report listeners */ public void addListeners(List allListeners) { listeners.addAll(allListeners); } + + + public static class ReportBuilderListener implements ThreadSafeAnalysisListener { + + private final Report report = new Report(); + private boolean done; + + /** + * Returns the final report. + * + * @throws IllegalStateException If {@link #finish()} has not been called yet + */ + public Report getReport() { + if (!done) { + throw new IllegalStateException("Reporting not done"); + } + return report; + } + + @Override + public void onRuleViolation(RuleViolation violation) { + report.addRuleViolation(violation); + } + + @Override + public void onSuppressedRuleViolation(SuppressedViolation violation) { + report.addSuppressedViolation(violation); + } + + @Override + public void onError(FileAnalysisException exception) { + report.addError(new ProcessingError(exception, exception.getFileName())); + } + + @Override + public void finish() { + done = true; + } + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java index de7bfd2001..65ab774426 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java @@ -5,9 +5,19 @@ package net.sourceforge.pmd; import java.io.File; +import java.text.MessageFormat; +import java.util.Objects; import java.util.logging.Logger; +import org.apache.commons.lang3.StringUtils; +import org.checkerframework.checker.nullness.qual.NonNull; + +import net.sourceforge.pmd.Report.ProcessingError; +import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.lang.LanguageVersion; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; +import net.sourceforge.pmd.lang.rule.RuleViolationFactory; /** * The RuleContext provides access to Rule processing state. This information @@ -25,7 +35,9 @@ import net.sourceforge.pmd.lang.LanguageVersion; * between calls to difference source files. Failure to do so, may result in * undefined behavior. */ -public class RuleContext { +public class RuleContext implements AutoCloseable { + + private static final Object[] NO_ARGS = new Object[0]; private static final Logger LOG = Logger.getLogger(RuleContext.class.getName()); @@ -34,25 +46,88 @@ public class RuleContext { private LanguageVersion languageVersion; private boolean ignoreExceptions = true; + private final ThreadSafeAnalysisListener listener; + /** * Default constructor. */ public RuleContext() { - // nothing to do + listener = ThreadSafeAnalysisListener.noop(); } /** * Constructor which shares attributes and report listeners with the given * RuleContext. * - * @param ruleContext - * the context from which the values are shared + * @param ruleContext the context from which the values are shared */ public RuleContext(RuleContext ruleContext) { - this.report.addListeners(ruleContext.getReport().getListeners()); + this.listener = ruleContext.listener; this.setIgnoreExceptions(ruleContext.ignoreExceptions); } + public RuleContext(ThreadSafeAnalysisListener listener) { + this.listener = listener; + } + + + @Override + public void close() throws Exception { + listener.finish(); + } + + public void addError(ProcessingError error) { + + } + + + public void addViolation(Rule rule, Node location) { + addViolationWithMessage(rule, location, rule.getMessage(), NO_ARGS); + } + + public void addViolation(Rule rule, Node location, Object... formatArgs) { + addViolationWithMessage(rule, location, rule.getMessage(), formatArgs); + } + + public void addViolationWithMessage(Rule rule, Node location, String message) { + addViolationWithPosition(rule, location, -1, -1, message, NO_ARGS); + } + + public void addViolationWithMessage(Rule rule, Node location, String message, Object... formatArgs) { + addViolationWithPosition(rule, location, -1, -1, message, formatArgs); + } + + public void addViolationWithPosition(Rule rule, Node location, int beginLine, int endLine, String message, Object... formatArgs) { + Objects.requireNonNull(rule); + Objects.requireNonNull(location); + Objects.requireNonNull(message); + Objects.requireNonNull(formatArgs); + + RuleViolationFactory fact = getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory(); + + RuleViolation violation = fact.createViolation(rule, location, getSourceCodeFilename(), makeMessage(message, formatArgs)); + if (beginLine != -1 && endLine != -1) { + // fixme, this is needed until we have actual Location objects + ((ParametricRuleViolation) violation).setLines(beginLine, endLine); + } + + SuppressedViolation suppressed = fact.suppressOrNull(location, violation); + + if (suppressed != null) { + listener.onSuppressedRuleViolation(suppressed); + } else { + listener.onRuleViolation(violation); + } + } + + private String makeMessage(@NonNull String message, Object[] args) { + // Escape PMD specific variable message format, specifically the { + // in the ${, so MessageFormat doesn't bitch. + final String escapedMessage = StringUtils.replace(message, "${", "$'{'"); + return MessageFormat.format(escapedMessage, args != null ? args : NO_ARGS); + } + + /** * Get the Report to which Rule Violations are sent. * diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java b/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java new file mode 100644 index 0000000000..39e427ea3a --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java @@ -0,0 +1,82 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd; + +import net.sourceforge.pmd.Report.ReportBuilderListener; +import net.sourceforge.pmd.Report.SuppressedViolation; +import net.sourceforge.pmd.lang.ast.FileAnalysisException; +import net.sourceforge.pmd.renderers.Renderer; + +/** + * A handler for analysis events. This must be thread safe. + */ +public interface ThreadSafeAnalysisListener { + + /** + * Handle a new violation (not suppressed). + */ + default void onRuleViolation(RuleViolation violation) { + + } + + /** + * Handle a new suppressed violation. + */ + default void onSuppressedRuleViolation(SuppressedViolation violation) { + + } + + + /** + * Handle an error that occurred while processing a file. + */ + default void onError(FileAnalysisException exception) { + + } + + + /** + * All files have been processed. + */ + default void finish() throws Exception { + + } + + + static ThreadSafeAnalysisListener noop() { + return new ThreadSafeAnalysisListener() {}; + } + + + static ThreadSafeAnalysisListener forReporter(Renderer renderer) { + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + return new ThreadSafeAnalysisListener() { + + @Override + public void onRuleViolation(RuleViolation violation) { + reportBuilder.onRuleViolation(violation); + } + + @Override + public void onSuppressedRuleViolation(SuppressedViolation violation) { + reportBuilder.onSuppressedRuleViolation(violation); + } + + @Override + public void onError(FileAnalysisException exception) { + reportBuilder.onError(exception); + } + + @Override + public void finish() throws Exception { + reportBuilder.finish(); + renderer.renderFileReport(reportBuilder.getReport()); + renderer.end(); + renderer.flush(); + } + }; + } + +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/JavaCCTokenizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/JavaCCTokenizer.java index 5a9b0bdaf7..04b9ab04ef 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/JavaCCTokenizer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/JavaCCTokenizer.java @@ -59,7 +59,7 @@ public abstract class JavaCCTokenizer implements Tokenizer { currentToken = tokenFilter.getNextToken(); } } catch (TokenMgrError e) { - throw e.withFileName(sourceCode.getFileName()); + throw e.setFileName(sourceCode.getFileName()); } finally { tokenEntries.add(TokenEntry.getEOF()); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java index 8d43032984..9315f1da2b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java @@ -4,7 +4,8 @@ package net.sourceforge.pmd.lang; -import java.util.Collections; +import static java.util.Collections.emptyList; + import java.util.List; import net.sourceforge.pmd.annotation.Experimental; @@ -41,7 +42,7 @@ public interface LanguageVersionHandler { */ @Experimental default List> getProcessingStages() { - return Collections.emptyList(); + return emptyList(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/FileAnalysisException.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/FileAnalysisException.java index b9781a938d..552e28975c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/FileAnalysisException.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/FileAnalysisException.java @@ -4,6 +4,10 @@ package net.sourceforge.pmd.lang.ast; +import java.util.Objects; + +import org.checkerframework.checker.nullness.qual.NonNull; + /** * An exception that occurs while processing a file. Subtypes include *
    @@ -15,6 +19,9 @@ package net.sourceforge.pmd.lang.ast; */ public class FileAnalysisException extends RuntimeException { + protected static final String UNKNOWN_FNAME = "(unknown file)"; + private String filename = UNKNOWN_FNAME; + public FileAnalysisException() { super(); } @@ -26,8 +33,24 @@ public class FileAnalysisException extends RuntimeException { public FileAnalysisException(Throwable cause) { super(cause); } + public FileAnalysisException(String message, Throwable cause) { super(message, cause); } + FileAnalysisException setFileName(String filename) { + this.filename = Objects.requireNonNull(filename); + return this; + } + + protected boolean hasFileName() { + return !UNKNOWN_FNAME.equals(filename); + } + + /** + * The name of the file in which the error occurred. + */ + public @NonNull String getFileName() { + return filename; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java index 65e135a055..acfca312cc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java @@ -16,14 +16,13 @@ public final class TokenMgrError extends FileAnalysisException { private final int line; private final int column; - private final String filename; /** * Create a new exception. * * @param line Line number * @param column Column number - * @param filename Filename. If unknown, it can be completed with {@link #withFileName(String)} later + * @param filename Filename. If unknown, it can be completed with {@link #setFileName(String)} later * @param message Message of the error * @param cause Cause of the error, if any */ @@ -31,7 +30,9 @@ public final class TokenMgrError extends FileAnalysisException { super(message, cause); this.line = line; this.column = column; - this.filename = filename; + if (filename != null) { + super.setFileName(filename); + } } /** @@ -42,7 +43,6 @@ public final class TokenMgrError extends FileAnalysisException { super(makeReason(eofSeen, lexStateName, errorAfter, curChar)); line = errorLine; column = errorColumn; - filename = null; // may be replaced with #withFileName } public int getLine() { @@ -53,14 +53,11 @@ public final class TokenMgrError extends FileAnalysisException { return column; } - public @Nullable String getFilename() { - return filename; - } @Override public String getMessage() { - String leader = filename != null ? "Lexical error in file " + filename : "Lexical error"; + String leader = hasFileName() ? "Lexical error in file " + getFileName() : "Lexical error"; return leader + " at line " + line + ", column " + column + ". Encountered: " + super.getMessage(); } @@ -71,8 +68,9 @@ public final class TokenMgrError extends FileAnalysisException { * * @return A new exception */ - public TokenMgrError withFileName(String filename) { - return new TokenMgrError(this.line, this.column, filename, this.getMessage(), this.getCause()); + public TokenMgrError setFileName(String filename) { + super.setFileName(filename); + return this; } private static String makeReason(boolean eofseen, String lexStateName, String errorAfter, char curChar) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java index b9ce1dcda6..d871f4906e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java @@ -36,7 +36,7 @@ public abstract class JjtreeParserAdapter implements Parser try { return parseImpl(charStream, task.getCommentMarker()); } catch (TokenMgrError tme) { - throw tme.withFileName(task.getFileDisplayName()); + throw tme.setFileName(task.getFileDisplayName()); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java index 8cb94b80a6..9f0737a473 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java @@ -288,63 +288,35 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul } /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) + * @see RuleContext#addViolation(Rule, Node, Object...) */ public void addViolation(Object data, Node node) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, this.getMessage(), null); + ((RuleContext) data).addViolation(this, node); } /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) + * @see RuleContext#addViolation(Rule, Node, Object...) */ - public void addViolation(Object data, Node node, String arg) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, this.getMessage(), new Object[]{arg}); + public void addViolation(Object data, Node node, Object... args) { + ((RuleContext) data).addViolation(this, node, args); } /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) - */ - public void addViolation(Object data, Node node, Object[] args) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, this.getMessage(), args); - } - - /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) + * @see RuleContext#addViolationWithMessage(Rule, Node, String, Object...) */ public void addViolationWithMessage(Object data, Node node, String message) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, message, null); + ((RuleContext) data).addViolationWithMessage(this, node, message); } - /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) - */ public void addViolationWithMessage(Object data, Node node, String message, int beginLine, int endLine) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, message, beginLine, endLine, null); + ((RuleContext) data).addViolationWithPosition(this, node, beginLine, endLine, message); } /** - * @see RuleViolationFactory#addViolation(RuleContext, Rule, Node, String, - * Object[]) + * @see RuleContext#addViolationWithMessage(Rule, Node, String, Object...) */ public void addViolationWithMessage(Object data, Node node, String message, Object[] args) { - RuleContext ruleContext = (RuleContext) data; - ruleContext.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory().addViolation(ruleContext, - this, node, message, args); + ((RuleContext) data).addViolationWithMessage(this, node, message, args); } /** diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ParametricRuleViolation.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ParametricRuleViolation.java index ad670478b2..386c76e0ff 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ParametricRuleViolation.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/ParametricRuleViolation.java @@ -4,10 +4,7 @@ package net.sourceforge.pmd.lang.rule; -import java.io.File; - import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -34,16 +31,11 @@ public class ParametricRuleViolation implements RuleViolation { // RuleViolationFactory to support identifying without a Node, and update // Rule base classes too. // TODO we never need a node. We just have to have a "position", ie line/column, or offset, + file, whatever - public ParametricRuleViolation(Rule theRule, RuleContext ctx, T node, String message) { + public ParametricRuleViolation(Rule theRule, String filename, T node, String message) { rule = theRule; description = message; + this.filename = filename == null ? "" : filename; - File file = ctx.getSourceCodeFile(); - if (file != null) { - filename = file.getPath(); - } else { - filename = ""; - } if (node != null) { beginLine = node.getBeginLine(); beginColumn = node.getBeginColumn(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleViolationFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleViolationFactory.java index f759cf69dd..6e81f61d2f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleViolationFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/RuleViolationFactory.java @@ -4,38 +4,26 @@ package net.sourceforge.pmd.lang.rule; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.checkerframework.checker.nullness.qual.NonNull; +import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.lang.LanguageVersionHandler; +import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.Node; /** - * This class handles of producing a Language specific RuleViolation and adding - * to a Report. + * Creates violations and controls suppression behavior for a language. * - *

    Since PMD 6.21.0, implementations of this interface are considered internal - * API and hence deprecated. Clients should exclusively use this interface and obtain - * instances through {@link LanguageVersionHandler#getRuleViolationFactory()}. + * TODO split this into violation decorators + violation suppressors. + * There is no need to have language-specific violation classes. */ public interface RuleViolationFactory { - /** - * Adds a violation to the report. - * - * @param ruleContext - * the RuleContext - * @param rule - * the rule - * @param node - * the node that produces the violation - * @param message - * specific message to put in the report - * @param args - * arguments to embed in the rule violation message - */ - void addViolation(RuleContext ruleContext, Rule rule, @Nullable Node node, String message, Object[] args); - void addViolation(RuleContext ruleContext, Rule rule, @Nullable Node node, String message, int beginLine, int endLine, Object[] args); + RuleViolation createViolation(Rule rule, @NonNull Node location, String filename, String formattedMessage); + + + SuppressedViolation suppressOrNull(Node location, RuleViolation violation); + + } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java index c5fa2d5e6e..799e745431 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java @@ -4,18 +4,15 @@ package net.sourceforge.pmd.lang.rule.impl; -import java.text.MessageFormat; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import org.apache.commons.lang3.StringUtils; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.checkerframework.checker.nullness.qual.NonNull; import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.ViolationSuppressor; import net.sourceforge.pmd.lang.ast.Node; @@ -31,58 +28,23 @@ import net.sourceforge.pmd.lang.rule.RuleViolationFactory; */ public class DefaultRuleViolationFactory implements RuleViolationFactory { - private static final Object[] NO_ARGS = new Object[0]; - private static final DefaultRuleViolationFactory DEFAULT = new DefaultRuleViolationFactory(); private Set allSuppressors; - private String cleanup(String message, Object[] args) { - - if (message != null) { - // Escape PMD specific variable message format, specifically the { - // in the ${, so MessageFormat doesn't bitch. - final String escapedMessage = StringUtils.replace(message, "${", "$'{'"); - return MessageFormat.format(escapedMessage, args != null ? args : NO_ARGS); - } else { - return message; - } - } - - @Override - public void addViolation(RuleContext ruleContext, Rule rule, Node node, String message, Object[] args) { - - String formattedMessage = cleanup(message, args); - - RuleViolation rv = createRuleViolation(rule, ruleContext, node, formattedMessage); - maybeSuppress(ruleContext, node, rv); + public RuleViolation createViolation(Rule rule, @NonNull Node location, String filename, String formattedMessage) { + return new ParametricRuleViolation<>(rule, filename, location, formattedMessage); } @Override - public void addViolation(RuleContext ruleContext, Rule rule, Node node, String message, int beginLine, int endLine, Object[] args) { - - String formattedMessage = cleanup(message, args); - - RuleViolation rv = createRuleViolation(rule, ruleContext, node, formattedMessage, beginLine, endLine); - maybeSuppress(ruleContext, node, rv); - } - - private void maybeSuppress(RuleContext ruleContext, @Nullable Node node, RuleViolation rv) { - - if (node != null) { - // note: no suppression when node is null. - // Node should in fact never be null, this is todo for later - - for (ViolationSuppressor suppressor : getAllSuppressors()) { - SuppressedViolation suppressed = suppressor.suppressOrNull(rv, node); - if (suppressed != null) { - ruleContext.getReport().addSuppressedViolation(suppressed); - return; - } + public final SuppressedViolation suppressOrNull(Node location, RuleViolation violation) { + for (ViolationSuppressor suppressor : getAllSuppressors()) { + SuppressedViolation suppressed = suppressor.suppressOrNull(violation, location); + if (suppressed != null) { + return suppressed; } } - - ruleContext.getReport().addRuleViolation(rv); + return null; } /** @@ -93,17 +55,6 @@ public class DefaultRuleViolationFactory implements RuleViolationFactory { return Collections.emptyList(); } - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message) { - return new ParametricRuleViolation<>(rule, ruleContext, node, message); - } - - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message, - int beginLine, int endLine) { - ParametricRuleViolation rv = new ParametricRuleViolation<>(rule, ruleContext, node, message); - rv.setLines(beginLine, endLine); - return rv; - } - private Set getAllSuppressors() { if (allSuppressors == null) { // lazy loaded because calling getSuppressors in constructor diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java index 4c9e8b83ee..74c9d114eb 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java @@ -62,7 +62,7 @@ public class RuleApplicator { rcto.close(1); } catch (RuntimeException e) { if (ctx.isIgnoreExceptions()) { - ctx.getReport().addError(new ProcessingError(e, ctx.getSourceCodeFilename())); + ctx.addError(new ProcessingError(e, ctx.getSourceCodeFilename())); if (LOG.isLoggable(Level.WARNING)) { LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java index 39e27560e2..f5c988db57 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java @@ -7,14 +7,16 @@ package net.sourceforge.pmd; import static net.sourceforge.pmd.properties.constraints.NumericConstraints.inRange; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; import java.io.File; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; import org.junit.Test; +import net.sourceforge.pmd.Report.ReportBuilderListener; +import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.lang.DummyLanguageModule; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.DummyNode; @@ -73,11 +75,9 @@ public class AbstractRuleTest { public void testCreateRV() { MyRule r = new MyRule(); r.setRuleSetName("foo"); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(5, 5, 5, 10); - RuleViolation rv = new ParametricRuleViolation(r, ctx, s, r.getMessage()); + RuleViolation rv = new ParametricRuleViolation<>(r, "filename", s, r.getMessage()); assertEquals("Line number mismatch!", 5, rv.getBeginLine()); assertEquals("Filename mismatch!", "filename", rv.getFilename()); assertEquals("Rule object mismatch!", r, rv.getRule()); @@ -88,11 +88,9 @@ public class AbstractRuleTest { @Test public void testCreateRV2() { MyRule r = new MyRule(); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(5, 5, 5, 10); - RuleViolation rv = new ParametricRuleViolation<>(r, ctx, s, "specificdescription"); + RuleViolation rv = new ParametricRuleViolation<>(r, "filename", s, "specificdescription"); assertEquals("Line number mismatch!", 5, rv.getBeginLine()); assertEquals("Filename mismatch!", "filename", rv.getFilename()); assertEquals("Rule object mismatch!", r, rv.getRule()); @@ -100,34 +98,32 @@ public class AbstractRuleTest { } @Test - public void testRuleWithVariableInMessage() { + public void testRuleWithVariableInMessage() throws Exception { MyRule r = new MyRule(); r.definePropertyDescriptor(PropertyFactory.intProperty("testInt").desc("description").require(inRange(0, 100)).defaultValue(10).build()); r.setMessage("Message ${packageName} ${className} ${methodName} ${variableName} ${testInt} ${noSuchProperty}"); - RuleContext ctx = new RuleContext(); - ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); - ctx.setReport(new Report()); - ctx.setSourceCodeFile(new File("filename")); - DummyNode s = new DummyRoot(); - s.setCoords(5, 1, 6, 1); - s.setImage("TestImage"); - r.addViolation(ctx, s); - RuleViolation rv = ctx.getReport().getViolations().get(0); + ReportBuilderListener listener = new ReportBuilderListener(); + try (RuleContext ctx = new RuleContext(listener)) { + ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + ctx.setSourceCodeFile(new File("filename")); + DummyNode s = new DummyRoot(); + s.setCoords(5, 1, 6, 1); + s.setImage("TestImage"); + r.addViolation(ctx, s); + } + RuleViolation rv = listener.getReport().getViolations().get(0); assertEquals("Message foo 10 ${noSuchProperty}", rv.getDescription()); } @Test public void testRuleSuppress() { - MyRule r = new MyRule(); - RuleContext ctx = new RuleContext(); - Map m = new HashMap<>(); - m.put(5, ""); - ctx.setSourceCodeFile(new File("filename")); + Map m = Collections.singletonMap(5, ""); DummyRoot n = new DummyRoot(m); n.setCoords(5, 1, 6, 1); - DefaultRuleViolationFactory.defaultInstance().addViolation(ctx, r, n, "specificdescription", new Object[0]); + RuleViolation violation = DefaultRuleViolationFactory.defaultInstance().createViolation(new MyRule(), n, "file", "specificdescription"); + SuppressedViolation suppressed = DefaultRuleViolationFactory.defaultInstance().suppressOrNull(n, violation); - assertTrue(ctx.getReport().getViolations().isEmpty()); + assertNotNull(suppressed); } @Test diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/ReportTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/ReportTest.java index 57a845fc8c..7a9335096f 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/ReportTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/ReportTest.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.StringWriter; @@ -38,15 +37,12 @@ public class ReportTest implements ThreadSafeReportListener { @Test public void testSortedReportFile() throws IOException { Report r = new Report(); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("foo")); Node s = getNode(10, 5); Rule rule1 = new MockRule("name", "desc", "msg", "rulesetname"); - r.addRuleViolation(new ParametricRuleViolation<>(rule1, ctx, s, rule1.getMessage())); - ctx.setSourceCodeFile(new File("bar")); + r.addRuleViolation(new ParametricRuleViolation<>(rule1, "foo", s, rule1.getMessage())); Node s1 = getNode(10, 5); Rule rule2 = new MockRule("name", "desc", "msg", "rulesetname"); - r.addRuleViolation(new ParametricRuleViolation<>(rule2, ctx, s1, rule2.getMessage())); + r.addRuleViolation(new ParametricRuleViolation<>(rule2, "bar", s1, rule2.getMessage())); Renderer rend = new XMLRenderer(); String result = render(rend, r); assertTrue("sort order wrong", result.indexOf("bar") < result.indexOf("foo")); @@ -55,16 +51,13 @@ public class ReportTest implements ThreadSafeReportListener { @Test public void testSortedReportLine() throws IOException { Report r = new Report(); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("foo1")); // same file!! Node node1 = getNode(20, 5); // line 20: after rule2 violation Rule rule1 = new MockRule("rule1", "rule1", "msg", "rulesetname"); - r.addRuleViolation(new ParametricRuleViolation<>(rule1, ctx, node1, rule1.getMessage())); + r.addRuleViolation(new ParametricRuleViolation<>(rule1, "foo1", node1, rule1.getMessage())); - ctx.setSourceCodeFile(new File("foo1")); // same file!! Node node2 = getNode(10, 5); // line 10: before rule1 violation Rule rule2 = new MockRule("rule2", "rule2", "msg", "rulesetname"); - r.addRuleViolation(new ParametricRuleViolation<>(rule2, ctx, node2, rule2.getMessage())); + r.addRuleViolation(new ParametricRuleViolation<>(rule2, "foo1", node2, rule2.getMessage())); // same file!! Renderer rend = new XMLRenderer(); String result = render(rend, r); assertTrue("sort order wrong", result.indexOf("rule2") < result.indexOf("rule1")); @@ -75,23 +68,20 @@ public class ReportTest implements ThreadSafeReportListener { Report rpt = new Report(); rpt.addListener(this); violationSemaphore = false; - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("file")); Node s = getNode(5, 5); Rule rule1 = new MockRule("name", "desc", "msg", "rulesetname"); - rpt.addRuleViolation(new ParametricRuleViolation<>(rule1, ctx, s, rule1.getMessage())); + rpt.addRuleViolation(new ParametricRuleViolation<>(rule1, "file", s, rule1.getMessage())); assertTrue(violationSemaphore); } @Test public void testIterator() { Report r = new Report(); - RuleContext ctx = new RuleContext(); Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); Node node1 = getNode(5, 5, true); - r.addRuleViolation(new ParametricRuleViolation<>(rule, ctx, node1, rule.getMessage())); + r.addRuleViolation(new ParametricRuleViolation<>(rule, "", node1, rule.getMessage())); Node node2 = getNode(5, 6, true); - r.addRuleViolation(new ParametricRuleViolation<>(rule, ctx, node2, rule.getMessage())); + r.addRuleViolation(new ParametricRuleViolation<>(rule, "", node2, rule.getMessage())); assertEquals(2, r.getViolations().size()); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java index 7ae5aba5eb..7a01a1b8c0 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java @@ -9,8 +9,12 @@ import static org.junit.Assert.assertNull; import java.io.File; +import org.junit.Assert; import org.junit.Test; +import net.sourceforge.pmd.Report.ReportBuilderListener; +import net.sourceforge.pmd.lang.ast.impl.DummyTreeUtil; + import junit.framework.JUnit4TestAdapter; public class RuleContextTest { @@ -33,6 +37,29 @@ public class RuleContextTest { assertEquals("filename mismatch", "foo.java", ctx.getSourceCodeFilename()); } + + @Test + public void testMessage() throws Exception { + ReportBuilderListener listener = new ReportBuilderListener(); + try (RuleContext ctx = new RuleContext(listener)) { + ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with \"'{'\""); + } + + RuleViolation violation = listener.getReport().getViolations().get(0); + Assert.assertEquals("message with \"{\"", violation.getDescription()); + } + + @Test + public void testMessageArgs() throws Exception { + ReportBuilderListener listener = new ReportBuilderListener(); + try (RuleContext ctx = new RuleContext(listener)) { + ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with 1 argument: \"{0}\"", "testarg1"); + } + + RuleViolation violation = listener.getReport().getViolations().get(0); + Assert.assertEquals("message with 1 argument: \"testarg1\"", violation.getDescription()); + } + @Test public void testSourceCodeFile() { RuleContext ctx = new RuleContext(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationComparatorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationComparatorTest.java index 45d3571c5c..1e810ae6ab 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationComparatorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationComparatorTest.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; -import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -71,10 +70,8 @@ public class RuleViolationComparatorTest { private RuleViolation createJavaRuleViolation(Rule rule, String fileName, int beginLine, String description, int beginColumn, int endLine, int endColumn) { - RuleContext ruleContext = new RuleContext(); - ruleContext.setSourceCodeFile(new File(fileName)); DummyNode simpleNode = new DummyNode(); simpleNode.setCoords(beginLine, beginColumn, endLine, endColumn); - return new ParametricRuleViolation(rule, ruleContext, simpleNode, description); + return new ParametricRuleViolation(rule, fileName, simpleNode, description); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationTest.java index d9717e3e4c..f47d766cf6 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationTest.java @@ -7,8 +7,6 @@ package net.sourceforge.pmd; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.io.File; - import org.junit.Ignore; import org.junit.Test; @@ -24,11 +22,9 @@ public class RuleViolationTest { @Test public void testConstructor1() { Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(2, 1, 2, 3); - RuleViolation r = new ParametricRuleViolation(rule, ctx, s, rule.getMessage()); + RuleViolation r = new ParametricRuleViolation(rule, "filename", s, rule.getMessage()); assertEquals("object mismatch", rule, r.getRule()); assertEquals("line number is wrong", 2, r.getBeginLine()); assertEquals("filename is wrong", "filename", r.getFilename()); @@ -37,11 +33,9 @@ public class RuleViolationTest { @Test public void testConstructor2() { Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(2, 1, 2, 3); - RuleViolation r = new ParametricRuleViolation(rule, ctx, s, "description"); + RuleViolation r = new ParametricRuleViolation(rule, "filename", s, "description"); assertEquals("object mismatch", rule, r.getRule()); assertEquals("line number is wrong", 2, r.getBeginLine()); assertEquals("filename is wrong", "filename", r.getFilename()); @@ -52,15 +46,12 @@ public class RuleViolationTest { public void testComparatorWithDifferentFilenames() { Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); RuleViolationComparator comp = RuleViolationComparator.INSTANCE; - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename1")); DummyNode s = new DummyNode(); s.setCoords(10, 1, 11, 3); - RuleViolation r1 = new ParametricRuleViolation(rule, ctx, s, "description"); - ctx.setSourceCodeFile(new File("filename2")); + RuleViolation r1 = new ParametricRuleViolation(rule, "filename1", s, "description"); DummyNode s1 = new DummyNode(); s1.setCoords(10, 1, 11, 3); - RuleViolation r2 = new ParametricRuleViolation(rule, ctx, s1, "description"); + RuleViolation r2 = new ParametricRuleViolation(rule, "filename2", s1, "description"); assertEquals(-1, comp.compare(r1, r2)); assertEquals(1, comp.compare(r2, r1)); } @@ -69,14 +60,12 @@ public class RuleViolationTest { public void testComparatorWithSameFileDifferentLines() { Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); RuleViolationComparator comp = RuleViolationComparator.INSTANCE; - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(10, 1, 15, 10); DummyNode s1 = new DummyNode(); s1.setCoords(20, 1, 25, 10); - RuleViolation r1 = new ParametricRuleViolation(rule, ctx, s, "description"); - RuleViolation r2 = new ParametricRuleViolation(rule, ctx, s1, "description"); + RuleViolation r1 = new ParametricRuleViolation(rule, "filename", s, "description"); + RuleViolation r2 = new ParametricRuleViolation(rule, "filename", s1, "description"); assertTrue(comp.compare(r1, r2) < 0); assertTrue(comp.compare(r2, r1) > 0); } @@ -86,14 +75,12 @@ public class RuleViolationTest { public void testComparatorWithSameFileSameLines() { Rule rule = new MockRule("name", "desc", "msg", "rulesetname"); RuleViolationComparator comp = RuleViolationComparator.INSTANCE; - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File("filename")); DummyNode s = new DummyNode(); s.setCoords(10, 1, 15, 10); DummyNode s1 = new DummyNode(); s.setCoords(10, 1, 15, 10); - RuleViolation r1 = new ParametricRuleViolation(rule, ctx, s, "description"); - RuleViolation r2 = new ParametricRuleViolation(rule, ctx, s1, "description"); + RuleViolation r1 = new ParametricRuleViolation(rule, "filename", s, "description"); + RuleViolation r2 = new ParametricRuleViolation(rule, "filename", s1, "description"); assertEquals(1, comp.compare(r1, r2)); assertEquals(1, comp.compare(r2, r1)); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java index f78e5b0e6b..3090e4f3b4 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java @@ -4,8 +4,9 @@ package net.sourceforge.pmd.lang; +import org.checkerframework.checker.nullness.qual.NonNull; + import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.DummyAstStages; import net.sourceforge.pmd.lang.ast.DummyRoot; @@ -58,26 +59,14 @@ public class DummyLanguageModule extends BaseLanguageModule { } public static class RuleViolationFactory extends DefaultRuleViolationFactory { - @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message) { - return createRuleViolation(rule, ruleContext, node, message, 0, 0); - } @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message, - int beginLine, int endLine) { - ParametricRuleViolation rv = new ParametricRuleViolation(rule, ruleContext, node, message) { + public RuleViolation createViolation(Rule rule, @NonNull Node location, String filename, String formattedMessage) { + return new ParametricRuleViolation(rule, filename, location, formattedMessage) { { this.packageName = "foo"; // just for testing variable expansion } - - @Override - public String getPackageName() { - return super.getPackageName(); - } }; - rv.setLines(beginLine, endLine); - return rv; } } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/DefaultRuleViolationFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/DefaultRuleViolationFactoryTest.java index baaed1c018..be4976ee99 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/DefaultRuleViolationFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/DefaultRuleViolationFactoryTest.java @@ -5,17 +5,16 @@ package net.sourceforge.pmd.lang.rule; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.lang.ast.DummyRoot; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.impl.DummyTreeUtil; import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory; public class DefaultRuleViolationFactoryTest { - private RuleContext ruleContext; - private RuleViolationFactory factory = DefaultRuleViolationFactory.defaultInstance(); private static class TestRule extends AbstractRule { @Override @@ -24,24 +23,11 @@ public class DefaultRuleViolationFactoryTest { } } - @Before - public void setup() { - ruleContext = new RuleContext(); - } - @Test public void testMessage() { - factory.addViolation(ruleContext, new TestRule(), null, "message with \"'{'\"", null); + RuleViolation violation = DefaultRuleViolationFactory.defaultInstance().createViolation(new TestRule(), DummyTreeUtil.tree(DummyRoot::new), "file", "Some message"); - RuleViolation violation = ruleContext.getReport().getViolations().get(0); - Assert.assertEquals("message with \"{\"", violation.getDescription()); + Assert.assertEquals("Some message", violation.getDescription()); } - @Test - public void testMessageArgs() { - factory.addViolation(ruleContext, new TestRule(), null, "message with 1 argument: \"{0}\"", new Object[] {"testarg1"}); - - RuleViolation violation = ruleContext.getReport().getViolations().get(0); - Assert.assertEquals("message with 1 argument: \"testarg1\"", violation.getDescription()); - } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java index cc959120ea..f8071db9f0 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java @@ -6,8 +6,6 @@ package net.sourceforge.pmd.renderers; import static org.junit.Assert.assertEquals; -import java.io.File; - import org.junit.Test; import net.sourceforge.pmd.FooRule; @@ -15,7 +13,6 @@ import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.RuleWithProperties; import net.sourceforge.pmd.lang.ast.DummyNode; @@ -76,9 +73,7 @@ public abstract class AbstractRendererTest { protected RuleViolation newRuleViolation(int endColumn) { DummyNode node = createNode(endColumn); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - return new ParametricRuleViolation(new FooRule(), ctx, node, "blah"); + return new ParametricRuleViolation(new FooRule(), getSourceCodeFilename(), node, "blah"); } protected static DummyNode createNode(int endColumn) { @@ -90,13 +85,11 @@ public abstract class AbstractRendererTest { @Test public void testRuleWithProperties() throws Exception { DummyNode node = createNode(1); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); Report report = new Report(); RuleWithProperties theRule = new RuleWithProperties(); theRule.setProperty(RuleWithProperties.STRING_PROPERTY_DESCRIPTOR, "the string value\nsecond line with \"quotes\""); - report.addRuleViolation(new ParametricRuleViolation(theRule, ctx, node, "blah")); + report.addRuleViolation(new ParametricRuleViolation(theRule, getSourceCodeFilename(), node, "blah")); String rendered = ReportTest.render(getRenderer(), report); assertEquals(filter(getExpectedWithProperties()), filter(rendered)); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java index ddf3c83589..217a7ce240 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java @@ -6,14 +6,11 @@ package net.sourceforge.pmd.renderers; import static org.junit.Assert.assertEquals; -import java.io.File; - import org.junit.Test; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; @@ -92,8 +89,6 @@ public class CodeClimateRendererTest extends AbstractRendererTest { @Test public void testXPathRule() throws Exception { DummyNode node = createNode(1); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); Report report = new Report(); XPathRule theRule = new XPathRule(XPathVersion.XPATH_3_1, "//dummyNode"); @@ -101,7 +96,7 @@ public class CodeClimateRendererTest extends AbstractRendererTest { theRule.setDescription("desc"); theRule.setName("Foo"); - report.addRuleViolation(new ParametricRuleViolation(theRule, ctx, node, "blah")); + report.addRuleViolation(new ParametricRuleViolation(theRule, getSourceCodeFilename(), node, "blah")); String rendered = ReportTest.render(getRenderer(), report); // Output should be the exact same as for non xpath rules diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java index 867868b007..388d2f99cb 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java @@ -6,7 +6,7 @@ package net.sourceforge.pmd.renderers; import static org.junit.Assert.assertEquals; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; import org.junit.Test; @@ -16,10 +16,12 @@ import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; +import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.ReportTest; import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.lang.DummyLanguageModule; +import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.DummyRoot; -import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory; public class SummaryHTMLRendererTest extends AbstractRendererTest { @@ -146,13 +148,15 @@ public class SummaryHTMLRendererTest extends AbstractRendererTest { assertEquals(getExpectedEmpty(), actual); } - private Report createEmptyReportWithSuppression() { - Map suppressions = new HashMap<>(); - suppressions.put(1, "test"); - RuleContext ctx = new RuleContext(); - DummyRoot root = new DummyRoot(suppressions); - root.setCoords(1, 10, 4, 5); - DefaultRuleViolationFactory.defaultInstance().addViolation(ctx, new FooRule(), root, "suppress test", 1, 1, new Object[0]); - return ctx.getReport(); + private Report createEmptyReportWithSuppression() throws Exception { + Map suppressions = Collections.singletonMap(1, "test"); + ReportBuilderListener listener = new ReportBuilderListener(); + try (RuleContext ctx = new RuleContext(listener)) { + ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + DummyRoot root = new DummyRoot(suppressions); + root.setCoords(1, 10, 4, 5); + ctx.addViolationWithPosition(new FooRule(), root, 1, 1, "suppress test"); + } + return listener.getReport(); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java index 2cfbd26ee1..44cbd72c8b 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.renderers; -import java.io.File; import java.io.StringReader; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -25,7 +24,6 @@ import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.Node; @@ -87,9 +85,7 @@ public class XMLRendererTest extends AbstractRendererTest { private RuleViolation createRuleViolation(String description) { DummyNode node = new DummyNode(); node.setCoords(1, 1, 1, 1); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - return new ParametricRuleViolation(new FooRule(), ctx, node, description); + return new ParametricRuleViolation(new FooRule(), getSourceCodeFilename(), node, description); } private void verifyXmlEscaping(Renderer renderer, String shouldContain, Charset charset) throws Exception { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XSLTRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XSLTRendererTest.java index 2cc3f0c9f4..98b79111f3 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XSLTRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XSLTRendererTest.java @@ -10,7 +10,6 @@ import org.junit.Test; import net.sourceforge.pmd.FooRule; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.Node; @@ -24,8 +23,7 @@ public class XSLTRendererTest { Report report = new Report(); DummyNode node = new DummyNode(); node.setCoords(1, 1, 1, 2); - RuleViolation rv = new ParametricRuleViolation(new FooRule(), new RuleContext(), node, - "violation message"); + RuleViolation rv = new ParametricRuleViolation(new FooRule(), "file", node, "violation message"); report.addRuleViolation(rv); String result = ReportTest.render(renderer, report); Assert.assertTrue(result.contains("violation message")); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java index 0d7f68341a..141d8452c7 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java @@ -25,7 +25,6 @@ import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.Node; @@ -70,9 +69,7 @@ public class YAHTMLRendererTest extends AbstractRendererTest { private RuleViolation newRuleViolation(int endColumn, final String packageNameArg, final String classNameArg) { DummyNode node = createNode(endColumn); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - return new ParametricRuleViolation(new FooRule(), ctx, node, "blah") { + return new ParametricRuleViolation(new FooRule(), getSourceCodeFilename(), node, "blah") { { packageName = packageNameArg; className = classNameArg; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolation.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolation.java index efa75cade2..2b8accaa26 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolation.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolation.java @@ -7,8 +7,9 @@ package net.sourceforge.pmd.lang.java.rule; import java.util.Iterator; import java.util.Set; +import org.checkerframework.checker.nullness.qual.NonNull; + import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; @@ -43,32 +44,24 @@ import net.sourceforge.pmd.lang.symboltable.Scope; @Deprecated public class JavaRuleViolation extends ParametricRuleViolation { - public JavaRuleViolation(Rule rule, RuleContext ctx, JavaNode node, String message, int beginLine, int endLine) { - this(rule, ctx, node, message); + public JavaRuleViolation(Rule rule, @NonNull JavaNode node, String filename, String message) { + super(rule, filename, node, message); - setLines(beginLine, endLine); - } + final Scope scope = node.getScope(); + final SourceFileScope sourceFileScope = scope.getEnclosingScope(SourceFileScope.class); - public JavaRuleViolation(Rule rule, RuleContext ctx, JavaNode node, String message) { - super(rule, ctx, node, message); + // Package name is on SourceFileScope + packageName = sourceFileScope.getPackageName() == null ? "" : sourceFileScope.getPackageName(); - if (node != null) { - final Scope scope = node.getScope(); - final SourceFileScope sourceFileScope = scope.getEnclosingScope(SourceFileScope.class); + // Class name is built from enclosing ClassScopes + setClassNameFrom(node); - // Package name is on SourceFileScope - packageName = sourceFileScope.getPackageName() == null ? "" : sourceFileScope.getPackageName(); - - // Class name is built from enclosing ClassScopes - setClassNameFrom(node); - - // Method name comes from 1st enclosing MethodScope - if (scope.getEnclosingScope(MethodScope.class) != null) { - methodName = scope.getEnclosingScope(MethodScope.class).getName(); - } - // Variable name node specific - setVariableNameIfExists(node); + // Method name comes from 1st enclosing MethodScope + if (scope.getEnclosingScope(MethodScope.class) != null) { + methodName = scope.getEnclosingScope(MethodScope.class).getName(); } + // Variable name node specific + setVariableNameIfExists(node); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleViolationFactory.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleViolationFactory.java index a7f2dc595b..24604ac01c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleViolationFactory.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleViolationFactory.java @@ -12,7 +12,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.ViolationSuppressor; import net.sourceforge.pmd.lang.ast.Node; @@ -49,14 +48,8 @@ public final class JavaRuleViolationFactory extends DefaultRuleViolationFactory } @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message) { - return new JavaRuleViolation(rule, ruleContext, (JavaNode) node, message); - } - - @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message, - int beginLine, int endLine) { - return new JavaRuleViolation(rule, ruleContext, (JavaNode) node, message, beginLine, endLine); + public RuleViolation createViolation(Rule rule, @NonNull Node location, String filename, String formattedMessage) { + return new JavaRuleViolation(rule, (JavaNode) location, filename, formattedMessage); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolationFactoryTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolationFactoryTest.java deleted file mode 100644 index 1ddbf886a9..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/JavaRuleViolationFactoryTest.java +++ /dev/null @@ -1,26 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule; - -import org.junit.Test; - -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.lang.java.rule.codestyle.DuplicateImportsRule; -import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleViolationFactory; -import net.sourceforge.pmd.lang.rule.RuleViolationFactory; - -/** - * @author guofei - * - */ -public class JavaRuleViolationFactoryTest { - - @Test - public void messageWithSingleBrace() { - RuleViolationFactory factory = JavaRuleViolationFactory.INSTANCE; - factory.addViolation(new RuleContext(), new DuplicateImportsRule(), null, "message with \"'{'\"", null); - } - -} diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/ReportTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/ReportTest.java index a74c4e18a8..4aa9ecae6b 100644 --- a/pmd-javascript/src/test/java/net/sourceforge/pmd/ReportTest.java +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/ReportTest.java @@ -23,7 +23,7 @@ public class ReportTest extends RuleTst { Rule rule = new AbstractEcmascriptRule() { @Override public Object visit(ASTFunctionNode node, Object data) { - EcmascriptLanguageModule.defaultHandler().getRuleViolationFactory().addViolation((RuleContext) data, this, node, "Test", null); + addViolationWithMessage(data, node, "Test"); return super.visit(node, data); } }; diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/test/lang/DummyLanguageModule.java b/pmd-test/src/main/java/net/sourceforge/pmd/test/lang/DummyLanguageModule.java index 3aab441e1e..d02f322bc7 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/test/lang/DummyLanguageModule.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/test/lang/DummyLanguageModule.java @@ -4,16 +4,11 @@ package net.sourceforge.pmd.test.lang; -import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.AbstractPmdLanguageVersionHandler; import net.sourceforge.pmd.lang.BaseLanguageModule; import net.sourceforge.pmd.lang.Parser; import net.sourceforge.pmd.lang.ParserOptions; -import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.RootNode; -import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory; import net.sourceforge.pmd.test.lang.ast.DummyNode; @@ -61,23 +56,6 @@ public class DummyLanguageModule extends BaseLanguageModule { public static class RuleViolationFactory extends DefaultRuleViolationFactory { - @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message) { - return createRuleViolation(rule, ruleContext, node, message, 0, 0); - } - @Override - protected RuleViolation createRuleViolation(Rule rule, RuleContext ruleContext, Node node, String message, - int beginLine, int endLine) { - ParametricRuleViolation rv = new ParametricRuleViolation(rule, ruleContext, node, message) { - @Override - public String getPackageName() { - this.packageName = "foo"; // just for testing variable expansion - return super.getPackageName(); - } - }; - rv.setLines(beginLine, endLine); - return rv; - } } }