Skip to content

Commit

Permalink
Review Picnic's refaster rule suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
ben-manes committed Oct 14, 2022
1 parent a998368 commit d74756d
Show file tree
Hide file tree
Showing 78 changed files with 157 additions and 268 deletions.
32 changes: 8 additions & 24 deletions .github/workflows/analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
java: [ 11, 18 ]
java: [ 11, 19 ]
env:
JAVA_VERSION: ${{ matrix.java }}
steps:
Expand All @@ -20,27 +20,11 @@ jobs:
with:
egress-policy: audit
- uses: actions/checkout@v3
- name: Set up JDK ${{ matrix.java }}
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: actions/setup-java@v3
with: |
distribution: temurin
java-version: ${{ matrix.java }}
attempt_limit: 3
attempt_delay: 2000
- name: Setup Gradle
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: gradle/gradle-build-action@v2
with: |
cache-read-only: false
generate-job-summary: false
arguments: help --daemon --no-scan
gradle-home-cache-strict-match: true
attempt_limit: 3
attempt_delay: 2000
- name: Run analyzers
run: ./.github/scripts/analyze.sh
uses: ./.github/actions/run-gradle
with:
early-access: false
java: ${{ matrix.java }}
arguments: >
pmdJavaPoet pmdMain pmdCodeGen pmdJmh -Dpmd
spotbugsJavaPoet spotbugsMain spotbugsCodeGen spotbugsJmh -Dspotbugs
2 changes: 1 addition & 1 deletion .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on: [ push, pull_request ]
env:
GRADLE_ENTERPRISE_CACHE_PASSWORD: ${{ secrets.GRADLE_ENTERPRISE_CACHE_PASSWORD }}
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
MAX_JVM: 18
MAX_JVM: 19
EA_JVM: 20

jobs:
Expand Down
31 changes: 6 additions & 25 deletions .github/workflows/dependency-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ permissions: read-all
env:
GRADLE_ENTERPRISE_CACHE_PASSWORD: ${{ secrets.GRADLE_ENTERPRISE_CACHE_PASSWORD }}
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
JAVA_VERSION: 18
JAVA_VERSION: 19

jobs:
dependency-check:
Expand All @@ -24,31 +24,12 @@ jobs:
with:
egress-policy: audit
- uses: actions/checkout@v3
- name: Set up JDK ${{ env.JAVA_VERSION }}
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: actions/setup-java@v3
with: |
cache: gradle
distribution: temurin
java-version: ${{ env.JAVA_VERSION }}
attempt_limit: 3
attempt_delay: 2000
- name: Setup Gradle
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: gradle/gradle-build-action@v2
with: |
cache-read-only: false
arguments: help --no-scan
generate-job-summary: false
attempt_limit: 3
attempt_delay: 2000
- name: Run dependency-check
run: ./gradlew dependencyCheckAggregate
continue-on-error: true
uses: ./.github/actions/run-gradle
with:
early-access: false
java: ${{ env.JAVA_VERSION }}
arguments: dependencyCheckAggregate
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@v2
continue-on-error: true
Expand Down
35 changes: 11 additions & 24 deletions .github/workflows/snyke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ permissions: read-all
env:
GRADLE_ENTERPRISE_CACHE_PASSWORD: ${{ secrets.GRADLE_ENTERPRISE_CACHE_PASSWORD }}
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
JAVA_VERSION: 18
JAVA_VERSION: 19

jobs:
snyke:
Expand All @@ -24,35 +24,22 @@ jobs:
with:
egress-policy: audit
- uses: actions/checkout@v3
- name: Set up JDK ${{ env.JAVA_VERSION }}
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: actions/setup-java@v3
with: |
distribution: temurin
java-version: ${{ env.JAVA_VERSION }}
attempt_limit: 3
attempt_delay: 2000
- name: Setup Gradle
uses: Wandalen/[email protected]
timeout-minutes: 5
with:
action: gradle/gradle-build-action@v2
with: |
cache-read-only: false
arguments: help --no-scan
generate-job-summary: false
attempt_limit: 3
attempt_delay: 2000
- name: Run Snyk test
uses: ./.github/actions/run-gradle
continue-on-error: true
run: ./gradlew snyk-test -PsnykArgs="--sarif-file-output=snyk.sarif"
with:
early-access: false
java: ${{ env.JAVA_VERSION }}
arguments: snyk-test -PsnykArgs="--sarif-file-output=snyk.sarif"
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@v2
continue-on-error: true
with:
sarif_file: snyk.sarif
- name: Run Snyk monitor
uses: ./.github/actions/run-gradle
continue-on-error: true
run: ./gradlew snyk-monitor
with:
early-access: false
java: ${{ env.JAVA_VERSION }}
arguments: snyk-monitor
14 changes: 6 additions & 8 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ buildscript {

dependencies {
classpath gradlePlugins.values()
classpath platforms.collect { platform(it) }

configurations.each { configuration ->
configurations.all { configuration ->
restrictions.each { module, version ->
constraints.add(configuration.name, module).version { require version }
}
platforms.each { dependency ->
add(configuration.name, platform(dependency))
}
}
}
}
Expand All @@ -43,13 +41,10 @@ allprojects {
}

dependencies {
configurations.each { configuration ->
configurations.all { configuration ->
restrictions.each { module, version ->
constraints.add(configuration.name, module).version { require version }
}
platforms.each { dependency ->
add(configuration.name, platform(dependency))
}
}
}
}
Expand All @@ -72,6 +67,9 @@ subprojects {
}

dependencies {
implementation platforms.collect { platform(it) }
annotationProcessor platforms.collect { platform(it) }

testImplementation libraries.guava
testImplementation testLibraries.junit
testImplementation testLibraries.truth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static java.util.stream.Collectors.joining;

import java.util.Collections;
import java.util.Set;

import com.google.common.base.CaseFormat;
Expand Down Expand Up @@ -100,6 +101,6 @@ public static boolean usesMaximum(Set<Feature> features) {
}

public static boolean usesFastPath(Set<Feature> features) {
return features.stream().noneMatch(fastPathIncompatible::contains) && usesMaximum(features);
return Collections.disjoint(features, fastPathIncompatible) && usesMaximum(features);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ default CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
*/
static <K, V> CompletableFuture<Map<K, V>> composeResult(Map<K, CompletableFuture<V>> futures) {
if (futures.isEmpty()) {
return CompletableFuture.completedFuture(Collections.emptyMap());
return CompletableFuture.completedFuture(Map.of());
}
@SuppressWarnings("rawtypes")
CompletableFuture<?>[] array = futures.values().toArray(new CompletableFuture[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import static com.github.benmanes.caffeine.testing.MapSubject.assertThat;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Maps.immutableEntry;
import static com.google.common.truth.Truth.assertThat;
import static java.util.Map.entry;
import static java.util.function.Function.identity;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -2567,7 +2565,7 @@ public void entrySet_contains_nullValue(Map<Int, Int> map, CacheContext context)
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void entrySet_contains_absent(Map<Int, Int> map, CacheContext context) {
var entry = entry(context.absentKey(), context.absentValue());
var entry = Map.entry(context.absentKey(), context.absentValue());
assertThat(map.entrySet().contains(entry)).isFalse();
}

Expand All @@ -2576,7 +2574,7 @@ public void entrySet_contains_absent(Map<Int, Int> map, CacheContext context) {
@CacheSpec(population = Population.FULL,
removalListener = { Listener.DISABLED, Listener.REJECTING })
public void entrySet_contains_present(Map<Int, Int> map, CacheContext context) {
var entry = entry(context.firstKey(), context.original().get(context.firstKey()));
var entry = Map.entry(context.firstKey(), context.original().get(context.firstKey()));
assertThat(map.entrySet().contains(entry)).isTrue();
}

Expand All @@ -2593,7 +2591,7 @@ public void entrySet_empty(Map<Int, Int> map, CacheContext context) {
@Test(dataProvider = "caches", expectedExceptions = UnsupportedOperationException.class)
public void entrySet_addIsNotSupported(Map<Int, Int> map, CacheContext context) {
try {
map.entrySet().add(immutableEntry(Int.valueOf(1), Int.valueOf(2)));
map.entrySet().add(Map.entry(Int.valueOf(1), Int.valueOf(2)));
} finally {
assertThat(map).isExhaustivelyEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Maps.immutableEntry;
import static com.google.common.truth.Truth.assertThat;
import static java.util.Map.entry;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toUnmodifiableMap;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -1393,8 +1390,8 @@ public void hashCode_self(AsyncCache<Int, Int> cache, CacheContext context) {
@CacheSpec(population = Population.EMPTY,
removalListener = { Listener.DISABLED, Listener.REJECTING })
public void equalsAndHashCodeFail_empty(AsyncCache<Int, Int> cache, CacheContext context) {
var other = Stream.of(1, 2, 3)
.collect(toUnmodifiableMap(Int::valueOf, key -> Int.futureOf(-key)));
var other = Map.of(Int.valueOf(1), Int.futureOf(-1),
Int.valueOf(2), Int.futureOf(-2), Int.valueOf(3), Int.futureOf(-3));
assertThat(cache.asMap().equals(other)).isFalse();
assertThat(other.equals(cache.asMap())).isFalse();
assertThat(cache.asMap().hashCode()).isNotEqualTo(other.hashCode());
Expand All @@ -1404,10 +1401,9 @@ public void equalsAndHashCodeFail_empty(AsyncCache<Int, Int> cache, CacheContext
@Test(dataProvider = "caches")
@CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL },
removalListener = { Listener.DISABLED, Listener.REJECTING })
public void equalsAndHashCodeFail_present(
AsyncCache<Int, Int> cache, CacheContext context) {
var other = Stream.of(1, 2, 3)
.collect(toUnmodifiableMap(Int::valueOf, key -> Int.futureOf(-key)));
public void equalsAndHashCodeFail_present(AsyncCache<Int, Int> cache, CacheContext context) {
var other = Map.of(Int.valueOf(1), Int.futureOf(-1),
Int.valueOf(2), Int.futureOf(-2), Int.valueOf(3), Int.futureOf(-3));
assertThat(cache.asMap().equals(other)).isFalse();
assertThat(other.equals(cache.asMap())).isFalse();
assertThat(cache.asMap().hashCode()).isNotEqualTo(other.hashCode());
Expand Down Expand Up @@ -2334,7 +2330,7 @@ public void entrySet_contains_nullValue(AsyncCache<Int, Int> cache, CacheContext
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void entrySet_contains_absent(AsyncCache<Int, Int> cache, CacheContext context) {
var entry = entry(context.absentKey(), context.absentValue().asFuture());
var entry = Map.entry(context.absentKey(), context.absentValue().asFuture());
assertThat(cache.asMap().entrySet().contains(entry)).isFalse();
}

Expand All @@ -2343,7 +2339,7 @@ public void entrySet_contains_absent(AsyncCache<Int, Int> cache, CacheContext co
@CacheSpec(population = Population.FULL,
removalListener = { Listener.DISABLED, Listener.REJECTING })
public void entrySet_contains_present(AsyncCache<Int, Int> cache, CacheContext context) {
var entry = entry(context.firstKey(), cache.asMap().get(context.firstKey()));
var entry = Map.entry(context.firstKey(), cache.asMap().get(context.firstKey()));
assertThat(cache.asMap().entrySet().contains(entry)).isTrue();
}

Expand All @@ -2352,7 +2348,7 @@ public void entrySet_contains_present(AsyncCache<Int, Int> cache, CacheContext c
@Test(dataProvider = "caches", expectedExceptions = UnsupportedOperationException.class)
public void entrySet_addIsNotSupported(AsyncCache<Int, Int> cache, CacheContext context) {
try {
cache.asMap().entrySet().add(immutableEntry(Int.valueOf(1), Int.valueOf(2).asFuture()));
cache.asMap().entrySet().add(Map.entry(Int.valueOf(1), Int.valueOf(2).asFuture()));
} finally {
assertThat(cache).isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static java.lang.Thread.State.BLOCKED;
import static java.util.Map.entry;
import static java.util.function.Function.identity;
import static org.hamcrest.Matchers.is;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -929,7 +928,7 @@ public void evict_resurrect_weight(Cache<Int, List<Int>> cache, CacheContext con
ConcurrentTestHarness.execute(() -> {
evictor.set(Thread.currentThread());
started.set(true);
cache.policy().eviction().get().setMaximum(0);
cache.policy().eviction().orElseThrow().setMaximum(0);
done.set(true);
});

Expand All @@ -943,7 +942,7 @@ public void evict_resurrect_weight(Cache<Int, List<Int>> cache, CacheContext con

assertThat(cache).containsEntry(key, List.of());
assertThat(context).removalNotifications().withCause(REPLACED)
.contains(entry(key, value)).exclusively();
.contains(Map.entry(key, value)).exclusively();
}

@Test(dataProvider = "caches")
Expand Down Expand Up @@ -994,14 +993,14 @@ public void evict_resurrect_expireAfterAccess(Cache<Int, Int> cache, CacheContex
ConcurrentTestHarness.execute(() -> {
evictor.set(Thread.currentThread());
started.set(true);
cache.policy().expireAfterAccess().get().setExpiresAfter(Duration.ZERO);
cache.policy().expireAfterAccess().orElseThrow().setExpiresAfter(Duration.ZERO);
done.set(true);
});

await().untilTrue(started);
var threadState = EnumSet.of(State.BLOCKED, State.WAITING);
await().until(() -> threadState.contains(evictor.get().getState()));
cache.policy().expireAfterAccess().get().setExpiresAfter(Duration.ofHours(1));
cache.policy().expireAfterAccess().orElseThrow().setExpiresAfter(Duration.ofHours(1));
return v;
});
await().untilTrue(done);
Expand All @@ -1025,14 +1024,14 @@ public void evict_resurrect_expireAfterWrite(Cache<Int, Int> cache, CacheContext
ConcurrentTestHarness.execute(() -> {
evictor.set(Thread.currentThread());
started.set(true);
cache.policy().expireAfterWrite().get().setExpiresAfter(Duration.ZERO);
cache.policy().expireAfterWrite().orElseThrow().setExpiresAfter(Duration.ZERO);
done.set(true);
});

await().untilTrue(started);
var threadState = EnumSet.of(State.BLOCKED, State.WAITING);
await().until(() -> threadState.contains(evictor.get().getState()));
cache.policy().expireAfterWrite().get().setExpiresAfter(Duration.ofHours(1));
cache.policy().expireAfterWrite().orElseThrow().setExpiresAfter(Duration.ofHours(1));
return v;
});
await().untilTrue(done);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static java.util.Map.entry;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -261,7 +260,7 @@ public void evict_zero_async(AsyncCache<Int, List<Int>> cache, CacheContext cont
await().untilTrue(done);
await().untilAsserted(() -> assertThat(cache).isEmpty());
assertThat(context).notifications().withCause(SIZE)
.contains(entry(context.absentKey(), Int.listOf(1, 2, 3, 4, 5)))
.contains(Map.entry(context.absentKey(), Int.listOf(1, 2, 3, 4, 5)))
.exclusively();
}

Expand Down
Loading

0 comments on commit d74756d

Please sign in to comment.