Skip to content

Conversation

Picnic-DevPla-Bot
Copy link
Contributor

This PR contains the following updates:

Package Type Update Change
Guava (source) patch 33.4.0-jre -> 33.4.5-jre
Guava (source) import patch 33.4.0-jre -> 33.4.5-jre

  • If you want to rebase/retry this PR, check this box

@Picnic-DevPla-Bot
Copy link
Contributor Author

Picnic-DevPla-Bot commented Mar 20, 2025

Suggested commit message:

Upgrade Guava 33.4.0-jre -> 33.4.7-jre (#1598)

See:
- https://guava.dev/releases/33.4.7-jre/api/diffs/
- https://github.com/google/guava/releases/tag/v33.4.1
- https://github.com/google/guava/releases/tag/v33.4.2
- https://github.com/google/guava/releases/tag/v33.4.3
- https://github.com/google/guava/releases/tag/v33.4.4
- https://github.com/google/guava/releases/tag/v33.4.5
- https://github.com/google/guava/releases/tag/v33.4.6
- https://github.com/google/guava/releases/tag/v33.4.7
- https://github.com/google/guava/compare/v33.4.0...v33.4.7

@rickie rickie added this to the 0.21.0 milestone Mar 20, 2025
Copy link

  • Surviving mutants in this change: 4
  • Killed mutants in this change: 5
class surviving killed
🧟tech.picnic.errorprone.utils.AnnotationAttributeMatcher 4 0
🎉tech.picnic.errorprone.bugpatterns.JUnitValueSource 0 2
🎉tech.picnic.errorprone.experimental.bugpatterns.MethodReferenceUsage 0 2
🎉tech.picnic.errorprone.bugpatterns.RedundantStringConversion 0 1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

  • Surviving mutants in this change: 4
  • Killed mutants in this change: 7
class surviving killed
🧟tech.picnic.errorprone.utils.AnnotationAttributeMatcher 4 0
🎉tech.picnic.errorprone.bugpatterns.JUnitValueSource 0 2
🎉tech.picnic.errorprone.guidelines.bugpatterns.BugPatternLink 0 2
🎉tech.picnic.errorprone.experimental.bugpatterns.MethodReferenceUsage 0 2
🎉tech.picnic.errorprone.bugpatterns.RedundantStringConversion 0 1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Hmm, added a few requireNonNulls. Not sure what we want as an approach. Require to have an extra message parameter to give a nicer message?

Before doing so everywhere, do we agree on the approach? I couldn't find a way around the Iterables.getOnlyElement nullability, while it is a bit strange to me that we now need to handle it like this becuase of a "ParameterizedNullness"...

I also added the -XepOpt:NullAway:JSpecifyMode=true, can move it to a different PR if desired, but it didn't have much effect it seems.

@Picnic-DevPla-Bot
Copy link
Contributor Author

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@rickie rickie force-pushed the renovate/guava-33.x branch from 1bf0db0 to 2ba0d38 Compare March 21, 2025 08:47
Copy link

  • Surviving mutants in this change: 4
  • Killed mutants in this change: 7
class surviving killed
🧟tech.picnic.errorprone.utils.AnnotationAttributeMatcher 4 0
🎉tech.picnic.errorprone.bugpatterns.JUnitValueSource 0 2
🎉tech.picnic.errorprone.guidelines.bugpatterns.BugPatternLink 0 2
🎉tech.picnic.errorprone.experimental.bugpatterns.MethodReferenceUsage 0 2
🎉tech.picnic.errorprone.bugpatterns.RedundantStringConversion 0 1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the renovate/guava-33.x branch from 2ba0d38 to de3e283 Compare March 22, 2025 14:11
Copy link

  • Surviving mutants in this change: 4
  • Killed mutants in this change: 1
class surviving killed
🧟tech.picnic.errorprone.utils.AnnotationAttributeMatcher 4 0
🎉tech.picnic.errorprone.bugpatterns.RedundantStringConversion 0 1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a commit.

Looks like both NullAway and IntelliJ IDEA do not interpret the @ParametricNullness annotations as expected. Adding -XepOpt:NullAway:JSpecifyMode=true doesn't improve the situation, but does flag additional things. I moved those changes to a separate PR: #1608.

CC @msridhar and @cpovirk; what are your thoughts on the newly introduced warnings? (Just checking: I assume that this is unrelated?)

@cpovirk
Copy link

cpovirk commented Mar 22, 2025

This is a little embarrassing, but, uh, where do I see the warnings? :)

@Stephan202
Copy link
Member

@cpovirk ah, they're now all suppressed; see the changes with accompanying XXX comments in the PR diff 😅.

(Let me try to produce a full overview in a follow-up comment; sec.)

@Stephan202
Copy link
Member

Stephan202 commented Mar 22, 2025

To reproduce:

[email protected]:PicnicSupermarket/error-prone-support.git
git checkout renovate/guava-33.x
# Undo the manual changes.
git diff origin/master..HEAD -- '*.java' | git apply -R
# Observe the warnings.
mvn clean install -DskipTests -Dverification.warn | grep NullAway

Output:

[WARNING] /path/to/error-prone-support/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/MoreJUnitMatchersTest.java:[211,69] [NullAway] passing @Nullable parameter 'annotation' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/MoreJUnitMatchersTest.java:[232,67] [NullAway] passing @Nullable parameter 'annotation' where @NonNull is required
[WARNING] /path/to/error-prone-support/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollection.java:[195,54] [NullAway] dereferenced expression Iterables.getOnlyElement(description.fixes) is @Nullable
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoStubbing.java:[54,39] [NullAway] passing @Nullable parameter 'Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/OptionalOrElseGet.java:[81,38] [NullAway] passing @Nullable parameter 'argument' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/OptionalOrElseGet.java:[82,65] [NullAway] passing @Nullable parameter 'argument' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java:[369,48] [NullAway] passing @Nullable parameter 'Iterables.getOnlyElement(methodInvocation.getArguments())' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLoggerDeclaration.java:[122,41] [NullAway] passing @Nullable parameter 'factoryArg' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java:[108,18] [NullAway] dereferenced expression separator is @Nullable
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java:[370,7] [NullAway] returning @Nullable expression from method with @NonNull return type
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java:[483,7] [NullAway] returning @Nullable expression from method with @NonNull return type
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java:[67,46] [NullAway] passing @Nullable parameter 'cell.getRowKey()' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java:[67,67] [NullAway] passing @Nullable parameter 'cell.getColumnKey()' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableTableRules.java:[67,84] [NullAway] passing @Nullable parameter 'cell.getValue()' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java:[90,21] [NullAway] passing @Nullable parameter 'annotation' where @NonNull is required
[WARNING] /path/to/error-prone-support/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java:[95,62] [NullAway] passing @Nullable parameter 'annotation' where @NonNull is required

@cpovirk
Copy link

cpovirk commented Mar 22, 2025

Ha, thanks, [the pointer to look for XXX in the PR is] all I needed :)

Copy link

Looks good. All 1 mutations in this change were killed.

class surviving killed
🎉tech.picnic.errorprone.bugpatterns.RedundantStringConversion 0 1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@msridhar
Copy link

I can't recall what NullAway does with @ParametricNullNess, will try to look later today if I have time, otherwise tomorrow

@cpovirk
Copy link

cpovirk commented Mar 23, 2025

If I point error-prone-support at an identical Guava but with all the @ParametricNullness usages removed, then the warnings go away. But that reintroduces the problem that NullAway's current treatment of @ParametricNullness was designed to solve: With this PR as it currently stands, I can edit the code to pass null for a @ParametricNullness parameter, and NullAway allows it. But if I remove Guava's @ParametricNullness annotations, then that edit produces a warning. This remains true even in JSpecify mode.

Here was a previous discussion about NullAway and @ParametericNullness, which links to other threads.

My guess is that NullAway is still doing the best it can with @ParametricNullness at the moment. If all goes well, the ongoing work on JSpecify mode will eventually let it ignore @ParametricNullness entirely.

@cpovirk
Copy link

cpovirk commented Mar 23, 2025

(Just checking: I assume that this is unrelated?)

And right, I don't think that's related.

@msridhar
Copy link

I think NullAway should ignore @ParametricNullness in JSpecify mode; I opened uber/NullAway#1173 on that. Hopefully with that, this project can switch over to JSpecify mode (once we fix the other bugs you found 🙂) and have a net better experience.

@Stephan202 Stephan202 modified the milestones: 0.21.0, 0.22.0 Mar 24, 2025
@msridhar
Copy link

FYI NullAway 0.12.5 included the change to ignore @ParametricNullness in JSpecify mode. We also just released 0.12.6 with a couple more fixes. You might want to try NullAway 0.12.6 in JSpecify mode, but there will still be cases where NullAway isn't smart enough to infer the right type arguments for calls to generic methods (we're working on it!). If those are too annoying, you might have to stick with not using JSpecify mode for now.

@Stephan202 Stephan202 force-pushed the renovate/guava-33.x branch from 59c8147 to bd7ba6f Compare March 29, 2025 14:36
@Stephan202 Stephan202 changed the base branch from master to sschroevers/guava-33.4.5-upgrade-with-jspecify-mode March 29, 2025 14:36
Copy link

  • Surviving mutants in this change: 4
  • Killed mutants in this change: 4
class surviving killed
🧟tech.picnic.errorprone.utils.AnnotationAttributeMatcher 4 0
🎉tech.picnic.errorprone.bugpatterns.JUnitValueSource 0 2
🎉tech.picnic.errorprone.experimental.bugpatterns.MethodReferenceUsage 0 2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 changed the title Upgrade Guava 33.4.0-jre -> 33.4.5-jre Upgrade Guava 33.4.0-jre -> 33.4.6-jre Mar 29, 2025
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

I rebased the PR on top of #1608 and bumped to version to 33.4.6. I resolved conflicts simply by dropping a bunch of commits; sorry about that. With JSpecify mode enabled, the upgrade now requires no changes, beyond the JSR-305 workaround described in uber/NullAway#1171. For that I ported the relevant changes from #1608, but with updated comments.

Also here: thanks for the rapid feedback and help @cpovirk and @msridhar!

@rickie rickie force-pushed the sschroevers/guava-33.4.5-upgrade-with-jspecify-mode branch from 60d9f56 to 6ef1c6e Compare April 2, 2025 11:55
Base automatically changed from sschroevers/guava-33.4.5-upgrade-with-jspecify-mode to renovate/version.nullaway April 2, 2025 12:23
@Stephan202 Stephan202 force-pushed the renovate/version.nullaway branch from aeb9587 to 14a1ee1 Compare April 3, 2025 08:14
@rickie rickie changed the base branch from renovate/version.nullaway to master April 4, 2025 12:55
@rickie rickie changed the base branch from master to renovate/version.nullaway April 4, 2025 12:56
@Stephan202 Stephan202 force-pushed the renovate/version.nullaway branch 3 times, most recently from a1dd062 to cb8b432 Compare April 14, 2025 07:26
Base automatically changed from renovate/version.nullaway to master April 14, 2025 07:42
@Stephan202 Stephan202 force-pushed the renovate/guava-33.x branch from 8b402f7 to 103e241 Compare April 14, 2025 07:55
@Stephan202 Stephan202 changed the title Upgrade Guava 33.4.0-jre -> 33.4.6-jre Upgrade Guava 33.4.0-jre -> 33.4.7-jre Apr 14, 2025
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Bumped to 33.4.7. The PR now targets master and is ready to merge.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

1 similar comment
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

@rickie rickie merged commit 07399bb into master Apr 14, 2025
17 checks passed
@rickie rickie deleted the renovate/guava-33.x branch April 14, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants