Merge branch 'pr-774'

2017-12-08 00:59:32 -03:00
parent 5a228d2b6f 9c832e3ac5
commit 83c15e2e45
6 changed files with 239 additions and 1 deletions
--- a/docs/pages/pmd/rules/java.md
+++ b/docs/pages/pmd/rules/java.md
@ -292,6 +292,7 @@ folder: pmd/rules
 *   [AddEmptyString](pmd_rules_java_performance.html#addemptystring): The conversion of literals to strings by concatenating them with empty strings is inefficient.It ...
 *   [AppendCharacterWithChar](pmd_rules_java_performance.html#appendcharacterwithchar): Avoid concatenating characters as strings in StringBuffer/StringBuilder.append methods.
 *   [AvoidArrayLoops](pmd_rules_java_performance.html#avoidarrayloops): Instead of manually copying data between two arrays, use the efficient System.arraycopy method in...
+*   [AvoidFileStream](pmd_rules_java_performance.html#avoidfilestream): The FileInputStream and FileOutputStream classes contains a finalizer method which will cause gar...
 *   [AvoidInstantiatingObjectsInLoops](pmd_rules_java_performance.html#avoidinstantiatingobjectsinloops): New objects created within loops should be checked to see if they can created outside them and re...
 *   [AvoidUsingShortType](pmd_rules_java_performance.html#avoidusingshorttype): Java uses the 'short' type to reduce memory usage, not to optimize calculation. In fact, the JVM ...
 *   [BigIntegerInstantiation](pmd_rules_java_performance.html#bigintegerinstantiation): Don't create instances of already existing BigInteger (BigInteger.ZERO, BigInteger.ONE) andfor Ja...
--- a/docs/pages/pmd/rules/java/performance.md
+++ b/docs/pages/pmd/rules/java/performance.md
@ -5,7 +5,7 @@ permalink: pmd_rules_java_performance.html
 folder: pmd/rules/java
 sidebaractiveurl: /pmd_rules_java.html
 editmepath: ../pmd-java/src/main/resources/category/java/performance.xml
-keywords: Performance, AddEmptyString, AppendCharacterWithChar, AvoidArrayLoops, AvoidInstantiatingObjectsInLoops, AvoidUsingShortType, BigIntegerInstantiation, BooleanInstantiation, ByteInstantiation, ConsecutiveAppendsShouldReuse, ConsecutiveLiteralAppends, InefficientEmptyStringCheck, InefficientStringBuffering, InsufficientStringBufferDeclaration, IntegerInstantiation, LongInstantiation, OptimizableToArrayCall, RedundantFieldInitializer, SimplifyStartsWith, ShortInstantiation, StringInstantiation, StringToString, TooFewBranchesForASwitchStatement, UnnecessaryWrapperObjectCreation, UseArrayListInsteadOfVector, UseArraysAsList, UseIndexOfChar, UselessStringValueOf, UseStringBufferForStringAppends, UseStringBufferLength
+keywords: Performance, AddEmptyString, AppendCharacterWithChar, AvoidArrayLoops, AvoidFileStream, AvoidInstantiatingObjectsInLoops, AvoidUsingShortType, BigIntegerInstantiation, BooleanInstantiation, ByteInstantiation, ConsecutiveAppendsShouldReuse, ConsecutiveLiteralAppends, InefficientEmptyStringCheck, InefficientStringBuffering, InsufficientStringBufferDeclaration, IntegerInstantiation, LongInstantiation, OptimizableToArrayCall, RedundantFieldInitializer, SimplifyStartsWith, ShortInstantiation, StringInstantiation, StringToString, TooFewBranchesForASwitchStatement, UnnecessaryWrapperObjectCreation, UseArrayListInsteadOfVector, UseArraysAsList, UseIndexOfChar, UselessStringValueOf, UseStringBufferForStringAppends, UseStringBufferLength
 ---
 ## AddEmptyString

@ -109,6 +109,59 @@ public class Test {
 <rule ref="category/java/performance.xml/AvoidArrayLoops" />
 ```

+## AvoidFileStream
+
+**Since:** PMD 6.0.0
+
+**Priority:** High (1)
+
+**Minimum Language Version:** Java 1.7
+
+The FileInputStream and FileOutputStream classes contains a finalizer method which will cause garbage collection pauses. See [JDK-8080225](https://bugs.openjdk.java.net/browse/JDK-8080225) for details.
+
+The FileReader and FileWriter constructors instantiate FileInputStream and FileOutputStream, again causing garbage collection issues while finalizer methods are called.
+
+* Use `Files.newInputStream(Paths.get(fileName))` instead of `new FileInputStream(fileName)`.
+* Use `Files.newOutputStream(Paths.get(fileName))` instead of `new FileOutputStream(fileName)`.
+* Use `Files.newBufferedReader(Paths.get(fileName))` instead of `new FileReader(fileName)`.
+* Use `Files.newBufferedWriter(Paths.get(fileName))` instead of `new FileWriter(fileName)`.
+
+```
+//PrimaryPrefix/AllocationExpression/ClassOrInterfaceType[
+       typeof(@Image, 'java.io.FileInputStream', 'FileInputStream')
+    or typeof(@Image, 'java.io.FileOutputStream', 'FileOutputStream')
+    or typeof(@Image, 'java.io.FileReader', 'FileReader')
+    or typeof(@Image, 'java.io.FileWriter', 'FileWriter')
+  ]
+```
+
+**Example(s):**
+
+``` java
+// these instantiations cause garbage collection pauses, even if properly closed
+
+    FileInputStream fis = new FileInputStream(fileName);
+    FileOutputStream fos = new FileOutputStream(fileName);
+    FileReader fr = new FileReader(fileName);
+    FileWriter fw = new FileWriter(fileName);
+
+    // the following instantiations help prevent Garbage Collection pauses, no finalization
+
+    try(InputStream is = Files.newInputStream(Paths.get(fileName))) {
+    }
+    try(OutputStream os = Files.newOutputStream(Paths.get(fileName))) {
+    }
+    try(BufferedReader br = Files.newBufferedReader(Paths.get(fileName), StandardCharsets.UTF_8)) {
+    }
+    try(BufferedWriter wr = Files.newBufferedWriter(Paths.get(fileName), StandardCharsets.UTF_8)) {
+    }
+```
+
+**Use this rule by referencing it:**
+``` xml
+<rule ref="category/java/performance.xml/AvoidFileStream" />
+```
+
 ## AvoidInstantiatingObjectsInLoops

 **Since:** PMD 2.2
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@ -177,6 +177,10 @@ The rule reference documentation has been updated to reflect these changes.
 *   The new Java rule `ForLoopCanBeForeach` (category `errorprone`) helps to identify those for-loops that can
    be safely refactored into for-each-loops available since java 1.5.

+*   The new Java rule `AvoidFileStream` (category `performance`) helps to identify code relying on `FileInputStream` / `FileOutputStream`
+    which, by using a finalizer, produces extra / unnecessary overhead to garbage collection, and should be replaced with
+    `Files.newInputStream` / `Files.newOutputStream` available since java 1.7.
+
 *   The new Apex rule `AvoidDirectAccessTriggerMap` (category `errorprone`) helps to identify direct array access to triggers,
    which can produce bugs by either accessing non-existing indexes, or leaving them out. You should use for-each-loops
    instead.
@ -567,4 +571,5 @@ a warning will now be produced suggesting users to adopt it for better performan
 *   [#749](https://github.com/pmd/pmd/pull/749): \[doc] Update the documentation for properties - [Clément Fournier](https://github.com/oowekyala)
 *   [#758](https://github.com/pmd/pmd/pull/758): \[core] Expose the full mapping from property type id to property extractor - [Clément Fournier](https://github.com/oowekyala)
 *   [#771](https://github.com/pmd/pmd/pull/771): \[apex] Fix Apex metrics framework failing on triggers, refs #768 - [Clément Fournier](https://github.com/oowekyala)
+*   [#774](https://github.com/pmd/pmd/pull/774): \[java] Avoid using FileInput/Output - see JDK-8080225 - [Chas Honton](https://github.com/chonton)

--- a/pmd-java/src/main/resources/category/java/performance.xml
+++ b/pmd-java/src/main/resources/category/java/performance.xml
@ -112,6 +112,61 @@ public class Test {
        </example>
    </rule>

+    <rule name="AvoidFileStream"
+          since="6.0.0"
+          message="Avoid instantiating FileInputStream, FileOutputStream, FileReader, or FileWriter"
+          language="java"
+          minimumLanguageVersion="1.7"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#avoidfilestream">
+        <description>
+The FileInputStream and FileOutputStream classes contains a finalizer method which will cause garbage collection pauses. See [JDK-8080225](https://bugs.openjdk.java.net/browse/JDK-8080225) for details.
+
+The FileReader and FileWriter constructors instantiate FileInputStream and FileOutputStream, again causing garbage collection issues while finalizer methods are called.
+
+* Use `Files.newInputStream(Paths.get(fileName))` instead of `new FileInputStream(fileName)`.
+* Use `Files.newOutputStream(Paths.get(fileName))` instead of `new FileOutputStream(fileName)`.
+* Use `Files.newBufferedReader(Paths.get(fileName))` instead of `new FileReader(fileName)`.
+* Use `Files.newBufferedWriter(Paths.get(fileName))` instead of `new FileWriter(fileName)`.
+        </description>
+        <priority>1</priority>
+        <properties>
+            <property name="xpath">
+                <value>
+<![CDATA[
+//PrimaryPrefix/AllocationExpression/ClassOrInterfaceType[
+       typeof(@Image, 'java.io.FileInputStream', 'FileInputStream')
+    or typeof(@Image, 'java.io.FileOutputStream', 'FileOutputStream')
+    or typeof(@Image, 'java.io.FileReader', 'FileReader')
+    or typeof(@Image, 'java.io.FileWriter', 'FileWriter')
+  ]
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+<![CDATA[
+    // these instantiations cause garbage collection pauses, even if properly closed
+
+    FileInputStream fis = new FileInputStream(fileName);
+    FileOutputStream fos = new FileOutputStream(fileName);
+    FileReader fr = new FileReader(fileName);
+    FileWriter fw = new FileWriter(fileName);
+
+    // the following instantiations help prevent Garbage Collection pauses, no finalization
+
+    try(InputStream is = Files.newInputStream(Paths.get(fileName))) {
+    }
+    try(OutputStream os = Files.newOutputStream(Paths.get(fileName))) {
+    }
+    try(BufferedReader br = Files.newBufferedReader(Paths.get(fileName), StandardCharsets.UTF_8)) {
+    }
+    try(BufferedWriter wr = Files.newBufferedWriter(Paths.get(fileName), StandardCharsets.UTF_8)) {
+    }
+]]>
+        </example>
+    </rule>
+
    <rule name="AvoidInstantiatingObjectsInLoops"
          since="2.2"
          message="Avoid instantiating new objects inside loops"
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/PerformanceRulesTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/PerformanceRulesTest.java
@ -18,6 +18,7 @@ public class PerformanceRulesTest extends SimpleAggregatorTst {
        addRule(RULESET, "AddEmptyString");
        addRule(RULESET, "AppendCharacterWithChar");
        addRule(RULESET, "AvoidArrayLoops");
+        addRule(RULESET, "AvoidFileStream");
        addRule(RULESET, "AvoidInstantiatingObjectsInLoops");
        addRule(RULESET, "AvoidUsingShortType");
        addRule(RULESET, "BigIntegerInstantiation");
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidFileStream.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidFileStream.xml
@ -0,0 +1,123 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+    xmlns="http://pmd.sourceforge.net/rule-tests"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+
+    <test-code>
+        <description><![CDATA[
+instantiating FileInputStream
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+import java.io.FileInputStream;
+public class Foo {
+ public void bar() {
+    FileInputStream is = new FileInputStream(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating java.io.FileInputStream
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+    java.io.FileInputStream is = new java.io.FileInputStream(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating FileOutputStream
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+import java.io.FileOutputStream;
+public class Foo {
+ public void bar() {
+    FileOutputStream os = new FileOutputStream(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating java.io.FileOutputStream
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+    java.io.FileOutputStream os = new java.io.FileOutputStream(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating FileReader
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+import java.io.FileReader;
+public class Foo {
+ public void bar() {
+    FileReader rd = new FileReader(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating java.io.FileReader
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+    java.io.FileReader rd = new java.io.FileReader(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating FileWriter
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+import java.io.FileWriter;
+public class Foo {
+ public void bar() {
+    FileWriter wr = new FileWriter(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description><![CDATA[
+instantiating java.io.FileWriter
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void bar() {
+    java.io.FileWriter wr = new java.io.FileWriter(fileName);
+ }
+}
+     ]]></code>
+    </test-code>
+
+</test-data>