From 72b701cae325ccd2d6efe27b9b5c9f29e7a87c27 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Tue, 24 Dec 2024 13:48:48 +0100 Subject: [PATCH 01/43] Upgrade OpenRewrite Templating 1.20.1 -> 1.20.2 (#1478) See: - https://github.com/openrewrite/rewrite-templating/releases/tag/v1.20.2 - https://github.com/openrewrite/rewrite-templating/compare/v1.20.1...v1.20.2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 18a639aa..710548b2 100644 --- a/pom.xml +++ b/pom.xml @@ -220,7 +220,7 @@ 1.0.1 0.12.2 1.1.4 - 1.20.1 + 1.20.2 3.2.3 From 12585a8969fa13af5eb5e0cbb6420045acd30cf4 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Wed, 25 Dec 2024 11:29:40 +0100 Subject: [PATCH 02/43] Upgrade AssertJ 3.26.3 -> 3.27.0 (#1472) See: - https://github.com/assertj/assertj/releases/tag/assertj-build-3.27.0 - https://github.com/assertj/assertj/compare/assertj-build-3.26.3...assertj-build-3.27.0 --- integration-tests/checkstyle-init.patch | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/checkstyle-init.patch b/integration-tests/checkstyle-init.patch index c3a72814..cf9a975f 100644 --- a/integration-tests/checkstyle-init.patch +++ b/integration-tests/checkstyle-init.patch @@ -7,7 +7,7 @@ + + org.assertj + assertj-core -+ 3.26.3 ++ 3.27.0 + test + diff --git a/pom.xml b/pom.xml index 710548b2..e5c2119c 100644 --- a/pom.xml +++ b/pom.xml @@ -445,7 +445,7 @@ org.assertj assertj-bom - 3.26.3 + 3.27.0 pom import From 83f3f8bedc7772c615a1f20fe40ab7db8aa22740 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 25 Dec 2024 13:40:22 +0100 Subject: [PATCH 03/43] Introduce `AssertThatString{Contains,DoesNotContain}` Refaster rules (#1479) While there, extend `AssertThatIterableIsEmpty`. --- .../refasterrules/AssertJRules.java | 1 + .../refasterrules/AssertJStringRules.java | 27 +++++++++++++++++++ .../AssertJStringRulesTestInput.java | 8 ++++++ .../AssertJStringRulesTestOutput.java | 8 ++++++ 4 files changed, 44 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java index c2e2f141..8e24755f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java @@ -264,6 +264,7 @@ final class AssertJRules { Refaster.anyOf( assertThat(iterable).hasSize(0), assertThat(iterable.iterator().hasNext()).isFalse(), + assertThat(iterable.iterator()).isExhausted(), assertThat(Iterables.size(iterable)).isEqualTo(0L), assertThat(Iterables.size(iterable)).isNotPositive()); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java index 4143884e..af4e03a3 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java @@ -12,6 +12,7 @@ import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractBooleanAssert; import org.assertj.core.api.AbstractStringAssert; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @@ -69,6 +70,32 @@ final class AssertJStringRules { } } + static final class AssertThatStringContains { + @BeforeTemplate + AbstractBooleanAssert before(String string, String substring) { + return assertThat(string.contains(substring)).isTrue(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + AbstractStringAssert after(String string, String substring) { + return assertThat(string).contains(substring); + } + } + + static final class AssertThatStringDoesNotContain { + @BeforeTemplate + AbstractBooleanAssert before(String string, String substring) { + return assertThat(string.contains(substring)).isFalse(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + AbstractStringAssert after(String string, String substring) { + return assertThat(string).doesNotContain(substring); + } + } + static final class AssertThatMatches { @BeforeTemplate AbstractAssert before(String string, String regex) { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestInput.java index 8c0d9278..a19b0779 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestInput.java @@ -33,6 +33,14 @@ final class AssertJStringRulesTest implements RefasterRuleCollectionTestCase { return assertThat("foo".isEmpty()).isFalse(); } + AbstractAssert testAssertThatStringContains() { + return assertThat("foo".contains("bar")).isTrue(); + } + + AbstractAssert testAssertThatStringDoesNotContain() { + return assertThat("foo".contains("bar")).isFalse(); + } + AbstractAssert testAssertThatMatches() { return assertThat("foo".matches(".*")).isTrue(); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestOutput.java index c9d019b2..7938bf16 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestOutput.java @@ -34,6 +34,14 @@ final class AssertJStringRulesTest implements RefasterRuleCollectionTestCase { return assertThat("foo").isNotEmpty(); } + AbstractAssert testAssertThatStringContains() { + return assertThat("foo").contains("bar"); + } + + AbstractAssert testAssertThatStringDoesNotContain() { + return assertThat("foo").doesNotContain("bar"); + } + AbstractAssert testAssertThatMatches() { return assertThat("foo").matches(".*"); } From 5fc7bc29ee00bac492a282b4955e5c7b169e2729 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 26 Dec 2024 17:47:57 +0100 Subject: [PATCH 04/43] Introduce `CharSequenceRules` Refaster rule collection (#1480) Resolves #1394. --- .../refasterrules/CharSequenceRules.java | 33 +++++++++++++++++++ .../errorprone/refasterrules/StringRules.java | 9 +++-- .../refasterrules/RefasterRulesTest.java | 1 + .../CharSequenceRulesTestInput.java | 16 +++++++++ .../CharSequenceRulesTestOutput.java | 16 +++++++++ .../refasterrules/StringRulesTestInput.java | 6 ++-- .../refasterrules/StringRulesTestOutput.java | 6 ++-- 7 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CharSequenceRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestOutput.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CharSequenceRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CharSequenceRules.java new file mode 100644 index 00000000..5e1bc518 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CharSequenceRules.java @@ -0,0 +1,33 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.errorprone.refaster.Refaster; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.AlsoNegation; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +/** Refaster rules related to expressions dealing with {@link CharSequence}s. */ +@OnlineDocumentation +final class CharSequenceRules { + private CharSequenceRules() {} + + /** + * Prefer {@link CharSequence#isEmpty()} over alternatives that consult the char sequence's + * length. + */ + // XXX: Drop this rule once we (and OpenRewrite) no longer support projects targeting Java 14 or + // below. + static final class CharSequenceIsEmpty { + @BeforeTemplate + boolean before(CharSequence charSequence) { + return Refaster.anyOf( + charSequence.length() == 0, charSequence.length() <= 0, charSequence.length() < 1); + } + + @AfterTemplate + @AlsoNegation + boolean after(CharSequence charSequence) { + return charSequence.isEmpty(); + } + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java index 2ae04b74..af81c81b 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java @@ -29,11 +29,14 @@ final class StringRules { private StringRules() {} /** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */ - // XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence` - // subtypes. This does require a mechanism (perhaps an annotation, or a separate Maven module) to - // make sure that non-String expressions are rewritten only if client code also targets JDK 15+. + // XXX: Drop this rule once we (and OpenRewrite) no longer support projects targeting Java 14 or + // below. The `CharSequenceIsEmpty` rule then suffices. (This rule exists so that e.g. projects + // that target JDK 11 can disable `CharSequenceIsEmpty` without losing a valuable rule.) + // XXX: Look into a more general approach to supporting different Java language levels, such as + // rule selection based on some annotation, or a separate Maven module. static final class StringIsEmpty { @BeforeTemplate + @SuppressWarnings("CharSequenceIsEmpty" /* This is a more specific template. */) boolean before(String str) { return Refaster.anyOf(str.length() == 0, str.length() <= 0, str.length() < 1); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index fd5cb296..3f9250eb 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -36,6 +36,7 @@ final class RefasterRulesTest { AssortedRules.class, BigDecimalRules.class, BugCheckerRules.class, + CharSequenceRules.class, ClassRules.class, CollectionRules.class, ComparatorRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestInput.java new file mode 100644 index 00000000..6a449f84 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestInput.java @@ -0,0 +1,16 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableSet; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase { + ImmutableSet testCharSequenceIsEmpty() { + return ImmutableSet.of( + new StringBuilder("foo").length() == 0, + new StringBuilder("bar").length() <= 0, + new StringBuilder("baz").length() < 1, + new StringBuilder("qux").length() != 0, + new StringBuilder("quux").length() > 0, + new StringBuilder("corge").length() >= 1); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestOutput.java new file mode 100644 index 00000000..0a506beb --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CharSequenceRulesTestOutput.java @@ -0,0 +1,16 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableSet; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase { + ImmutableSet testCharSequenceIsEmpty() { + return ImmutableSet.of( + new StringBuilder("foo").isEmpty(), + new StringBuilder("bar").isEmpty(), + new StringBuilder("baz").isEmpty(), + !new StringBuilder("qux").isEmpty(), + !new StringBuilder("quux").isEmpty(), + !new StringBuilder("corge").isEmpty()); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestInput.java index 39476e36..d189a5df 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestInput.java @@ -28,9 +28,9 @@ final class StringRulesTest implements RefasterRuleCollectionTestCase { "foo".length() == 0, "bar".length() <= 0, "baz".length() < 1, - "foo".length() != 0, - "bar".length() > 0, - "baz".length() >= 1); + "qux".length() != 0, + "quux".length() > 0, + "corge".length() >= 1); } boolean testStringIsEmptyPredicate() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestOutput.java index 7973d10d..3717a85f 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestOutput.java @@ -31,9 +31,9 @@ final class StringRulesTest implements RefasterRuleCollectionTestCase { "foo".isEmpty(), "bar".isEmpty(), "baz".isEmpty(), - !"foo".isEmpty(), - !"bar".isEmpty(), - !"baz".isEmpty()); + !"qux".isEmpty(), + !"quux".isEmpty(), + !"corge".isEmpty()); } boolean testStringIsEmptyPredicate() { From 7daa39a0b52ea2245008873691d704daa1ffab9a Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 27 Dec 2024 05:30:30 +0100 Subject: [PATCH 05/43] Update comments referencing Refaster rule limitation (#46) Issue google/error-prone#2706 has been resolved; we have yet to decide how to proceed. --- .../refasterrules/ImmutableListMultimapRules.java | 3 +-- .../picnic/errorprone/refasterrules/ImmutableListRules.java | 3 +-- .../picnic/errorprone/refasterrules/ImmutableMapRules.java | 3 +-- .../errorprone/refasterrules/ImmutableMultisetRules.java | 3 +-- .../errorprone/refasterrules/ImmutableSetMultimapRules.java | 3 +-- .../picnic/errorprone/refasterrules/ImmutableSetRules.java | 3 +-- .../errorprone/refasterrules/ImmutableSortedMapRules.java | 6 ++---- 7 files changed, 8 insertions(+), 16 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java index 3a943ab8..fc5e8b06 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java @@ -36,8 +36,7 @@ final class ImmutableListMultimapRules { * Prefer {@link ImmutableListMultimap#builder()} over the associated constructor on constructions * that produce a less-specific type. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableListMultimapBuilder { @BeforeTemplate ImmutableMultimap.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java index aca741d3..80732d38 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListRules.java @@ -28,8 +28,7 @@ final class ImmutableListRules { private ImmutableListRules() {} /** Prefer {@link ImmutableList#builder()} over the associated constructor. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableListBuilder { @BeforeTemplate ImmutableList.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java index 8c0f9c62..fa976d33 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java @@ -31,8 +31,7 @@ final class ImmutableMapRules { private ImmutableMapRules() {} /** Prefer {@link ImmutableMap#builder()} over the associated constructor. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableMapBuilder { @BeforeTemplate ImmutableMap.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMultisetRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMultisetRules.java index fee76fd8..9880ad7c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMultisetRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMultisetRules.java @@ -21,8 +21,7 @@ final class ImmutableMultisetRules { private ImmutableMultisetRules() {} /** Prefer {@link ImmutableMultiset#builder()} over the associated constructor. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableMultisetBuilder { @BeforeTemplate ImmutableMultiset.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetMultimapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetMultimapRules.java index 1c704943..a07d4a09 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetMultimapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetMultimapRules.java @@ -29,8 +29,7 @@ final class ImmutableSetMultimapRules { private ImmutableSetMultimapRules() {} /** Prefer {@link ImmutableSetMultimap#builder()} over the associated constructor. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableSetMultimapBuilder { @BeforeTemplate ImmutableSetMultimap.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetRules.java index 628bb1cf..351f93d5 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSetRules.java @@ -29,8 +29,7 @@ final class ImmutableSetRules { private ImmutableSetRules() {} /** Prefer {@link ImmutableSet#builder()} over the associated constructor. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableSetBuilder { @BeforeTemplate ImmutableSet.Builder before() { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java index 36975c70..e2cf20ef 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java @@ -37,8 +37,7 @@ final class ImmutableSortedMapRules { * Prefer {@link ImmutableSortedMap#naturalOrder()} over the alternative that requires explicitly * providing the {@link Comparator}. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableSortedMapNaturalOrderBuilder, V> { @BeforeTemplate ImmutableSortedMap.Builder before() { @@ -55,8 +54,7 @@ final class ImmutableSortedMapRules { * Prefer {@link ImmutableSortedMap#reverseOrder()} over the alternative that requires explicitly * providing the {@link Comparator}. */ - // XXX: This drops generic type information, sometimes leading to non-compilable code. See - // https://github.com/google/error-prone/pull/2706. + // XXX: This rule may drop generic type information, leading to non-compilable code. static final class ImmutableSortedMapReverseOrderBuilder, V> { @BeforeTemplate ImmutableSortedMap.Builder before() { From 6cfa9a0dfeeaef926b8188f34dcc6fc22ee44c2f Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sun, 29 Dec 2024 20:05:43 +0100 Subject: [PATCH 06/43] Upgrade Modernizer Maven Plugin 2.9.0 -> 3.0.0 (#1486) See: - https://github.com/gaul/modernizer-maven-plugin/releases/tag/modernizer-maven-plugin-3.0.0 - https://github.com/gaul/modernizer-maven-plugin/compare/modernizer-maven-plugin-2.9.0...modernizer-maven-plugin-3.0.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index e5c2119c..d2f13606 100644 --- a/pom.xml +++ b/pom.xml @@ -1462,7 +1462,7 @@ org.gaul modernizer-maven-plugin - 2.9.0 + 3.0.0 ++ 3.27.1 + test + diff --git a/pom.xml b/pom.xml index a1fc2e33..756e30ea 100644 --- a/pom.xml +++ b/pom.xml @@ -445,7 +445,7 @@ org.assertj assertj-bom - 3.27.0 + 3.27.1 pom import From 9493f2d59ab8f50ce242bf1d56ed434e71d86334 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Fri, 3 Jan 2025 11:18:47 +0100 Subject: [PATCH 18/43] Introduce `ImmutableTableRules` Refaster rule collection (#1489) --- .../ExplicitArgumentEnumeration.java | 2 +- .../bugpatterns/NonStaticImport.java | 2 +- .../refasterrules/ImmutableTableRules.java | 114 ++++++++++++++++++ .../refasterrules/RefasterRulesTest.java | 1 + .../ImmutableTableRulesTestInput.java | 48 ++++++++ .../ImmutableTableRulesTestOutput.java | 45 +++++++ 6 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestOutput.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java index 7770efe9..8deed251 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java @@ -113,7 +113,7 @@ public final class ExplicitArgumentEnumeration extends BugChecker .put(OBJECT_ENUMERABLE_ASSERT, "doesNotContainAnyElementsOf", "doesNotContain") .put(OBJECT_ENUMERABLE_ASSERT, "hasSameElementsAs", "containsOnly") .put(STEP_VERIFIER_STEP, "expectNextSequence", "expectNext") - .build(); + .buildOrThrow(); /** Instantiates a new {@link ExplicitArgumentEnumeration} instance. */ public ExplicitArgumentEnumeration() {} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java index eba7253b..aca3b6af 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java @@ -182,7 +182,7 @@ public final class NonStaticImport extends BugChecker implements CompilationUnit } } - return imports.build(); + return imports.buildOrThrow(); } private static boolean shouldNotBeStaticallyImported(String type, String member) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java new file mode 100644 index 00000000..910fd565 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java @@ -0,0 +1,114 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.common.collect.ImmutableTable.toImmutableTable; +import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; + +import com.google.common.collect.ImmutableTable; +import com.google.common.collect.Table; +import com.google.common.collect.Tables; +import com.google.errorprone.refaster.Refaster; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.refaster.annotation.MayOptionallyUse; +import com.google.errorprone.refaster.annotation.Placeholder; +import com.google.errorprone.refaster.annotation.UseImportPolicy; +import java.util.stream.Stream; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +/** Refaster rules related to expressions dealing with {@link ImmutableTable}s. */ +@OnlineDocumentation +final class ImmutableTableRules { + private ImmutableTableRules() {} + + /** Prefer {@link ImmutableTable#builder()} over the associated constructor. */ + // XXX: This rule may drop generic type information, leading to non-compilable code. + static final class ImmutableTableBuilder { + @BeforeTemplate + ImmutableTable.Builder before() { + return new ImmutableTable.Builder<>(); + } + + @AfterTemplate + ImmutableTable.Builder after() { + return ImmutableTable.builder(); + } + } + + /** + * Prefer {@link ImmutableTable.Builder#buildOrThrow()} over the less explicit {@link + * ImmutableTable.Builder#build()}. + */ + static final class ImmutableTableBuilderBuildOrThrow { + @BeforeTemplate + ImmutableTable before(ImmutableTable.Builder builder) { + return builder.build(); + } + + @AfterTemplate + ImmutableTable after(ImmutableTable.Builder builder) { + return builder.buildOrThrow(); + } + } + + /** Prefer {@link ImmutableTable#of(Object, Object, Object)} over more contrived alternatives. */ + static final class CellToImmutableTable { + @BeforeTemplate + ImmutableTable before(Table.Cell cell) { + return Refaster.anyOf( + ImmutableTable.builder().put(cell).buildOrThrow(), + Stream.of(cell) + .collect( + toImmutableTable( + Table.Cell::getRowKey, Table.Cell::getColumnKey, Table.Cell::getValue))); + } + + @AfterTemplate + ImmutableTable after(Table.Cell cell) { + return ImmutableTable.of(cell.getRowKey(), cell.getColumnKey(), cell.getValue()); + } + } + + /** + * Don't map a stream's elements to table cells, only to subsequently collect them into an {@link + * ImmutableTable}. The collection can be performed directly. + */ + abstract static class StreamOfCellsToImmutableTable { + @Placeholder(allowsIdentity = true) + abstract R rowFunction(@MayOptionallyUse E element); + + @Placeholder(allowsIdentity = true) + abstract C columnFunction(@MayOptionallyUse E element); + + @Placeholder(allowsIdentity = true) + abstract V valueFunction(@MayOptionallyUse E element); + + @BeforeTemplate + ImmutableTable before(Stream stream) { + return stream + .map(e -> Tables.immutableCell(rowFunction(e), columnFunction(e), valueFunction(e))) + .collect( + toImmutableTable( + Table.Cell::getRowKey, Table.Cell::getColumnKey, Table.Cell::getValue)); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + ImmutableTable after(Stream stream) { + return stream.collect( + toImmutableTable(e -> rowFunction(e), e -> columnFunction(e), e -> valueFunction(e))); + } + } + + /** Prefer {@link ImmutableTable#of()} over more contrived alternatives . */ + static final class ImmutableTableOf { + @BeforeTemplate + ImmutableTable before() { + return ImmutableTable.builder().buildOrThrow(); + } + + @AfterTemplate + ImmutableTable after() { + return ImmutableTable.of(); + } + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index 3f9250eb..d7b47d30 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -54,6 +54,7 @@ final class RefasterRulesTest { ImmutableSortedMapRules.class, ImmutableSortedMultisetRules.class, ImmutableSortedSetRules.class, + ImmutableTableRules.class, IntStreamRules.class, JUnitRules.class, JUnitToAssertJRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestInput.java new file mode 100644 index 00000000..74707c4b --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestInput.java @@ -0,0 +1,48 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.common.collect.ImmutableTable.toImmutableTable; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableTable; +import com.google.common.collect.Table; +import com.google.common.collect.Tables; +import java.util.stream.Stream; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class ImmutableTableRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(Table.class); + } + + ImmutableTable.Builder testImmutableTableBuilder() { + return new ImmutableTable.Builder<>(); + } + + ImmutableTable testImmutableTableBuilderBuildOrThrow() { + return ImmutableTable.builder().build(); + } + + ImmutableSet> testCellToImmutableTable() { + return ImmutableSet.of( + ImmutableTable.builder() + .put(Tables.immutableCell("foo", 1, "bar")) + .buildOrThrow(), + Stream.of(Tables.immutableCell("baz", 2, "qux")) + .collect( + toImmutableTable( + Table.Cell::getRowKey, Table.Cell::getColumnKey, Table.Cell::getValue))); + } + + ImmutableTable testStreamOfCellsToImmutableTable() { + return Stream.of(1, 2, 3) + .map(n -> Tables.immutableCell(n, n.toString(), n * 2)) + .collect( + toImmutableTable( + Table.Cell::getRowKey, Table.Cell::getColumnKey, Table.Cell::getValue)); + } + + ImmutableTable testImmutableTableOf() { + return ImmutableTable.builder().buildOrThrow(); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestOutput.java new file mode 100644 index 00000000..8b801bfe --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableTableRulesTestOutput.java @@ -0,0 +1,45 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.common.collect.ImmutableTable.toImmutableTable; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableTable; +import com.google.common.collect.Table; +import com.google.common.collect.Tables; +import java.util.stream.Stream; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class ImmutableTableRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(Table.class); + } + + ImmutableTable.Builder testImmutableTableBuilder() { + return ImmutableTable.builder(); + } + + ImmutableTable testImmutableTableBuilderBuildOrThrow() { + return ImmutableTable.builder().buildOrThrow(); + } + + ImmutableSet> testCellToImmutableTable() { + return ImmutableSet.of( + ImmutableTable.of( + Tables.immutableCell("foo", 1, "bar").getRowKey(), + Tables.immutableCell("foo", 1, "bar").getColumnKey(), + Tables.immutableCell("foo", 1, "bar").getValue()), + ImmutableTable.of( + Tables.immutableCell("baz", 2, "qux").getRowKey(), + Tables.immutableCell("baz", 2, "qux").getColumnKey(), + Tables.immutableCell("baz", 2, "qux").getValue())); + } + + ImmutableTable testStreamOfCellsToImmutableTable() { + return Stream.of(1, 2, 3).collect(toImmutableTable(n -> n, n -> n.toString(), n -> n * 2)); + } + + ImmutableTable testImmutableTableOf() { + return ImmutableTable.of(); + } +} From 19d3ba0505c4b53351a0716ea1af917c9746c3e0 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Fri, 3 Jan 2025 11:53:24 +0100 Subject: [PATCH 19/43] Extend `ImmutableMapRules` Refaster rule collection (#1488) Resolves #1223. --- .../bugpatterns/SpringMvcAnnotation.java | 2 +- .../refasterrules/ImmutableMapRules.java | 76 ++++++++++++--- .../ImmutableSortedMapRules.java | 10 +- .../ImmutableMapRulesTestInput.java | 59 +++++++++--- .../ImmutableMapRulesTestOutput.java | 45 ++++++--- .../ImmutableSortedMapRulesTestInput.java | 10 +- .../checkstyle-expected-changes.patch | 92 ++++++++++++++----- ...metheus-java-client-expected-changes.patch | 55 ++++++++++- 8 files changed, 273 insertions(+), 76 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java index dbc27412..df6a1917 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java @@ -54,7 +54,7 @@ public final class SpringMvcAnnotation extends BugChecker implements AnnotationT .put("PATCH", "PatchMapping") .put("POST", "PostMapping") .put("PUT", "PutMapping") - .build(); + .buildOrThrow(); /** Instantiates a new {@link SpringMvcAnnotation} instance. */ public SpringMvcAnnotation() {} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java index 3afed69c..792a137b 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java @@ -15,6 +15,7 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.google.errorprone.refaster.annotation.Matches; import com.google.errorprone.refaster.annotation.MayOptionallyUse; import com.google.errorprone.refaster.annotation.Placeholder; +import com.google.errorprone.refaster.annotation.Repeated; import com.google.errorprone.refaster.annotation.UseImportPolicy; import java.util.Collection; import java.util.Iterator; @@ -44,12 +45,28 @@ final class ImmutableMapRules { } } + /** + * Prefer {@link ImmutableMap.Builder#buildOrThrow()} over the less explicit {@link + * ImmutableMap.Builder#build()}. + */ + static final class ImmutableMapBuilderBuildOrThrow { + @BeforeTemplate + ImmutableMap before(ImmutableMap.Builder builder) { + return builder.build(); + } + + @AfterTemplate + ImmutableMap after(ImmutableMap.Builder builder) { + return builder.buildOrThrow(); + } + } + /** Prefer {@link ImmutableMap#of(Object, Object)} over more contrived alternatives. */ static final class EntryToImmutableMap { @BeforeTemplate ImmutableMap before(Map.Entry entry) { return Refaster.anyOf( - ImmutableMap.builder().put(entry).build(), + ImmutableMap.builder().put(entry).buildOrThrow(), Stream.of(entry).collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue))); } @@ -104,16 +121,17 @@ final class ImmutableMapRules { /** Prefer {@link ImmutableMap#copyOf(Iterable)} over more contrived alternatives. */ static final class EntryIterableToImmutableMap { @BeforeTemplate - ImmutableMap before(Map iterable) { + Map before(Map iterable) { return Refaster.anyOf( ImmutableMap.copyOf(iterable.entrySet()), - ImmutableMap.builder().putAll(iterable).build()); + ImmutableMap.builder().putAll(iterable).buildOrThrow(), + Map.copyOf(iterable)); } @BeforeTemplate ImmutableMap before(Iterable> iterable) { return Refaster.anyOf( - ImmutableMap.builder().putAll(iterable).build(), + ImmutableMap.builder().putAll(iterable).buildOrThrow(), Streams.stream(iterable).collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue))); } @@ -139,8 +157,6 @@ final class ImmutableMapRules { @Placeholder(allowsIdentity = true) abstract V valueFunction(@MayOptionallyUse E element); - // XXX: We could add variants in which the entry is created some other way, but we have another - // rule that covers canonicalization to `Map.entry`. @BeforeTemplate ImmutableMap before(Stream stream) { return stream @@ -224,7 +240,11 @@ final class ImmutableMapRules { static final class ImmutableMapOf { @BeforeTemplate Map before() { - return Refaster.anyOf(ImmutableMap.builder().build(), emptyMap(), Map.of()); + return Refaster.anyOf( + ImmutableMap.builder().buildOrThrow(), + ImmutableMap.ofEntries(), + emptyMap(), + Map.of()); } @AfterTemplate @@ -243,7 +263,10 @@ final class ImmutableMapRules { @BeforeTemplate Map before(K k1, V v1) { return Refaster.anyOf( - ImmutableMap.builder().put(k1, v1).build(), singletonMap(k1, v1), Map.of(k1, v1)); + ImmutableMap.builder().put(k1, v1).buildOrThrow(), + ImmutableMap.ofEntries(Map.entry(k1, v1)), + singletonMap(k1, v1), + Map.of(k1, v1)); } @AfterTemplate @@ -261,7 +284,8 @@ final class ImmutableMapRules { static final class ImmutableMapOf2 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2) { - return Map.of(k1, v1, k2, v2); + return Refaster.anyOf( + ImmutableMap.ofEntries(Map.entry(k1, v1), Map.entry(k2, v2)), Map.of(k1, v1, k2, v2)); } @AfterTemplate @@ -279,7 +303,9 @@ final class ImmutableMapRules { static final class ImmutableMapOf3 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2, K k3, V v3) { - return Map.of(k1, v1, k2, v2, k3, v3); + return Refaster.anyOf( + ImmutableMap.ofEntries(Map.entry(k1, v1), Map.entry(k2, v2), Map.entry(k3, v3)), + Map.of(k1, v1, k2, v2, k3, v3)); } @AfterTemplate @@ -299,7 +325,10 @@ final class ImmutableMapRules { static final class ImmutableMapOf4 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) { - return Map.of(k1, v1, k2, v2, k3, v3, k4, v4); + return Refaster.anyOf( + ImmutableMap.ofEntries( + Map.entry(k1, v1), Map.entry(k2, v2), Map.entry(k3, v3), Map.entry(k4, v4)), + Map.of(k1, v1, k2, v2, k3, v3, k4, v4)); } @AfterTemplate @@ -319,7 +348,14 @@ final class ImmutableMapRules { static final class ImmutableMapOf5 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) { - return Map.of(k1, v1, k2, v2, k3, v3, k4, v4, k5, v5); + return Refaster.anyOf( + ImmutableMap.ofEntries( + Map.entry(k1, v1), + Map.entry(k2, v2), + Map.entry(k3, v3), + Map.entry(k4, v4), + Map.entry(k5, v5)), + Map.of(k1, v1, k2, v2, k3, v3, k4, v4, k5, v5)); } @AfterTemplate @@ -370,6 +406,22 @@ final class ImmutableMapRules { } } + /** + * Prefer {@link ImmutableMap#ofEntries(Map.Entry[])} over alternatives that don't communicate the + * immutability of the resulting map at the type level. + */ + static final class ImmutableMapOfEntries { + @BeforeTemplate + Map before(@Repeated Map.Entry entries) { + return Map.ofEntries(entries); + } + + @AfterTemplate + ImmutableMap after(@Repeated Map.Entry entries) { + return ImmutableMap.ofEntries(entries); + } + } + // XXX: Add a rule for this: // Maps.transformValues(streamOfEntries.collect(groupBy(fun)), ImmutableMap::copyOf) // -> diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java index e2cf20ef..2a30e31a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRules.java @@ -71,7 +71,7 @@ final class ImmutableSortedMapRules { static final class EmptyImmutableSortedMap, V> { @BeforeTemplate ImmutableSortedMap before() { - return ImmutableSortedMap.naturalOrder().build(); + return ImmutableSortedMap.naturalOrder().buildOrThrow(); } @AfterTemplate @@ -89,7 +89,7 @@ final class ImmutableSortedMapRules { static final class PairToImmutableSortedMap, V> { @BeforeTemplate ImmutableSortedMap before(K key, V value) { - return ImmutableSortedMap.naturalOrder().put(key, value).build(); + return ImmutableSortedMap.naturalOrder().put(key, value).buildOrThrow(); } @AfterTemplate @@ -105,7 +105,7 @@ final class ImmutableSortedMapRules { @BeforeTemplate ImmutableSortedMap before(Map.Entry entry) { return Refaster.anyOf( - ImmutableSortedMap.naturalOrder().put(entry).build(), + ImmutableSortedMap.naturalOrder().put(entry).buildOrThrow(), Stream.of(entry) .collect( toImmutableSortedMap(naturalOrder(), Map.Entry::getKey, Map.Entry::getValue))); @@ -126,7 +126,7 @@ final class ImmutableSortedMapRules { return Refaster.anyOf( ImmutableSortedMap.copyOf(iterable, naturalOrder()), ImmutableSortedMap.copyOf(iterable.entrySet()), - ImmutableSortedMap.naturalOrder().putAll(iterable).build()); + ImmutableSortedMap.naturalOrder().putAll(iterable).buildOrThrow()); } @BeforeTemplate @@ -134,7 +134,7 @@ final class ImmutableSortedMapRules { Iterable> iterable) { return Refaster.anyOf( ImmutableSortedMap.copyOf(iterable, naturalOrder()), - ImmutableSortedMap.naturalOrder().putAll(iterable).build(), + ImmutableSortedMap.naturalOrder().putAll(iterable).buildOrThrow(), Streams.stream(iterable) .collect( toImmutableSortedMap( diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java index 2302bb4c..7531a5f8 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java @@ -24,9 +24,13 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { return new ImmutableMap.Builder<>(); } + ImmutableMap testImmutableMapBuilderBuildOrThrow() { + return ImmutableMap.builder().build(); + } + ImmutableSet> testEntryToImmutableMap() { return ImmutableSet.of( - ImmutableMap.builder().put(Map.entry("foo", 1)).build(), + ImmutableMap.builder().put(Map.entry("foo", 1)).buildOrThrow(), Stream.of(Map.entry("foo", 1)) .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue))); } @@ -51,13 +55,14 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { ImmutableMap.copyOf(Maps.asMap(ImmutableSet.of(10), Integer::valueOf))); } - ImmutableSet> testEntryIterableToImmutableMap() { + ImmutableSet> testEntryIterableToImmutableMap() { return ImmutableSet.of( ImmutableMap.copyOf(ImmutableMap.of("foo", 1).entrySet()), - ImmutableMap.builder().putAll(ImmutableMap.of("foo", 1)).build(), + ImmutableMap.builder().putAll(ImmutableMap.of("foo", 1)).buildOrThrow(), + Map.copyOf(ImmutableMap.of("foo", 1)), ImmutableMap.builder() .putAll(ImmutableMap.of("foo", 1).entrySet()) - .build(), + .buildOrThrow(), ImmutableMap.of("foo", 1).entrySet().stream() .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)), Streams.stream(Iterables.cycle(Map.entry("foo", 1))) @@ -100,32 +105,51 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { ImmutableSet> testImmutableMapOf() { return ImmutableSet.of( - ImmutableMap.builder().build(), + ImmutableMap.builder().buildOrThrow(), + ImmutableMap.ofEntries(), Collections.emptyMap(), Map.of()); } ImmutableSet> testImmutableMapOf1() { return ImmutableSet.of( - ImmutableMap.builder().put("k1", "v1").build(), + ImmutableMap.builder().put("k1", "v1").buildOrThrow(), + ImmutableMap.ofEntries(Map.entry("k1", "v1")), Collections.singletonMap("k1", "v1"), Map.of("k1", "v1")); } - Map testImmutableMapOf2() { - return Map.of("k1", "v1", "k2", "v2"); + ImmutableSet> testImmutableMapOf2() { + return ImmutableSet.of( + ImmutableMap.ofEntries(Map.entry("k1", "v1"), Map.entry("k2", "v2")), + Map.of("k1", "v1", "k2", "v2")); } - Map testImmutableMapOf3() { - return Map.of("k1", "v1", "k2", "v2", "k3", "v3"); + ImmutableSet> testImmutableMapOf3() { + return ImmutableSet.of( + ImmutableMap.ofEntries(Map.entry("k1", "v1"), Map.entry("k2", "v2"), Map.entry("k3", "v3")), + Map.of("k1", "v1", "k2", "v2", "k3", "v3")); } - Map testImmutableMapOf4() { - return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"); + ImmutableSet> testImmutableMapOf4() { + return ImmutableSet.of( + ImmutableMap.ofEntries( + Map.entry("k1", "v1"), + Map.entry("k2", "v2"), + Map.entry("k3", "v3"), + Map.entry("k4", "v4")), + Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4")); } - Map testImmutableMapOf5() { - return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"); + ImmutableSet> testImmutableMapOf5() { + return ImmutableSet.of( + ImmutableMap.ofEntries( + Map.entry("k1", "v1"), + Map.entry("k2", "v2"), + Map.entry("k3", "v3"), + Map.entry("k4", "v4"), + Map.entry("k5", "v5")), + Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5")); } ImmutableMap testImmutableMapCopyOfMapsFilterKeys() { @@ -139,4 +163,11 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { .filter(entry -> entry.getValue() > 0) .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } + + ImmutableSet> testImmutableMapOfEntries() { + return ImmutableSet.of( + Map.ofEntries(), + Map.ofEntries(Map.entry("foo", 1)), + Map.ofEntries(Map.entry("bar", 2), Map.entry("baz", 3))); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java index 3365f3fd..0482e072 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java @@ -24,6 +24,10 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { return ImmutableMap.builder(); } + ImmutableMap testImmutableMapBuilderBuildOrThrow() { + return ImmutableMap.builder().buildOrThrow(); + } + ImmutableSet> testEntryToImmutableMap() { return ImmutableSet.of( ImmutableMap.of(Map.entry("foo", 1).getKey(), Map.entry("foo", 1).getValue()), @@ -46,8 +50,9 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { Maps.toMap(ImmutableSet.of(10), Integer::valueOf)); } - ImmutableSet> testEntryIterableToImmutableMap() { + ImmutableSet> testEntryIterableToImmutableMap() { return ImmutableSet.of( + ImmutableMap.copyOf(ImmutableMap.of("foo", 1)), ImmutableMap.copyOf(ImmutableMap.of("foo", 1)), ImmutableMap.copyOf(ImmutableMap.of("foo", 1)), ImmutableMap.copyOf(ImmutableMap.of("foo", 1).entrySet()), @@ -83,28 +88,39 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { } ImmutableSet> testImmutableMapOf() { - return ImmutableSet.of(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); + return ImmutableSet.of( + ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); } ImmutableSet> testImmutableMapOf1() { return ImmutableSet.of( - ImmutableMap.of("k1", "v1"), ImmutableMap.of("k1", "v1"), ImmutableMap.of("k1", "v1")); + ImmutableMap.of("k1", "v1"), + ImmutableMap.of("k1", "v1"), + ImmutableMap.of("k1", "v1"), + ImmutableMap.of("k1", "v1")); } - Map testImmutableMapOf2() { - return ImmutableMap.of("k1", "v1", "k2", "v2"); + ImmutableSet> testImmutableMapOf2() { + return ImmutableSet.of( + ImmutableMap.of("k1", "v1", "k2", "v2"), ImmutableMap.of("k1", "v1", "k2", "v2")); } - Map testImmutableMapOf3() { - return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3"); + ImmutableSet> testImmutableMapOf3() { + return ImmutableSet.of( + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3"), + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3")); } - Map testImmutableMapOf4() { - return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"); + ImmutableSet> testImmutableMapOf4() { + return ImmutableSet.of( + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"), + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4")); } - Map testImmutableMapOf5() { - return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"); + ImmutableSet> testImmutableMapOf5() { + return ImmutableSet.of( + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"), + ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5")); } ImmutableMap testImmutableMapCopyOfMapsFilterKeys() { @@ -114,4 +130,11 @@ final class ImmutableMapRulesTest implements RefasterRuleCollectionTestCase { ImmutableMap testImmutableMapCopyOfMapsFilterValues() { return ImmutableMap.copyOf(Maps.filterValues(ImmutableMap.of("foo", 1), v -> v > 0)); } + + ImmutableSet> testImmutableMapOfEntries() { + return ImmutableSet.of( + ImmutableMap.ofEntries(), + ImmutableMap.ofEntries(Map.entry("foo", 1)), + ImmutableMap.ofEntries(Map.entry("bar", 2), Map.entry("baz", 3))); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRulesTestInput.java index 3dc40717..46e9ffda 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableSortedMapRulesTestInput.java @@ -32,16 +32,16 @@ final class ImmutableSortedMapRulesTest implements RefasterRuleCollectionTestCas } ImmutableSortedMap testEmptyImmutableSortedMap() { - return ImmutableSortedMap.naturalOrder().build(); + return ImmutableSortedMap.naturalOrder().buildOrThrow(); } ImmutableSortedMap testPairToImmutableSortedMap() { - return ImmutableSortedMap.naturalOrder().put("foo", 1).build(); + return ImmutableSortedMap.naturalOrder().put("foo", 1).buildOrThrow(); } ImmutableSet> testEntryToImmutableSortedMap() { return ImmutableSet.of( - ImmutableSortedMap.naturalOrder().put(Map.entry("foo", 1)).build(), + ImmutableSortedMap.naturalOrder().put(Map.entry("foo", 1)).buildOrThrow(), Stream.of(Map.entry("foo", 1)) .collect(toImmutableSortedMap(naturalOrder(), Map.Entry::getKey, Map.Entry::getValue))); } @@ -52,10 +52,10 @@ final class ImmutableSortedMapRulesTest implements RefasterRuleCollectionTestCas ImmutableSortedMap.copyOf(ImmutableSortedMap.of("foo", 1).entrySet()), ImmutableSortedMap.naturalOrder() .putAll(ImmutableSortedMap.of("foo", 1)) - .build(), + .buildOrThrow(), ImmutableSortedMap.naturalOrder() .putAll(ImmutableSortedMap.of("foo", 1).entrySet()) - .build(), + .buildOrThrow(), ImmutableSortedMap.of("foo", 1).entrySet().stream() .collect(toImmutableSortedMap(naturalOrder(), Map.Entry::getKey, Map.Entry::getValue)), Streams.stream(Iterables.cycle(Map.entry("foo", 1))) diff --git a/integration-tests/checkstyle-expected-changes.patch b/integration-tests/checkstyle-expected-changes.patch index a7b24179..252a145d 100644 --- a/integration-tests/checkstyle-expected-changes.patch +++ b/integration-tests/checkstyle-expected-changes.patch @@ -14875,7 +14875,7 @@ final String defaultValue = getDefaultValue(propertyName, field, instance); --- a/src/main/java/com/puppycrawl/tools/checkstyle/site/SiteUtil.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/site/SiteUtil.java -@@ -19,6 +19,14 @@ +@@ -19,6 +19,15 @@ package com.puppycrawl.tools.checkstyle.site; @@ -14886,11 +14886,12 @@ + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; ++import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.puppycrawl.tools.checkstyle.Checker; import com.puppycrawl.tools.checkstyle.DefaultConfiguration; -@@ -55,7 +63,6 @@ import java.net.URI; +@@ -55,7 +64,6 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -14898,7 +14899,7 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; -@@ -72,7 +79,6 @@ import java.util.Optional; +@@ -72,7 +80,6 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.regex.Pattern; @@ -14906,7 +14907,31 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import javax.annotation.Nullable; -@@ -156,7 +162,7 @@ public final class SiteUtil { +@@ -131,12 +138,17 @@ public final class SiteUtil { + + /** Class name and their corresponding parent module name. */ + private static final Map, String> CLASS_TO_PARENT_MODULE = +- Map.ofEntries( +- Map.entry(AbstractCheck.class, TreeWalker.class.getSimpleName()), +- Map.entry(TreeWalkerFilter.class, TreeWalker.class.getSimpleName()), +- Map.entry(AbstractFileSetCheck.class, Checker.class.getSimpleName()), +- Map.entry(Filter.class, Checker.class.getSimpleName()), +- Map.entry(BeforeExecutionFileFilter.class, Checker.class.getSimpleName())); ++ ImmutableMap.of( ++ AbstractCheck.class, ++ TreeWalker.class.getSimpleName(), ++ TreeWalkerFilter.class, ++ TreeWalker.class.getSimpleName(), ++ AbstractFileSetCheck.class, ++ Checker.class.getSimpleName(), ++ Filter.class, ++ Checker.class.getSimpleName(), ++ BeforeExecutionFileFilter.class, ++ Checker.class.getSimpleName()); + + /** Set of properties that every check has. */ + private static final Set CHECK_PROPERTIES = getProperties(AbstractCheck.class); +@@ -156,7 +168,7 @@ public final class SiteUtil { /** Set of properties that are undocumented. Those are internal properties. */ private static final Set UNDOCUMENTED_PROPERTIES = @@ -14915,7 +14940,7 @@ "SuppressWithNearbyCommentFilter.fileContents", "SuppressionCommentFilter.fileContents"); /** Properties that can not be gathered from class instance. */ -@@ -294,27 +300,25 @@ public final class SiteUtil { +@@ -294,27 +306,25 @@ public final class SiteUtil { /** Path to main source code folder. */ private static final String MAIN_FOLDER_PATH = @@ -14932,16 +14957,16 @@ + new File(Path.of(MAIN_FOLDER_PATH, CHECKS, NAMING, "AbstractNameCheck.java").toString()), new File( - Paths.get(MAIN_FOLDER_PATH, CHECKS, NAMING, "AbstractNameCheck.java").toString()), -- new File( -- Paths.get(MAIN_FOLDER_PATH, CHECKS, "javadoc", "AbstractJavadocCheck.java") -- .toString()), -- new File(Paths.get(MAIN_FOLDER_PATH, "api", "AbstractFileSetCheck.java").toString()), + Path.of(MAIN_FOLDER_PATH, CHECKS, "javadoc", "AbstractJavadocCheck.java").toString()), + new File(Path.of(MAIN_FOLDER_PATH, "api", "AbstractFileSetCheck.java").toString()), new File( -- Paths.get(MAIN_FOLDER_PATH, CHECKS, "header", "AbstractHeaderCheck.java").toString()), +- Paths.get(MAIN_FOLDER_PATH, CHECKS, "javadoc", "AbstractJavadocCheck.java") +- .toString()), +- new File(Paths.get(MAIN_FOLDER_PATH, "api", "AbstractFileSetCheck.java").toString()), + Path.of(MAIN_FOLDER_PATH, CHECKS, "header", "AbstractHeaderCheck.java").toString()), new File( +- Paths.get(MAIN_FOLDER_PATH, CHECKS, "header", "AbstractHeaderCheck.java").toString()), +- new File( - Paths.get(MAIN_FOLDER_PATH, CHECKS, "metrics", "AbstractClassCouplingCheck.java") + Path.of(MAIN_FOLDER_PATH, CHECKS, "metrics", "AbstractClassCouplingCheck.java") .toString()), @@ -14951,7 +14976,7 @@ .toString())); /** Private utility constructor. */ -@@ -475,7 +479,7 @@ public final class SiteUtil { +@@ -475,7 +485,7 @@ public final class SiteUtil { * @throws MacroExecutionException if an I/O error occurs. */ public static Set getXdocsTemplatesFilePaths() throws MacroExecutionException { @@ -14960,7 +14985,7 @@ try (Stream stream = Files.find( directory, -@@ -483,7 +487,7 @@ public final class SiteUtil { +@@ -483,7 +493,7 @@ public final class SiteUtil { (path, attr) -> { return attr.isRegularFile() && path.toString().endsWith(".xml.template"); })) { @@ -14969,7 +14994,7 @@ } catch (IOException ioException) { throw new MacroExecutionException("Failed to find xdocs templates", ioException); } -@@ -510,7 +514,7 @@ public final class SiteUtil { +@@ -510,7 +520,7 @@ public final class SiteUtil { } // If parent class is not found, check interfaces @@ -14978,7 +15003,7 @@ final Class[] interfaces = moduleClass.getInterfaces(); for (Class interfaceClass : interfaces) { parentModuleName = CLASS_TO_PARENT_MODULE.get(interfaceClass); -@@ -520,7 +524,7 @@ public final class SiteUtil { +@@ -520,7 +530,7 @@ public final class SiteUtil { } } @@ -14987,7 +15012,7 @@ final String message = String.format( Locale.ROOT, "Failed to find parent module for %s", moduleClass.getSimpleName()); -@@ -544,7 +548,7 @@ public final class SiteUtil { +@@ -544,7 +554,7 @@ public final class SiteUtil { prop -> { return !isGlobalProperty(clss, prop) && !isUndocumentedProperty(clss, prop); }) @@ -14996,7 +15021,7 @@ properties.addAll(getNonExplicitProperties(instance, clss)); return new TreeSet<>(properties); } -@@ -663,7 +667,7 @@ public final class SiteUtil { +@@ -663,7 +673,7 @@ public final class SiteUtil { treeWalkerConfig.addChild(scraperCheckConfig); try { checker.configure(defaultConfiguration); @@ -15005,7 +15030,7 @@ checker.process(filesToProcess); checker.destroy(); } catch (CheckstyleException checkstyleException) { -@@ -986,9 +990,7 @@ public final class SiteUtil { +@@ -986,9 +996,7 @@ public final class SiteUtil { if (value != null && Array.getLength(value) > 0) { result = removeSquareBrackets( @@ -15016,7 +15041,7 @@ } if (result.isEmpty()) { -@@ -1020,8 +1022,7 @@ public final class SiteUtil { +@@ -1020,8 +1028,7 @@ public final class SiteUtil { result = ""; } else { try (Stream valuesStream = getValuesStream(value)) { @@ -15026,7 +15051,7 @@ } } -@@ -1062,10 +1063,7 @@ public final class SiteUtil { +@@ -1062,10 +1069,7 @@ public final class SiteUtil { private static String getIntArrayPropertyValue(Object value) { try (IntStream stream = getIntStream(value)) { String result = @@ -15038,7 +15063,7 @@ if (result.isEmpty()) { result = CURLY_BRACKETS; } -@@ -1170,11 +1168,11 @@ public final class SiteUtil { +@@ -1170,11 +1174,11 @@ public final class SiteUtil { */ public static List getDifference(int[] tokens, int... subtractions) { final Set subtractionsSet = @@ -15052,7 +15077,7 @@ } /** -@@ -1221,7 +1219,7 @@ public final class SiteUtil { +@@ -1221,7 +1225,7 @@ public final class SiteUtil { throw new MacroExecutionException("Failed to get parent path for " + templatePath); } return templatePathParent @@ -15418,12 +15443,13 @@ } --- a/src/main/java/com/puppycrawl/tools/checkstyle/utils/UnmodifiableCollectionUtil.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/utils/UnmodifiableCollectionUtil.java -@@ -19,13 +19,15 @@ +@@ -19,13 +19,16 @@ package com.puppycrawl.tools.checkstyle.utils; +import static java.util.stream.Collectors.toUnmodifiableList; + ++import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import java.util.Arrays; import java.util.Collection; @@ -15435,7 +15461,7 @@ /** *
Note: it simply wraps the existing JDK methods to provide a workaround for Pitest survival -@@ -57,7 +59,7 @@ public final class UnmodifiableCollectionUtil { +@@ -57,7 +60,7 @@ public final class UnmodifiableCollectionUtil { * @return An unmodifiable List containing elements of the specified type. */ public static List unmodifiableList(Collection items, Class elementType) { @@ -15444,7 +15470,16 @@ } /** -@@ -92,6 +94,6 @@ public final class UnmodifiableCollectionUtil { +@@ -81,7 +84,7 @@ public final class UnmodifiableCollectionUtil { + * @return an immutable copy of the input map + */ + public static Map copyOfMap(Map map) { +- return Map.copyOf(map); ++ return ImmutableMap.copyOf(map); + } + + /** +@@ -92,6 +95,6 @@ public final class UnmodifiableCollectionUtil { * @return immutable set */ public static Set singleton(T obj) { @@ -56937,6 +56972,15 @@ private static final Map> FULLY_QUALIFIED_CLASS_NAMES = ImmutableMap.>builder() .put("int", int.class) +@@ -97,7 +96,7 @@ public class XdocsJavaDocsTest extends AbstractModuleTestSupport { + .put("URI", URI.class) + .put("WrapOption", WrapOption.class) + .put("PARAM_LITERAL", int[].class) +- .build(); ++ .buildOrThrow(); + + private static final List> CHECK_PROPERTIES = new ArrayList<>(); + private static final Map CHECK_PROPERTY_DOC = new HashMap<>(); @@ -115,14 +114,14 @@ public class XdocsJavaDocsTest extends AbstractModuleTestSupport { } diff --git a/integration-tests/prometheus-java-client-expected-changes.patch b/integration-tests/prometheus-java-client-expected-changes.patch index 8c83b749..0d5a79ba 100644 --- a/integration-tests/prometheus-java-client-expected-changes.patch +++ b/integration-tests/prometheus-java-client-expected-changes.patch @@ -4459,6 +4459,15 @@ "values from otel have precedence over builder", new TestCase() .expectedProperties( +@@ -113,7 +113,7 @@ class OtelAutoConfigTest { + .put("otel.exporter.otlp.timeout", Optional.of("13s")) + .put("otel.exporter.otlp.metrics.timeout", Optional.empty()) + .put("otel.service.name", Optional.of("otel-service")) +- .build()) ++ .buildOrThrow()) + .expectedResourceAttributes( + ImmutableMap.of( + "key", @@ -128,7 +128,7 @@ class OtelAutoConfigTest { "otel-version")) .exporterBuilder(OtelAutoConfigTest::setBuilderValues) @@ -4468,15 +4477,44 @@ "values from prom properties have precedence over builder and otel", new TestCase() .expectedProperties( -@@ -177,7 +177,7 @@ class OtelAutoConfigTest { +@@ -143,7 +143,7 @@ class OtelAutoConfigTest { + .put("otel.exporter.otlp.metrics.timeout", Optional.of("23s")) + .put("otel.exporter.otlp.timeout", Optional.of("13s")) + .put("otel.service.name", Optional.of("prom-service")) +- .build()) ++ .buildOrThrow()) + .expectedResourceAttributes( + ImmutableMap.of( + "key", +@@ -176,8 +176,8 @@ class OtelAutoConfigTest { + .put( "io.prometheus.exporter.opentelemetry.resourceAttributes", "key=prom-value") - .build())), +- .build())), - Arguments.of( ++ .buildOrThrow())), + arguments( "values from prom properties builder have precedence over builder and otel", new TestCase() .expectedProperties( +@@ -192,7 +192,7 @@ class OtelAutoConfigTest { + .put("otel.exporter.otlp.metrics.timeout", Optional.of("23s")) + .put("otel.exporter.otlp.timeout", Optional.of("13s")) + .put("otel.service.name", Optional.of("prom-service")) +- .build()) ++ .buildOrThrow()) + .expectedResourceAttributes( + ImmutableMap.of( + "key", +@@ -233,7 +233,7 @@ class OtelAutoConfigTest { + .put( + "otel.resource.attributes", + "key=otel-value,service.namespace=otel-namespace,service.instance.id=otel-instance,service.version=otel-version") +- .build(); ++ .buildOrThrow(); + } + + private static void setBuilderValues(OpenTelemetryExporter.Builder builder) { @@ -250,8 +250,8 @@ class OtelAutoConfigTest { .resourceAttribute("key", "builder-value"); } @@ -4565,7 +4603,12 @@ } --- a/prometheus-metrics-exporter-opentelemetry/src/test/java/io/prometheus/metrics/exporter/opentelemetry/otelmodel/PrometheusMetricDataTest.java +++ b/prometheus-metrics-exporter-opentelemetry/src/test/java/io/prometheus/metrics/exporter/opentelemetry/otelmodel/PrometheusMetricDataTest.java -@@ -7,7 +7,7 @@ import io.prometheus.metrics.model.snapshots.Unit; +@@ -3,14 +3,15 @@ package io.prometheus.metrics.exporter.opentelemetry.otelmodel; + import static java.util.Map.entry; + import static org.junit.jupiter.api.Assertions.*; + ++import com.google.common.collect.ImmutableMap; + import io.prometheus.metrics.model.snapshots.Unit; import java.util.Map; import org.junit.jupiter.api.Test; @@ -4573,7 +4616,11 @@ +final class PrometheusMetricDataTest { Map translations = - Map.ofEntries( +- Map.ofEntries( ++ ImmutableMap.ofEntries( + entry("days", "d"), + entry("hours", "h"), + entry("minutes", "min"), --- a/prometheus-metrics-exporter-pushgateway/src/main/java/io/prometheus/metrics/exporter/pushgateway/DefaultJobLabelDetector.java +++ b/prometheus-metrics-exporter-pushgateway/src/main/java/io/prometheus/metrics/exporter/pushgateway/DefaultJobLabelDetector.java @@ -3,7 +3,6 @@ package io.prometheus.metrics.exporter.pushgateway; From c3ae1b5b92c6f15b42d34e9e0f6a040491747914 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sun, 5 Jan 2025 13:28:00 +0100 Subject: [PATCH 20/43] Upgrade AssertJ 3.27.1 -> 3.27.2 (#1501) See: - https://github.com/assertj/assertj/releases/tag/assertj-build-3.27.2 - https://github.com/assertj/assertj/compare/assertj-build-3.27.1...assertj-build-3.27.2 --- integration-tests/checkstyle-init.patch | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/checkstyle-init.patch b/integration-tests/checkstyle-init.patch index 5688dad0..a31ca2b2 100644 --- a/integration-tests/checkstyle-init.patch +++ b/integration-tests/checkstyle-init.patch @@ -7,7 +7,7 @@ + + org.assertj + assertj-core -+ 3.27.1 ++ 3.27.2 + test + diff --git a/pom.xml b/pom.xml index 756e30ea..5d57b25e 100644 --- a/pom.xml +++ b/pom.xml @@ -445,7 +445,7 @@ org.assertj assertj-bom - 3.27.1 + 3.27.2 pom import From 623bdd7344d586b88c59f19124a093b47ad119d6 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sun, 5 Jan 2025 17:27:38 +0100 Subject: [PATCH 21/43] Upgrade Mockito 5.14.2 -> 5.15.2 (#1498) See: - https://github.com/mockito/mockito/releases/tag/v5.15.0 - https://github.com/mockito/mockito/releases/tag/v5.15.1 - https://github.com/mockito/mockito/releases/tag/v5.15.2 - https://github.com/mockito/mockito/compare/v5.14.2...v5.15.2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 5d57b25e..32d78a92 100644 --- a/pom.xml +++ b/pom.xml @@ -216,7 +216,7 @@ 1.0 17 3.9.9 - 5.14.2 + 5.15.2 1.0.1 0.12.2 1.1.4 From 7eb46dd21893e5638fc5c03bc02386dd65aeecbd Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Mon, 6 Jan 2025 08:54:01 +0100 Subject: [PATCH 22/43] Upgrade Checker Framework Annotations 3.48.3 -> 3.48.4 (#1497) See: - https://github.com/typetools/checker-framework/releases/tag/checker-framework-3.48.4 - https://github.com/typetools/checker-framework/compare/checker-framework-3.48.3...checker-framework-3.48.4 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 32d78a92..78124988 100644 --- a/pom.xml +++ b/pom.xml @@ -452,7 +452,7 @@ org.checkerframework checker-qual - 3.48.3 + 3.48.4 org.hamcrest From 37d8ea92c6fde14e537eaa534edc87a8a9408005 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Tue, 7 Jan 2025 18:01:31 +0100 Subject: [PATCH 23/43] Upgrade NullAway 0.12.2 -> 0.12.3 (#1502) See: - https://github.com/uber/NullAway/blob/master/CHANGELOG.md - https://github.com/uber/NullAway/releases/tag/v0.12.3 - https://github.com/uber/NullAway/compare/v0.12.2...v0.12.3 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 78124988..8c8d7b3f 100644 --- a/pom.xml +++ b/pom.xml @@ -218,7 +218,7 @@ 3.9.9 5.15.2 1.0.1 - 0.12.2 + 0.12.3 1.1.4 1.20.2 3.2.3 From dec22b86334965cc03472486d399626686433267 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Tue, 7 Jan 2025 19:15:54 +0100 Subject: [PATCH 24/43] Introduce assorted `Iterator` and `Iterable` Refaster rules (#1487) --- .../refasterrules/AssertJEnumerableRules.java | 101 ++++++++++++++++- .../refasterrules/AssertJIterableRules.java | 90 +++++++++++++++ .../refasterrules/AssertJIteratorRules.java | 43 +++++++ .../refasterrules/AssertJRules.java | 106 ------------------ .../refasterrules/RefasterRulesTest.java | 4 +- .../AssertJEnumerableRulesTestInput.java | 39 ++++++- .../AssertJEnumerableRulesTestOutput.java | 40 ++++++- .../AssertJIterableRulesTestInput.java | 35 ++++++ .../AssertJIterableRulesTestOutput.java | 34 ++++++ .../AssertJIteratorRulesTestInput.java | 17 +++ .../AssertJIteratorRulesTestOutput.java | 17 +++ 11 files changed, 404 insertions(+), 122 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIterableRules.java create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIteratorRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestOutput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestOutput.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJEnumerableRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJEnumerableRules.java index bc19fd72..b4318931 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJEnumerableRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJEnumerableRules.java @@ -5,6 +5,9 @@ import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import java.util.Collection; +import org.assertj.core.api.AbstractIntegerAssert; +import org.assertj.core.api.AbstractIterableAssert; +import org.assertj.core.api.AbstractIterableSizeAssert; import org.assertj.core.api.EnumerableAssert; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @@ -21,6 +24,11 @@ final class AssertJEnumerableRules { enumAssert.hasSizeLessThan(1)); } + @BeforeTemplate + void before(AbstractIterableAssert enumAssert) { + enumAssert.size().isNotPositive(); + } + @AfterTemplate void after(EnumerableAssert enumAssert) { enumAssert.isEmpty(); @@ -34,30 +42,113 @@ final class AssertJEnumerableRules { enumAssert.hasSizeGreaterThan(0), enumAssert.hasSizeGreaterThanOrEqualTo(1)); } + @BeforeTemplate + AbstractIntegerAssert before(AbstractIterableAssert enumAssert) { + return Refaster.anyOf(enumAssert.size().isNotEqualTo(0), enumAssert.size().isPositive()); + } + @AfterTemplate EnumerableAssert after(EnumerableAssert enumAssert) { return enumAssert.isNotEmpty(); } } - static final class EnumerableAssertHasSameSizeAs { + static final class EnumerableAssertHasSize { @BeforeTemplate - EnumerableAssert before(EnumerableAssert enumAssert, Iterable iterable) { + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int size) { + return enumAssert.size().isEqualTo(size); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int size) { + return enumAssert.hasSize(size); + } + } + + static final class EnumerableAssertHasSizeLessThan { + @BeforeTemplate + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int size) { + return enumAssert.size().isLessThan(size); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int size) { + return enumAssert.hasSizeLessThan(size); + } + } + + static final class EnumerableAssertHasSizeLessThanOrEqualTo { + @BeforeTemplate + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int size) { + return enumAssert.size().isLessThanOrEqualTo(size); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int size) { + return enumAssert.hasSizeLessThanOrEqualTo(size); + } + } + + static final class EnumerableAssertHasSizeGreaterThan { + @BeforeTemplate + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int size) { + return enumAssert.size().isGreaterThan(size); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int size) { + return enumAssert.hasSizeGreaterThan(size); + } + } + + static final class EnumerableAssertHasSizeGreaterThanOrEqualTo { + @BeforeTemplate + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int size) { + return enumAssert.size().isGreaterThanOrEqualTo(size); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int size) { + return enumAssert.hasSizeGreaterThanOrEqualTo(size); + } + } + + static final class EnumerableAssertHasSizeBetween { + @BeforeTemplate + AbstractIterableSizeAssert before( + AbstractIterableAssert enumAssert, int lower, int upper) { + return enumAssert.size().isBetween(lower, upper); + } + + @AfterTemplate + EnumerableAssert after(EnumerableAssert enumAssert, int lower, int upper) { + return enumAssert.hasSizeBetween(lower, upper); + } + } + + static final class EnumerableAssertHasSameSizeAs { + @BeforeTemplate + EnumerableAssert before(EnumerableAssert enumAssert, Iterable iterable) { return enumAssert.hasSize(Iterables.size(iterable)); } @BeforeTemplate - EnumerableAssert before(EnumerableAssert enumAssert, Collection iterable) { + EnumerableAssert before(EnumerableAssert enumAssert, Collection iterable) { return enumAssert.hasSize(iterable.size()); } @BeforeTemplate - EnumerableAssert before(EnumerableAssert enumAssert, T[] iterable) { + EnumerableAssert before(EnumerableAssert enumAssert, E[] iterable) { return enumAssert.hasSize(iterable.length); } @AfterTemplate - EnumerableAssert after(EnumerableAssert enumAssert, Iterable iterable) { + EnumerableAssert after(EnumerableAssert enumAssert, Iterable iterable) { return enumAssert.hasSameSizeAs(iterable); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIterableRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIterableRules.java new file mode 100644 index 00000000..70bc5ed1 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIterableRules.java @@ -0,0 +1,90 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.Iterables; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.refaster.annotation.UseImportPolicy; +import java.util.Collection; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractIntegerAssert; +import org.assertj.core.api.IterableAssert; +import org.assertj.core.api.ObjectAssert; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +@OnlineDocumentation +final class AssertJIterableRules { + private AssertJIterableRules() {} + + static final class AssertThatIterableIsEmpty { + @BeforeTemplate + void before(Iterable iterable) { + assertThat(iterable.iterator()).isExhausted(); + } + + @BeforeTemplate + void before(Collection iterable) { + assertThat(iterable.isEmpty()).isTrue(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(Collection iterable) { + assertThat(iterable).isEmpty(); + } + } + + static final class AssertThatIterableIsNotEmpty { + @BeforeTemplate + AbstractAssert before(Iterable iterable) { + return assertThat(iterable.iterator()).hasNext(); + } + + @BeforeTemplate + AbstractAssert before(Collection iterable) { + return assertThat(iterable.isEmpty()).isFalse(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + IterableAssert after(Iterable iterable) { + return assertThat(iterable).isNotEmpty(); + } + } + + static final class AssertThatIterableSize { + @BeforeTemplate + AbstractIntegerAssert before(Iterable iterable) { + return assertThat(Iterables.size(iterable)); + } + + @BeforeTemplate + AbstractIntegerAssert before(Collection iterable) { + return assertThat(iterable.size()); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + AbstractIntegerAssert after(Iterable iterable) { + return assertThat(iterable).size(); + } + } + + // XXX: In practice this rule isn't very useful, as it only matches invocations of + // `assertThat(E)`. In most cases a more specific overload of `assertThat` is invoked, in which + // case this rule won't match. Look into a more robust approach. + static final class AssertThatIterableHasOneElementEqualTo { + @BeforeTemplate + ObjectAssert before(Iterable iterable, E element) { + return assertThat(Iterables.getOnlyElement(iterable)).isEqualTo(element); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + IterableAssert after(Iterable iterable, E element) { + return assertThat(iterable).containsExactly(element); + } + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIteratorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIteratorRules.java new file mode 100644 index 00000000..2b2e503f --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJIteratorRules.java @@ -0,0 +1,43 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.refaster.annotation.UseImportPolicy; +import java.util.Iterator; +import org.assertj.core.api.AbstractBooleanAssert; +import org.assertj.core.api.IteratorAssert; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +@OnlineDocumentation +final class AssertJIteratorRules { + private AssertJIteratorRules() {} + + static final class AssertThatHasNext { + @BeforeTemplate + AbstractBooleanAssert before(Iterator iterator) { + return assertThat(iterator.hasNext()).isTrue(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + IteratorAssert after(Iterator iterator) { + return assertThat(iterator).hasNext(); + } + } + + static final class AssertThatIsExhausted { + @BeforeTemplate + AbstractBooleanAssert before(Iterator iterator) { + return assertThat(iterator.hasNext()).isFalse(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + IteratorAssert after(Iterator iterator) { + return assertThat(iterator).isExhausted(); + } + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java index c325dcec..c45c912c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java @@ -3,7 +3,6 @@ package tech.picnic.errorprone.refasterrules; import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; import static org.assertj.core.api.Assertions.assertThat; -import com.google.common.collect.Iterables; import com.google.common.collect.Multiset; import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; @@ -12,7 +11,6 @@ import com.google.errorprone.refaster.annotation.Matches; import com.google.errorprone.refaster.annotation.NotMatches; import com.google.errorprone.refaster.annotation.Repeated; import com.google.errorprone.refaster.annotation.UseImportPolicy; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.OptionalDouble; @@ -22,9 +20,7 @@ import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collector; import java.util.stream.Stream; -import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.AbstractCollectionAssert; -import org.assertj.core.api.AbstractComparableAssert; import org.assertj.core.api.AbstractDoubleAssert; import org.assertj.core.api.AbstractIntegerAssert; import org.assertj.core.api.AbstractLongAssert; @@ -254,108 +250,6 @@ final class AssertJRules { } } - // - // Iterable - // - - static final class AssertThatIterableIsEmpty { - @BeforeTemplate - void before(Iterable iterable) { - Refaster.anyOf( - assertThat(iterable).hasSize(0), - assertThat(iterable.iterator().hasNext()).isFalse(), - assertThat(iterable.iterator()).isExhausted(), - assertThat(Iterables.size(iterable)).isEqualTo(0L), - assertThat(Iterables.size(iterable)).isNotPositive()); - } - - @BeforeTemplate - void before(Collection iterable) { - Refaster.anyOf( - assertThat(iterable.isEmpty()).isTrue(), - assertThat(iterable.size()).isEqualTo(0L), - assertThat(iterable.size()).isNotPositive()); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Collection iterable) { - assertThat(iterable).isEmpty(); - } - } - - static final class AssertThatIterableIsNotEmpty { - @BeforeTemplate - AbstractAssert before(Iterable iterable) { - return Refaster.anyOf( - assertThat(iterable.iterator().hasNext()).isTrue(), - assertThat(Iterables.size(iterable)).isNotEqualTo(0), - assertThat(Iterables.size(iterable)).isPositive()); - } - - @BeforeTemplate - AbstractAssert before(Collection iterable) { - return Refaster.anyOf( - assertThat(iterable.isEmpty()).isFalse(), - assertThat(iterable.size()).isNotEqualTo(0), - assertThat(iterable.size()).isPositive()); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - IterableAssert after(Iterable iterable) { - return assertThat(iterable).isNotEmpty(); - } - } - - static final class AssertThatIterableHasSize { - @BeforeTemplate - AbstractIntegerAssert before(Iterable iterable, int length) { - return assertThat(Iterables.size(iterable)).isEqualTo(length); - } - - @BeforeTemplate - AbstractIntegerAssert before(Collection iterable, int length) { - return assertThat(iterable.size()).isEqualTo(length); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - IterableAssert after(Iterable iterable, int length) { - return assertThat(iterable).hasSize(length); - } - } - - static final class AssertThatIterableHasOneElementEqualTo { - @BeforeTemplate - ObjectAssert before(Iterable iterable, T element) { - return assertThat(Iterables.getOnlyElement(iterable)).isEqualTo(element); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - IterableAssert after(Iterable iterable, T element) { - return assertThat(iterable).containsExactly(element); - } - } - - // XXX: This overload is here because `assertThat` has an overload for `Comparable` types. - // Unfortunately this still doesn't convince Refaster to match this rule in the context of - // Comparable types. Figure out why! Note that this also affects the `AssertThatOptional` rule. - static final class AssertThatIterableHasOneComparableElementEqualTo< - S extends Comparable, T extends S> { - @BeforeTemplate - AbstractComparableAssert before(Iterable iterable, T element) { - return assertThat(Iterables.getOnlyElement(iterable)).isEqualTo(element); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - IterableAssert after(Iterable iterable, T element) { - return assertThat(iterable).containsExactly(element); - } - } - // // List // diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index d7b47d30..0d7046f9 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -23,6 +23,8 @@ final class RefasterRulesTest { AssertJEnumerableRules.class, AssertJFloatRules.class, AssertJIntegerRules.class, + AssertJIterableRules.class, + AssertJIteratorRules.class, AssertJLongRules.class, AssertJMapRules.class, AssertJNumberRules.class, @@ -43,7 +45,6 @@ final class RefasterRulesTest { DoubleStreamRules.class, EqualityRules.class, FileRules.class, - InputStreamRules.class, ImmutableEnumSetRules.class, ImmutableListRules.class, ImmutableListMultimapRules.class, @@ -55,6 +56,7 @@ final class RefasterRulesTest { ImmutableSortedMultisetRules.class, ImmutableSortedSetRules.class, ImmutableTableRules.class, + InputStreamRules.class, IntStreamRules.class, JUnitRules.class, JUnitToAssertJRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestInput.java index bab44b4e..f2e603e4 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestInput.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.EnumerableAssert; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -14,14 +15,42 @@ final class AssertJEnumerableRulesTest implements RefasterRuleCollectionTestCase } void testEnumerableAssertIsEmpty() { - assertThat(ImmutableSet.of()).hasSize(0); - assertThat(ImmutableSet.of()).hasSizeLessThanOrEqualTo(0); - assertThat(ImmutableSet.of()).hasSizeLessThan(1); + assertThat(ImmutableSet.of(1)).hasSize(0); + assertThat(ImmutableSet.of(2)).hasSizeLessThanOrEqualTo(0); + assertThat(ImmutableSet.of(3)).hasSizeLessThan(1); + assertThat(ImmutableSet.of(4)).size().isNotPositive(); } - ImmutableSet> testEnumerableAssertIsNotEmpty() { + ImmutableSet> testEnumerableAssertIsNotEmpty() { return ImmutableSet.of( - assertThat("foo").hasSizeGreaterThan(0), assertThat("bar").hasSizeGreaterThanOrEqualTo(1)); + assertThat(ImmutableSet.of(1)).hasSizeGreaterThan(0), + assertThat(ImmutableSet.of(2)).hasSizeGreaterThanOrEqualTo(1), + assertThat(ImmutableSet.of(3)).size().isNotEqualTo(0), + assertThat(ImmutableSet.of(4)).size().isPositive()); + } + + AbstractAssert testEnumerableAssertHasSize() { + return assertThat(ImmutableSet.of(1)).size().isEqualTo(2); + } + + AbstractAssert testEnumerableAssertHasSizeLessThan() { + return assertThat(ImmutableSet.of(1)).size().isLessThan(2); + } + + AbstractAssert testEnumerableAssertHasSizeLessThanOrEqualTo() { + return assertThat(ImmutableSet.of(1)).size().isLessThanOrEqualTo(2); + } + + AbstractAssert testEnumerableAssertHasSizeGreaterThan() { + return assertThat(ImmutableSet.of(1)).size().isGreaterThan(2); + } + + AbstractAssert testEnumerableAssertHasSizeGreaterThanOrEqualTo() { + return assertThat(ImmutableSet.of(1)).size().isGreaterThanOrEqualTo(2); + } + + AbstractAssert testEnumerableAssertHasSizeBetween() { + return assertThat(ImmutableSet.of(1)).size().isBetween(2, 3); } ImmutableSet> testEnumerableAssertHasSameSizeAs() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestOutput.java index 892f2f69..93f7a685 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJEnumerableRulesTestOutput.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.EnumerableAssert; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -14,13 +15,42 @@ final class AssertJEnumerableRulesTest implements RefasterRuleCollectionTestCase } void testEnumerableAssertIsEmpty() { - assertThat(ImmutableSet.of()).isEmpty(); - assertThat(ImmutableSet.of()).isEmpty(); - assertThat(ImmutableSet.of()).isEmpty(); + assertThat(ImmutableSet.of(1)).isEmpty(); + assertThat(ImmutableSet.of(2)).isEmpty(); + assertThat(ImmutableSet.of(3)).isEmpty(); + assertThat(ImmutableSet.of(4)).isEmpty(); } - ImmutableSet> testEnumerableAssertIsNotEmpty() { - return ImmutableSet.of(assertThat("foo").isNotEmpty(), assertThat("bar").isNotEmpty()); + ImmutableSet> testEnumerableAssertIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableSet.of(1)).isNotEmpty(), + assertThat(ImmutableSet.of(2)).isNotEmpty(), + assertThat(ImmutableSet.of(3)).isNotEmpty(), + assertThat(ImmutableSet.of(4)).isNotEmpty()); + } + + AbstractAssert testEnumerableAssertHasSize() { + return assertThat(ImmutableSet.of(1)).hasSize(2); + } + + AbstractAssert testEnumerableAssertHasSizeLessThan() { + return assertThat(ImmutableSet.of(1)).hasSizeLessThan(2); + } + + AbstractAssert testEnumerableAssertHasSizeLessThanOrEqualTo() { + return assertThat(ImmutableSet.of(1)).hasSizeLessThanOrEqualTo(2); + } + + AbstractAssert testEnumerableAssertHasSizeGreaterThan() { + return assertThat(ImmutableSet.of(1)).hasSizeGreaterThan(2); + } + + AbstractAssert testEnumerableAssertHasSizeGreaterThanOrEqualTo() { + return assertThat(ImmutableSet.of(1)).hasSizeGreaterThanOrEqualTo(2); + } + + AbstractAssert testEnumerableAssertHasSizeBetween() { + return assertThat(ImmutableSet.of(1)).hasSizeBetween(2, 3); } ImmutableSet> testEnumerableAssertHasSameSizeAs() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestInput.java new file mode 100644 index 00000000..9e3b3fb1 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestInput.java @@ -0,0 +1,35 @@ +package tech.picnic.errorprone.refasterrules; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractIntegerAssert; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class AssertJIterableRulesTest implements RefasterRuleCollectionTestCase { + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(Iterables.class); + } + + void testAssertThatIterableIsEmpty() { + assertThat(ImmutableSet.of(1).iterator()).isExhausted(); + assertThat(ImmutableSet.of(2).isEmpty()).isTrue(); + } + + ImmutableSet> testAssertThatIterableIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableSet.of(1).iterator()).hasNext(), + assertThat(ImmutableSet.of(2).isEmpty()).isFalse()); + } + + ImmutableSet> testAssertThatIterableSize() { + return ImmutableSet.of( + assertThat(Iterables.size(ImmutableSet.of(1))), assertThat(ImmutableSet.of(2).size())); + } + + AbstractAssert testAssertThatIterableHasOneElementEqualTo() { + return assertThat(Iterables.getOnlyElement(ImmutableSet.of(new Object()))).isEqualTo("foo"); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestOutput.java new file mode 100644 index 00000000..e4757b4f --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIterableRulesTestOutput.java @@ -0,0 +1,34 @@ +package tech.picnic.errorprone.refasterrules; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractIntegerAssert; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class AssertJIterableRulesTest implements RefasterRuleCollectionTestCase { + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(Iterables.class); + } + + void testAssertThatIterableIsEmpty() { + assertThat(ImmutableSet.of(1)).isEmpty(); + assertThat(ImmutableSet.of(2)).isEmpty(); + } + + ImmutableSet> testAssertThatIterableIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableSet.of(1)).isNotEmpty(), assertThat(ImmutableSet.of(2)).isNotEmpty()); + } + + ImmutableSet> testAssertThatIterableSize() { + return ImmutableSet.of( + assertThat(ImmutableSet.of(1)).size(), assertThat(ImmutableSet.of(2)).size()); + } + + AbstractAssert testAssertThatIterableHasOneElementEqualTo() { + return assertThat(ImmutableSet.of(new Object())).containsExactly("foo"); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestInput.java new file mode 100644 index 00000000..fbbb2e58 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestInput.java @@ -0,0 +1,17 @@ +package tech.picnic.errorprone.refasterrules; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableSet; +import org.assertj.core.api.AbstractAssert; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class AssertJIteratorRulesTest implements RefasterRuleCollectionTestCase { + AbstractAssert testAssertThatHasNext() { + return assertThat(ImmutableSet.of().iterator().hasNext()).isTrue(); + } + + AbstractAssert testAssertThatIsExhausted() { + return assertThat(ImmutableSet.of().iterator().hasNext()).isFalse(); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestOutput.java new file mode 100644 index 00000000..e616358b --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJIteratorRulesTestOutput.java @@ -0,0 +1,17 @@ +package tech.picnic.errorprone.refasterrules; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableSet; +import org.assertj.core.api.AbstractAssert; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class AssertJIteratorRulesTest implements RefasterRuleCollectionTestCase { + AbstractAssert testAssertThatHasNext() { + return assertThat(ImmutableSet.of().iterator()).hasNext(); + } + + AbstractAssert testAssertThatIsExhausted() { + return assertThat(ImmutableSet.of().iterator()).isExhausted(); + } +} From 4aacc5838258350556fb6f771b720d03a9ababc4 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 8 Jan 2025 09:35:09 +0100 Subject: [PATCH 25/43] Extend `JUnitToAssertJRules` Refaster rule collection (#1484) By migrating all remaining `assertArrayEquals` methods. --- .../refasterrules/JUnitToAssertJRules.java | 468 ++++++++++++++++-- .../JUnitToAssertJRulesTestInput.java | 136 +++++ .../JUnitToAssertJRulesTestOutput.java | 148 ++++++ 3 files changed, 718 insertions(+), 34 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java index e7915237..05456e4e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java @@ -5,6 +5,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; +import static org.assertj.core.api.Assertions.offset; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -34,7 +36,8 @@ import tech.picnic.errorprone.refaster.annotation.TypeMigration; *

Note that, while both libraries throw an {@link AssertionError} in case of an assertion * failure, the exact subtype used generally differs. */ -// XXX: Not all JUnit `Assertions` methods have an associated Refaster rule yet; expand this class. +// XXX: The `AssertThat*Array*ContainsExactly*` rules assume that `expected` and `actual` are not +// both `null`. // XXX: Introduce a `@Matcher` on `Executable` and `ThrowingSupplier` expressions, such that they // are only matched if they are also compatible with the `ThrowingCallable` functional interface. // When implementing such a matcher, note that expressions with a non-void return type such as @@ -50,39 +53,6 @@ import tech.picnic.errorprone.refaster.annotation.TypeMigration; "assertAll(String, Collection)", "assertAll(String, Executable[])", "assertAll(String, Stream)", - "assertArrayEquals(boolean[], boolean[])", - "assertArrayEquals(boolean[], boolean[], String)", - "assertArrayEquals(boolean[], boolean[], Supplier)", - "assertArrayEquals(byte[], byte[])", - "assertArrayEquals(byte[], byte[], String)", - "assertArrayEquals(byte[], byte[], Supplier)", - "assertArrayEquals(char[], char[])", - "assertArrayEquals(char[], char[], String)", - "assertArrayEquals(char[], char[], Supplier)", - "assertArrayEquals(double[], double[])", - "assertArrayEquals(double[], double[], double)", - "assertArrayEquals(double[], double[], double, String)", - "assertArrayEquals(double[], double[], double, Supplier)", - "assertArrayEquals(double[], double[], String)", - "assertArrayEquals(double[], double[], Supplier)", - "assertArrayEquals(float[], float[])", - "assertArrayEquals(float[], float[], float)", - "assertArrayEquals(float[], float[], float, String)", - "assertArrayEquals(float[], float[], float, Supplier)", - "assertArrayEquals(float[], float[], String)", - "assertArrayEquals(float[], float[], Supplier)", - "assertArrayEquals(int[], int[])", - "assertArrayEquals(int[], int[], String)", - "assertArrayEquals(int[], int[], Supplier)", - "assertArrayEquals(long[], long[])", - "assertArrayEquals(long[], long[], String)", - "assertArrayEquals(long[], long[], Supplier)", - "assertArrayEquals(Object[], Object[])", - "assertArrayEquals(Object[], Object[], String)", - "assertArrayEquals(Object[], Object[], Supplier)", - "assertArrayEquals(short[], short[])", - "assertArrayEquals(short[], short[], String)", - "assertArrayEquals(short[], short[], Supplier)", "assertEquals(Byte, Byte)", "assertEquals(Byte, byte)", "assertEquals(byte, Byte)", @@ -302,6 +272,436 @@ import tech.picnic.errorprone.refaster.annotation.TypeMigration; final class JUnitToAssertJRules { private JUnitToAssertJRules() {} + static final class AssertThatBooleanArrayContainsExactly { + @BeforeTemplate + void before(boolean[] actual, boolean[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean[] actual, boolean[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatBooleanArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(boolean[] actual, String message, boolean[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean[] actual, String message, boolean[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatBooleanArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(boolean[] actual, Supplier message, boolean[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean[] actual, Supplier message, boolean[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatByteArrayContainsExactly { + @BeforeTemplate + void before(byte[] actual, byte[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(byte[] actual, byte[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatByteArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(byte[] actual, String message, byte[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(byte[] actual, String message, byte[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatByteArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(byte[] actual, Supplier message, byte[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(byte[] actual, Supplier message, byte[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatCharArrayContainsExactly { + @BeforeTemplate + void before(char[] actual, char[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(char[] actual, char[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatCharArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(char[] actual, String message, char[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(char[] actual, String message, char[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatCharArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(char[] actual, Supplier message, char[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(char[] actual, Supplier message, char[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatShortArrayContainsExactly { + @BeforeTemplate + void before(short[] actual, short[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(short[] actual, short[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatShortArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(short[] actual, String message, short[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(short[] actual, String message, short[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatShortArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(short[] actual, Supplier message, short[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(short[] actual, Supplier message, short[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatIntArrayContainsExactly { + @BeforeTemplate + void before(int[] actual, int[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int[] actual, int[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatIntArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(int[] actual, String message, int[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int[] actual, String message, int[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatIntArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(int[] actual, Supplier message, int[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int[] actual, Supplier message, int[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatLongArrayContainsExactly { + @BeforeTemplate + void before(long[] actual, long[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(long[] actual, long[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatLongArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(long[] actual, String message, long[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(long[] actual, String message, long[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatLongArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(long[] actual, Supplier message, long[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(long[] actual, Supplier message, long[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatFloatArrayContainsExactly { + @BeforeTemplate + void before(float[] actual, float[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, float[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatFloatArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(float[] actual, String message, float[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, String message, float[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatFloatArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(float[] actual, Supplier message, float[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, Supplier message, float[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatFloatArrayContainsExactlyWithOffset { + @BeforeTemplate + void before(float[] actual, float[] expected, float delta) { + assertArrayEquals(expected, actual, delta); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, float[] expected, float delta) { + assertThat(actual).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatFloatArrayWithFailMessageContainsExactlyWithOffset { + @BeforeTemplate + void before(float[] actual, String message, float[] expected, float delta) { + assertArrayEquals(expected, actual, delta, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, String message, float[] expected, float delta) { + assertThat(actual).withFailMessage(message).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatFloatArrayWithFailMessageSupplierContainsExactlyWithOffset { + @BeforeTemplate + void before(float[] actual, Supplier message, float[] expected, float delta) { + assertArrayEquals(expected, actual, delta, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(float[] actual, Supplier message, float[] expected, float delta) { + assertThat(actual).withFailMessage(message).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatDoubleArrayContainsExactly { + @BeforeTemplate + void before(double[] actual, double[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, double[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatDoubleArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(double[] actual, String message, double[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, String message, double[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatDoubleArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(double[] actual, Supplier message, double[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, Supplier message, double[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatDoubleArrayContainsExactlyWithOffset { + @BeforeTemplate + void before(double[] actual, double[] expected, double delta) { + assertArrayEquals(expected, actual, delta); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, double[] expected, double delta) { + assertThat(actual).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatDoubleArrayWithFailMessageContainsExactlyWithOffset { + @BeforeTemplate + void before(double[] actual, String message, double[] expected, double delta) { + assertArrayEquals(expected, actual, delta, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, String message, double[] expected, double delta) { + assertThat(actual).withFailMessage(message).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatDoubleArrayWithFailMessageSupplierContainsExactlyWithOffset { + @BeforeTemplate + void before( + double[] actual, Supplier messageSupplier, double[] expected, double delta) { + assertArrayEquals(expected, actual, delta, messageSupplier); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(double[] actual, Supplier messageSupplier, double[] expected, double delta) { + assertThat(actual).withFailMessage(messageSupplier).containsExactly(expected, offset(delta)); + } + } + + static final class AssertThatObjectArrayContainsExactly { + @BeforeTemplate + void before(Object[] actual, Object[] expected) { + assertArrayEquals(expected, actual); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(Object[] actual, Object[] expected) { + assertThat(actual).containsExactly(expected); + } + } + + static final class AssertThatObjectArrayWithFailMessageContainsExactly { + @BeforeTemplate + void before(Object[] actual, String message, Object[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(Object[] actual, String message, Object[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + + static final class AssertThatObjectArrayWithFailMessageSupplierContainsExactly { + @BeforeTemplate + void before(Object[] actual, Supplier message, Object[] expected) { + assertArrayEquals(expected, actual, message); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(Object[] actual, Supplier message, Object[] expected) { + assertThat(actual).withFailMessage(message).containsExactly(expected); + } + } + static final class Fail { @BeforeTemplate T before() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestInput.java index 8abb82ce..e696308e 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestInput.java @@ -1,5 +1,7 @@ package tech.picnic.errorprone.refasterrules; +import static org.assertj.core.api.Assertions.offset; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -24,6 +26,8 @@ final class JUnitToAssertJRulesTest implements RefasterRuleCollectionTestCase { assertInstanceOf(null, null), assertThrows(null, null), assertThrowsExactly(null, null), + offset(0.0), + (Runnable) () -> assertArrayEquals((int[]) null, null), (Runnable) () -> assertFalse(true), (Runnable) () -> assertNotNull(null), (Runnable) () -> assertNotSame(null, null), @@ -32,6 +36,138 @@ final class JUnitToAssertJRulesTest implements RefasterRuleCollectionTestCase { (Runnable) () -> assertTrue(true)); } + void testAssertThatBooleanArrayContainsExactly() { + assertArrayEquals(new boolean[] {true}, new boolean[] {false}); + } + + void testAssertThatBooleanArrayWithFailMessageContainsExactly() { + assertArrayEquals(new boolean[] {true}, new boolean[] {false}, "foo"); + } + + void testAssertThatBooleanArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new boolean[] {true}, new boolean[] {false}, () -> "foo"); + } + + void testAssertThatByteArrayContainsExactly() { + assertArrayEquals(new byte[] {1}, new byte[] {2}); + } + + void testAssertThatByteArrayWithFailMessageContainsExactly() { + assertArrayEquals(new byte[] {1}, new byte[] {2}, "foo"); + } + + void testAssertThatByteArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new byte[] {1}, new byte[] {2}, () -> "foo"); + } + + void testAssertThatCharArrayContainsExactly() { + assertArrayEquals(new char[] {'a'}, new char[] {'b'}); + } + + void testAssertThatCharArrayWithFailMessageContainsExactly() { + assertArrayEquals(new char[] {'a'}, new char[] {'b'}, "foo"); + } + + void testAssertThatCharArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new char[] {'a'}, new char[] {'b'}, () -> "foo"); + } + + void testAssertThatShortArrayContainsExactly() { + assertArrayEquals(new short[] {1}, new short[] {2}); + } + + void testAssertThatShortArrayWithFailMessageContainsExactly() { + assertArrayEquals(new short[] {1}, new short[] {2}, "foo"); + } + + void testAssertThatShortArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new short[] {1}, new short[] {2}, () -> "foo"); + } + + void testAssertThatIntArrayContainsExactly() { + assertArrayEquals(new int[] {1}, new int[] {2}); + } + + void testAssertThatIntArrayWithFailMessageContainsExactly() { + assertArrayEquals(new int[] {1}, new int[] {2}, "foo"); + } + + void testAssertThatIntArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new int[] {1}, new int[] {2}, () -> "foo"); + } + + void testAssertThatLongArrayContainsExactly() { + assertArrayEquals(new long[] {1L}, new long[] {2L}); + } + + void testAssertThatLongArrayWithFailMessageContainsExactly() { + assertArrayEquals(new long[] {1L}, new long[] {2L}, "foo"); + } + + void testAssertThatLongArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new long[] {1L}, new long[] {2L}, () -> "foo"); + } + + void testAssertThatFloatArrayContainsExactly() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}); + } + + void testAssertThatFloatArrayWithFailMessageContainsExactly() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}, "foo"); + } + + void testAssertThatFloatArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}, () -> "foo"); + } + + void testAssertThatFloatArrayContainsExactlyWithOffset() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}, 0.1f); + } + + void testAssertThatFloatArrayWithFailMessageContainsExactlyWithOffset() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}, 0.1f, "foo"); + } + + void testAssertThatFloatArrayWithFailMessageSupplierContainsExactlyWithOffset() { + assertArrayEquals(new float[] {1.0F}, new float[] {2.0F}, 0.1f, () -> "foo"); + } + + void testAssertThatDoubleArrayContainsExactly() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}); + } + + void testAssertThatDoubleArrayWithFailMessageContainsExactly() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}, "foo"); + } + + void testAssertThatDoubleArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}, () -> "foo"); + } + + void testAssertThatDoubleArrayContainsExactlyWithOffset() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}, 0.1); + } + + void testAssertThatDoubleArrayWithFailMessageContainsExactlyWithOffset() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}, 0.1, "foo"); + } + + void testAssertThatDoubleArrayWithFailMessageSupplierContainsExactlyWithOffset() { + assertArrayEquals(new double[] {1.0}, new double[] {2.0}, 0.1, () -> "foo"); + } + + void testAssertThatObjectArrayContainsExactly() { + assertArrayEquals(new Object[] {"foo"}, new Object[] {"bar"}); + } + + void testAssertThatObjectArrayWithFailMessageContainsExactly() { + assertArrayEquals(new Object[] {"foo"}, new Object[] {"bar"}, "foo"); + } + + void testAssertThatObjectArrayWithFailMessageSupplierContainsExactly() { + assertArrayEquals(new Object[] {"foo"}, new Object[] {"bar"}, () -> "foo"); + } + Object testFail() { return Assertions.fail(); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestOutput.java index 66ce892c..64854f2e 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/JUnitToAssertJRulesTestOutput.java @@ -3,6 +3,8 @@ package tech.picnic.errorprone.refasterrules; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.offset; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -27,6 +29,8 @@ final class JUnitToAssertJRulesTest implements RefasterRuleCollectionTestCase { assertInstanceOf(null, null), assertThrows(null, null), assertThrowsExactly(null, null), + offset(0.0), + (Runnable) () -> assertArrayEquals((int[]) null, null), (Runnable) () -> assertFalse(true), (Runnable) () -> assertNotNull(null), (Runnable) () -> assertNotSame(null, null), @@ -35,6 +39,150 @@ final class JUnitToAssertJRulesTest implements RefasterRuleCollectionTestCase { (Runnable) () -> assertTrue(true)); } + void testAssertThatBooleanArrayContainsExactly() { + assertThat(new boolean[] {false}).containsExactly(new boolean[] {true}); + } + + void testAssertThatBooleanArrayWithFailMessageContainsExactly() { + assertThat(new boolean[] {false}).withFailMessage("foo").containsExactly(new boolean[] {true}); + } + + void testAssertThatBooleanArrayWithFailMessageSupplierContainsExactly() { + assertThat(new boolean[] {false}) + .withFailMessage(() -> "foo") + .containsExactly(new boolean[] {true}); + } + + void testAssertThatByteArrayContainsExactly() { + assertThat(new byte[] {2}).containsExactly(new byte[] {1}); + } + + void testAssertThatByteArrayWithFailMessageContainsExactly() { + assertThat(new byte[] {2}).withFailMessage("foo").containsExactly(new byte[] {1}); + } + + void testAssertThatByteArrayWithFailMessageSupplierContainsExactly() { + assertThat(new byte[] {2}).withFailMessage(() -> "foo").containsExactly(new byte[] {1}); + } + + void testAssertThatCharArrayContainsExactly() { + assertThat(new char[] {'b'}).containsExactly(new char[] {'a'}); + } + + void testAssertThatCharArrayWithFailMessageContainsExactly() { + assertThat(new char[] {'b'}).withFailMessage("foo").containsExactly(new char[] {'a'}); + } + + void testAssertThatCharArrayWithFailMessageSupplierContainsExactly() { + assertThat(new char[] {'b'}).withFailMessage(() -> "foo").containsExactly(new char[] {'a'}); + } + + void testAssertThatShortArrayContainsExactly() { + assertThat(new short[] {2}).containsExactly(new short[] {1}); + } + + void testAssertThatShortArrayWithFailMessageContainsExactly() { + assertThat(new short[] {2}).withFailMessage("foo").containsExactly(new short[] {1}); + } + + void testAssertThatShortArrayWithFailMessageSupplierContainsExactly() { + assertThat(new short[] {2}).withFailMessage(() -> "foo").containsExactly(new short[] {1}); + } + + void testAssertThatIntArrayContainsExactly() { + assertThat(new int[] {2}).containsExactly(new int[] {1}); + } + + void testAssertThatIntArrayWithFailMessageContainsExactly() { + assertThat(new int[] {2}).withFailMessage("foo").containsExactly(new int[] {1}); + } + + void testAssertThatIntArrayWithFailMessageSupplierContainsExactly() { + assertThat(new int[] {2}).withFailMessage(() -> "foo").containsExactly(new int[] {1}); + } + + void testAssertThatLongArrayContainsExactly() { + assertThat(new long[] {2L}).containsExactly(new long[] {1L}); + } + + void testAssertThatLongArrayWithFailMessageContainsExactly() { + assertThat(new long[] {2L}).withFailMessage("foo").containsExactly(new long[] {1L}); + } + + void testAssertThatLongArrayWithFailMessageSupplierContainsExactly() { + assertThat(new long[] {2L}).withFailMessage(() -> "foo").containsExactly(new long[] {1L}); + } + + void testAssertThatFloatArrayContainsExactly() { + assertThat(new float[] {2.0F}).containsExactly(new float[] {1.0F}); + } + + void testAssertThatFloatArrayWithFailMessageContainsExactly() { + assertThat(new float[] {2.0F}).withFailMessage("foo").containsExactly(new float[] {1.0F}); + } + + void testAssertThatFloatArrayWithFailMessageSupplierContainsExactly() { + assertThat(new float[] {2.0F}).withFailMessage(() -> "foo").containsExactly(new float[] {1.0F}); + } + + void testAssertThatFloatArrayContainsExactlyWithOffset() { + assertThat(new float[] {2.0F}).containsExactly(new float[] {1.0F}, offset(0.1f)); + } + + void testAssertThatFloatArrayWithFailMessageContainsExactlyWithOffset() { + assertThat(new float[] {2.0F}) + .withFailMessage("foo") + .containsExactly(new float[] {1.0F}, offset(0.1f)); + } + + void testAssertThatFloatArrayWithFailMessageSupplierContainsExactlyWithOffset() { + assertThat(new float[] {2.0F}) + .withFailMessage(() -> "foo") + .containsExactly(new float[] {1.0F}, offset(0.1f)); + } + + void testAssertThatDoubleArrayContainsExactly() { + assertThat(new double[] {2.0}).containsExactly(new double[] {1.0}); + } + + void testAssertThatDoubleArrayWithFailMessageContainsExactly() { + assertThat(new double[] {2.0}).withFailMessage("foo").containsExactly(new double[] {1.0}); + } + + void testAssertThatDoubleArrayWithFailMessageSupplierContainsExactly() { + assertThat(new double[] {2.0}).withFailMessage(() -> "foo").containsExactly(new double[] {1.0}); + } + + void testAssertThatDoubleArrayContainsExactlyWithOffset() { + assertThat(new double[] {2.0}).containsExactly(new double[] {1.0}, offset(0.1)); + } + + void testAssertThatDoubleArrayWithFailMessageContainsExactlyWithOffset() { + assertThat(new double[] {2.0}) + .withFailMessage("foo") + .containsExactly(new double[] {1.0}, offset(0.1)); + } + + void testAssertThatDoubleArrayWithFailMessageSupplierContainsExactlyWithOffset() { + assertThat(new double[] {2.0}) + .withFailMessage(() -> "foo") + .containsExactly(new double[] {1.0}, offset(0.1)); + } + + void testAssertThatObjectArrayContainsExactly() { + assertThat(new Object[] {"bar"}).containsExactly(new Object[] {"foo"}); + } + + void testAssertThatObjectArrayWithFailMessageContainsExactly() { + assertThat(new Object[] {"bar"}).withFailMessage("foo").containsExactly(new Object[] {"foo"}); + } + + void testAssertThatObjectArrayWithFailMessageSupplierContainsExactly() { + assertThat(new Object[] {"bar"}) + .withFailMessage(() -> "foo") + .containsExactly(new Object[] {"foo"}); + } + Object testFail() { return org.assertj.core.api.Assertions.fail(); } From ff0e30c8852f137c67268bfb74a8691b9b2c2775 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 9 Jan 2025 09:20:39 +0100 Subject: [PATCH 26/43] Make Mockito setup IntelliJ IDEA-compatible (#1500) See https://github.com/mockito/mockito/issues/3037#issuecomment-2569680756 --- README.md | 5 +++-- pom.xml | 12 +++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1826bbe3..76a48614 100644 --- a/README.md +++ b/README.md @@ -235,8 +235,9 @@ Other highly relevant commands: against _all_ code in the current working directory. For more information check the [PIT Maven plugin][pitest-maven]. -When running the project's tests in IntelliJ IDEA, you might see the following -error: +Opening the project in IntelliJ IDEA may require running `mvn clean install` +first. Additionally, when running the project's tests using the IDE, you might +see the following error: ``` java: exporting a package from system module jdk.compiler is not allowed with --release diff --git a/pom.xml b/pom.xml index 8c8d7b3f..0b47ffcb 100644 --- a/pom.xml +++ b/pom.xml @@ -113,7 +113,7 @@ -Xmx${argLine.xmx} - -javaagent:${org.mockito:mockito-core:jar} + -javaagent:${project.build.directory}/test-agents/mockito-core.jar - - com.google.errorprone 2024-11-03T15:58:19Z @@ -210,7 +205,7 @@ 1.1.1 1.11.0 ${version.error-prone-orig} - v${version.error-prone-orig}-picnic-1 + ${version.error-prone-orig}-picnic-2 2.36.0 0.1.28 1.0 @@ -226,31 +221,6 @@ - - ${groupId.error-prone} - error_prone_annotation - ${version.error-prone} - - - ${groupId.error-prone} - error_prone_annotations - ${version.error-prone} - - - ${groupId.error-prone} - error_prone_check_api - ${version.error-prone} - - - ${groupId.error-prone} - error_prone_core - ${version.error-prone} - - - ${groupId.error-prone} - error_prone_test_helpers - ${version.error-prone} - ${project.groupId} documentation-support @@ -333,6 +303,31 @@ auto-value-annotations ${version.auto-value} + + com.google.errorprone + error_prone_annotation + ${version.error-prone} + + + com.google.errorprone + error_prone_annotations + ${version.error-prone} + + + com.google.errorprone + error_prone_check_api + ${version.error-prone} + + + com.google.errorprone + error_prone_core + ${version.error-prone} + + + com.google.errorprone + error_prone_test_helpers + ${version.error-prone} + com.google.googlejavaformat google-java-format @@ -1607,63 +1602,8 @@ error-prone-fork - com.github.PicnicSupermarket.error-prone ${version.error-prone-fork} - - - - - com.google.errorprone - error_prone_annotations - ${version.error-prone-orig} - - - - - - - - org.apache.maven.plugins - maven-enforcer-plugin - - - - - - - com.google.errorprone - error_prone_annotations - - * - - - - - - - - - org.apache.maven.plugins - maven-surefire-plugin - - - - true - - - - - - - ${groupId.error-prone} + com.google.errorprone error_prone_core ${version.error-prone} diff --git a/refaster-compiler/pom.xml b/refaster-compiler/pom.xml index 2946dbb4..fdf4afdb 100644 --- a/refaster-compiler/pom.xml +++ b/refaster-compiler/pom.xml @@ -15,19 +15,6 @@ https://error-prone.picnic.tech - - ${groupId.error-prone} - error_prone_annotations - provided - - - ${groupId.error-prone} - error_prone_check_api - - - ${groupId.error-prone} - error_prone_core - ${project.groupId} refaster-support @@ -37,6 +24,19 @@ auto-service-annotations provided + + com.google.errorprone + error_prone_annotations + provided + + + com.google.errorprone + error_prone_check_api + + + com.google.errorprone + error_prone_core + com.google.guava guava diff --git a/refaster-runner/pom.xml b/refaster-runner/pom.xml index 459a314e..f75ed47b 100644 --- a/refaster-runner/pom.xml +++ b/refaster-runner/pom.xml @@ -15,26 +15,6 @@ https://error-prone.picnic.tech - - ${groupId.error-prone} - error_prone_annotation - provided - - - ${groupId.error-prone} - error_prone_annotations - provided - - - ${groupId.error-prone} - error_prone_check_api - provided - - - ${groupId.error-prone} - error_prone_test_helpers - test - ${project.groupId} refaster-compiler @@ -53,6 +33,26 @@ auto-service-annotations provided + + com.google.errorprone + error_prone_annotation + provided + + + com.google.errorprone + error_prone_annotations + provided + + + com.google.errorprone + error_prone_check_api + provided + + + com.google.errorprone + error_prone_test_helpers + test + com.google.guava guava diff --git a/refaster-support/pom.xml b/refaster-support/pom.xml index 3176949f..6370d8f1 100644 --- a/refaster-support/pom.xml +++ b/refaster-support/pom.xml @@ -15,43 +15,36 @@ https://error-prone.picnic.tech - - - - ${groupId.error-prone} - error_prone_annotations - provided - - - - ${groupId.error-prone} - error_prone_annotation - provided - - - ${groupId.error-prone} - error_prone_check_api - provided - - - ${groupId.error-prone} - error_prone_core - provided - - - ${groupId.error-prone} - error_prone_test_helpers - test - com.google.auto.value auto-value-annotations provided + + com.google.errorprone + error_prone_annotation + provided + + + com.google.errorprone + error_prone_annotations + provided + + + com.google.errorprone + error_prone_check_api + provided + + + com.google.errorprone + error_prone_core + provided + + + com.google.errorprone + error_prone_test_helpers + test + com.google.guava guava diff --git a/refaster-test-support/pom.xml b/refaster-test-support/pom.xml index 89b7795c..46dca581 100644 --- a/refaster-test-support/pom.xml +++ b/refaster-test-support/pom.xml @@ -15,28 +15,6 @@ https://error-prone.picnic.tech - - ${groupId.error-prone} - error_prone_annotation - - - ${groupId.error-prone} - error_prone_annotations - provided - - - ${groupId.error-prone} - error_prone_check_api - - - ${groupId.error-prone} - error_prone_core - test - - - ${groupId.error-prone} - error_prone_test_helpers - ${project.groupId} refaster-runner @@ -46,6 +24,28 @@ auto-service-annotations provided + + com.google.errorprone + error_prone_annotation + + + com.google.errorprone + error_prone_annotations + provided + + + com.google.errorprone + error_prone_check_api + + + com.google.errorprone + error_prone_core + test + + + com.google.errorprone + error_prone_test_helpers + com.google.guava guava diff --git a/settings.xml b/settings.xml index dc0a0822..799f0750 100644 --- a/settings.xml +++ b/settings.xml @@ -5,13 +5,13 @@ + Prone. This fork is hosted using GitHub Packages. See + https://github.com/PicnicSupermarket/error-prone/packages. --> error-prone-fork - jitpack.io - https://jitpack.io + error-prone-fork + https://maven.pkg.github.com/PicnicSupermarket/error-prone @@ -29,4 +29,11 @@ + + + error-prone-fork + ${env.GITHUB_ACTOR} + ${env.GITHUB_TOKEN} + + diff --git a/website/generate-version-compatibility-overview.sh b/website/generate-version-compatibility-overview.sh index c0716db6..19cb110b 100755 --- a/website/generate-version-compatibility-overview.sh +++ b/website/generate-version-compatibility-overview.sh @@ -86,7 +86,7 @@ for eps_version in ${eps_versions}; do -Ppatch \ -Pself-check \ -Dverification.skip \ - -Dversion.error-prone-orig="${ep_version}" \ + -Dversion.error-prone="${ep_version}" \ && echo "SUCCESS: { \"eps_version\": \"${eps_version}\", \"ep_version\": \"${ep_version}\" }" || true # Undo any changes applied by the checks. git checkout -- '*.java' From d2ce18e33e488442eaf097203656e5b418e88cdd Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sat, 11 Jan 2025 12:36:15 +0100 Subject: [PATCH 31/43] Upgrade actions/upload-artifact v4.5.0 -> v4.6.0 (#1506) See: - https://github.com/actions/upload-artifact/releases/tag/v4.6.0 --- .github/workflows/pitest-analyze-pr.yml | 2 +- .github/workflows/run-integration-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pitest-analyze-pr.yml b/.github/workflows/pitest-analyze-pr.yml index 61ddab0a..76d9041a 100644 --- a/.github/workflows/pitest-analyze-pr.yml +++ b/.github/workflows/pitest-analyze-pr.yml @@ -38,7 +38,7 @@ jobs: - name: Aggregate Pitest reports run: mvn pitest-git:aggregate -DkilledEmoji=":tada:" -DmutantEmoji=":zombie:" -DtrailingText="Mutation testing report by [Pitest](https://pitest.org/). Review any surviving mutants by inspecting the line comments under [_Files changed_](${{ github.event.number }}/files)." - name: Upload Pitest reports as artifact - uses: actions/upload-artifact@6f51ac03b9356f520e9adb1b1b7802705f340c2b # v4.5.0 + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: name: pitest-reports path: ./target/pit-reports-ci diff --git a/.github/workflows/run-integration-tests.yml b/.github/workflows/run-integration-tests.yml index 3f3e28f8..e0ef2660 100644 --- a/.github/workflows/run-integration-tests.yml +++ b/.github/workflows/run-integration-tests.yml @@ -55,7 +55,7 @@ jobs: run: xvfb-run "./integration-tests/${{ matrix.integration-test }}.sh" "${{ runner.temp }}/artifacts" - name: Upload artifacts on failure if: ${{ failure() }} - uses: actions/upload-artifact@6f51ac03b9356f520e9adb1b1b7802705f340c2b # v4.5.0 + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: name: "integration-test-${{ matrix.integration-test }}" path: "${{ runner.temp }}/artifacts" From a42353b41a13e6e045159dc68b133c162c347349 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 11 Jan 2025 14:42:22 +0100 Subject: [PATCH 32/43] Introduce `EagerStringFormatting` check (#1139) This new check flags code that can be simplified and/or optimized by deferring certain string formatting operations. --- .../bugpatterns/EagerStringFormatting.java | 325 ++++++++++++++++++ .../bugpatterns/Slf4jLogStatement.java | 2 - .../EagerStringFormattingTest.java | 286 +++++++++++++++ .../picnic/errorprone/utils/SourceCode.java | 4 +- .../checkstyle-expected-changes.patch | 12 +- .../checkstyle-expected-warnings.txt | 4 + ...metheus-java-client-expected-changes.patch | 22 +- 7 files changed, 636 insertions(+), 19 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java create mode 100644 error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/EagerStringFormattingTest.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java new file mode 100644 index 00000000..036bdb70 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java @@ -0,0 +1,325 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.isSubtypeOf; +import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; +import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.joining; +import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; +import com.google.common.base.Verify; +import com.google.common.base.VerifyException; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.annotations.Var; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; +import java.util.Collections; +import java.util.Formattable; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Stream; +import tech.picnic.errorprone.utils.SourceCode; + +/** + * A {@link BugChecker} that flags {@link String#format} and {@link String#formatted} invocations + * that can be omitted by delegating to another format method. + */ +// XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see +// https://www.slf4j.org/faq.html#paramException. That should be documented. +// XXX: Some of the `Matcher`s defined here are also declared by the `Slf4jLogStatement` and +// `RedundantStringConversion` checks. Look into deduplicating them. +// XXX: Should we also simplify e.g. `LOG.error(String.join("sep", arg1, arg2), throwable)`? Perhaps +// that's too obscure. +// XXX: This check currently only flags string format expressions that are a direct argument to +// another format-capable method invocation. Indirect cases, such as where the result is assigned to +// a variable, are currently not covered. +@AutoService(BugChecker.class) +@BugPattern( + summary = "String formatting can be deferred", + link = BUG_PATTERNS_BASE_URL + "EagerStringFormatting", + linkType = CUSTOM, + severity = WARNING, + tags = {PERFORMANCE, SIMPLIFICATION}) +public final class EagerStringFormatting extends BugChecker implements MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher FORMATTABLE = isSubtypeOf(Formattable.class); + private static final Matcher LOCALE = isSubtypeOf(Locale.class); + private static final Matcher SLF4J_MARKER = isSubtypeOf("org.slf4j.Marker"); + private static final Matcher THROWABLE = isSubtypeOf(Throwable.class); + private static final Matcher REQUIRE_NON_NULL_INVOCATION = + staticMethod().onClass(Objects.class.getCanonicalName()).named("requireNonNull"); + private static final Matcher GUAVA_GUARD_INVOCATION = + anyOf( + staticMethod() + .onClass(Preconditions.class.getCanonicalName()) + .namedAnyOf("checkArgument", "checkNotNull", "checkState"), + staticMethod() + .onClass(Verify.class.getCanonicalName()) + .namedAnyOf("verify", "verifyNotNull")); + private static final Matcher SLF4J_LOGGER_INVOCATION = + instanceMethod() + .onDescendantOf("org.slf4j.Logger") + .namedAnyOf("trace", "debug", "info", "warn", "error"); + private static final Matcher STATIC_FORMAT_STRING = + staticMethod().onClass(String.class.getCanonicalName()).named("format"); + private static final Matcher INSTANCE_FORMAT_STRING = + instanceMethod().onDescendantOf(String.class.getCanonicalName()).named("formatted"); + private static final String MESSAGE_NEVER_NULL_ARGUMENT = + "String formatting never yields `null` expression"; + + /** Instantiates a new {@link EagerStringFormatting} instance. */ + public EagerStringFormatting() {} + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + Tree parent = state.getPath().getParentPath().getLeaf(); + if (!(parent instanceof MethodInvocationTree methodInvocation)) { + /* + * Fast path: this isn't a method invocation whose result is an argument to another method + * invocation. + */ + // XXX: This logic assumes that the string format operation isn't redundantly wrapped in + // parentheses. Similar assumptions likely exist throughout the code base. Investigate how to + // structurally cover such cases. + return Description.NO_MATCH; + } + + return StringFormatExpression.tryCreate(tree, state) + .map(expr -> analyzeFormatStringContext(expr, methodInvocation, state)) + .orElse(Description.NO_MATCH); + } + + private Description analyzeFormatStringContext( + StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) { + if (REQUIRE_NON_NULL_INVOCATION.matches(context, state)) { + return analyzeRequireNonNullStringFormatContext(stringFormat, context); + } + + if (GUAVA_GUARD_INVOCATION.matches(context, state)) { + return analyzeGuavaGuardStringFormatContext(stringFormat, context, state); + } + + if (SLF4J_LOGGER_INVOCATION.matches(context, state)) { + return analyzeSlf4jLoggerStringFormatContext(stringFormat, context, state); + } + + /* + * The string formatting operation does not appear to happen in a context that admits of + * simplification or optimization. + */ + return Description.NO_MATCH; + } + + private Description analyzeRequireNonNullStringFormatContext( + StringFormatExpression stringFormat, MethodInvocationTree context) { + List arguments = context.getArguments(); + if (arguments.size() != 2 || arguments.get(0).equals(stringFormat.expression())) { + /* Vacuous validation that string formatting doesn't yield `null`. */ + return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build(); + } + + if (stringFormat.arguments().stream() + .anyMatch(EagerStringFormatting::isNonFinalLocalVariable)) { + /* + * The format operation depends on a variable that isn't final or effectively final; moving + * it into a lambda expression would cause a compilation error. + */ + return buildDescription(context) + .setMessage(message() + " (but this requires introducing an effectively final variable)") + .build(); + } + + /* Suggest that the string formatting is deferred. */ + return describeMatch(context, SuggestedFix.prefixWith(stringFormat.expression(), "() -> ")); + } + + private Description analyzeGuavaGuardStringFormatContext( + StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) { + List arguments = context.getArguments(); + if (arguments.get(0).equals(stringFormat.expression())) { + /* + * Vacuous `checkNotNull` or `verifyNotNull` validation that string formatting doesn't yield + * `null`. + */ + return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build(); + } + + if (stringFormat.simplifiableFormatString().isEmpty() || arguments.size() > 2) { + /* + * The format string cannot be simplified, or the format string produces a format string + * itself, or its result is the input to another format operation. These are complex cases + * that we'll only flag. + */ + return createSimplificationSuggestion(context, "Guava"); + } + + return describeMatch(context, stringFormat.suggestFlattening("%s", state)); + } + + private Description analyzeSlf4jLoggerStringFormatContext( + StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) { + if (stringFormat.simplifiableFormatString().isEmpty()) { + /* We can't simplify this case; only flag it. */ + return createSimplificationSuggestion(context, "SLF4J"); + } + + List arguments = context.getArguments(); + int leftOffset = SLF4J_MARKER.matches(arguments.get(0), state) ? 1 : 0; + int rightOffset = THROWABLE.matches(arguments.get(arguments.size() - 1), state) ? 1 : 0; + if (arguments.size() != leftOffset + 1 + rightOffset) { + /* + * The format string produces a format string itself, or its result is the input to another + * format operation. This is a complex case that we'll only flag. + */ + return createSimplificationSuggestion(context, "SLF4J"); + } + + return describeMatch(context, stringFormat.suggestFlattening("{}", state)); + } + + private static boolean isNonFinalLocalVariable(Tree tree) { + Symbol symbol = ASTHelpers.getSymbol(tree); + return symbol instanceof VarSymbol + && symbol.owner instanceof MethodSymbol + && !ASTHelpers.isConsideredFinal(symbol); + } + + private Description createSimplificationSuggestion(MethodInvocationTree context, String library) { + return buildDescription(context) + .setMessage( + "%s (assuming that %s's simplified formatting support suffices)" + .formatted(message(), library)) + .build(); + } + + /** Description of a string format expression. */ + @AutoValue + abstract static class StringFormatExpression { + /** The full string format expression. */ + abstract MethodInvocationTree expression(); + + /** The format string expression. */ + abstract Tree formatString(); + + /** The string format arguments to be plugged into its format string. */ + abstract ImmutableList arguments(); + + /** + * The constant format string, if it contains only {@code %s} placeholders, and the number of + * said placeholders matches the number of format arguments. + */ + abstract Optional simplifiableFormatString(); + + private SuggestedFix suggestFlattening(String newPlaceholder, VisitorState state) { + return SuggestedFix.replace( + expression(), + Stream.concat( + Stream.of(deriveFormatStringExpression(newPlaceholder, state)), + arguments().stream().map(arg -> SourceCode.treeToString(arg, state))) + .collect(joining(", "))); + } + + private String deriveFormatStringExpression(String newPlaceholder, VisitorState state) { + String formatString = + String.format( + simplifiableFormatString() + .orElseThrow(() -> new VerifyException("Format string cannot be simplified")), + Collections.nCopies(arguments().size(), newPlaceholder).toArray()); + + /* + * If the suggested replacement format string is the same as the original, then use the + * expression's existing source code representation. This way string constant references are + * not unnecessarily replaced. + */ + return formatString.equals(ASTHelpers.constValue(formatString(), String.class)) + ? SourceCode.treeToString(formatString(), state) + : SourceCode.toStringConstantExpression(formatString, state); + } + + private static Optional tryCreate( + MethodInvocationTree tree, VisitorState state) { + if (INSTANCE_FORMAT_STRING.matches(tree, state)) { + return Optional.of( + create( + tree, + requireNonNull(ASTHelpers.getReceiver(tree), "Receiver unexpectedly absent"), + ImmutableList.copyOf(tree.getArguments()), + state)); + } + + if (STATIC_FORMAT_STRING.matches(tree, state)) { + List arguments = tree.getArguments(); + int argOffset = LOCALE.matches(arguments.get(0), state) ? 1 : 0; + return Optional.of( + create( + tree, + arguments.get(argOffset), + ImmutableList.copyOf(arguments.subList(argOffset + 1, arguments.size())), + state)); + } + + return Optional.empty(); + } + + private static StringFormatExpression create( + MethodInvocationTree expression, + Tree formatString, + ImmutableList arguments, + VisitorState state) { + return new AutoValue_EagerStringFormatting_StringFormatExpression( + expression, + formatString, + arguments, + Optional.ofNullable(ASTHelpers.constValue(formatString, String.class)) + .filter(template -> isSimplifiable(template, arguments, state))); + } + + private static boolean isSimplifiable( + String formatString, ImmutableList arguments, VisitorState state) { + if (arguments.stream().anyMatch(arg -> FORMATTABLE.matches(arg, state))) { + /* `Formattable` arguments can have arbitrary format semantics. */ + return false; + } + + @Var int placeholderCount = 0; + for (int p = formatString.indexOf('%'); p != -1; p = formatString.indexOf('%', p + 2)) { + if (p == formatString.length() - 1) { + /* Malformed format string with trailing `%`. */ + return false; + } + + char modifier = formatString.charAt(p + 1); + if (modifier == 's') { + placeholderCount++; + } else if (modifier != '%') { + /* Only `%s` and `%%` (a literal `%`) are supported. */ + return false; + } + } + + return placeholderCount == arguments.size(); + } + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java index 7836961f..382d5592 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java @@ -28,8 +28,6 @@ import tech.picnic.errorprone.utils.SourceCode; /** A {@link BugChecker} that flags SLF4J usages that are likely to be in error. */ // XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see // https://www.slf4j.org/faq.html#paramException. That should be documented. -// XXX: Also simplify `LOG.error(String.format("Something %s", arg), throwable)`. -// XXX: Also simplify `LOG.error(String.join("sep", arg1, arg2), throwable)`? Perhaps too obscure. // XXX: Write a similar checker for Spring RestTemplates, String.format and friends, Guava // preconditions, ... @AutoService(BugChecker.class) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/EagerStringFormattingTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/EagerStringFormattingTest.java new file mode 100644 index 00000000..97af5abd --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/EagerStringFormattingTest.java @@ -0,0 +1,286 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class EagerStringFormattingTest { + @Test + void identification() { + CompilationTestHelper.newInstance(EagerStringFormatting.class, getClass()) + .expectErrorMessage("DEFER", m -> m.contains("String formatting can be deferred\n")) + .expectErrorMessage( + "DEFER_EXTRA_VARIABLE", + m -> + m.contains( + "String formatting can be deferred (but this requires introducing an effectively final variable)")) + .expectErrorMessage( + "DEFER_SIMPLIFIED_GUAVA", + m -> + m.contains( + "String formatting can be deferred (assuming that Guava's simplified formatting support suffices)")) + .expectErrorMessage( + "DEFER_SIMPLIFIED_SLF4J", + m -> + m.contains( + "String formatting can be deferred (assuming that SLF4J's simplified formatting support suffices)")) + .expectErrorMessage( + "VACUOUS", m -> m.contains("String formatting never yields `null` expression")) + .addSourceLines( + "A.java", + "import static com.google.common.base.Preconditions.checkArgument;", + "import static com.google.common.base.Preconditions.checkNotNull;", + "import static com.google.common.base.Preconditions.checkState;", + "import static com.google.common.base.Verify.verify;", + "import static com.google.common.base.Verify.verifyNotNull;", + "import static java.util.Objects.requireNonNull;", + "", + "import java.util.Formattable;", + "import java.util.Locale;", + "import org.slf4j.Logger;", + "import org.slf4j.LoggerFactory;", + "import org.slf4j.Marker;", + "", + "class A {", + " private static final Logger LOG = LoggerFactory.getLogger(A.class);", + "", + " private int nonFinalField = 0;", + "", + " void m() {", + " Formattable formattable = (formatter, flags, width, precision) -> {};", + " int effectivelyFinalLocal = 0;", + " /* A local variable that is also not effectively final. */", + " int nonFinalLocal = 0;", + " nonFinalLocal = 1;", + "", + " String.format(\"%s\", \"foo\");", + " String.format(Locale.US, \"%s\", \"foo\");", + " \"%s\".formatted(\"foo\");", + " String.format(\"%s\", \"foo\", \"bar\");", + " String.format(\"%s %s\", \"foo\", \"bar\");", + " String.format(\"%s %s %%\", \"foo\", \"bar\");", + "", + " System.out.println(String.format(\"%s\", nonFinalLocal));", + "", + " requireNonNull(\"never-null\");", + " requireNonNull(\"never-null\", () -> String.format(\"Format string: %s\", nonFinalField));", + " // BUG: Diagnostic matches: VACUOUS", + " requireNonNull(String.format(\"Never-null format string: %s\", nonFinalField));", + " // BUG: Diagnostic matches: VACUOUS", + " requireNonNull(\"Never-null format string: %s\".formatted(nonFinalField), \"message\");", + " // BUG: Diagnostic matches: VACUOUS", + " requireNonNull(", + " String.format(\"Never-null format string\"), String.format(\"Malformed format string: %\"));", + " // BUG: Diagnostic matches: DEFER_EXTRA_VARIABLE", + " requireNonNull(\"never-null\", String.format(\"Format string: %s\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " requireNonNull(\"never-null\", String.format(\"Format string: %s\", effectivelyFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " requireNonNull(", + " \"never-null\",", + " String.format(", + " \"Custom format string: %s, %d, %s\", getClass(), nonFinalField, \"string-constant\"));", + "", + " checkArgument(true);", + " checkNotNull(\"never-null\");", + " checkState(false);", + " verify(true);", + " verifyNotNull(\"never-null\");", + " checkArgument(false, \"Without format string\");", + " checkNotNull(\"never-null\", \"Without format string\");", + " checkState(true, \"Without format string\");", + " verify(false, \"Without format string\");", + " verifyNotNull(\"never-null\", \"Without format string\");", + " checkArgument(true, \"With format string: %s\", nonFinalLocal);", + " checkNotNull(\"never-null\", \"With format string: %s\", nonFinalLocal);", + " checkState(false, \"With format string: %s\", nonFinalLocal);", + " verify(true, \"With format string: %s\", nonFinalLocal);", + " verifyNotNull(\"never-null\", \"With format string: %s\", nonFinalLocal);", + " // BUG: Diagnostic matches: VACUOUS", + " checkNotNull(String.format(\"Never-null format string: %s\", nonFinalLocal));", + " // BUG: Diagnostic matches: VACUOUS", + " verifyNotNull(\"Never-null format string: %s\".formatted(nonFinalLocal), \"message\");", + " // BUG: Diagnostic matches: VACUOUS", + " checkNotNull(", + " String.format(\"Never-null format string\"), String.format(\"Malformed format string: %\"));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkArgument(true, String.format(toString()));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkNotNull(\"never-null\", toString().formatted());", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkState(true, String.format(\"Custom format string: %d\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " verify(true, \"Mismatched format string:\".formatted(nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " verifyNotNull(\"never-null\", \"Mismatched format string: %d\".formatted());", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkArgument(true, String.format(\"Malformed format string: %\"));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkNotNull(\"never-null\", \"Format string with `Formattable`: %s\".formatted(formattable));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " checkState(true, String.format(\"Generated format string: %%s\"), nonFinalLocal);", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA", + " verify(", + " true,", + " \"Format string with format string argument: %s\",", + " String.format(\"Format string argument: %s\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " verifyNotNull(", + " \"never-null\", String.format(\"Format string: %s, %s\", nonFinalLocal, nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " checkArgument(true, \"Format string: %s%%\".formatted(nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " checkNotNull(", + " \"never-null\", String.format(Locale.US, \"Format string with locale: %s\", nonFinalLocal));", + "", + " LOG.trace(\"Without format string\");", + " LOG.debug(\"With format string: {}\", nonFinalLocal);", + " LOG.info((Marker) null, \"With marker\");", + " LOG.warn((Marker) null, \"With marker and format string: {}\", nonFinalLocal);", + " LOG.error(\"With throwable\", new RuntimeException());", + " LOG.trace(\"With throwable and format string: {}\", nonFinalLocal, new RuntimeException());", + " LOG.debug((Marker) null, \"With marker and throwable\", new RuntimeException());", + " LOG.info(", + " (Marker) null,", + " \"With marker, throwable and format string: {}\",", + " nonFinalLocal,", + " new RuntimeException());", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.warn(String.format(toString()));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.error(toString().formatted());", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.trace(String.format(\"Custom format string: %d\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.debug(\"Mismatched format string:\".formatted(nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.info(\"Mismatched format string %d:\".formatted());", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.warn(String.format(\"Malformed format string: %\"));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.error(\"Format string with `Formattable`: %s\".formatted(formattable));", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.trace(String.format(\"Generated format string: {}\"), nonFinalLocal);", + " // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J", + " LOG.debug(", + " \"Format string with format string argument: {}\",", + " String.format(\"Format string argument: %s\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " LOG.info(String.format(\"Vacuous format string %%\"));", + " // BUG: Diagnostic matches: DEFER", + " LOG.warn(String.format(\"With format string: %s, %s\", nonFinalLocal, nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " LOG.error(String.format(Locale.ROOT, \"With vacuous localized format string %%\"));", + " // BUG: Diagnostic matches: DEFER", + " LOG.trace((Marker) null, String.format(\"With marker and format string: %s\", nonFinalLocal));", + " // BUG: Diagnostic matches: DEFER", + " LOG.debug(", + " String.format(\"With throwable and format string: %s\", nonFinalLocal),", + " new RuntimeException());", + " // BUG: Diagnostic matches: DEFER", + " LOG.info(", + " (Marker) null,", + " String.format(\"With marker, throwable and format string: %s\", nonFinalLocal),", + " new RuntimeException());", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + BugCheckerRefactoringTestHelper.newInstance(EagerStringFormatting.class, getClass()) + .addInputLines( + "A.java", + "import static com.google.common.base.Preconditions.checkArgument;", + "import static com.google.common.base.Preconditions.checkNotNull;", + "import static com.google.common.base.Preconditions.checkState;", + "import static com.google.common.base.Verify.verify;", + "import static com.google.common.base.Verify.verifyNotNull;", + "import static java.util.Objects.requireNonNull;", + "", + "import java.util.Locale;", + "import org.slf4j.Logger;", + "import org.slf4j.LoggerFactory;", + "import org.slf4j.Marker;", + "", + "class A {", + " private static final Logger LOG = LoggerFactory.getLogger(A.class);", + " private static final String GUAVA_COMPATIBLE_PATTERN = \"with-only-%s-placeholder\";", + " private static final String GUAVA_INCOMPATIBLE_PATTERN = \"with-%%-marker\";", + "", + " void m() {", + " requireNonNull(\"never-null\", String.format(\"Format string: %s\", 0));", + "", + " checkArgument(true, String.format(\"Vacuous format string %%\"));", + " checkNotNull(\"never-null\", \"Format string: %s %s%%\".formatted(1, 2));", + " checkState(false, String.format(Locale.US, \"Format string with locale: %s\", 3));", + " verify(true, GUAVA_COMPATIBLE_PATTERN.formatted(4));", + " verifyNotNull(\"never-null\", String.format(Locale.ENGLISH, GUAVA_COMPATIBLE_PATTERN, 5));", + " checkArgument(false, GUAVA_INCOMPATIBLE_PATTERN.formatted());", + " checkNotNull(\"never-null\", String.format(GUAVA_INCOMPATIBLE_PATTERN));", + "", + " LOG.trace(\"Vacuous format string %%\".formatted());", + " LOG.debug(String.format(\"With format string: %s, %s%%\", 6, 7));", + " LOG.info(String.format(Locale.ROOT, \"With vacuous localized format string %%\"));", + " LOG.warn((Marker) null, \"With marker and format string: %s\".formatted(8));", + " LOG.error(", + " String.format(Locale.US, \"With throwable and format string: %s, %s\", 9, 10),", + " new RuntimeException());", + " LOG.trace(", + " (Marker) null,", + " \"With marker, throwable and format string: %s\".formatted(11),", + " new RuntimeException());", + " LOG.debug(GUAVA_COMPATIBLE_PATTERN.formatted(12));", + " LOG.info(String.format(Locale.ENGLISH, GUAVA_COMPATIBLE_PATTERN, 13));", + " LOG.warn(GUAVA_INCOMPATIBLE_PATTERN.formatted());", + " LOG.error(String.format(GUAVA_INCOMPATIBLE_PATTERN));", + " }", + "}") + .addOutputLines( + "A.java", + "import static com.google.common.base.Preconditions.checkArgument;", + "import static com.google.common.base.Preconditions.checkNotNull;", + "import static com.google.common.base.Preconditions.checkState;", + "import static com.google.common.base.Verify.verify;", + "import static com.google.common.base.Verify.verifyNotNull;", + "import static java.util.Objects.requireNonNull;", + "", + "import java.util.Locale;", + "import org.slf4j.Logger;", + "import org.slf4j.LoggerFactory;", + "import org.slf4j.Marker;", + "", + "class A {", + " private static final Logger LOG = LoggerFactory.getLogger(A.class);", + " private static final String GUAVA_COMPATIBLE_PATTERN = \"with-only-%s-placeholder\";", + " private static final String GUAVA_INCOMPATIBLE_PATTERN = \"with-%%-marker\";", + "", + " void m() {", + " requireNonNull(\"never-null\", () -> String.format(\"Format string: %s\", 0));", + "", + " checkArgument(true, \"Vacuous format string %\");", + " checkNotNull(\"never-null\", \"Format string: %s %s%\", 1, 2);", + " checkState(false, \"Format string with locale: %s\", 3);", + " verify(true, GUAVA_COMPATIBLE_PATTERN, 4);", + " verifyNotNull(\"never-null\", GUAVA_COMPATIBLE_PATTERN, 5);", + " checkArgument(false, \"with-%-marker\");", + " checkNotNull(\"never-null\", \"with-%-marker\");", + "", + " LOG.trace(\"Vacuous format string %\");", + " LOG.debug(\"With format string: {}, {}%\", 6, 7);", + " LOG.info(\"With vacuous localized format string %\");", + " LOG.warn((Marker) null, \"With marker and format string: {}\", 8);", + " LOG.error(\"With throwable and format string: {}, {}\", 9, 10, new RuntimeException());", + " LOG.trace(", + " (Marker) null, \"With marker, throwable and format string: {}\", 11, new RuntimeException());", + " LOG.debug(\"with-only-{}-placeholder\", 12);", + " LOG.info(\"with-only-{}-placeholder\", 13);", + " LOG.warn(\"with-%-marker\");", + " LOG.error(\"with-%-marker\");", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java index 741a50a6..5266385a 100644 --- a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java +++ b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java @@ -63,9 +63,11 @@ public final class SourceCode { * found. * @return A non-{@code null} string. * @apiNote This method differs from {@link com.sun.tools.javac.util.Constants#format(Object)} in - * that it does not superfluously escape single quote characters. It is different from {@link + * that it does not superfluously escape single quote characters (the latter only does the + * "clean thing" starting from JDK 23). It is different from {@link * VisitorState#getConstantExpression(Object)} in that it is more performant and accepts any * {@link CharSequence} instance. + * @see JDK-8325078 */ // XXX: Drop this method if https://github.com/google/error-prone/pull/4586 is merged and released // with the proposed `CharSequence` compatibility change. diff --git a/integration-tests/checkstyle-expected-changes.patch b/integration-tests/checkstyle-expected-changes.patch index 252a145d..0c191d93 100644 --- a/integration-tests/checkstyle-expected-changes.patch +++ b/integration-tests/checkstyle-expected-changes.patch @@ -15402,7 +15402,7 @@ - if (name == null) { - throw new IllegalArgumentException(String.format(Locale.ROOT, TOKEN_ID_EXCEPTION_FORMAT, id)); - } -+ checkArgument(name != null, String.format(Locale.ROOT, TOKEN_ID_EXCEPTION_FORMAT, id)); ++ checkArgument(name != null, TOKEN_ID_EXCEPTION_FORMAT, id); return name; } @@ -15414,11 +15414,11 @@ - throw new IllegalArgumentException( - String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name)); - } -+ checkArgument(id != null, String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name)); ++ checkArgument(id != null, TOKEN_NAME_EXCEPTION_FORMAT, name); return id; } -@@ -165,10 +163,9 @@ public final class TokenUtil { +@@ -165,10 +163,7 @@ public final class TokenUtil { * @throws IllegalArgumentException when name is unknown */ public static String getShortDescription(String name) { @@ -15426,13 +15426,11 @@ - throw new IllegalArgumentException( - String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name)); - } -+ checkArgument( -+ TOKEN_NAME_TO_VALUE.containsKey(name), -+ String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name)); ++ checkArgument(TOKEN_NAME_TO_VALUE.containsKey(name), TOKEN_NAME_EXCEPTION_FORMAT, name); final String tokenTypes = "com.puppycrawl.tools.checkstyle.api.tokentypes"; final ResourceBundle bundle = ResourceBundle.getBundle(tokenTypes, Locale.ROOT); -@@ -344,7 +341,7 @@ public final class TokenUtil { +@@ -344,7 +339,7 @@ public final class TokenUtil { public static BitSet asBitSet(String... tokens) { return Arrays.stream(tokens) .map(String::trim) diff --git a/integration-tests/checkstyle-expected-warnings.txt b/integration-tests/checkstyle-expected-warnings.txt index 1b8b8bd8..e78365a8 100644 --- a/integration-tests/checkstyle-expected-warnings.txt +++ b/integration-tests/checkstyle-expected-warnings.txt @@ -1,3 +1,5 @@ +src/it/java/com/google/checkstyle/test/base/AbstractIndentationTestSupport.java:[100,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices) +src/it/java/com/google/checkstyle/test/base/AbstractIndentationTestSupport.java:[85,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices) src/it/java/com/google/checkstyle/test/chapter7javadoc/rule734nonrequiredjavadoc/NonRequiredJavadocTest.java:[33,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) src/it/java/com/sun/checkstyle/test/chapter5comments/rule52documentationcomments/InvalidJavadocPositionTest.java:[35,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) src/it/java/org/checkstyle/suppressionxpathfilter/XpathRegressionAbbreviationAsWordInNameTest.java:[116,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `enum` is not a valid identifier) @@ -105,6 +107,8 @@ src/test/java/com/puppycrawl/tools/checkstyle/checks/design/InterfaceIsTypeCheck src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java:[54,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java:[100,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `null` is not a valid identifier) src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheckTest.java:[81,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) +src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java:[69,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices) +src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java:[80,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices) src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/InvalidJavadocPositionCheckTest.java:[59,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocContentLocationCheckTest.java:[57,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier) src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocContentLocationCheckTest.java:[75,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `package` is not a valid identifier) diff --git a/integration-tests/prometheus-java-client-expected-changes.patch b/integration-tests/prometheus-java-client-expected-changes.patch index 0d5a79ba..a7324f5d 100644 --- a/integration-tests/prometheus-java-client-expected-changes.patch +++ b/integration-tests/prometheus-java-client-expected-changes.patch @@ -5968,7 +5968,7 @@ import static io.prometheus.metrics.instrumentation.dropwizard5.labels.MapperConfig.METRIC_GLOB_REGEX; import java.util.HashMap; -@@ -32,10 +33,9 @@ class GraphiteNamePattern { +@@ -32,10 +33,11 @@ class GraphiteNamePattern { * @param pattern The glob style pattern to be used. */ GraphiteNamePattern(final String pattern) throws IllegalArgumentException { @@ -5978,11 +5978,13 @@ - } + checkArgument( + VALIDATION_PATTERN.matcher(pattern).matches(), -+ String.format("Provided pattern [%s] does not matches [%s]", pattern, METRIC_GLOB_REGEX)); ++ "Provided pattern [%s] does not matches [%s]", ++ pattern, ++ METRIC_GLOB_REGEX); initializePattern(pattern); } -@@ -84,7 +84,7 @@ class GraphiteNamePattern { +@@ -84,7 +86,7 @@ class GraphiteNamePattern { escapedPattern.append("([^.]*)").append(quoted); } @@ -6001,7 +6003,7 @@ import java.util.HashMap; import java.util.Map; import java.util.regex.Pattern; -@@ -106,21 +108,19 @@ public final class MapperConfig { +@@ -106,21 +108,21 @@ public final class MapperConfig { } private void validateMatch(final String match) { @@ -6013,9 +6015,9 @@ - } + checkArgument( + MATCH_EXPRESSION_PATTERN.matcher(match).matches(), -+ String.format( -+ "Match expression [%s] does not match required pattern %s", -+ match, MATCH_EXPRESSION_PATTERN)); ++ "Match expression [%s] does not match required pattern %s", ++ match, ++ MATCH_EXPRESSION_PATTERN); } private void validateLabels(final Map labels) { @@ -6027,11 +6029,13 @@ - } + checkArgument( + LABEL_PATTERN.matcher(key).matches(), -+ String.format("Label [%s] does not match required pattern %s", match, LABEL_PATTERN)); ++ "Label [%s] does not match required pattern %s", ++ match, ++ LABEL_PATTERN); } } } -@@ -149,7 +149,6 @@ public final class MapperConfig { +@@ -149,7 +151,6 @@ public final class MapperConfig { public int hashCode() { int result = match != null ? match.hashCode() : 0; result = 31 * result + (name != null ? name.hashCode() : 0); From 3e4a01a673e30409e67e4b7c577c8b622ce2f3a9 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 11 Jan 2025 14:54:28 +0100 Subject: [PATCH 33/43] Have `TimeZoneUsage` check also flag illegal method references (#1507) While there, reorder a few things. --- .../errorprone/bugpatterns/TimeZoneUsage.java | 24 +++++-- .../bugpatterns/TimeZoneUsageTest.java | 69 +++++++++++++++++-- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsage.java index eb9a3b07..ae8797c4 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsage.java @@ -16,10 +16,12 @@ 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.bugpatterns.BugChecker.MemberReferenceTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberReferenceTree; import com.sun.source.tree.MethodInvocationTree; import java.time.Clock; import java.time.Instant; @@ -41,7 +43,8 @@ import java.time.ZonedDateTime; linkType = CUSTOM, severity = WARNING, tags = FRAGILE_CODE) -public final class TimeZoneUsage extends BugChecker implements MethodInvocationTreeMatcher { +public final class TimeZoneUsage extends BugChecker + implements MethodInvocationTreeMatcher, MemberReferenceTreeMatcher { private static final long serialVersionUID = 1L; private static final Matcher BANNED_TIME_METHOD = anyOf( @@ -59,6 +62,10 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT "tickMillis", "tickMinutes", "tickSeconds"), + staticMethod() + .onClassAny(Instant.class.getCanonicalName()) + .named("now") + .withNoParameters(), staticMethod() .onClassAny( LocalDate.class.getCanonicalName(), @@ -67,17 +74,22 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT OffsetDateTime.class.getCanonicalName(), OffsetTime.class.getCanonicalName(), ZonedDateTime.class.getCanonicalName()) - .named("now"), - staticMethod() - .onClassAny(Instant.class.getCanonicalName()) - .named("now") - .withNoParameters()); + .named("now")); /** Instantiates a new {@link TimeZoneUsage} instance. */ public TimeZoneUsage() {} @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + return getDescription(tree, state); + } + + @Override + public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) { + return getDescription(tree, state); + } + + private Description getDescription(ExpressionTree tree, VisitorState state) { return BANNED_TIME_METHOD.matches(tree, state) ? buildDescription(tree).build() : Description.NO_MATCH; diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageTest.java index abca2d4e..fcf0c383 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageTest.java @@ -21,6 +21,7 @@ final class TimeZoneUsageTest { "import java.time.OffsetTime;", "import java.time.ZoneId;", "import java.time.ZonedDateTime;", + "import reactor.core.publisher.Mono;", "", "class A {", " void m() {", @@ -31,68 +32,122 @@ final class TimeZoneUsageTest { " Clock.tick(clock, Duration.ZERO);", "", " // BUG: Diagnostic contains:", - " Clock.systemUTC();", + " clock.getZone();", " // BUG: Diagnostic contains:", - " Clock.systemDefaultZone();", + " Mono.fromSupplier(clock::getZone);", + " // BUG: Diagnostic contains:", + " clock.withZone(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(clock::withZone);", " // BUG: Diagnostic contains:", " Clock.system(UTC);", " // BUG: Diagnostic contains:", + " Mono.empty().map(Clock::system);", + " // BUG: Diagnostic contains:", + " Clock.systemDefaultZone();", + " // BUG: Diagnostic contains:", + " Mono.fromSupplier(Clock::systemDefaultZone);", + " // BUG: Diagnostic contains:", + " Clock.systemUTC();", + " // BUG: Diagnostic contains:", + " Mono.fromSupplier(Clock::systemUTC);", + " // BUG: Diagnostic contains:", " Clock.tickMillis(UTC);", " // BUG: Diagnostic contains:", + " Mono.empty().map(Clock::tickMillis);", + " // BUG: Diagnostic contains:", " Clock.tickMinutes(UTC);", " // BUG: Diagnostic contains:", + " Mono.empty().map(Clock::tickMinutes);", + " // BUG: Diagnostic contains:", " Clock.tickSeconds(UTC);", " // BUG: Diagnostic contains:", - " clock.getZone();", - " // BUG: Diagnostic contains:", - " clock.withZone(UTC);", + " Mono.empty().map(Clock::tickSeconds);", "", + " Instant.now(clock);", + " Mono.empty().map(Instant::now);", " // BUG: Diagnostic contains:", " Instant.now();", - " // This is equivalent to `clock.instant()`, which is fine.", - " Instant.now(clock);", + " // BUG: Diagnostic contains:", + " Mono.fromSupplier(Instant::now);", "", " // BUG: Diagnostic contains:", " LocalDate.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(LocalDate::now);", + " // BUG: Diagnostic contains:", " LocalDate.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalDate::now);", + " // BUG: Diagnostic contains:", " LocalDate.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalDate::now);", "", " // BUG: Diagnostic contains:", " LocalDateTime.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(LocalDateTime::now);", + " // BUG: Diagnostic contains:", " LocalDateTime.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalDateTime::now);", + " // BUG: Diagnostic contains:", " LocalDateTime.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalDateTime::now);", "", " // BUG: Diagnostic contains:", " LocalTime.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(LocalTime::now);", + " // BUG: Diagnostic contains:", " LocalTime.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalTime::now);", + " // BUG: Diagnostic contains:", " LocalTime.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(LocalTime::now);", "", " // BUG: Diagnostic contains:", " OffsetDateTime.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(OffsetDateTime::now);", + " // BUG: Diagnostic contains:", " OffsetDateTime.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(OffsetDateTime::now);", + " // BUG: Diagnostic contains:", " OffsetDateTime.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(OffsetDateTime::now);", "", " // BUG: Diagnostic contains:", " OffsetTime.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(OffsetTime::now);", + " // BUG: Diagnostic contains:", " OffsetTime.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(OffsetTime::now);", + " // BUG: Diagnostic contains:", " OffsetTime.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(OffsetTime::now);", "", " // BUG: Diagnostic contains:", " ZonedDateTime.now();", " // BUG: Diagnostic contains:", + " Mono.fromSupplier(ZonedDateTime::now);", + " // BUG: Diagnostic contains:", " ZonedDateTime.now(clock);", " // BUG: Diagnostic contains:", + " Mono.empty().map(ZonedDateTime::now);", + " // BUG: Diagnostic contains:", " ZonedDateTime.now(UTC);", + " // BUG: Diagnostic contains:", + " Mono.empty().map(ZonedDateTime::now);", " }", "", " abstract class ForwardingClock extends Clock {", From 6211e7e65156fe10cddb48a20db0f4c352d9692d Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sun, 12 Jan 2025 11:54:41 +0100 Subject: [PATCH 34/43] Upgrade pitest-maven-plugin 1.17.3 -> 1.17.4 (#1508) See: - https://github.com/hcoles/pitest/releases/tag/1.17.4 - https://github.com/hcoles/pitest/compare/1.17.3...1.17.4 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1e217db8..efc1dca6 100644 --- a/pom.xml +++ b/pom.xml @@ -1524,7 +1524,7 @@ org.pitest pitest-maven - 1.17.3 + 1.17.4 From f4562c7106ef456e3c72b1c032b1a2f0c9fe1bc4 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Sun, 12 Jan 2025 12:31:32 +0100 Subject: [PATCH 35/43] Upgrade MongoDB driver 5.2.1 -> 5.3.0 (#1509) See: - https://jira.mongodb.org/issues/?jql=project%20%3D%20JAVA%20AND%20fixVersion%20%3E%205.2.1%20AND%20fixVersion%20%3C%3D%205.3.0 - https://github.com/mongodb/mongo-java-driver/releases/tag/r5.3.0-beta0 - https://github.com/mongodb/mongo-java-driver/releases/tag/r5.3.0 - https://github.com/mongodb/mongo-java-driver/compare/r5.2.1...r5.3.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index efc1dca6..26282aee 100644 --- a/pom.xml +++ b/pom.xml @@ -486,7 +486,7 @@ org.mongodb mongodb-driver-core - 5.2.1 + 5.3.0 org.openrewrite From b123282778b4137efe09def1ff10c0ecfc99b10c Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Mon, 13 Jan 2025 09:51:58 +0100 Subject: [PATCH 36/43] Upgrade OpenRewrite Templating 1.20.2 -> 1.21.0 (#1510) See: - https://github.com/openrewrite/rewrite-templating/releases/tag/v1.21.0 - https://github.com/openrewrite/rewrite-templating/compare/v1.20.2...v1.21.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 26282aee..959d2d4e 100644 --- a/pom.xml +++ b/pom.xml @@ -215,7 +215,7 @@ 1.0.1 0.12.3 1.1.4 - 1.20.2 + 1.21.0 3.2.3 From 54c56a4cc808c9ee7ce97da7fe00758c6feabd3c Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Mon, 13 Jan 2025 14:17:40 +0100 Subject: [PATCH 37/43] Upgrade OpenRewrite 2.23.2 -> 3.0.0 (#1511) See: - https://github.com/openrewrite/rewrite-recipe-bom/releases/tag/v3.0.0 - https://github.com/openrewrite/rewrite-recipe-bom/compare/v2.23.2...v3.0.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 959d2d4e..029d8652 100644 --- a/pom.xml +++ b/pom.xml @@ -496,7 +496,7 @@ org.openrewrite.recipe rewrite-recipe-bom - 2.23.2 + 3.0.0 pom import From 90b4baba120cf656dbe7025ac69f918336e985c1 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:20:12 +0100 Subject: [PATCH 38/43] Upgrade Micrometer 1.14.2 -> 1.14.3 (#1513) See: - https://github.com/micrometer-metrics/micrometer/releases/tag/v1.14.3 - https://github.com/micrometer-metrics/micrometer/compare/v1.14.2...v1.14.3 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 029d8652..a6a1778e 100644 --- a/pom.xml +++ b/pom.xml @@ -363,7 +363,7 @@ io.micrometer micrometer-bom - 1.14.2 + 1.14.3 pom import From 56db8adb5ae4feb4df160703da4a055cc4e90774 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:44:33 +0100 Subject: [PATCH 39/43] Upgrade OpenRewrite 3.0.0 -> 3.0.1 (#1514) See: - https://github.com/openrewrite/rewrite-recipe-bom/releases/tag/v3.0.1 - https://github.com/openrewrite/rewrite-recipe-bom/compare/v3.0.0...v3.0.1 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a6a1778e..c90b749f 100644 --- a/pom.xml +++ b/pom.xml @@ -496,7 +496,7 @@ org.openrewrite.recipe rewrite-recipe-bom - 3.0.0 + 3.0.1 pom import From 78141b252803defdbb741288df0b30f266bb1229 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:40:35 +0100 Subject: [PATCH 40/43] Upgrade Project Reactor 2024.0.1 -> 2024.0.2 (#1515) See: - https://github.com/reactor/reactor/releases/tag/2024.0.2 - https://github.com/reactor/reactor/compare/2024.0.1...2024.0.2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c90b749f..672e2dee 100644 --- a/pom.xml +++ b/pom.xml @@ -370,7 +370,7 @@ io.projectreactor reactor-bom - 2024.0.1 + 2024.0.2 pom import From 126159878ce9dc06a5f6b1e31dab98fe179ed626 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:33:55 +0100 Subject: [PATCH 41/43] Upgrade Spring 6.2.1 -> 6.2.2 (#1516) See: - https://github.com/spring-projects/spring-framework/releases/tag/v6.2.2 - https://github.com/spring-projects/spring-framework/compare/v6.2.1...v6.2.2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 672e2dee..aa0b038f 100644 --- a/pom.xml +++ b/pom.xml @@ -510,7 +510,7 @@ org.springframework spring-framework-bom - 6.2.1 + 6.2.2 pom import From 9f83eec36b19f804c2c792d6d7f4956a66cf2508 Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Fri, 17 Jan 2025 12:40:07 +0100 Subject: [PATCH 42/43] Upgrade jOOQ 3.19.17 -> 3.19.18 (#1518) See: - https://www.jooq.org/notes - https://github.com/jOOQ/jOOQ/releases/tag/version-3.19.18 - https://github.com/jOOQ/jOOQ/compare/version-3.19.17...version-3.19.18 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index aa0b038f..f6a7969c 100644 --- a/pom.xml +++ b/pom.xml @@ -462,7 +462,7 @@ org.jooq jooq - 3.19.17 + 3.19.18 org.jspecify From 3767341605acf3ed3ea9053ccbc2651cb4f7056b Mon Sep 17 00:00:00 2001 From: Picnic-DevPla-Bot <168541957+Picnic-DevPla-Bot@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:05:37 +0100 Subject: [PATCH 43/43] Upgrade swagger-annotations 1.6.14 -> 1.6.15 (#1517) See: - https://github.com/swagger-api/swagger-core/releases/tag/v1.6.15 - https://github.com/swagger-api/swagger-core/compare/v1.6.14...v1.6.15 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f6a7969c..56c315dd 100644 --- a/pom.xml +++ b/pom.xml @@ -382,7 +382,7 @@ io.swagger swagger-annotations - 1.6.14 + 1.6.15 io.swagger.core.v3