Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package tech.picnic.errorprone.refasterrules;

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.NotMatches;
import java.util.Deque;
import java.util.Iterator;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsList;

/** Refaster rules related to expressions dealing with {@link Deque} instances. */
// XXX: Introduce similar rules for `BlockingDeque`.
@OnlineDocumentation
final class DequeRules {
private DequeRules() {}

/** Prefer {@link Deque#addLast(Object)} over less clear alternatives. */
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. 🤞

}

@AfterTemplate
void after(Deque<S> deque, T element) {
deque.addFirst(element);
}
}

/**
* Prefer {@link Deque#addLast(Object)} over less clear alternatives.
*
* <p>Note: this rule does not match instances of type {@link java.util.List} (including {@link
* java.util.LinkedList}, which also implements {@link Deque}), as {@link
* java.util.List#add(Object)} is the idiomatic method for those types.
*/
static final class DequeAddLast<S, T extends S> {
@BeforeTemplate
void before(@NotMatches(IsList.class) Deque<S> deque, T element) {
deque.add(element);
}

@AfterTemplate
void after(Deque<S> deque, T element) {
deque.addLast(element);
}
}

/** Prefer {@link Deque#removeFirst()} over less clear alternatives. */
static final class DequeRemoveFirst<S, T extends S> {
@BeforeTemplate
S before(Deque<T> deque) {
return Refaster.anyOf(deque.pop(), deque.remove());
}

@AfterTemplate
S after(Deque<T> deque) {
return deque.removeFirst();
}
}

/** Prefer {@link Deque#offerLast(Object)} over less clear alternatives. */
static final class DequeOfferLast<S, T extends S> {
@BeforeTemplate
boolean before(Deque<S> deque, T element) {
return deque.offer(element);
}

@AfterTemplate
boolean after(Deque<S> deque, T element) {
return deque.offerLast(element);
}
}

/** Prefer {@link Deque#pollFirst()} over less clear alternatives. */
static final class DequePollFirst<S, T extends S> {
@BeforeTemplate
@Nullable S before(Deque<T> deque) {
return deque.poll();
}

@AfterTemplate
@Nullable S after(Deque<T> deque) {
return deque.pollFirst();
}
}

/** Prefer {@link Deque#pollFirst()} over less clear alternatives. */
static final class DequeGetFirst<S, T extends S> {
@BeforeTemplate
S before(Deque<T> deque) {
return deque.element();
}

@AfterTemplate
S after(Deque<T> deque) {
return deque.getFirst();
}
}

/** Prefer {@link Deque#peekFirst()} over less clear alternatives. */
static final class DequePeekFirst<S, T extends S> {
@BeforeTemplate
@Nullable S before(Deque<T> deque) {
return deque.peek();
}

@AfterTemplate
@Nullable S after(Deque<T> deque) {
return deque.peekFirst();
}
}

/** Prefer {@link Deque#removeFirstOccurrence(Object)} over less clear alternatives. */
static final class DequeRemoveFirstOccurrence<S, T extends S> {
@BeforeTemplate
boolean before(Deque<S> deque, T element) {
return deque.remove(element);
}

@AfterTemplate
boolean after(Deque<S> deque, T element) {
return deque.removeFirstOccurrence(element);
}
}

/** Prefer {@link Deque#iterator()} over more contrived alternatives. */
static final class DequeIterator<T> {
@BeforeTemplate
Iterator<T> before(Deque<T> deque) {
return deque.reversed().descendingIterator();
}

@AfterTemplate
Iterator<T> after(Deque<T> deque) {
return deque.iterator();
}
}

/** Prefer {@link Deque#descendingIterator()} over more contrived alternatives. */
static final class DequeDescendingIterator<T> {
@BeforeTemplate
Iterator<T> before(Deque<T> deque) {
return deque.reversed().iterator();
}

@AfterTemplate
Iterator<T> after(Deque<T> deque) {
return deque.descendingIterator();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ final class RefasterRulesTest {
ClassRules.class,
CollectionRules.class,
ComparatorRules.class,
DequeRules.class,
DoubleStreamRules.class,
EqualityRules.class,
FileRules.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import java.util.ArrayDeque;
import java.util.Iterator;
import java.util.LinkedList;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class DequeRulesTest implements RefasterRuleCollectionTestCase {
void testDequeAddFirst() {
new ArrayDeque<String>().push("foo");
}

void testDequeAddLast() {
new ArrayDeque<String>().add("foo");
new LinkedList<String>().add("bar");
}

ImmutableSet<String> testDequeRemoveFirst() {
return ImmutableSet.of(new ArrayDeque<String>(0).pop(), new ArrayDeque<String>(1).remove());
}

boolean testDequeOfferLast() {
return new ArrayDeque<String>().offer("foo");
}

String testDequePollFirst() {
return new ArrayDeque<String>().poll();
}

String testDequeGetFirst() {
return new ArrayDeque<String>().element();
}

String testDequePeekFirst() {
return new ArrayDeque<String>().peek();
}

boolean testDequeRemoveFirstOccurrence() {
return new ArrayDeque<>().remove("foo");
}

Iterator<String> testDequeIterator() {
return new ArrayDeque<String>().reversed().descendingIterator();
}

Iterator<String> testDequeDescendingIterator() {
return new ArrayDeque<String>().reversed().iterator();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import java.util.ArrayDeque;
import java.util.Iterator;
import java.util.LinkedList;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class DequeRulesTest implements RefasterRuleCollectionTestCase {
void testDequeAddFirst() {
new ArrayDeque<String>().addFirst("foo");
}

void testDequeAddLast() {
new ArrayDeque<String>().addLast("foo");
new LinkedList<String>().add("bar");
}

ImmutableSet<String> testDequeRemoveFirst() {
return ImmutableSet.of(
new ArrayDeque<String>(0).removeFirst(), new ArrayDeque<String>(1).removeFirst());
}

boolean testDequeOfferLast() {
return new ArrayDeque<String>().offerLast("foo");
}

String testDequePollFirst() {
return new ArrayDeque<String>().pollFirst();
}

String testDequeGetFirst() {
return new ArrayDeque<String>().getFirst();
}

String testDequePeekFirst() {
return new ArrayDeque<String>().peekFirst();
}

boolean testDequeRemoveFirstOccurrence() {
return new ArrayDeque<>().removeFirstOccurrence("foo");
}

Iterator<String> testDequeIterator() {
return new ArrayDeque<String>().iterator();
}

Iterator<String> testDequeDescendingIterator() {
return new ArrayDeque<String>().descendingIterator();
}
}
Loading