Merge pull request #3246 from vszholobov:visibleStaticFieldsMustBeFinal

[java] New Rule: MutableStaticState #3246

docs/pages/release_notes.md

@@ -30,6 +30,13 @@ to analyze JavaScript code. Note that PMD core still only requires Java 7.
     <rule ref="category/java/bestpractices.xml/JUnit5TestShouldBePackagePrivate" />
 ```
 
+*   The new Java rule {% rule "java/design/MutableStaticState" %} finds non-private static fields
+    that are not final. These fields break encapsulation since these fields can be modified from anywhere
+    within the program. You can try out this rule like so:
+```xml
+    <rule ref="category/java/design.xml/MutableStaticState" />
+```
+
 #### Modified rules
 
 *   The Java rule {% rule "java/errorprone/CompareObjectsWithEquals" %} has now a new property
@@ -93,6 +100,7 @@ to analyze JavaScript code. Note that PMD core still only requires Java 7.
     *   [#3275](https://github.com/pmd/pmd/pull/3275): \[java] UnnecessaryLocalBeforeReturn: false negatives with lambda and anon class
 *   java-design
     *   [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative
+    *   [#2987](https://github.com/pmd/pmd/issues/2987): \[java] New Rule: Public and protected static fields must be final
 *   java-errorprone
     *   [#3110](https://github.com/pmd/pmd/issues/3110): \[java] Enhance CompareObjectsWithEquals with list of exceptions
     *   [#3112](https://github.com/pmd/pmd/issues/3112): \[java] Deprecate rule CloneThrowsCloneNotSupportedException
@@ -108,5 +116,6 @@ to analyze JavaScript code. Note that PMD core still only requires Java 7.
 
 ### External Contributions
 *   [#3272](https://github.com/pmd/pmd/pull/3272): \[apex] correction for ApexUnitTestMethodShouldHaveIsTestAnnotation false positives - [William Brockhus](https://github.com/YodaDaCoda)
+*   [#3246](https://github.com/pmd/pmd/pull/3246): \[java] New Rule: MutableStaticState - [Vsevolod Zholobov](https://github.com/vszholobov)
 
 {% endtocmaker %}

pmd-core/src/main/resources/rulesets/releases/6350.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+
+<ruleset name="6350"
+    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+  <description>
+This ruleset contains links to rules that are new in PMD v6.35.0
+  </description>
+
+    <rule ref="category/java/design.xml/MutableStaticState" />
+
+</ruleset>

pmd-java/src/main/resources/category/java/design.xml

@@ -1731,6 +1731,44 @@ public class MaybeAUtility {
   public static void foo() {}
   public static void bar() {}
 }
+]]>
+        </example>
+    </rule>
+
+    <rule name="MutableStaticState"
+          language="java"
+          since="6.35.0"
+          message="Do not use non-final non-private static fields"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#mutablestaticstate">
+        <description>
+Non-private static fields should be made constants (or immutable references) by
+declaring them final.
+
+Non-private non-final static fields break encapsulation and can lead to hard to find
+bugs, since these fields can be modified from anywhere within the program.
+Callers can trivially access and modify non-private non-final static fields. Neither
+accesses nor modifications can be guarded against, and newly set values cannot
+be validated.
+
+If you are using this rule, then you don't need this
+rule {% rule java/errorprone/AssignmentToNonFinalStatic %}.
+        </description>
+        <priority>3</priority>
+        <properties>
+            <property name="version" value="2.0"/>
+            <property name="xpath">
+                <value>
+                    <![CDATA[
+//FieldDeclaration[@Static = true() and @Private = false() and @Final = false()]
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+public class Greeter { public static Foo foo = new Foo(); ... }       // avoid this
+public class Greeter { public static final Foo FOO = new Foo(); ... } // use this instead
 ]]>
         </example>
     </rule>

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/MutableStaticStateTest.java

@@ -0,0 +1,11 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.design;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class MutableStaticStateTest extends PmdRuleTst {
+    // no additional unit tests
+}

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/MutableStaticState.xml

@@ -0,0 +1,53 @@
+<?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>Public fields</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>2</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+    public static Foo foo = new Foo();
+    public static final Foo FOO = new Foo();
+}
+        ]]></code>
+        <source-type>java 11</source-type>
+    </test-code>
+
+    <test-code>
+        <description>Default fields</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>2</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+    static Foo foo = new Foo();
+    static final Foo FOO = new Foo();
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Protected fields</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>2</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+    protected static Foo foo = new Foo();
+    protected static final Foo FOO = new Foo();
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Private fields</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    private static Foo foo = new Foo();
+    private static final Foo FOO = new Foo();
+}
+        ]]></code>
+    </test-code>
+
+</test-data>