Skip to content

Commit

Permalink
Respect order when --codemod-includes are set (#464)
Browse files Browse the repository at this point in the history
  • Loading branch information
nahsra authored Nov 10, 2024
1 parent 882f436 commit 0ecf40e
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,26 @@ public CodemodLoader(
// sort the codemods according to their priority
List<Class<? extends CodeChanger>> orderedCodemodTypes = new ArrayList<>(unorderedCodemodTypes);

// sort according to the codemod execution priority of each codemod type
orderedCodemodTypes.sort(
(c1, c2) -> {
CodemodExecutionPriority p1 = c1.getAnnotation(Codemod.class).executionPriority();
CodemodExecutionPriority p2 = c2.getAnnotation(Codemod.class).executionPriority();
return CodemodExecutionPriority.priorityOrderComparator.compare(p1, p2);
});
// if there's an order from --codemod-includes, honor that
Optional<List<String>> desiredOrder = codemodRegulator.desiredCodemodIdOrder();
if (desiredOrder.isPresent()) {
orderedCodemodTypes.sort(
(c1, c2) -> {
String id1 = c1.getAnnotation(Codemod.class).id();
String id2 = c2.getAnnotation(Codemod.class).id();
int index1 = desiredOrder.get().indexOf(id1);
int index2 = desiredOrder.get().indexOf(id2);
return Integer.compare(index1, index2);
});
} else {
// sort according to the codemod execution priority of each codemod type
orderedCodemodTypes.sort(
(c1, c2) -> {
CodemodExecutionPriority p1 = c1.getAnnotation(Codemod.class).executionPriority();
CodemodExecutionPriority p2 = c2.getAnnotation(Codemod.class).executionPriority();
return CodemodExecutionPriority.priorityOrderComparator.compare(p1, p2);
});
}

// get all the injectable parameters
Set<String> packagesScanned = new HashSet<>();
Expand Down Expand Up @@ -127,7 +140,6 @@ public CodemodLoader(
codemods.add(new CodemodIdPair(codemodId, codeChanger));
}
}

this.codemods = Collections.unmodifiableList(codemods);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.List;
import java.util.Objects;
import java.util.Optional;

/** A type that is relied on to inform our in-flight analysis on whether codemods are allowed. */
public interface CodemodRegulator {
Expand All @@ -14,15 +15,23 @@ public interface CodemodRegulator {
*/
boolean isAllowed(String codemodId);

Optional<List<String>> desiredCodemodIdOrder();

class DefaultCodemodRegulator implements CodemodRegulator {

private final DefaultRuleSetting setting;
private final List<String> exceptions;
private final List<String> desiredOrder;

DefaultCodemodRegulator(
final DefaultRuleSetting defaultCodemodSetting, final List<String> codemodExceptions) {
this.setting = Objects.requireNonNull(defaultCodemodSetting);
this.exceptions = Objects.requireNonNull(codemodExceptions);
if (DefaultRuleSetting.ENABLED.equals(defaultCodemodSetting)) {
this.desiredOrder = null;
} else {
this.desiredOrder = codemodExceptions;
}
}

@Override
Expand All @@ -32,6 +41,11 @@ public boolean isAllowed(final String codemodId) {
}
return exceptions.contains(codemodId);
}

@Override
public Optional<List<String>> desiredCodemodIdOrder() {
return Optional.ofNullable(desiredOrder);
}
}

static CodemodRegulator of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static io.codemodder.CodemodLoader.isValidCodemodId;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.github.javaparser.JavaParser;
Expand Down Expand Up @@ -216,6 +217,81 @@ public boolean supports(Path file) {
}
}

/** Confirm the codemods respect the order of the regulator. */
@Test
void it_handles_codemod_orders(final @TempDir Path tmpDir) throws IOException {

// create an ordered list of codemods
List<Class<? extends CodeChanger>> codemodTypes =
List.of(
// test:java/changes-file
ChangesFile.class,

// test:java/changes-file-again
ChangesFileAgain.class,

// test:java/changes-file-yet-again
ChangesFileYetAgain.class);

CodemodLoader loader = createLoader(codemodTypes, tmpDir);
List<CodemodIdPair> codemods = loader.getCodemods();
assertThat(codemods.get(0).getId(), equalTo("test:java/changes-file"));
assertThat(codemods.get(1).getId(), equalTo("test:java/changes-file-again"));
assertThat(codemods.get(2).getId(), equalTo("test:java/changes-file-yet-again"));

// now we specify a --codemod-includes order to be in backwards order
CodemodRegulator regulator =
CodemodRegulator.of(
DefaultRuleSetting.DISABLED,
List.of(
"test:java/changes-file-yet-again",
"test:java/changes-file-again",
"test:java/changes-file"));
loader =
new CodemodLoader(
codemodTypes,
regulator,
tmpDir,
List.of("**"),
List.of(),
Files.list(tmpDir).toList(),
Map.of(),
List.of(),
List.of(),
null,
null,
null);

codemods = loader.getCodemods();
assertThat(codemods.get(0).getId(), equalTo("test:java/changes-file-yet-again"));
assertThat(codemods.get(1).getId(), equalTo("test:java/changes-file-again"));
assertThat(codemods.get(2).getId(), equalTo("test:java/changes-file"));

// now do it again, with only the B and C codemods
regulator =
CodemodRegulator.of(
DefaultRuleSetting.ENABLED,
List.of("test:java/changes-file-again", "test:java/changes-file"));
loader =
new CodemodLoader(
codemodTypes,
regulator,
tmpDir,
List.of("**"),
List.of(),
Files.list(tmpDir).toList(),
Map.of(),
List.of(),
List.of(),
null,
null,
null);

codemods = loader.getCodemods();
assertThat(codemods, hasSize(1));
assertThat(codemods.get(0).getId(), equalTo("test:java/changes-file-yet-again"));
}

/**
* We create a file, and then run two codemods on it. The first codemod adds a line, the second
* adds another. This will ensure that the codemods are run in the order they are provided and
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.codemodder;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import java.util.Optional;
import org.junit.jupiter.api.Test;

final class CodemodRegulatorTest {

@Test
void it_respects_order_on_includes() {
CodemodRegulator regulator =
CodemodRegulator.of(DefaultRuleSetting.DISABLED, List.of("c", "a", "b"));
Optional<List<String>> desiredCodemodIdOrder = regulator.desiredCodemodIdOrder();
assertThat(desiredCodemodIdOrder).isPresent();
List<String> includeOrder = desiredCodemodIdOrder.get();
assertThat(includeOrder).containsExactly("c", "a", "b");
}

@Test
void it_doesnt_opine_on_order_when_excludes() {
CodemodRegulator regulator =
CodemodRegulator.of(DefaultRuleSetting.ENABLED, List.of("c", "a", "b"));
Optional<List<String>> desiredCodemodIdOrder = regulator.desiredCodemodIdOrder();
assertThat(desiredCodemodIdOrder).isEmpty();
}
}

0 comments on commit 0ecf40e

Please sign in to comment.