NonComparableStreamSort validates direct types

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/NonComparableStreamSort.java

@@ -28,6 +28,7 @@
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 import java.util.stream.Stream;
 
@@ -36,7 +37,7 @@
         name = "NonComparableStreamSort",
         link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
         linkType = BugPattern.LinkType.CUSTOM,
-        severity = SeverityLevel.ERROR,
+        severity = SeverityLevel.WARNING,
         summary = "Stream.sorted() should only be called on streams of Comparable types.")
 public final class NonComparableStreamSort extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
     private static final long serialVersionUID = 1L;
@@ -53,11 +54,19 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
             return Description.NO_MATCH;
         }
         Type returnType = ASTHelpers.getReturnType(tree);
-        if (returnType == null || returnType.getTypeArguments().size() != 1) {
+        if (returnType == null) {
             return Description.NO_MATCH;
         }
-        Type streamParameterType = Iterables.getOnlyElement(returnType.getTypeArguments());
-        if (ASTHelpers.isCastable(streamParameterType, state.getTypeFromString(Comparable.class.getName()), state)) {
+        Symbol streamSymbol = state.getSymbolFromString(Stream.class.getName());
+        if (streamSymbol == null) {
+            return Description.NO_MATCH;
+        }
+        Type streamType = state.getTypes().asSuper(returnType, streamSymbol);
+        if (streamType.getTypeArguments().size() != 1) {
+            return Description.NO_MATCH;
+        }
+        Type streamParameterType = Iterables.getOnlyElement(streamType.getTypeArguments());
+        if (ASTHelpers.isSubtype(streamParameterType, state.getTypeFromString(Comparable.class.getName()), state)) {
             return Description.NO_MATCH;
         }
         return buildDescription(tree)

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/NonComparableStreamSortTests.java

@@ -41,7 +41,7 @@ public void should_fail_on_non_comparable_stream() {
                 "class Test {",
                 "   public static final void main(String[] args) {",
                 "       List<Object> list = new ArrayList<>();",
-                "       // BUG: Stream.sorted() should only be called on streams of Comparable types.",
+                "       // BUG: Diagnostic contains: Stream.sorted() should only be called on streams of Comparable",
                 "       list.stream().sorted();",
                 "   }",
                 "}"
@@ -62,4 +62,21 @@ public void should_on_comparable_stream() {
                 "}"
         ).doTest();
     }
+
+    @Test
+    public void should_fail_without_direct_types() {
+        compilationHelper.addSourceLines(
+                "Test.java",
+                "import java.util.List;",
+                "import java.util.ArrayList;",
+                "class Test {",
+                "   public static final void main(String[] args) {",
+                "       List<Iface> list = new ArrayList<>();",
+                "       // BUG: Diagnostic contains: Stream.sorted() should only be called on streams of Comparable",
+                "       list.stream().sorted();",
+                "   }",
+                "   interface Iface {}",
+                "}"
+        ).doTest();
+    }
 }

changelog/@unreleased/pr-1070.v2.yml

@@ -0,0 +1,13 @@
+type: improvement
+improvement:
+  description: |-
+    Improve `NonComparableStreamSort` to validate that stream types implement comparable, as opposed to validating that casting to comparable does not cause a compiler error.
+
+    This commit reduces the severity to WARNING because it's
+    possible that the check will flag code that happens to work
+    today, but we strongly recommend against sorting streams of
+    a type that is not directly comparable without a custom
+    comparator because it is likely to break later due to lack
+    of enforcement by the type system.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1070