Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off UnnecessaryLambda, ThrowSpecificity on Java 13 #1095

Closed
wants to merge 3 commits into from
Closed

Turn off UnnecessaryLambda, ThrowSpecificity on Java 13 #1095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02d278d
Turn off UnnecessaryLambda, ThrowSpecificity on Java 13
iamdanfox Dec 5, 2019
1ed9810
Add generated changelog entries
iamdanfox Dec 5, 2019
ff58fa7
Add generated changelog entries
iamdanfox Dec 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/@unreleased/pr-1095.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
type: fix
fix:
description: When building with Java13, two more error-prone checks (`UnnecessaryLambda`
and `ThrowSpecificity`) are turned off to avoid NoSuchMethodErrors in the compiler.
They still run if your project builds on Java8 or 11.
links:
- https://github.com/palantir/gradle-baseline/pull/1095
2 changes: 2 additions & 0 deletions gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineErrorProne.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ private static void configureErrorProneOptions(
// https://github.com/google/error-prone/issues/1106
errorProneOptions.check("TypeParameterUnusedInFormals", CheckSeverity.OFF);
errorProneOptions.check("PreferCollectionConstructors", CheckSeverity.OFF);
errorProneOptions.check("UnnecessaryLambda", CheckSeverity.OFF);
errorProneOptions.check("ThrowSpecificity", CheckSeverity.OFF);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the two where we've seen failures on java13 projects so far, but all checks using SuggestedFixes.qualifyType and SuggestedFixes.prettyType are impacted:

[gradle-baseline]$ git grep "SuggestedFixes\..*Type"
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/AssertjPrimitiveComparison.java:        String cast = '(' + SuggestedFixes.prettyType(state, null, targetType) + ") ";
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CatchSpecificity.java:                                .map(type -> SuggestedFixes.prettyType(state, fix, type))
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java:        return toMethodReference(SuggestedFixes.qualifyType(state, builder, symbol))
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreASTHelpers.java:                .sorted(Comparator.comparing(type -> SuggestedFixes.prettyType(state, null, type)))
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java:                            + SuggestedFixes.qualifyType(state, fix, "org.assertj.core.api.HamcrestCondition")
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java:                            + SuggestedFixes.qualifyType(state, fix, "org.assertj.core.api.HamcrestCondition")
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java:            String qualifiedMap = SuggestedFixes.prettyType(
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferAssertj.java:            return SuggestedFixes.qualifyType(state, fix, "org.assertj.core.api.Assertions.assertThat");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferBuiltInConcurrentKeySet.java:            String qualifiedType = SuggestedFixes.qualifyType(state, fix, "java.util.concurrent.ConcurrentHashMap");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferCollectionConstructors.java:        String collectionType = SuggestedFixes.qualifyType(state, fixBuilder, collectionClass.getName());
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferCollectionTransform.java:                    qualifiedType = SuggestedFixes.qualifyType(state, fix, "com.google.common.collect.Lists");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferCollectionTransform.java:                    qualifiedType = SuggestedFixes.qualifyType(state, fix, "com.google.common.collect.Collections2");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferListsPartition.java:                String qualifiedType = SuggestedFixes.qualifyType(state, fix, "com.google.common.collect.Lists");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLoggingPreconditions.java:        String logSafeQualifiedClassName = SuggestedFixes.qualifyType(state, fix, "com.palantir.logsafe.Preconditions");
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ReadReturnValueIgnored.java:        String qualifiedReference = SuggestedFixes.qualifyType(state, fix, fullyQualifiedReplacement);
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamOfEmpty.java:                    .addFix(MoreSuggestedFixes.renameInvocationRetainingTypeArguments(tree, "empty", state))
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StrictCollectionIncompatibleType.java:        return SuggestedFixes.prettyType(null, null, type);
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ThrowError.java:            String qualifiedName = SuggestedFixes.qualifyType(state, fix, IllegalStateException.class.getName());
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ThrowError.java:            String qualifiedName = SuggestedFixes.qualifyType(state, fix,
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ThrowSpecificity.java:                        .map(checkedException -> SuggestedFixes.prettyType(state, fix, checkedException))

Should we disable all these checks on java13? We should probably migrate our calls to our MoreSuggestedFixes utility with a fallback for java13 which simply adds an import and returns the class name. It would be nice to get tests running on java13 in this repo so we ca be more confident moving forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems much more reasonable - too much late night hacking for Dan.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here: #1110

I think we still may want to disable UnnecessaryLambda, UnnecessaryAbstractClass, etc on jdk13 because we can't modify upstream code.

}

if (javaCompile.equals(compileRefaster)) {
Expand Down