From f62a411a45ee94bb8ae629c5fc36b21c97e46b05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Jul 2020 14:48:02 +0200 Subject: [PATCH 1/4] Deprecate some Report methods --- docs/pages/release_notes.md | 7 +++++++ .../src/main/java/net/sourceforge/pmd/Report.java | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..417a52096c 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,13 @@ This is a {{ site.pmd.release_type }} release. ### API Changes +#### Deprecated API + +- {% jdoc !!core::Report#treeSize() %} +- {% jdoc !!core::Report#treeIterator() %} +- {% jdoc !!core::Report#treeIsEmpty() %} +- {% jdoc !!core::Report#getCountSummary() %} + ### External Contributions {% endtocmaker %} 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 f25a68bae1..bb7b192204 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -258,7 +258,10 @@ public class Report implements Iterable { * Calculate a summary of violation counts per fully classified class name. * * @return violations per class name + * + * @deprecated This is too specific. Not every violation has a qualified name. */ + @Deprecated public Map getCountSummary() { Map summary = new HashMap<>(); for (RuleViolation rv : violationTree) { @@ -465,7 +468,10 @@ public class Report implements Iterable { * * @return true if no violations have been reported, * false otherwise + * + * @deprecated The {@link ReportTree} is deprecated */ + @Deprecated public boolean treeIsEmpty() { return !violationTree.iterator().hasNext(); } @@ -474,7 +480,10 @@ public class Report implements Iterable { * Returns an iteration over the reported violations. * * @return an iterator + * + * @deprecated The {@link ReportTree} is deprecated */ + @Deprecated public Iterator treeIterator() { return violationTree.iterator(); } @@ -506,7 +515,10 @@ public class Report implements Iterable { * The number of violations. * * @return number of violations. + * + * @deprecated The {@link ReportTree} is deprecated */ + @Deprecated public int treeSize() { return violationTree.size(); } From ce3257309073b683299fc3cfcc4233e6c19b5300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Jul 2020 14:55:01 +0200 Subject: [PATCH 2/4] Deprecate more methods --- .../main/java/net/sourceforge/pmd/Report.java | 62 +++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-) 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 bb7b192204..0095987b92 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -45,8 +45,8 @@ public class Report implements Iterable { private final List violations = new ArrayList<>(); private final Set metrics = new HashSet<>(); private final List listeners = new ArrayList<>(); - private List errors; - private List configErrors; + private List errors = new ArrayList<>(); + private List configErrors = new ArrayList<>(); private Map linesToSuppress = new HashMap<>(); private long start; private long end; @@ -439,6 +439,10 @@ public class Report implements Iterable { return metrics.iterator(); } + /** + * @deprecated Use {@link #getViolations()} or {@link #getProcessingErrors()} + */ + @Deprecated public boolean isEmpty() { return !violations.iterator().hasNext() && !hasErrors(); } @@ -448,9 +452,12 @@ public class Report implements Iterable { * * @return true if there were any processing errors, * false otherwise + * + * @deprecated Use {@link #getProcessingErrors()}.isEmpty() */ + @Deprecated public boolean hasErrors() { - return errors != null && !errors.isEmpty(); + return getProcessingErrors().isEmpty(); } /** @@ -458,9 +465,12 @@ public class Report implements Iterable { * * @return true if there were any configuration errors, * false otherwise + * + * @deprecated Use {@link #getConfigErrors()}.isEmpty() */ + @Deprecated public boolean hasConfigErrors() { - return configErrors != null && !configErrors.isEmpty(); + return getConfigErrors().isEmpty(); } /** @@ -488,27 +498,64 @@ public class Report implements Iterable { return violationTree.iterator(); } + /** + * @deprecated Use {@link #getViolations()} + */ + @Deprecated @Override public Iterator iterator() { return violations.iterator(); } + + /** + * Returns an unmodifiable list of violations that have been + * recorded until now. + */ + public List getViolations() { + return Collections.unmodifiableList(violations); + } + + + /** + * Returns an unmodifiable list of processing errors that have been + * recorded until now. + */ + public List getProcessingErrors() { + return Collections.unmodifiableList(errors); + } + + + /** + * Returns an unmodifiable list of configuration errors that have + * been recorded until now. + */ + public List getConfigErrors() { + return Collections.unmodifiableList(configErrors); + } + + /** * Returns an iterator of the reported processing errors. * * @return the iterator + * + * @deprecated Use {@link #getProcessingErrors()} */ + @Deprecated public Iterator errors() { - return errors == null ? Collections.emptyIterator() : errors.iterator(); + return getProcessingErrors().iterator(); } /** * Returns an iterator of the reported configuration errors. * * @return the iterator + * @deprecated Use {@link #getConfigErrors()} */ + @Deprecated public Iterator configErrors() { - return configErrors == null ? Collections.emptyIterator() : configErrors.iterator(); + return getConfigErrors().iterator(); } /** @@ -527,7 +574,10 @@ public class Report implements Iterable { * The number of violations. * * @return number of violations. + * + * @deprecated Use {@link #getViolations()} */ + @Deprecated public int size() { return violations.size(); } From 772bc1346ca41585a9fa8f38f28a9f050fdff952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 28 Jul 2020 19:33:01 +0200 Subject: [PATCH 3/4] Deprecate more things --- docs/pages/release_notes.md | 8 +-- .../main/java/net/sourceforge/pmd/Report.java | 70 +++++++++---------- .../pmd/renderers/SummaryHTMLRenderer.java | 25 ++++++- .../java/net/sourceforge/pmd/RuleSetTest.java | 4 +- 4 files changed, 64 insertions(+), 43 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 417a52096c..bee7e4572f 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,10 +20,10 @@ This is a {{ site.pmd.release_type }} release. #### Deprecated API -- {% jdoc !!core::Report#treeSize() %} -- {% jdoc !!core::Report#treeIterator() %} -- {% jdoc !!core::Report#treeIsEmpty() %} -- {% jdoc !!core::Report#getCountSummary() %} +- Many methods of {% jdoc !!core::Report %}. They are replaced by accessors +that produce a List. For example, {% jdoc !a!core::Report#iterator() %} +(and implementing Iterable) and {% jdoc !a!core::Report#isEmpty() %} are both +replaced by {% jdoc !a!core::Report#getViolations() %}. ### External Contributions 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 0095987b92..fe7319e1d4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -45,12 +45,12 @@ public class Report implements Iterable { private final List violations = new ArrayList<>(); private final Set metrics = new HashSet<>(); private final List listeners = new ArrayList<>(); - private List errors = new ArrayList<>(); - private List configErrors = new ArrayList<>(); + private final List errors = new ArrayList<>(); + private final List configErrors = new ArrayList<>(); private Map linesToSuppress = new HashMap<>(); private long start; private long end; - private List suppressedRuleViolations = new ArrayList<>(); + private final List suppressedRuleViolations = new ArrayList<>(); /** * Creates a new, initialized, empty report for the given file name. @@ -272,6 +272,10 @@ public class Report implements Iterable { return summary; } + /** + * @deprecated The {@link ReportTree} is deprecated + */ + @Deprecated public ReportTree getViolationTree() { return this.violationTree; } @@ -281,7 +285,10 @@ public class Report implements Iterable { * * @return a Map summarizing the Report: String (rule name) -> Integer (count * of violations) + * + * @deprecated This is too specific, only used by one renderer. */ + @Deprecated public Map getSummary() { Map summary = new HashMap<>(); for (RuleViolation rv : violations) { @@ -305,10 +312,6 @@ public class Report implements Iterable { listeners.add(listener); } - public List getSuppressedRuleViolations() { - return suppressedRuleViolations; - } - /** * Adds a new rule violation to the report and notify the listeners. * @@ -360,9 +363,6 @@ public class Report implements Iterable { * the error to add */ public void addConfigError(ConfigurationError error) { - if (configErrors == null) { - configErrors = new ArrayList<>(); - } configErrors.add(error); } @@ -373,9 +373,6 @@ public class Report implements Iterable { * the error to add */ public void addError(ProcessingError error) { - if (errors == null) { - errors = new ArrayList<>(); - } errors.add(error); } @@ -389,29 +386,16 @@ public class Report implements Iterable { * @see AbstractAccumulatingRenderer */ public void merge(Report r) { - Iterator i = r.errors(); - while (i.hasNext()) { - addError(i.next()); - } - Iterator ce = r.configErrors(); - while (ce.hasNext()) { - addConfigError(ce.next()); - } - Iterator m = r.metrics(); - while (m.hasNext()) { - addMetric(m.next()); - } - Iterator v = r.iterator(); - while (v.hasNext()) { - RuleViolation violation = v.next(); + errors.addAll(r.errors); + configErrors.addAll(r.configErrors); + metrics.addAll(r.metrics); + suppressedRuleViolations.addAll(r.suppressedRuleViolations); + + for (RuleViolation violation : r.getViolations()) { int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE); violations.add(index < 0 ? -index - 1 : index, violation); violationTree.addRuleViolation(violation); } - Iterator s = r.getSuppressedRuleViolations().iterator(); - while (s.hasNext()) { - suppressedRuleViolations.add(s.next()); - } } /** @@ -457,7 +441,7 @@ public class Report implements Iterable { */ @Deprecated public boolean hasErrors() { - return getProcessingErrors().isEmpty(); + return !getProcessingErrors().isEmpty(); } /** @@ -470,7 +454,7 @@ public class Report implements Iterable { */ @Deprecated public boolean hasConfigErrors() { - return getConfigErrors().isEmpty(); + return !getConfigErrors().isEmpty(); } /** @@ -508,9 +492,16 @@ public class Report implements Iterable { } + /** + * Returns an unmodifiable list of violations that were suppressed. + */ + public List getSuppressedRuleViolations() { + return Collections.unmodifiableList(suppressedRuleViolations); + } + /** * Returns an unmodifiable list of violations that have been - * recorded until now. + * recorded until now. None of those violations were suppressed. */ public List getViolations() { return Collections.unmodifiableList(violations); @@ -587,7 +578,10 @@ public class Report implements Iterable { * in the end. * * @see #getElapsedTimeInMillis() + * + * @deprecated Not used, {@link #getElapsedTimeInMillis()} will be removed */ + @Deprecated public void start() { start = System.currentTimeMillis(); } @@ -596,11 +590,17 @@ public class Report implements Iterable { * Mark the end time of the report. This is ued to get the elapsed time. * * @see #getElapsedTimeInMillis() + * @deprecated Not used, {@link #getElapsedTimeInMillis()} will be removed */ + @Deprecated public void end() { end = System.currentTimeMillis(); } + /** + * @deprecated Unused + */ + @Deprecated public long getElapsedTimeInMillis() { return end - start; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java index fb6d8cf272..8f4881669f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java @@ -5,9 +5,15 @@ package net.sourceforge.pmd.renderers; import java.io.IOException; +import java.util.HashMap; import java.util.Map; +import java.util.Map.Entry; + +import org.apache.commons.lang3.mutable.MutableInt; import net.sourceforge.pmd.PMD; +import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.RuleViolation; /** * Renderer to a summarized HTML format. @@ -59,8 +65,8 @@ public class SummaryHTMLRenderer extends AbstractAccumulatingRenderer { writer.write("

Summary

" + PMD.EOL); writer.write("" + PMD.EOL); writer.write("" + PMD.EOL); - Map summary = report.getSummary(); - for (Map.Entry entry : summary.entrySet()) { + Map summary = getSummary(report); + for (Entry entry : summary.entrySet()) { String ruleName = entry.getKey(); writer.write("
Rule nameNumber of violations
"); writer.write(ruleName); @@ -70,4 +76,19 @@ public class SummaryHTMLRenderer extends AbstractAccumulatingRenderer { } writer.write("
" + PMD.EOL); } + + private static Map getSummary(Report report) { + Map summary = new HashMap<>(); + for (RuleViolation rv : report.getViolations()) { + String name = rv.getRule().getName(); + MutableInt count = summary.get(name); + if (count == null) { + count = new MutableInt(0); + summary.put(name, count); + } + count.increment(); + } + return summary; + } + } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java index f19014a2c8..b07deb6b10 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -526,8 +526,8 @@ public class RuleSetTest { context.setIgnoreExceptions(true); // the default ruleset.apply(makeCompilationUnits(), context); - assertTrue("Report should have processing errors", context.getReport().hasErrors()); - List errors = CollectionUtil.toList(context.getReport().errors()); + List errors = context.getReport().getProcessingErrors(); + assertTrue("Report should have processing errors", !errors.isEmpty()); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); From 34d961b79421e6c84728a133dc7b6e502a1764b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 29 Jul 2020 17:44:55 +0200 Subject: [PATCH 4/4] Deprecate ReadableDuration --- docs/pages/release_notes.md | 1 + pmd-core/src/main/java/net/sourceforge/pmd/Report.java | 3 +++ 2 files changed, 4 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index bee7e4572f..148cf916cb 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -24,6 +24,7 @@ This is a {{ site.pmd.release_type }} release. that produce a List. For example, {% jdoc !a!core::Report#iterator() %} (and implementing Iterable) and {% jdoc !a!core::Report#isEmpty() %} are both replaced by {% jdoc !a!core::Report#getViolations() %}. +- {% jdoc !!core::Report.ReadableDuration %} ### External Contributions 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 fe7319e1d4..6f33d5ea31 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -74,7 +74,10 @@ public class Report implements Iterable { /** * Represents a duration. Useful for reporting processing time. + * + * @deprecated Not used within PMD. Rendering durations is format-specific. */ + @Deprecated public static class ReadableDuration { private final long duration;