From 958d5901896fb9ad668d43f6b4938ff110e5dc21 Mon Sep 17 00:00:00 2001 From: Luke Bemish Date: Fri, 30 Aug 2024 22:32:17 -0500 Subject: [PATCH 1/5] Add API for reacting to collected replacements --- .../net/neoforged/jst/api/Replacements.java | 12 +++++- .../neoforged/jst/api/SourceTransformer.java | 14 +++++++ .../net/neoforged/jst/cli/Replacement.java | 31 --------------- .../jst/cli/SourceFileProcessor.java | 39 +++++++++++++++---- 4 files changed, 56 insertions(+), 40 deletions(-) delete mode 100644 cli/src/main/java/net/neoforged/jst/cli/Replacement.java diff --git a/api/src/main/java/net/neoforged/jst/api/Replacements.java b/api/src/main/java/net/neoforged/jst/api/Replacements.java index 2885813..645e48b 100644 --- a/api/src/main/java/net/neoforged/jst/api/Replacements.java +++ b/api/src/main/java/net/neoforged/jst/api/Replacements.java @@ -4,10 +4,19 @@ import com.intellij.psi.PsiElement; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public final class Replacements { - private final List replacements = new ArrayList<>(); + private final List replacements; + + public Replacements(List replacements) { + this.replacements = replacements; + } + + public Replacements() { + this(new ArrayList<>()); + } public boolean isEmpty() { return replacements.isEmpty(); @@ -81,5 +90,4 @@ public String apply(CharSequence originalContent) { writer.append(originalContent, replacements.get(replacements.size() - 1).range().getEndOffset(), originalContent.length()); return writer.toString(); } - } diff --git a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java index c595219..978c19e 100644 --- a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java +++ b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java @@ -2,6 +2,8 @@ import com.intellij.psi.PsiFile; +import java.util.List; + /** * Transformers are created through {@link SourceTransformerPlugin plugins}, and handle source replacements. *

@@ -19,6 +21,18 @@ public interface SourceTransformer { default void beforeRun(TransformContext context) { } + /** + * Invoke after replacements are collected for a given file, but before they are applied. + *

+ * Can be used to react to or verify the replacements that were collected. + * @param fileEntry the file entry being transformed + * @param replacements the replacements that were collected; read-only + * @return {@code true} if the transformation should continue, {@code false} otherwise + */ + default boolean beforeReplacement(FileEntry fileEntry, List replacements) { + return true; + } + /** * Invoked after all source transformations are finished. *

diff --git a/cli/src/main/java/net/neoforged/jst/cli/Replacement.java b/cli/src/main/java/net/neoforged/jst/cli/Replacement.java deleted file mode 100644 index 9fad609..0000000 --- a/cli/src/main/java/net/neoforged/jst/cli/Replacement.java +++ /dev/null @@ -1,31 +0,0 @@ -package net.neoforged.jst.cli; - -import com.intellij.openapi.util.TextRange; -import com.intellij.psi.PsiElement; - -import java.util.Comparator; - -public record Replacement(TextRange range, String newText) { - - public static final Comparator COMPARATOR = Comparator.comparingInt(replacement -> replacement.range.getStartOffset()); - - public static Replacement replace(PsiElement element, String newText) { - return new Replacement(element.getTextRange(), newText); - } - - public static Replacement insertBefore(PsiElement element, String newText) { - var startOffset = element.getTextRange().getStartOffset(); - return new Replacement(new TextRange( - startOffset, - startOffset - ), newText); - } - - public static Replacement insertAfter(PsiElement element, String newText) { - var endOffset = element.getTextRange().getEndOffset(); - return new Replacement(new TextRange( - endOffset, - endOffset - ), newText); - } -} diff --git a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java index c97b113..e38e6bf 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -6,6 +6,7 @@ import net.neoforged.jst.api.FileSink; import net.neoforged.jst.api.FileSource; import net.neoforged.jst.api.Logger; +import net.neoforged.jst.api.Replacement; import net.neoforged.jst.api.Replacements; import net.neoforged.jst.api.SourceTransformer; import net.neoforged.jst.api.TransformContext; @@ -19,6 +20,7 @@ import java.nio.file.attribute.FileTime; import java.time.Instant; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -63,16 +65,23 @@ public boolean process(FileSource source, FileSink sink, List }); } } else { + boolean[] status = {true}; try (var asyncOut = new OrderedParallelWorkQueue(sink, maxQueueDepth); var stream = source.streamEntries()) { stream.forEach(entry -> asyncOut.submitAsync(parallelSink -> { try { - processEntry(entry, sourceRoot, transformers, parallelSink); + var isOk = processEntry(entry, sourceRoot, transformers, parallelSink); + if (!isOk) { + status[0] = false; + } } catch (IOException e) { throw new UncheckedIOException(e); } })); } + if (!status[0]) { + return false; + } } boolean isOk = true; @@ -83,11 +92,13 @@ public boolean process(FileSource source, FileSink sink, List return isOk; } - private void processEntry(FileEntry entry, VirtualFile sourceRoot, List transformers, FileSink sink) throws IOException { + private boolean processEntry(FileEntry entry, VirtualFile sourceRoot, List transformers, FileSink sink) throws IOException { if (entry.directory()) { sink.putDirectory(entry.relativePath()); - return; + return true; } + + boolean[] isOk = {true}; try (var in = entry.openInputStream()) { byte[] content = in.readAllBytes(); @@ -95,13 +106,17 @@ private void processEntry(FileEntry entry, VirtualFile sourceRoot, List transformers, byte[] originalContentBytes) { + private byte[] transformSource(VirtualFile contentRoot, FileEntry entry, List transformers, byte[] originalContentBytes, boolean[] status) { // Instead of parsing the content we actually read from the file, we read the virtual file that is // visible to IntelliJ from adding the source jar. The reasoning is that IntelliJ will cache this internally // and reuse it when cross-referencing type-references. If we parsed from a String instead, it would parse // the same file twice. + var path = entry.relativePath(); var sourceFile = contentRoot.findFileByRelativePath(path); if (sourceFile == null) { System.err.println("Can't transform " + path + " since IntelliJ doesn't see it in the source jar."); @@ -130,14 +146,23 @@ byte[] transformSource(VirtualFile contentRoot, String path, List replacementsList = new ArrayList<>(); + var replacements = new Replacements(replacementsList); for (var transformer : transformers) { transformer.visitFile(psiFile, replacements); } + var readOnlyReplacements = Collections.unmodifiableList(replacementsList); + boolean isOk = true; + for (var transformer : transformers) { + isOk = isOk && transformer.beforeReplacement(entry, readOnlyReplacements); + } + + status[0] = isOk; + // If no replacements were made, just stream the original content into the destination file - if (replacements.isEmpty()) { + if (!isOk || replacements.isEmpty()) { return originalContentBytes; } From fc700d2de2c6eb3914b1f14b2f0c0cc3addce19f Mon Sep 17 00:00:00 2001 From: Luke Bemish Date: Sun, 1 Sep 2024 13:43:46 -0500 Subject: [PATCH 2/5] Make threaded case use AtomicBoolean --- .../java/net/neoforged/jst/cli/SourceFileProcessor.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java index e38e6bf..d6ee99c 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; /** * Reference for out-of-IDE usage of the IntelliJ Java parser is from the Kotlin compiler @@ -65,21 +66,21 @@ public boolean process(FileSource source, FileSink sink, List }); } } else { - boolean[] status = {true}; + AtomicBoolean status = new AtomicBoolean(true); try (var asyncOut = new OrderedParallelWorkQueue(sink, maxQueueDepth); var stream = source.streamEntries()) { stream.forEach(entry -> asyncOut.submitAsync(parallelSink -> { try { var isOk = processEntry(entry, sourceRoot, transformers, parallelSink); if (!isOk) { - status[0] = false; + status.set(false); } } catch (IOException e) { throw new UncheckedIOException(e); } })); } - if (!status[0]) { + if (!status.get()) { return false; } } From b6aa4c502cb04ceff11e7fca2d2ebdee07fc42e5 Mon Sep 17 00:00:00 2001 From: Luke Bemish Date: Sun, 1 Sep 2024 20:11:00 -0500 Subject: [PATCH 3/5] Update docs --- .../main/java/net/neoforged/jst/api/SourceTransformer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java index 978c19e..6535422 100644 --- a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java +++ b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java @@ -27,7 +27,7 @@ default void beforeRun(TransformContext context) { * Can be used to react to or verify the replacements that were collected. * @param fileEntry the file entry being transformed * @param replacements the replacements that were collected; read-only - * @return {@code true} if the transformation should continue, {@code false} otherwise + * @return {@code true} if the transformation should continue, {@code false} if it should fail */ default boolean beforeReplacement(FileEntry fileEntry, List replacements) { return true; @@ -39,7 +39,7 @@ default boolean beforeReplacement(FileEntry fileEntry, List replace * Can be used for post-transformation validation. * * @param context the transform context - * @return {@code true} if the transformation was successful, {@code false} otherwise + * @return {@code true} if the transformation was successful, {@code false} if it failed */ default boolean afterRun(TransformContext context) { return true; From 590e6f5fb754b48d990186936951653b9771b58f Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Wed, 4 Sep 2024 22:29:41 +0200 Subject: [PATCH 4/5] Renames --- .../neoforged/jst/cli/SourceFileProcessor.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java index d6ee99c..fbc78a8 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -99,7 +99,7 @@ private boolean processEntry(FileEntry entry, VirtualFile sourceRoot, List transformers, byte[] originalContentBytes, boolean[] status) { + private byte[] transformSource(VirtualFile contentRoot, FileEntry entry, List transformers, byte[] originalContentBytes, boolean[] successOut) { // Instead of parsing the content we actually read from the file, we read the virtual file that is // visible to IntelliJ from adding the source jar. The reasoning is that IntelliJ will cache this internally // and reuse it when cross-referencing type-references. If we parsed from a String instead, it would parse @@ -155,15 +155,15 @@ private byte[] transformSource(VirtualFile contentRoot, FileEntry entry, List Date: Wed, 4 Sep 2024 22:31:20 +0200 Subject: [PATCH 5/5] Renames --- .../java/net/neoforged/jst/cli/SourceFileProcessor.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java index fbc78a8..5e7dd06 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -66,21 +66,20 @@ public boolean process(FileSource source, FileSink sink, List }); } } else { - AtomicBoolean status = new AtomicBoolean(true); + var success = new AtomicBoolean(true); try (var asyncOut = new OrderedParallelWorkQueue(sink, maxQueueDepth); var stream = source.streamEntries()) { stream.forEach(entry -> asyncOut.submitAsync(parallelSink -> { try { - var isOk = processEntry(entry, sourceRoot, transformers, parallelSink); - if (!isOk) { - status.set(false); + if (!processEntry(entry, sourceRoot, transformers, parallelSink)) { + success.set(false); } } catch (IOException e) { throw new UncheckedIOException(e); } })); } - if (!status.get()) { + if (!success.get()) { return false; } }