Update UseDiamondOperator

Convert it to a java rule

pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java

@@ -61,6 +61,14 @@ public class LanguageVersion implements Comparable<LanguageVersion> {
         return version.length() > 0 ? language.getTerseName() + ' ' + version : language.getTerseName();
     }
 
+    public int compareToVersion(String versionString) {
+        LanguageVersion otherVersion = language.getVersion(versionString);
+        if (otherVersion == null) {
+            throw new IllegalArgumentException("No such version '" + versionString + "' for language " + language.getName());
+        }
+        return this.compareTo(otherVersion);
+    }
+
     @Override
     public int compareTo(LanguageVersion o) {
         if (o == null) {

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UseDiamondOperatorRule.java

@@ -0,0 +1,79 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.codestyle;
+
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
+import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
+import net.sourceforge.pmd.lang.java.ast.ASTTypeArguments;
+import net.sourceforge.pmd.lang.java.ast.ExprContext;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
+import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol;
+import net.sourceforge.pmd.lang.java.types.JTypeMirror;
+import net.sourceforge.pmd.lang.java.types.TypeOps;
+
+public class UseDiamondOperatorRule extends AbstractJavaRulechainRule {
+
+    public UseDiamondOperatorRule() {
+        super(ASTConstructorCall.class);
+    }
+
+    public Object visit(ASTConstructorCall ctorCall, Object data) {
+        ASTClassOrInterfaceType newTypeNode = ctorCall.getTypeNode();
+        JTypeMirror newType = newTypeNode.getTypeMirror();
+
+        ASTTypeArguments targs = newTypeNode.getTypeArguments();
+        if (targs != null && targs.isDiamond()
+            // if unresolved we can't know whether the class is generic or not
+            || TypeOps.isUnresolved(newType)) {
+            return null;
+        }
+
+        // targs may be null, in which case this would be a raw type
+        if (!newType.isGeneric()) {
+            return null;
+        }
+
+        ExprContext exprCtx = ctorCall.getConversionContextType();
+        if (exprCtx == null) {
+            return null; // cannot be converted
+        }
+
+        if (!supportsDiamondOnAnonymousClass(ctorCall) && ctorCall.isAnonymousClass()
+            || useJava7Rules(ctorCall) && isNecessaryInJava7(newType, exprCtx)) {
+            return null;
+        }
+
+        if (targs != null) {
+            addViolation(data, targs);
+        } else {
+            addViolation(data, newTypeNode);
+        }
+
+        return null;
+    }
+
+    private boolean useJava7Rules(ASTConstructorCall ctorCall) {
+        return ctorCall.getAstInfo().getLanguageVersion().compareToVersion("1.8") < 0;
+    }
+
+    private boolean isNecessaryInJava7(JTypeMirror newType, ExprContext exprContext) {
+        JTypeMirror target = exprContext.getTargetType();
+        if (target == null) {
+            return false;
+        }
+        JTypeDeclSymbol targetSym = target.getSymbol();
+        if (!(targetSym instanceof JClassSymbol)) {
+            return false;
+        }
+        JTypeMirror asSuperType = newType.getAsSuper((JClassSymbol) targetSym);
+        return !TypeOps.isSameType(asSuperType, target);
+    }
+
+    private boolean supportsDiamondOnAnonymousClass(ASTConstructorCall ctorCall) {
+        return ctorCall.getAstInfo().getLanguageVersion().compareToVersion("9") >= 0;
+    }
+
+}

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

@@ -1661,44 +1661,16 @@ public class Foo {
     <rule name="UseDiamondOperator"
           language="java"
           since="6.11.0"
-          message="Explicit type arguments can be replaced by Diamond Operator"
-          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          message="Explicit type arguments can be replaced by a diamond: &lt;&gt;"
+          class="net.sourceforge.pmd.lang.java.rule.codestyle.UseDiamondOperatorRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#usediamondoperator"
           minimumLanguageVersion="1.7">
-        <description>
-Use the diamond operator to let the type be inferred automatically. With the Diamond operator it is possible
-to avoid duplication of the type parameters.
-Instead, the compiler is now able to infer the parameter types for constructor calls,
-which makes the code also more readable.
-
-The diamond operator has been introduced with java 7. However, type inference has been improved further
-with java8, rendering more type parameters unnecessary. This is only possible with java8 and the resulting
-code won't compile with java7. If you use java7, make sure to enable `java7Compatibility` for this rule to avoid
-false positives.
-        </description>
+        <description><![CDATA[
+Use the diamond operator (`<>`) to let type arguments be inferred automatically
+from the context. This avoids duplication of the type arguments in the type of
+a variable and in its constructor, which makes the code more concise and readable.
+            ]]></description>
         <priority>3</priority>
-        <properties>
-            <property name="version" value="2.0" />
-            <property name="xpath">
-                <value>
-                    <![CDATA[
-(
-//VariableInitializer[preceding-sibling::VariableDeclaratorId[1]/@TypeInferred=false()]
-|
-//StatementExpression[AssignmentOperator and PrimaryExpression/PrimaryPrefix[not(Expression)]]
-)
-/(Expression | Expression[$java7Compatibility = false()]/ConditionalExpression | Expression[$java7Compatibility = false()]/ConditionalExpression/Expression)
-/PrimaryExpression[not(PrimarySuffix) and not(ancestor::ArgumentList)]
-/PrimaryPrefix
-/AllocationExpression
-    [@AnonymousClass=false()]
-    [ClassOrInterfaceType/TypeArguments[@Diamond=false() and not($java7Compatibility = true() and .//TypeArgument[@Wildcard=true()])]]
-    [not(ArrayDimsAndInits)]
-]]>
-                </value>
-            </property>
-            <property name="java7Compatibility" type="Boolean" description="If disabled, the rule shows also violations that are applicable for java8+" value="false" />
-        </properties>
         <example>
             <![CDATA[
 List<String> strings = new ArrayList<String>(); // unnecessary duplication of type parameters

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/UseDiamondOperatorTest.java

@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class UseDiamondOperatorTest extends PmdRuleTst {
     // no additional unit tests
 }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UseDiamondOperator.xml

@@ -17,7 +17,7 @@ public class Foo {
        List<String> strings = new ArrayList<String>();
        List<String> strings2 = new ArrayList<>();
        List<List<String>> strings3 = new ArrayList<>();
-       List<List<String>> strings4 = new ArrayList<List<List<String>>>();
+       List<List<List<String>>> strings4 = new ArrayList<List<List<String>>>();
        this.field = new ArrayList<String>();
     }
 }
@@ -25,42 +25,93 @@ public class Foo {
     </test-code>
 
     <test-code>
-        <description>False positive cases: anonymous classes, methods calls</description>
-        <expected-problems>0</expected-problems>
+        <description>False positive cases</description>
+        <expected-problems>2</expected-problems>
         <code><![CDATA[
-public class Foo {
-    public void foo() {
-        Collections.sort(files, new Comparator<DataSource>() {
-            @Override
-            public int compare(DataSource left, DataSource right) {
-                String leftString = left.getNiceFileName(useShortNames, inputPaths);
-                String rightString = right.getNiceFileName(useShortNames, inputPaths);
-                return leftString.compareTo(rightString);
-            }
-        });
-        final TreeSet<Map.Entry<String, TimedResult>> sortedKeySet = new TreeSet<>(
-            new Comparator<Map.Entry<String, TimedResult>>() {
-                @Override
-                public int compare(final Entry<String, TimedResult> o1, final Entry<String, TimedResult> o2) {
-                    return Long.compare(o1.getValue().selfTimeNanos.get(), o2.getValue().selfTimeNanos.get());
-                }
-            });
-        new ThreadLocal<Queue<TimerEntry>>() {
-            @Override
-            protected Queue<TimerEntry> initialValue() {
-                return Collections.asLifoQueue(new LinkedList<TimerEntry>());
-            }
-        };
-        Iterator<Node> EMPTY_ITERATOR = new ArrayList<Node>().iterator();
-        ((ListNode<E>) rev).reverseCache = new SoftReference<ImmutableList<E>>(this);
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+public class E {
+    public void foo(List<E> files) {
+        Iterator<E> EMPTY_ITERATOR = new ArrayList<E>().iterator(); // necessary
+        ((ListNode<E>) null).list = new ArrayList<E>();             // violation
     }
-    public Map<PropertyDescriptor<?>, Object> getOverriddenPropertiesByPropertyDescriptor() {
-        return propertyValues == null ? new HashMap<PropertyDescriptor<?>, Object>() : new HashMap<>(propertyValues);
+
+    public Map<ListNode<?>, Object> getOverriddenPropertiesByPropertyDescriptor() {
+        // assignment context
+        return Integer.compare(1,2) == 0 ? new HashMap<ListNode<?>, Object>() // violation
+                                         : new HashMap<>();
+    }
+
+    static class ListNode<E> {
+        List<E> list;
     }
 }
         ]]></code>
     </test-code>
 
+    <test-code>
+        <description>Anonymous classes before java 9</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+public class E {
+    public void foo(List<E> files) {
+        Collections.sort(files, new Comparator<E>() {
+            @Override
+            public int compare(E left, E right) {
+                return left.toString().compareTo(right.toString());
+            }
+        });
+        new ThreadLocal<List<E>>() {
+            @Override
+            protected List<E> initialValue() {
+                return null;
+            }
+        };
+    }
+}
+        ]]></code>
+        <source-type>java 8</source-type>
+    </test-code>
+
+    <test-code>
+        <description>Anonymous classes after java 9</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>8,17</expected-linenumbers>
+        <code><![CDATA[
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+public class E {
+    public void foo(List<E> files) {
+        Collections.sort(files, new Comparator<E>() {
+            @Override
+            public int compare(E left, E right) {
+                return left.toString().compareTo(right.toString());
+            }
+        });
+        new ThreadLocal<List<E>>() { // not this one, it has no context
+            @Override
+            protected List<E> initialValue() {
+                return new ArrayList<E>();
+            }
+        };
+    }
+}
+        ]]></code>
+        <source-type>java 9</source-type>
+    </test-code>
+
     <test-code>
         <description>#1624[java] UseDiamondOperator doesn't work with var</description>
         <expected-problems>1</expected-problems>
@@ -98,6 +149,7 @@ public class Buzz {
         <description>#1723 FP with var inside lambda (declaration)</description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
+import java.util.ArrayList;
 class Foo {
     {
         Runnable someAction = () -> {
@@ -113,6 +165,7 @@ class Foo {
         <description>#1723 FP with var inside lambda (assignment)</description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
+import java.util.ArrayList;
 class Foo {
     {
         Runnable someAction;
@@ -145,7 +198,6 @@ class Foo {
 
     <test-code>
         <description>(J7) Version sensitive tests - avoid possible false positives on Java7</description>
-        <rule-property name="java7Compatibility">true</rule-property>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 import java.lang.ref.WeakReference;
@@ -166,6 +218,7 @@ public class Foo {
     }
 }
         ]]></code>
+        <source-type>java 7</source-type>
     </test-code>
 
     <test-code>
@@ -184,11 +237,11 @@ public class Foo {
     }
 }
         ]]></code>
+        <source-type>java 8</source-type>
     </test-code>
 
     <test-code>
         <description>False negative for nested type parameters (#2545)</description>
-        <rule-property name="java7Compatibility">true</rule-property>
         <expected-problems>3</expected-problems>
         <expected-linenumbers>7,8,17</expected-linenumbers>
         <code><![CDATA[
@@ -212,6 +265,7 @@ public class UseDiamondOperatorFalseNegative {
     }
 }
         ]]></code>
+        <source-type>java 7</source-type>
     </test-code>
 
     <test-code>