Skip to content

Introduce DequeRules Refaster rule collection#1946

Merged
rickie merged 6 commits intomasterfrom
sschroevers/introduce-DequeRules
Jan 6, 2026
Merged

Introduce DequeRules Refaster rule collection#1946
rickie merged 6 commits intomasterfrom
sschroevers/introduce-DequeRules

Conversation

@Stephan202
Copy link
Copy Markdown
Member

@Stephan202 Stephan202 commented Nov 1, 2025

❗ This PR is on top of #1927. ❗

Suggested commit message:

Introduce `DequeRules` Refaster rule collection (#1946)

@Stephan202 Stephan202 added this to the 0.27.0 milestone Nov 1, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 1, 2025

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.

2 similar comments
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 1, 2025

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 1, 2025

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
Copy Markdown
Collaborator

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

From my days of preparing for solving coding algorithms, that set of rules change a lot of the common terminology used 😄

But if I think about it, being more "explicit" about what each API does also makes things easier and alleviates some assumptions.

static final class DequeAddFirst<S, T extends S> {
@BeforeTemplate
void before(Deque<S> deque, T element) {
deque.push(element);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some people think of Deque as a replacement for Stack interface; wouldn't push (and pop) keywords be more relevant here? 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, that's a fair concern. My thinking is that the suggestions here will make the intent of the code clear in all cases, independently from (a) the specific combination of operations applied in a given context and (b) whether the user is familiar with stack operations.

It wouldn't surprise me if this change receives some pushback... let's see. 🤞

@rickie rickie force-pushed the sschroevers/target-jdk-21 branch 2 times, most recently from 96f070e to 67d7d5f Compare November 6, 2025 15:45
Base automatically changed from sschroevers/target-jdk-21 to master November 6, 2025 16:09
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 7, 2025

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 force-pushed the sschroevers/introduce-DequeRules branch from 262f5f2 to 9cf3ff4 Compare November 8, 2025 11:38
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 8, 2025

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 rickie force-pushed the sschroevers/introduce-DequeRules branch from 9cf3ff4 to 5a11037 Compare November 10, 2025 14:57
@github-actions
Copy link
Copy Markdown

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 force-pushed the sschroevers/introduce-DequeRules branch from 5a11037 to 7820b05 Compare November 10, 2025 21:23
@rickie rickie force-pushed the sschroevers/introduce-DequeRules branch from 7820b05 to 77afb73 Compare November 27, 2025 10:29
@rickie
Copy link
Copy Markdown
Member

rickie commented Nov 27, 2025

/integration-test --> Build 🔴

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 27, 2025

Suggested commit message:

Introduce `DequeRules` Refaster rule collection (#1946)

@github-actions
Copy link
Copy Markdown

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.

@github-actions
Copy link
Copy Markdown

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.

@github-actions
Copy link
Copy Markdown

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
Copy Markdown
Member

rickie commented Nov 28, 2025

Interesting, for prometheus-java-client we end up with a loop!

       r += item.g;
     }
     while (i < toIndex) {
-      samples.addLast(new Sample(sortedBuffer[i], 0));
+      samples.add(new Sample(sortedBuffer[i], 0));
       i++;
       n++;
     }

commit 3d0ba102bec88df0c27faf3e539084511408f44e
Author: Rick Ossendrijver <rick.ossendrijver@gmail.com>
Date:   Fri Nov 28 13:34:35 2025 +0100

    minor: Apply patches

diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CKMSQuantiles.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CKMSQuanti
les.java
index c16383f..ab99c21 100644
--- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CKMSQuantiles.java
+++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CKMSQuantiles.java
@@ -110,7 +110,7 @@ final class CKMSQuantiles {
       r += item.g;
     }
     while (i < toIndex) {
-      samples.add(new Sample(sortedBuffer[i], 0));
+      samples.addLast(new Sample(sortedBuffer[i], 0));
       i++;
       n++;
     }

Didn't investigate further yet.

@rickie
Copy link
Copy Markdown
Member

rickie commented Nov 28, 2025

Was in meetings mostly so saw a nice opportunity for Windsurf 😄.

Root Cause Analysis
There are two conflicting rules:

`DequeAddLast` in `DequeRules.java`  (lines 31-41):

> Transforms: deque.add(element) → deque.addLast(element) for Deque<S> types

`ListAdd`  in `CollectionRules.java`  (lines 553-568):
> Transforms: list.addLast(element) → list.add(element) for List<S> types

The Problem:

LinkedList<T> implements both List<T> and Deque<T>

When `samples.add(...)` is called on a LinkedList:
The `DequeAddLast` rule matches and transforms it to `samples.addLast(...)`
The `ListAdd`  rule then matches and transforms it back to `samples.add(...)`

This creates an infinite loop of transformations

Created a new matcher. Think makes sense as there is this overlap for this specific method on both of the types.
Feel free to revert or decide to do otherwise :), as I said, mostly windsurf-ed it. :)

@github-actions
Copy link
Copy Markdown

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

class surviving killed
🎉tech.picnic.errorprone.refaster.matchers.IsList 0 2

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

@rickie
Copy link
Copy Markdown
Member

rickie commented Nov 28, 2025

/integration-test -> build ✔️

@github-actions
Copy link
Copy Markdown

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

class surviving killed
🎉tech.picnic.errorprone.refaster.matchers.IsList 0 2

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

@sonarqubecloud
Copy link
Copy Markdown

@Stephan202 Stephan202 modified the milestones: 0.27.0, 0.28.0 Nov 29, 2025
@Stephan202 Stephan202 force-pushed the sschroevers/introduce-DequeRules branch from 81a0355 to 3d971ed Compare January 5, 2026 19:36
@Stephan202
Copy link
Copy Markdown
Member Author

I finally had a look at your changes; sorry for the delay @rickie. I rebased and added a small commit.

@Stephan202
Copy link
Copy Markdown
Member Author

Stephan202 commented Jan 5, 2026

/integration-test -> build ✔️

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 5, 2026

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

class surviving killed
🎉tech.picnic.errorprone.refaster.matchers.IsList 0 2

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

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jan 5, 2026

@rickie rickie merged commit e4d8aca into master Jan 6, 2026
19 checks passed
@rickie rickie deleted the sschroevers/introduce-DequeRules branch January 6, 2026 09:35
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