From fff505eec03c6f35f1e5a2abb827a7567b6dc595 Mon Sep 17 00:00:00 2001 From: Shibi Balamurugan Date: Tue, 2 Apr 2024 14:18:39 -0400 Subject: [PATCH 1/2] Check eclipse collection api factory --- baseline-error-prone/build.gradle | 2 + .../EclipseCollectionsApiFactoryUsage.java | 58 +++++++++++++++++ ...EclipseCollectionsApiFactoryUsageTest.java | 63 +++++++++++++++++++ versions.props | 1 + 4 files changed, 124 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsageTest.java diff --git a/baseline-error-prone/build.gradle b/baseline-error-prone/build.gradle index bc5792c54..9f38083c2 100644 --- a/baseline-error-prone/build.gradle +++ b/baseline-error-prone/build.gradle @@ -41,6 +41,8 @@ dependencies { testRuntimeOnly 'com.palantir.conjure.java.runtime:conjure-java-annotations' // for PreferCommonAnnotations testRuntimeOnly 'org.jetbrains:annotations' + // for eclipse-collections classloading bug + testRuntimeOnly 'org.eclipse.collections:eclipse-collections' annotationProcessor 'com.google.auto.service:auto-service' compileOnly 'org.immutables:value::annotations' diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java new file mode 100644 index 000000000..9fd7fb165 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java @@ -0,0 +1,58 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ImportTree; +import com.sun.tools.javac.code.Type; + +@AutoService(BugChecker.class) +@BugPattern( + summary = "Import of type uses non-canonical name", + severity = BugPattern.SeverityLevel.ERROR, + documentSuppression = false) +public final class EclipseCollectionsApiFactoryUsage extends BugChecker implements BugChecker.ImportTreeMatcher { + private static final String BAD_ECLIPSE_COLLECTIONS_USAGE_SUBSTRING = "org.eclipse.collections.api.factory."; + + public EclipseCollectionsApiFactoryUsage() {} + + @Override + public Description matchImport(ImportTree tree, VisitorState state) { + Type importType = ASTHelpers.getType(tree.getQualifiedIdentifier()); + + if (importType == null) { + return Description.NO_MATCH; + } + + String importName = importType.toString(); + if (importName.contains(BAD_ECLIPSE_COLLECTIONS_USAGE_SUBSTRING)) { + String fixedImport = importName.replace(".api.", ".impl."); + SuggestedFix fix = SuggestedFix.builder() + .removeImport(importName) + .addImport(fixedImport) + .build(); + return describeMatch(tree, fix); + } + return Description.NO_MATCH; + } +} \ No newline at end of file diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsageTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsageTest.java new file mode 100644 index 000000000..65fd98fd3 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsageTest.java @@ -0,0 +1,63 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import org.junit.jupiter.api.Test; + +public class EclipseCollectionsApiFactoryUsageTest { + @Test + public void desired_import_remains_unchanged() { + fix().addInputLines( + "Client.java", + "package com.google.frobber;", + "import org.eclipse.collections.impl.factory.primitive.LongLists;", + "public final class Client {", + " public int getOrder() {", + " return 66;", + " }", + "}") + .expectUnchanged() + .doTest(); + } + + @Test + public void bad_import_changes() { + fix().addInputLines( + "Client.java", + "package com.google.frobber;", + "import org.eclipse.collections.api.factory.primitive.LongLists;", + "public final class Client {", + " public int getOrder() {", + " return 66;", + " }", + "}") + .addOutputLines( + "Client.java", + "package com.google.frobber;", + "import org.eclipse.collections.impl.factory.primitive.LongLists;", + "public final class Client {", + " public int getOrder() {", + " return 66;", + " }", + "}") + .doTest(); + } + + private RefactoringValidator fix() { + return RefactoringValidator.of(EclipseCollectionsApiFactoryUsage.class, getClass()); + } +} diff --git a/versions.props b/versions.props index 3cdcfd13b..dc5833e20 100644 --- a/versions.props +++ b/versions.props @@ -35,6 +35,7 @@ net.ltgt.gradle:gradle-errorprone-plugin = 3.1.0 one.util:streamex = 0.8.2 org.apache.commons:commons-lang3 = 3.14.0 org.assertj:assertj-core = 3.25.3 +org.eclipse.collections:* = 11.1.0 org.hamcrest:hamcrest-core = 2.2 org.junit.jupiter:* = 5.10.2 org.junit.vintage:* = 5.10.2 From a4163f43883316280f7ad2c40d0c30512d4ab8ec Mon Sep 17 00:00:00 2001 From: Shibi Balamurugan Date: Tue, 2 Apr 2024 14:25:10 -0400 Subject: [PATCH 2/2] Add summary --- .../errorprone/EclipseCollectionsApiFactoryUsage.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java index 9fd7fb165..330e17b2f 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/EclipseCollectionsApiFactoryUsage.java @@ -28,7 +28,8 @@ @AutoService(BugChecker.class) @BugPattern( - summary = "Import of type uses non-canonical name", + summary = "Import eclipse-collections uses api factory instead of impl factory. This could result in" + + "classloading deadlocks as explained here: https://github.com/palantir/atlasdb/pull/7073", severity = BugPattern.SeverityLevel.ERROR, documentSuppression = false) public final class EclipseCollectionsApiFactoryUsage extends BugChecker implements BugChecker.ImportTreeMatcher { @@ -55,4 +56,4 @@ public Description matchImport(ImportTree tree, VisitorState state) { } return Description.NO_MATCH; } -} \ No newline at end of file +}