Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Arcmutate 1.0.1 -> 1.0.2 #418

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

Picnic-Bot
Copy link
Contributor

@Picnic-Bot Picnic-Bot commented Dec 16, 2022

This PR contains the following updates:

Package Type Update Change
com.groupcdg.arcmutate:base build patch 1.0.1 -> 1.0.2

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

@Picnic-Bot
Copy link
Contributor Author

cody-christmas-banner

The Java Platform team wishes you a happy festive period. Here is the suggested commit message:

Upgrade base 1.0.1 -> 1.0.2
Renovate be like

image
Header generated and modified from Sweaterify.

@github-actions
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.

@Stephan202
Copy link
Member

The JDK 20-ea build failed with:

/*
 *  ERROR: Did not encounter a test in `CollectionRulesTestInput.java` for the following rule(s):
 *  - CollectionAddAllToCollectionBlock
 *  - CollectionAddAllToCollectionExpression
 *  - CollectionForEach
 *  - CollectionIsEmpty
 *  - CollectionRemoveAllFromCollectionExpression
 *  - CollectionSize
 *  - CollectionToArray
 *  - ImmutableCollectionAsList
 *  - ImmutableCollectionContains
 *  - ImmutableCollectionIterator
 *  - ImmutableCollectionParallelStream
 *  - ImmutableCollectionStream
 *  - ImmutableCollectionToArrayWithArray
 *  - ImmutableCollectionToArrayWithGenerator
 *  - ImmutableCollectionToString
 *  - NewArrayListFromCollection
 *  - OptionalFirstCollectionElement
 *  - OptionalFirstQueueElement
 *  - RemoveOptionalFirstNavigableSetElement
 *  - RemoveOptionalFirstQueueElement
 *  - SetRemoveAllCollection
 */

That's not related to this PR. Restarted to understand whether this might have been a fluke, or requires a closer look.

@Stephan202
Copy link
Member

Nope, failed again. That requires separate investigation (I likely won't have time today).

@Picnic-Bot Picnic-Bot force-pushed the renovate/com.groupcdg.arcmutate-base-1.x branch from dc8b0ce to 21f5961 Compare December 17, 2022 02:01
@Stephan202
Copy link
Member

Can reproduce the issue with OpenJDK 20-ea+27, but not with 20-ea+26. So it's due to some change in this diff.

@Stephan202
Copy link
Member

Based on the diff I made an educated guess. These and these @BeforeTemplate methods are the only rules we have that contain an enhanced for loop. If I drop those the problem goes away; the test then (correctly) fails with:

[ERROR] Failures: 
[ERROR]   RefasterRulesTest.validateRuleCollection:83 value of:
    maybeFormat(...)
diff (-expected +actual):
    @@ -42,8 +42,12 @@
     
       void testCollectionAddAllToCollectionBlock() {
         new ArrayList<>().addAll(ImmutableSet.of("foo"));
    -    new ArrayList<Number>().addAll(ImmutableSet.of(1));
    -    new ArrayList<Number>().addAll(ImmutableSet.of(2));
    +    for (Number element : ImmutableSet.of(1)) {
    +      new ArrayList<Number>().add(element);
    +    }
    +    for (Integer element : ImmutableSet.of(2)) {
    +      new ArrayList<Number>().add(element);
    +    }
       }
     
       boolean testCollectionRemoveAllFromCollectionExpression() {
    @@ -52,8 +56,12 @@
     
       void testSetRemoveAllCollection() {
         new HashSet<>().removeAll(ImmutableSet.of("foo"));
    -    new HashSet<Number>().removeAll(ImmutableList.of(1));
    -    new HashSet<Number>().removeAll(ImmutableSet.of(2));
    +    for (Number element : ImmutableList.of(1)) {
    +      new HashSet<Number>().remove(element);
    +    }
    +    for (Integer element : ImmutableSet.of(2)) {
    +      new HashSet<Number>().remove(element);
    +    }
       }
     
       ArrayList<String> testNewArrayListFromCollection() {

Attaching a debugger, I see that Refaster triggers this AbstractMethodError (which is swallowed here):

Receiver class com.google.errorprone.refaster.AutoValue_UEnhancedForLoop does not define or inherit an implementation of the resolved method 'abstract com.sun.source.tree.Tree getVariableOrRecordPattern()' of interface com.sun.source.tree.EnhancedForLoopTree.

All this points to openjdk/jdk20@2cb64a7 being the "culprit".

@Stephan202
Copy link
Member

Stack trace for completeness:

java.lang.AbstractMethodError: Receiver class com.google.errorprone.refaster.AutoValue_UEnhancedForLoop does not define or inherit an implementation of the resolved method 'abstract com.sun.source.tree.Tree getVariableOrRecordPattern()' of interface com.sun.source.tree.EnhancedForLoopTree.
	at jdk.compiler/com.sun.source.util.TreeScanner.visitEnhancedForLoop(TreeScanner.java:336)
	at com.google.errorprone.refaster.UEnhancedForLoop.accept(UEnhancedForLoop.java:49)
	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:92)
	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
	at com.google.errorprone.refaster.BlockTemplate.lambda$matchesStartingAtBeginning$1(BlockTemplate.java:141)
	at com.google.errorprone.refaster.Choice$2.thenChoose(Choice.java:122)
	at com.google.errorprone.refaster.BlockTemplate.matchesStartingAtBeginning(BlockTemplate.java:122)
	at com.google.errorprone.refaster.BlockTemplate.matchesStartingAnywhere(BlockTemplate.java:169)
	at com.google.errorprone.refaster.BlockTemplate.match(BlockTemplate.java:101)
	at com.google.errorprone.refaster.RefasterScanner.scan(RefasterScanner.java:107)
	at com.google.errorprone.refaster.RefasterScanner.scan(RefasterScanner.java:51)
	at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:96)
	at jdk.compiler/com.sun.source.util.TreeScanner.visitMethod(TreeScanner.java:224)
	at com.google.errorprone.refaster.RefasterScanner.visitMethod(RefasterScanner.java:88)
	at com.google.errorprone.refaster.RefasterScanner.visitMethod(RefasterScanner.java:51)
	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:944)
	at com.google.errorprone.refaster.RefasterScanner.visitClass(RefasterScanner.java:75)
	at com.google.errorprone.refaster.RefasterScanner.visitClass(RefasterScanner.java:51)
	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:851)
	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:92)
	at com.google.errorprone.refaster.RefasterScanner.scan(RefasterScanner.java:132)
	at com.google.errorprone.refaster.RefasterScanner.scan(RefasterScanner.java:51)
	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
	at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:119)
	at jdk.compiler/com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:152)
	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:619)
	at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:92)
	at com.google.errorprone.refaster.RefasterScanner.scan(RefasterScanner.java:132)
	at com.google.errorprone.refaster.RefasterRule.apply(RefasterRule.java:139)
	at tech.picnic.errorprone.refaster.AnnotatedCompositeCodeTransformer.apply(AnnotatedCompositeCodeTransformer.java:72)
	at com.google.errorprone.CompositeCodeTransformer.apply(CompositeCodeTransformer.java:45)
	at tech.picnic.errorprone.refaster.runner.Refaster.matchCompilationUnit(Refaster.java:87)
	at tech.picnic.errorprone.refaster.test.RefasterRuleCollection.matchCompilationUnit(RefasterRuleCollection.java:135)
	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:449)
	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:555)
	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:150)
	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:619)
	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:66)
	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
	at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
	at com.google.errorprone.BugCheckerRefactoringTestHelper.applyDiff(BugCheckerRefactoringTestHelper.java:325)
	at com.google.errorprone.BugCheckerRefactoringTestHelper.runTestOnPair(BugCheckerRefactoringTestHelper.java:268)
	at com.google.errorprone.BugCheckerRefactoringTestHelper.doTest(BugCheckerRefactoringTestHelper.java:250)
	at tech.picnic.errorprone.refaster.test.RefasterRuleCollection.validate(RefasterRuleCollection.java:127)
	at tech.picnic.errorprone.refasterrules.RefasterRulesTest.validateRuleCollection(RefasterRulesTest.java:84)
	...

@Stephan202
Copy link
Member

So this issue is already fixed in Error Prone (google/error-prone@df033f0), but not yet released. I did file google/error-prone#3610 just now to make also the Error Prone build itself compatible with JDK 20-ea. For this repository I filed #419 to workaround the issue for now.

@rickie rickie force-pushed the renovate/com.groupcdg.arcmutate-base-1.x branch from 21f5961 to a815a04 Compare December 19, 2022 10:55
@github-actions
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.

@rickie
Copy link
Member

rickie commented Dec 19, 2022

Nice analysis and PR in EP @Stephan202 😄!

@github-actions
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.

@Stephan202 Stephan202 added this to the 0.7.0 milestone Dec 19, 2022
@rickie rickie force-pushed the renovate/com.groupcdg.arcmutate-base-1.x branch from 2c54fd0 to d2ab293 Compare December 19, 2022 14:08
@github-actions
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.

@rickie
Copy link
Member

rickie commented Dec 19, 2022

Suggested commit message:

Upgrade Pitest Git base plugin 1.0.1 -> 1.0.2 (#418)

We haven't had this one before, suggested commit message based on: #380 (comment).

@Stephan202
Copy link
Member

Hmm, how about:

Upgrade Arcmutate base plugin 1.0.1 -> 1.0.2 (#418)

Or even just:

Upgrade Arcmutate plugin 1.0.1 -> 1.0.2 (#418)

Or even just:

Upgrade Arcmutate 1.0.1 -> 1.0.2 (#418)

@rickie
Copy link
Member

rickie commented Dec 19, 2022

I like the:

Upgrade Arcmutate 1.0.1 -> 1.0.2 (#418)

The most actually. Let's roll with that one.

@Stephan202
Copy link
Member

Stephan202 commented Dec 19, 2022

SGTM! (You'll open a Renovate PR?)

@rickie
Copy link
Member

rickie commented Dec 19, 2022

You'll open a Renovate PR?

Yes of course 😉. Done!

@rickie rickie changed the title Upgrade com.groupcdg.arcmutate:base 1.0.1 -> 1.0.2 Upgrade Arcmutate 1.0.1 -> 1.0.2 Dec 19, 2022
@rickie rickie merged commit 7c40fdc into master Dec 19, 2022
@rickie rickie deleted the renovate/com.groupcdg.arcmutate-base-1.x branch December 19, 2022 20:02
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.

3 participants