Merge pull request #845 from gjoseph/archunit-rule-to-avoid-guava

Archunit rule to avoid guava
gjoseph · Dec 18, 2024 · 032dca0 · 032dca0
2 parents 6419e52 + 5af5ccf
commit 032dca0
Show file tree

Hide file tree

Showing 26 changed files with 143 additions and 49 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -43,7 +43,9 @@ jobs:
           distribution: "${{ matrix.java-distrib }}"
           cache: "maven"
       - name: Backend Build
-        run: ./mvnw clean verify -B -V
+        run: ./mvnw clean verify -B -V -Darchunit.freeze.store.default.allowStoreUpdate=false
+      - name: OpenRewriteCheck
+        run: ./mvnw rewrite:dryRun -DfailOnDryRunResults=true # Ensure no OpenRewrite rule/change hasn't been committed
 
   build-frontend:
     name: Frontend ${{ matrix.npm-command }} 📍

diff --git a/pom.xml b/pom.xml
@@ -218,8 +218,6 @@
                     <version>3.13.0</version>
                     <configuration>
                         <release>21</release>
-                        <source>21</source>
-                        <target>21</target>
                     </configuration>
                 </plugin>
                 <plugin>
@@ -252,6 +250,18 @@
                     <artifactId>jacoco-maven-plugin</artifactId>
                     <version>0.8.12</version>
                 </plugin>
+                <plugin>
+                    <groupId>org.openrewrite.maven</groupId>
+                    <artifactId>rewrite-maven-plugin</artifactId>
+                    <version>5.46.1</version>
+                    <dependencies>
+                        <dependency>
+                            <groupId>org.openrewrite.recipe</groupId>
+                            <artifactId>rewrite-migrate-java</artifactId>
+                            <version>2.30.1</version>
+                        </dependency>
+                    </dependencies>
+                </plugin>
             </plugins>
         </pluginManagement>
         <plugins>
@@ -326,6 +336,19 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.openrewrite.maven</groupId>
+                <artifactId>rewrite-maven-plugin</artifactId>
+                <configuration>
+                    <configLocation>${maven.multiModuleProjectDirectory}/src/openrewrite/rewrite.yml</configLocation>
+                    <failOnDryRunResults>true</failOnDryRunResults>
+                    <failOnInvalidActiveRecipes>true</failOnInvalidActiveRecipes>
+                    <exportDatatables>true</exportDatatables>
+                    <activeRecipes>
+                        <recipe>net.incongru.tichu.rewrite.RecipeBook</recipe>
+                    </activeRecipes>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 </project>
diff --git a/src/openrewrite/rewrite.yml b/src/openrewrite/rewrite.yml
@@ -0,0 +1,23 @@
+---
+type: specs.openrewrite.org/v1beta/recipe
+name: net.incongru.tichu.rewrite.RecipeBook
+recipeList:
+  - org.openrewrite.java.migrate.UpgradeToJava21
+  - org.openrewrite.java.migrate.guava.NoGuavaJava21
+  - net.incongru.tichu.rewrite.RemoveJakartaAnnotationDependency
+
+---
+type: specs.openrewrite.org/v1beta/recipe
+name: net.incongru.tichu.rewrite.RemoveJakartaAnnotationDependency
+description: |
+  Somewhere in the depths of UpgradeToJava21 via Java8toJava11,
+  dependencies to jakarta.annotation:jakarta.annotation-api are
+  added because we use javax.annotation..*. Except, we only use
+  @Nonnull and @Nullable, which are _not_ in this jar. Since
+  individual recipes can't be disabled when they're declared
+  within others, here's a recipe that does the inverse.
+  See https://github.com/openrewrite/rewrite/discussions/4211
+recipeList:
+  - org.openrewrite.java.dependencies.RemoveDependency:
+      groupId: jakarta.annotation
+      artifactId: jakarta.annotation-api
diff --git a/tichu-actions/src/test/java/net/incongru/tichu/action/impl/CheatDealTest.java b/tichu-actions/src/test/java/net/incongru/tichu/action/impl/CheatDealTest.java
@@ -6,18 +6,18 @@
 import static net.incongru.tichu.model.util.DeckConstants.G2;
 import static net.incongru.tichu.model.util.DeckConstants.G5;
 import static net.incongru.tichu.model.util.DeckConstants.MahJong;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 
-import com.google.common.collect.Sets;
 import java.util.Set;
 import net.incongru.tichu.action.GameContext;
 import net.incongru.tichu.action.param.CheatDealParam;
 import net.incongru.tichu.action.param.InitialiseGameParam;
 import net.incongru.tichu.action.param.JoinTableParam;
 import net.incongru.tichu.model.HandAssert;
 import net.incongru.tichu.model.UserId;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
@@ -45,13 +45,13 @@ void failsInNormalGame() {
         new JoinTable()
             .exec(ctx, JoinTableParam.withActor(UserId.of("quinn"), 1));
 
-        Assertions.assertThatThrownBy(() -> {
+        assertThatThrownBy(() -> {
             new CheatDeal()
                 .exec(
                     ctx,
                     CheatDealParam.withActor(
                         UserId.of("jules"),
-                        Sets.newHashSet(MahJong, B2)
+                        Set.of(MahJong, B2)
                     )
                 );
         })
@@ -73,13 +73,13 @@ void worksInSimulatedGame() {
         new JoinTable()
             .exec(ctx, JoinTableParam.withActor(UserId.of("quinn"), 1));
 
-        Assertions.assertThatCode(() -> {
+        assertThatCode(() -> {
             new CheatDeal()
                 .exec(
                     ctx,
                     CheatDealParam.withActor(
                         UserId.of("jules"),
-                        Sets.newHashSet(MahJong, B2)
+                        Set.of(MahJong, B2)
                     )
                 );
         }).doesNotThrowAnyException();

diff --git a/tichu-actions/src/test/java/net/incongru/tichu/action/impl/PlayerIsReadyTest.java b/tichu-actions/src/test/java/net/incongru/tichu/action/impl/PlayerIsReadyTest.java
@@ -7,7 +7,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
-import com.google.common.collect.Sets;
+import java.util.Set;
 import net.incongru.tichu.action.GameContext;
 import net.incongru.tichu.action.param.CheatDealParam;
 import net.incongru.tichu.action.param.InitialiseGameParam;
@@ -72,7 +72,7 @@ void gameStartsWhenAllPlayersAreReady() {
                 ctx,
                 CheatDealParam.withActor(
                     UserId.of("jules"),
-                    Sets.newHashSet(MahJong, B2)
+                    Set.of(MahJong, B2)
                 )
             );
 

diff --git a/tichu-actions/src/test/java/net/incongru/tichu/action/impl/TestGameContext.java b/tichu-actions/src/test/java/net/incongru/tichu/action/impl/TestGameContext.java
@@ -68,6 +68,6 @@ TestGameContext allReady() {
 
     @Override
     public void log(String msg, Object... args) {
-        System.out.println(String.format(msg, args));
+        System.out.println(msg.formatted(args));
     }
 }
diff --git a/tichu-archunit/src/test/archunit-store/c4bf907c-94d9-4516-9a39-70254e191191 b/tichu-archunit/src/test/archunit-store/c4bf907c-94d9-4516-9a39-70254e191191
@@ -0,0 +1,29 @@
+Constructor <net.incongru.tichu.model.Play$PlayResult.<init>(net.incongru.tichu.model.Play$PlayResult$Result, java.lang.String)> calls method <com.google.common.base.Preconditions.checkArgument(boolean, java.lang.String, java.lang.Object)> in (Play.java:72)
+Constructor <net.incongru.tichu.model.Play$PlayResult.<init>(net.incongru.tichu.model.Play, net.incongru.tichu.model.Play$PlayResult$Result, java.lang.String)> calls method <com.google.common.base.Preconditions.checkNotNull(java.lang.Object, java.lang.String, java.lang.Object)> in (Play.java:83)
+Constructor <net.incongru.tichu.model.card.CardImpl.<init>(net.incongru.tichu.model.card.CardValue, net.incongru.tichu.model.card.CardSuit)> calls method <com.google.common.base.Preconditions.checkState(boolean, java.lang.Object)> in (CardImpl.java:10)
+Constructor <net.incongru.tichu.model.plays.Single.<init>(java.util.Set)> calls method <com.google.common.base.Preconditions.checkArgument(boolean, java.lang.Object)> in (Single.java:22)
+Method <net.incongru.tichu.model.CardDeck.allRemaining()> calls method <com.google.common.collect.ImmutableSet.copyOf(java.util.Collection)> in (CardDeck.java:48)
+Method <net.incongru.tichu.model.Functions.lastNMatches(java.util.Deque, int, java.util.function.Predicate)> calls method <com.google.common.collect.Iterators.all(java.util.Iterator, com.google.common.base.Predicate)> in (Functions.java:23)
+Method <net.incongru.tichu.model.Functions.lastNMatches(java.util.Deque, int, java.util.function.Predicate)> calls method <com.google.common.collect.Iterators.limit(java.util.Iterator, int)> in (Functions.java:24)
+Method <net.incongru.tichu.model.Players.cycleFrom(net.incongru.tichu.model.Player)> calls method <com.google.common.collect.Iterators.cycle(java.lang.Iterable)> in (Players.java:89)
+Method <net.incongru.tichu.model.Players.cycleFrom(net.incongru.tichu.model.Player)> calls method <com.google.common.collect.Iterators.peekingIterator(java.util.Iterator)> in (Players.java:88)
+Method <net.incongru.tichu.model.Players.cycleFrom(net.incongru.tichu.model.Player)> calls method <com.google.common.collect.PeekingIterator.next()> in (Players.java:93)
+Method <net.incongru.tichu.model.Players.cycleFrom(net.incongru.tichu.model.Player)> calls method <com.google.common.collect.PeekingIterator.peek()> in (Players.java:92)
+Method <net.incongru.tichu.model.Players.getTeam(int)> calls method <com.google.common.base.Preconditions.checkArgument(boolean, java.lang.String, int)> in (Players.java:112)
+Method <net.incongru.tichu.model.Trick.currentPlayer()> calls method <com.google.common.base.Preconditions.checkState(boolean, java.lang.Object)> in (Trick.java:118)
+Method <net.incongru.tichu.model.Trick.currentPlayer()> calls method <com.google.common.base.Preconditions.checkState(boolean, java.lang.Object)> in (Trick.java:122)
+Method <net.incongru.tichu.model.Trick.isDone()> calls method <com.google.common.base.Predicates.compose(com.google.common.base.Predicate, com.google.common.base.Function)> in (Trick.java:157)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.from([Ljava.lang.Object;)> in (CardTest.java:51)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.from([Ljava.lang.Object;)> in (CardTest.java:54)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.from(java.lang.Iterable)> in (CardTest.java:57)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.from(java.lang.Iterable)> in (CardTest.java:62)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.toSortedList(java.util.Comparator)> in (CardTest.java:51)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.toSortedList(java.util.Comparator)> in (CardTest.java:57)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.toSortedSet(java.util.Comparator)> in (CardTest.java:54)
+Method <net.incongru.tichu.model.card.CardTest.suitComparatorDoesNotMessUpSpecialCards()> calls method <com.google.common.collect.FluentIterable.toSortedSet(java.util.Comparator)> in (CardTest.java:62)
+Method <net.incongru.tichu.model.plays.NSameValue$NSameValuesFactory.is(java.util.Set)> calls method <com.google.common.collect.Collections2.transform(java.util.Collection, com.google.common.base.Function)> in (NSameValue.java:49)
+Method <net.incongru.tichu.model.plays.Straight$Factory.is(java.util.Set)> calls method <com.google.common.collect.Collections2.transform(java.util.Collection, com.google.common.base.Function)> in (Straight.java:107)
+Method <net.incongru.tichu.model.plays.Straight.getCardValuesWithPhoenix()> calls method <com.google.common.collect.Collections2.transform(java.util.Collection, com.google.common.base.Function)> in (Straight.java:55)
+Method <net.incongru.tichu.simu.parse.SimulationFileParserTest.sampleIsLoadable()> calls method <com.google.common.collect.Lists.reverse(java.util.List)> in (SimulationFileParserTest.java:33)
+Method <net.incongru.tichu.simu.util.NameableEnum.allNamesOf(java.lang.Class)> calls method <com.google.common.collect.Lists.transform(java.util.List, com.google.common.base.Function)> in (NameableEnum.java:45)
+Method <net.incongru.tichu.simu.util.NameableEnum.byName(java.lang.Class, java.lang.String)> calls method <com.google.common.collect.Lists.transform(java.util.List, com.google.common.base.Function)> in (NameableEnum.java:21)
diff --git a/tichu-archunit/src/test/archunit-store/stored.rules b/tichu-archunit/src/test/archunit-store/stored.rules
@@ -0,0 +1,3 @@
+#
+#Wed Dec 18 14:10:11 AEDT 2024
+avoid_using_Guava=c4bf907c-94d9-4516-9a39-70254e191191
diff --git a/tichu-archunit/src/test/java/net/incongru/tichu/archunit/ArchUnitTest.java b/tichu-archunit/src/test/java/net/incongru/tichu/archunit/ArchUnitTest.java
@@ -1,12 +1,15 @@
 package net.incongru.tichu.archunit;
 
 import static com.tngtech.archunit.core.domain.JavaModifier.ABSTRACT;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.constructors;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.members;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
 
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchRule;
+import com.tngtech.archunit.library.freeze.FreezingArchRule;
 import net.incongru.tichu.action.Action;
 import net.incongru.tichu.model.Play;
 import net.incongru.tichu.model.plays.InvalidPlay;
@@ -77,4 +80,13 @@ class ArchUnitTest {
             .andShould()
             .beDeclaredInClassesThat()
             .arePackagePrivate();
+
+    @ArchTest
+    static final ArchRule avoid_using_Guava = FreezingArchRule.freeze(
+        noClasses()
+            .should()
+            .accessClassesThat()
+            .resideInAnyPackage("com.google.common..")
+            .as("avoid_using_Guava")
+    );
 }
diff --git a/tichu-archunit/src/test/resources/archunit.properties b/tichu-archunit/src/test/resources/archunit.properties
@@ -0,0 +1,7 @@
+freeze.store.default.allowStoreCreation=false
+freeze.refreeze=false
+freeze.store.default.path=src/test/archunit-store
+
+# Allows violations to be resolved; new violations will not be allowed.
+# False in CI to ensure we commit freeze file changes
+freeze.store.default.allowStoreUpdate=true
diff --git a/tichu-model/src/main/java/net/incongru/tichu/model/CardDeck.java b/tichu-model/src/main/java/net/incongru/tichu/model/CardDeck.java
@@ -41,7 +41,7 @@ protected List<Card> shuffle(Set<Card> cards) {
     }
 
     public Card take() {
-        return cards.remove(0);
+        return cards.removeFirst();
     }
 
     public Set<Card> allRemaining() {

diff --git a/tichu-model/src/main/java/net/incongru/tichu/model/Player.java b/tichu-model/src/main/java/net/incongru/tichu/model/Player.java
@@ -1,8 +1,8 @@
 package net.incongru.tichu.model;
 
-import com.google.common.base.Preconditions;
 import java.util.Collections;
 import java.util.LinkedHashSet;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -20,7 +20,7 @@ public class Player {
     private final Set<Card> wonCards = new LinkedHashSet<>();
 
     public Player(@Nonnull UserId id) {
-        Preconditions.checkNotNull(id, "Player name can't be null");
+        Objects.requireNonNull(id, "Player name can't be null");
         this.id = id;
         this.ready = false;
     }

diff --git a/tichu-model/src/main/java/net/incongru/tichu/model/Score.java b/tichu-model/src/main/java/net/incongru/tichu/model/Score.java
@@ -3,6 +3,6 @@
 public record Score(int team1, int team2) {
     @Override
     public String toString() {
-        return String.format("%d:%d", team1(), team2());
+        return "%d:%d".formatted(team1(), team2());
     }
 }
diff --git a/tichu-model/src/main/java/net/incongru/tichu/model/Team.java b/tichu-model/src/main/java/net/incongru/tichu/model/Team.java
@@ -1,6 +1,5 @@
 package net.incongru.tichu.model;
 
-import com.google.common.base.Preconditions;
 import java.util.Objects;
 
 /**
@@ -12,7 +11,7 @@ public class Team {
     private final Player[] players = new Player[2];
 
     public Team(String name) {
-        Preconditions.checkNotNull(name, "Team name can't be null");
+        Objects.requireNonNull(name, "Team name can't be null");
         this.name = name;
     }
 

diff --git a/tichu-model/src/main/java/net/incongru/tichu/model/plays/Straight.java b/tichu-model/src/main/java/net/incongru/tichu/model/plays/Straight.java
@@ -8,7 +8,7 @@
 import static net.incongru.tichu.model.card.SubstituteCardValue.substituteFor;
 
 import com.google.common.collect.Collections2;
-import com.google.common.collect.Lists;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -51,8 +51,8 @@ public CardValue getHigherBound() {
     }
 
     private Collection<CardValue> getCardValuesWithPhoenix() {
-        final Collection<CardValue> values = Lists.newArrayList(
-            Collections2.transform(getCards(), card -> card.val())
+        final Collection<CardValue> values = new ArrayList<>(
+            Collections2.transform(getCards(), Card::val)
         );
         values.removeIf(v -> v == Phoenix);
         if (phoenixSubstitute != null) {
@@ -103,7 +103,7 @@ public Straight is(Set<Card> cards) {
                 return null;
             }
 
-            final List<CardValue> values = Lists.newArrayList(
+            final List<CardValue> values = new ArrayList<>(
                 Collections2.transform(cards, Card::val)
             );
             values.sort(CardComparators.V_BY_PLAY_ORDER);
@@ -140,8 +140,8 @@ public Straight is(Set<Card> cards) {
 
             // If phoenix hasn't been used, we use it for the highest possible position
             if (phoenixIsAvail) {
-                final CardValue last = values.get(values.size() - 1);
-                final CardValue first = values.get(0);
+                final CardValue last = values.getLast();
+                final CardValue first = values.getFirst();
                 if (last == Ace && first.playOrder() > Two.playOrder()) {
                     // then we use it "before" the first card
                     sub = substituteFor(first.playOrder() - 1);

diff --git a/tichu-model/src/test/java/net/incongru/tichu/model/TichuRulesTest.java b/tichu-model/src/test/java/net/incongru/tichu/model/TichuRulesTest.java
@@ -51,7 +51,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import com.google.common.collect.Sets;
+import java.util.Set;
 import net.incongru.tichu.model.card.Card;
 import net.incongru.tichu.model.card.CardNumbers;
 import net.incongru.tichu.model.plays.ConsecutivePairs;
@@ -438,6 +438,6 @@ void consecutivePairs() {
     }
 
     private Play newPlay(Card... cards) {
-        return new TichuRules().validate(Sets.newHashSet(cards));
+        return new TichuRules().validate(Set.of(cards));
     }
 }
diff --git a/tichu-model/src/test/java/net/incongru/tichu/model/TrickTest.java b/tichu-model/src/test/java/net/incongru/tichu/model/TrickTest.java
@@ -7,8 +7,8 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import com.google.common.collect.Sets;
 import java.util.Collections;
+import java.util.Set;
 import net.incongru.tichu.model.util.DeckConstants;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
@@ -36,7 +36,7 @@ void isDoneOnceEveryBodyPassed() {
 
         // 1st player plays
         assertThat(
-            trick.play(alex, Sets.newHashSet(DeckConstants.B2)).result()
+            trick.play(alex, Set.of(DeckConstants.B2)).result()
         ).isEqualTo(Play.PlayResult.Result.NEXTGOES);
 
         // players 2 and 3 pass, we're not done yet
@@ -93,7 +93,7 @@ void prevPlayReturnsPreviousNonPassPlay() {
 
         // 1st player plays
         assertThat(
-            trick.play(alex, Sets.newHashSet(DeckConstants.B2)).result()
+            trick.play(alex, Set.of(DeckConstants.B2)).result()
         ).isEqualTo(Play.PlayResult.Result.NEXTGOES);
 
         // players 2 and 3 pass, we're not done yet
@@ -111,7 +111,7 @@ void prevPlayReturnsPreviousNonPassPlay() {
         );
         // player 4 plays another single, that's valid
         assertThat(
-            trick.play(quinn, Sets.newHashSet(DeckConstants.G5)).result()
+            trick.play(quinn, Set.of(DeckConstants.G5)).result()
         ).isEqualTo(Play.PlayResult.Result.NEXTGOES);
         assertFalse(trick.isDone());
     }

diff --git a/tichu-model/src/test/java/net/incongru/tichu/model/card/CardTest.java b/tichu-model/src/test/java/net/incongru/tichu/model/card/CardTest.java
@@ -13,7 +13,6 @@
 import com.google.common.collect.FluentIterable;
 import net.incongru.tichu.model.CardDeck;
 import net.incongru.tichu.model.util.DeckConstants;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -53,7 +52,7 @@ void suitComparatorDoesNotMessUpSpecialCards() {
         assertThat(
             FluentIterable.from(array).toSortedSet(BY_SUIT)
         ).containsOnly(array);
-        Assertions.assertThat(
+        assertThat(
             FluentIterable.from(new CardDeck().allRemaining()).toSortedList(
                 BY_SUIT
             )