Introduce `LexicographicalPermitsListing` check by mohamedsamehsalah · Pull Request #2017 · PicnicSupermarket/error-prone-support

mohamedsamehsalah · 2025-12-03T14:26:37Z

Suggested commit message

Introduce `LexicographicalSealedInterfacePermitsListing` check

github-actions · 2025-12-03T14:29:45Z

Suggested commit message:

Introduce `LexicographicalPermitsListing` check (#2017)

Add a new check that enforces lexicographical ordering of sealed
`permits` listings to keep these declarations consistent and reduce
merge conflicts.

github-actions · 2025-12-03T14:32:39Z

Surviving mutants in this change: 8
Killed mutants in this change: 11

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	8	11

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

github-actions · 2025-12-03T14:36:03Z

Surviving mutants in this change: 8
Killed mutants in this change: 11

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	8	11

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

sonarqubecloud · 2025-12-03T17:02:23Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2025-12-03T17:03:11Z

Surviving mutants in this change: 8
Killed mutants in this change: 11

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	8	11

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

github-actions · 2026-01-20T08:35:35Z

Surviving mutants in this change: 3
Killed mutants in this change: 11

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	3	11

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

github-actions · 2026-01-20T08:36:28Z

Surviving mutants in this change: 3
Killed mutants in this change: 11

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	3	11

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

github-actions · 2026-01-20T08:43:25Z

Surviving mutants in this change: 2
Killed mutants in this change: 12

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	2	12

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

mohamedsamehsalah

@rickie, should be ready for review now 🙏

mohamedsamehsalah · 2026-01-20T08:47:10Z

...in/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java

+
+  @Override
+  public Description matchClass(ClassTree tree, VisitorState state) {
+    if (!IS_INTERFACE.matches(tree, state)) {


@rickie I thought this would be addressed by adding the class G in the unit tests, but I am probably missing something 🤔

Fixed it in my previous commit :)

I think we can actually also support classes, right?

mohamedsamehsalah · 2026-01-20T08:48:01Z

...in/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java

+                    .replace(original, SourceCode.treeToString(replacement, state)))
+        .reduce(SuggestedFix.Builder::merge)
+        .map(SuggestedFix.Builder::build)
+        .orElseThrow(() -> new VerifyException("No clauses were provided"));


Should this be an IllegalStateException? After all the compiler will complain if no clauses were provided.

github-actions · 2026-01-21T09:37:40Z

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

class	surviving	killed
🎉`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	0	13

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

github-actions · 2026-01-31T09:00:02Z

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

class	surviving	killed
🎉`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	0	13

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

coderabbitai · 2026-01-31T09:00:23Z

📝 Walkthrough

Walkthrough

Introduces a new BugChecker that validates lexicographic ordering of permitted non-sealed interfaces in Java interface declarations. When permits are unordered, the checker reports a diagnostic and provides an automated fix to reorder them lexicographically. Includes comprehensive test coverage for diagnostic detection and source refactoring scenarios.

Changes

Cohort / File(s)	Summary
Bug Checker Implementation `error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java`	New BugChecker extending ClassTreeMatcher that enforces lexicographic ordering of permits in sealed interfaces. Includes matchClass method for validation, sort method for ordering permits by their string representation, and fixOrdering method to generate multi-part SuggestedFix.
Test Suite `error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListingTest.java`	Comprehensive test class with three test methods: identificationIgnoresClasses (verifies inner class exclusion), identification (validates diagnostic detection with mixed interface/class permits), and replacement (verifies source refactoring correctly reorders permits).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	⚠️ Warning	The PR title 'Introduce `LexicographicalPermitsListing` check' is partially related to the changeset but uses an incomplete class name. The actual implementation is named `LexicographicalSealedInterfacePermitsListing`, making the title misleading and imprecise.	Update the PR title to 'Introduce `LexicographicalSealedInterfacePermitsListing` check' to accurately reflect the actual class name and feature being introduced.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description provides the commit message which relates to the changeset by naming the new check being introduced, though it lacks implementation details.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch mohamedsamehsalah/lexico-sealed-permits-listing

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java`:
- Around line 26-31: Update the class Javadoc for
LexicographicalSealedInterfacePermitsListing to accurately describe its scope:
state that the checker verifies the lexicographic ordering of the entire sealed
interface "permits" list (not only "permitted non-sealed interfaces") and that
it flags out-of-order entries in that full permits list; keep the existing
explanation about why sorted ordering helps conflict resolution but change the
wording to reference the entire permits list rather than only non-sealed
permitted interfaces.

coderabbitai · 2026-01-31T09:00:26Z

...in/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java

+/**
+ * A {@link BugChecker} that flags permitted non-sealed interfaces that are not lexicographically
+ * sorted.
+ *
+ * <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
+ * resolution, and can even avoid it if two branches add the same annotation.


⚠️ Potential issue | 🟡 Minor

Clarify the checker’s scope in Javadoc.

The implementation sorts the entire permits list for sealed interfaces, not just “permitted non-sealed interfaces.” Tighten the wording to avoid misleading readers.

✏️ Suggested wording update

- * A {`@link` BugChecker} that flags permitted non-sealed interfaces that are not lexicographically - * sorted. + * A {`@link` BugChecker} that flags sealed interfaces whose {`@code` permits} clause is not + * lexicographically sorted.

🤖 Prompt for AI Agents

In `@error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java` around lines 26 - 31, Update the class Javadoc for LexicographicalSealedInterfacePermitsListing to accurately describe its scope: state that the checker verifies the lexicographic ordering of the entire sealed interface "permits" list (not only "permitted non-sealed interfaces") and that it flags out-of-order entries in that full permits list; keep the existing explanation about why sorted ordering helps conflict resolution but change the wording to reference the entire permits list rather than only non-sealed permitted interfaces.

rickie

Didn't write all comments on the PR as I was on the ✈️ , but I think it's in a good shape!

I added support for also sorting classes. Now we need to rename the check 🤔.

rickie · 2026-02-01T11:03:52Z

...in/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java

+
+  @Override
+  public Description matchClass(ClassTree tree, VisitorState state) {
+    if (!IS_INTERFACE.matches(tree, state)) {


Fixed it in my previous commit :)

I think we can actually also support classes, right?

rickie · 2026-02-01T11:03:54Z

...ava/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListingTest.java

+final class LexicographicalSealedInterfacePermitsListingTest {
+  @Test
+  void identificationIgnoresClasses() {
+    CompilationTestHelper.newInstance(


I'll add this to the identification test :)

rickie · 2026-02-01T11:03:56Z

...ava/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListingTest.java

+    CompilationTestHelper.newInstance(
+            LexicographicalSealedInterfacePermitsListing.class, getClass())
+        .addSourceLines(
+            "pkg/A.java",


Suggested change

"pkg/A.java",

"A.java",

We can omit this :)

rickie · 2026-02-01T11:04:04Z

...in/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedInterfacePermitsListing.java

+ * sorted.
+ *
+ * <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
+ * resolution, and can even avoid it if two branches add the same annotation.


Let's drop the last (copied 😉) sentence.

github-actions · 2026-02-01T15:49:11Z

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

class	surviving	killed
🎉`tech.picnic.errorprone.bugpatterns.LexicographicalSealedInterfacePermitsListing`	0	10

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

github-actions · 2026-02-09T09:44:23Z

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

class	surviving	killed
🎉`tech.picnic.errorprone.bugpatterns.LexicographicalSealedPermitsListing`	0	10

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

rickie

Nice one @mohamedsamehsalah 🚀

Thanks for the beautiful PR :)

Stephan202

I rebased and added a commit. Suggested commit message:

Introduce `LexicographicalPermitsListing` check (#2017)

As this new check is structurally similar to
`LexicographicalAnnotationListing`, the shared logic is factored out and 
exposed through `SourceCode#sortTrees`.

While there, optimize both checks for the common case in which no 
violation is found, by reducing the number of allocations for compliant
code.

Stephan202 · 2026-02-22T16:48:59Z

...ib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListing.java

+
+  @Override
+  public Description matchClass(ClassTree tree, VisitorState state) {
+    List<? extends Tree> originalOrderPermitClauses = tree.getPermitsClause();


We can make this a bit shorter:

Suggested change

List<? extends Tree> originalOrderPermitClauses = tree.getPermitsClause();

List<? extends Tree> originalClauses = tree.getPermitsClause();

Stephan202 · 2026-02-22T16:49:59Z

...ib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListing.java

+  @Override
+  public Description matchClass(ClassTree tree, VisitorState state) {
+    List<? extends Tree> originalOrderPermitClauses = tree.getPermitsClause();
+    ImmutableList<? extends Tree> sortedPermitClauses = sort(originalOrderPermitClauses, state);


Like in LexicographicalAnnotationListing we can skip the check if there are fewer than two clauses.

Edit actually, perhaps we should use Comparators.isInOrder. Quite some logic is duplicated now, so let's factor this out in a helper method.

Stephan202 · 2026-02-22T19:18:10Z

...ib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListing.java

+/**
+ * A {@link BugChecker} that flags permitted non-sealed interfaces and classes that are not
+ * lexicographically sorted.


The permitted clause lists permitted direct subtypes; whether those are non-sealed interfaces has nothing to do with it.

Stephan202 · 2026-02-22T19:20:45Z

...ib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListing.java

+    linkType = CUSTOM,
+    severity = SUGGESTION,
+    tags = STYLE)
+public final class LexicographicalSealedPermitsListing extends BugChecker


Let's just call it LexicographicalPermitsListing.

Stephan202 · 2026-02-22T19:25:43Z

...rc/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListingTest.java

+            "  class G {}",
+            "}")
+        .addSourceLines(
+            "Foo.java",


We generally use one top-level class.

Stephan202 · 2026-02-22T19:26:44Z

...rc/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListingTest.java

+            "import pkg.A.C;",
+            "import pkg.A.D;",
+            "",
+            "// BUG: Diagnostic contains:",


Generally we list the non-violating cases first.

Stephan202 · 2026-02-22T19:29:36Z

...rc/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalSealedPermitsListingTest.java

+            "package pkg;",
+            "",
+            "import pkg.A.B;",
+            "import pkg.A.C;",
+            "import pkg.A.D;",


We can omit this package statement and the imports.

sonarqubecloud · 2026-02-22T20:28:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2026-02-22T20:29:50Z

Surviving mutants in this change: 2
Killed mutants in this change: 15

class	surviving	killed
🧟`tech.picnic.errorprone.utils.SourceCode`	1	5
🧟`tech.picnic.errorprone.bugpatterns.LexicographicalAnnotationListing`	1	6
🎉`tech.picnic.errorprone.bugpatterns.LexicographicalPermitsListing`	0	4

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

Stephan202 · 2026-02-22T20:37:44Z

The two reported mutants are unkillable: both branches exist for performance reasons only.

Stephan202 · 2026-02-22T20:38:41Z

...-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalPermitsListing.java

+  public Description matchClass(ClassTree tree, VisitorState state) {
+    SuggestedFix fix =
+        SourceCode.sortTrees(
+            tree.getPermitsClause(),
+            comparing(annotation -> SourceCode.treeToString(annotation, state)),
+            state);
+    return fix.isEmpty()
+        ? Description.NO_MATCH
+        : describeMatch(tree.getPermitsClause().getFirst(), fix);
+  }


ClassTrees are way less common than ModifierTrees, so I omitted the if (tree.getPermitsClause().size() < 2) case here.

mohamedsamehsalah · 2026-02-23T07:35:50Z

Thanks for review and suggestions @Stephan202 🙏

mohamedsamehsalah added this to the 0.28.0 milestone Dec 3, 2025

mohamedsamehsalah requested review from Stephan202 and rickie December 3, 2025 14:26

mohamedsamehsalah temporarily deployed to codex December 3, 2025 14:26 — with GitHub Actions Inactive

mohamedsamehsalah temporarily deployed to codex December 3, 2025 14:32 — with GitHub Actions Inactive

rickie added the new feature label Dec 3, 2025

mohamedsamehsalah temporarily deployed to codex December 3, 2025 16:59 — with GitHub Actions Inactive

Stephan202 modified the milestones: 0.28.0, 0.29.0 Jan 9, 2026

mohamedsamehsalah force-pushed the mohamedsamehsalah/lexico-sealed-permits-listing branch from 15653b8 to 942753a Compare January 20, 2026 08:32

mohamedsamehsalah temporarily deployed to codex January 20, 2026 08:32 — with GitHub Actions Inactive

mohamedsamehsalah force-pushed the mohamedsamehsalah/lexico-sealed-permits-listing branch from 942753a to fb46fbe Compare January 20, 2026 08:33

mohamedsamehsalah temporarily deployed to codex January 20, 2026 08:33 — with GitHub Actions Inactive

mohamedsamehsalah temporarily deployed to codex January 20, 2026 08:39 — with GitHub Actions Inactive

mohamedsamehsalah commented Jan 20, 2026

View reviewed changes

rickie temporarily deployed to codex January 21, 2026 09:34 — with GitHub Actions Inactive

rickie force-pushed the mohamedsamehsalah/lexico-sealed-permits-listing branch from 37489d8 to b4c75c3 Compare January 31, 2026 08:56

rickie temporarily deployed to codex January 31, 2026 08:56 — with GitHub Actions Inactive

coderabbitai bot reviewed Jan 31, 2026

View reviewed changes

rickie temporarily deployed to codex February 1, 2026 15:46 — with GitHub Actions Inactive

rickie reviewed Feb 1, 2026

View reviewed changes

rickie temporarily deployed to codex February 9, 2026 09:41 — with GitHub Actions Inactive

rickie changed the title ~~Introduce LexicographicalSealedInterfacePermitsListing check~~ Introduce LexicographicalSealedPermitsListing check Feb 9, 2026

rickie temporarily deployed to codex February 9, 2026 09:44 — with GitHub Actions Inactive

rickie approved these changes Feb 9, 2026

View reviewed changes

mohamedsamehsalah and others added 11 commits February 22, 2026 17:41

Introduce LexicographicalSealedInterfacePermitsListing check

74c50b8

Improve summary

d05a3a0

Oopsie

b7e6d39

Drop redundant checks to kill mutations

ee2f5a2

Improve tests

49aedb2

Kill one more mutation

7f1a539

Kill some mutants?

9fd0303

Suggestions

f1d9dc1

Format

506d42a

Last fixes

e846f4e

Suggestions

d2d621d

Stephan202 force-pushed the mohamedsamehsalah/lexico-sealed-permits-listing branch from a44fe61 to d2d621d Compare February 22, 2026 20:26

Stephan202 had a problem deploying to codex February 22, 2026 20:26 — with GitHub Actions Error

Stephan202 approved these changes Feb 22, 2026

View reviewed changes

Stephan202 changed the title ~~Introduce LexicographicalSealedPermitsListing check~~ Introduce LexicographicalPermitsListing check Feb 22, 2026

Stephan202 temporarily deployed to codex February 22, 2026 20:26 — with GitHub Actions Inactive

Stephan202 reviewed Feb 22, 2026

View reviewed changes

	List<? extends Tree> originalOrderPermitClauses = tree.getPermitsClause();
	List<? extends Tree> originalClauses = tree.getPermitsClause();

Comments

Conversation

mohamedsamehsalah commented Dec 3, 2025

Suggested commit message

Uh oh!

github-actions bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 3, 2025

Uh oh!

github-actions bot commented Dec 3, 2025

Uh oh!

sonarqubecloud bot commented Dec 3, 2025

Quality Gate passed

Uh oh!

github-actions bot commented Dec 3, 2025

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

mohamedsamehsalah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

github-actions bot commented Jan 31, 2026

Uh oh!

coderabbitai bot commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

rickie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 1, 2026

Uh oh!

github-actions bot commented Feb 9, 2026

Uh oh!

rickie left a comment

Choose a reason for hiding this comment

Uh oh!

Stephan202 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 3, 2025 •

edited

Loading

coderabbitai bot commented Jan 31, 2026 •

edited

Loading