Reorganize refactorings

Cleaning up Refactoring interface, splitting it into 2 interfaces.
Removed lkt_refactor flag from some tests since the commit revealed a
bug that made them pass while they shouldn't.

Author: Killian Perlin

lkql_jit/cli/src/main/java/com/adacore/lkql_jit/cli/LKQLRefactor.java

@@ -9,29 +9,40 @@
 
 import com.adacore.lkql_jit.options.Refactorings.LKQLToLkt;
 import com.adacore.lkql_jit.options.Refactorings.Refactoring;
+import com.adacore.lkql_jit.options.Refactorings.TokenBasedRefactoring;
+import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Callable;
 import picocli.CommandLine;
 
 /**
- * Refactor command for LKQL. Allows to run automatic migrations, allowing the LKQL team to create
- * migrators to adapt to syntactic or semantic changes in LKQL.
+ * Refactor command for LKQL. Allows to run automatic migrations, allowing the
+ * LKQL team to create migrators to adapt to syntactic or semantic changes in
+ * LKQL.
  *
- * <p>Migrators work on the stream of tokens directly. Migrator implementer will attach actions
- * (append/prepend/replace) on tokens, which allows to modify the output stream without working on
- * text directly, which simplifies the expression of refactorings.
+ * <p>
+ * Migrators are just an implementation of the functionnal interface
+ * 'Refactoring'. It expects an LKQL unit as input and produces the new unit
+ * content as a 'String'.
  *
- * <p>A migrator implementer will typically:
+ * <p>
+ * In practice, a 'Refactoring' implementation will be of one of 2 kinds:
+ * 1. A 'TokenBasedRefactoring' working on the stream of tokens directly and
+ * producing rewriting actions on each token of the stream.
+ * 2. A 'TreeBasedRefactoring' working on the Liblkqllang syntax tree and
+ * producing a new 'String' directly.
  *
- * <p>1. Find the nodes he wants to refactor in the LKQL tree 2. From those nodes, find the tokens
- * inside the nodes that need to be altered 3. Attach actions to those tokens
+ * <p>
+ * Then the rewriter will emit a new file for every LKQL unit, either inplace or
+ * on stdout.
  *
- * <p>Then the rewriter will emit a new file for every LKQL unit, either inplace or on stdout.
+ * <p>
+ * To add a refactoring, you need to extend the 'RefactoringKind' enum to add a
+ * new refactoring id, and then extend the 'getRefactoring' method to return the
+ * corresponding implementation of 'Refactoring' to apply.
  *
- * <p>To add a refactoring, you need to extend the 'refactoringKind' enum to add a new refactoring
- * id, and then extend the 'getRefactoring' method to return an anonymous function that will add
- * actions to the list of actions.
  */
 @CommandLine.Command(
     name = "refactor",
@@ -66,7 +77,7 @@ private enum RefactoringKind {
      */
     public Refactoring getRefactoring(AnalysisUnit unit) {
         return switch (refactoring) {
-            case IS_TO_COLON -> (Refactoring.State state) -> {
+            case IS_TO_COLON -> (TokenBasedRefactoring) state -> {
                 Refactoring.stream(state.unit.getRoot())
                     .filter(n -> n instanceof NodePatternDetail)
                     .forEach(det -> {
@@ -89,15 +100,26 @@ public Refactoring getRefactoring(AnalysisUnit unit) {
         };
     }
 
+    /**
+     * Rewrite a unit by printing refactored file, either to stdout, or to the
+     * original file, depending on the value of the '-i' command line flag.
+     */
     @Override
     public Integer call() {
         var ctx = AnalysisContext.create();
         for (var file : files) {
             var unit = ctx.getUnitFromFile(file);
-            var refactoring = getRefactoring(unit);
-            var state = new Refactoring.State(unit);
-            refactoring.applyRefactor(state);
-            state.printAllTokens(this.inPlace);
+            var result = getRefactoring(unit).apply(unit);
+
+            if (inPlace) {
+                try (var writer = new PrintWriter(unit.getFileName(), StandardCharsets.UTF_8)) {
+                    writer.print(result);
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
+            } else {
+                System.out.print(result);
+            }
         }
         return 0;
     }

lkql_jit/options/src/main/java/com/adacore/lkql_jit/options/Refactorings/LKQLToLkt.java

@@ -9,18 +9,24 @@
 
 import com.adacore.liblkqllang.Liblkqllang;
 
-public class LKQLToLkt implements Refactoring {
+public class LKQLToLkt implements TreeBasedRefactoring {
 
     /** Pointer to last-entered selector during rewriting. */
     private Liblkqllang.SelectorDecl currentSelector = Liblkqllang.SelectorDecl.NONE;
 
     @Override
-    public void applyRefactor(Refactoring.State state) {
-        final var root = state.unit.getRoot();
-        state.prepend(root.tokenStart(), "# lkql version: 2\n\n");
+    public String apply(Liblkqllang.AnalysisUnit unit) {
+        var root = unit.getRoot();
+        return (
+            "# lkql version: 2\n\n" +
+            textRange(unit.getFirstToken(), root.tokenStart().previous()) +
+            apply(root) +
+            textRange(root.tokenEnd().next(), unit.getLastToken())
+        );
+    }
 
-        // Because this rewriting doesn't use the token/action API, this is a hack
-        state.replaceRange(root.tokenStart(), root.tokenEnd(), refactorNode(root));
+    public String apply(Liblkqllang.LkqlNode root) {
+        return refactorNode(root);
     }
 
     /** Computes the text representing the refactored node. */

lkql_jit/options/src/main/java/com/adacore/lkql_jit/options/Refactorings/Refactoring.java

@@ -6,167 +6,14 @@
 package com.adacore.lkql_jit.options.Refactorings;
 
 import com.adacore.liblkqllang.Liblkqllang;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 
+/** Class offering an abstraction over all refactoring processes. */
+@FunctionalInterface
 public interface Refactoring {
-    class State {
-
-        public final Liblkqllang.AnalysisUnit unit;
-
-        /** Kind of actions that can be attached to a token. */
-        enum ActionKind {
-            APPEND,
-            PREPEND,
-            REPLACE,
-        }
-
-        /**
-         * Action record, encapsulating an action that can be attached to a token. When applied, the
-         * action will either append the given text, prepend the given text, or replace the token text
-         * with the given text, depending on the kind.
-         *
-         * <p>To remove a token, simply use a REPLACE action with an empty text.
-         */
-        private record Action(ActionKind kind, String text) {}
-
-        /**
-         * List of actions accumulated in this state object.
-         */
-        public final HashMap<String, List<Action>> actions = new HashMap<>();
-
-        public State(Liblkqllang.AnalysisUnit unit) {
-            this.unit = unit;
-        }
-
-        /** Add an action to the list of actions to apply */
-        private void addAction(Liblkqllang.Token token, Action action) {
-            List<Action> actionList;
-            var tokenId = getTokenId(token);
-            if (actions.containsKey(tokenId)) {
-                actionList = actions.get(tokenId);
-            } else {
-                actionList = new ArrayList<>();
-                actions.put(tokenId, actionList);
-            }
-
-            actionList.add(action);
-        }
-
-        public void delete(Liblkqllang.Token token) {
-            addAction(token, new Action(ActionKind.REPLACE, ""));
-        }
-
-        public void replace(Liblkqllang.Token token, String text) {
-            addAction(token, new Action(ActionKind.REPLACE, text));
-        }
-
-        public void replaceRange(Liblkqllang.Token start, Liblkqllang.Token end, String text) {
-            var cur = start;
-            while (cur.tokenIndex != end.tokenIndex) {
-                delete(cur);
-                cur = cur.next();
-            }
-            // append new code
-            replace(end, text);
-        }
-
-        public void append(Liblkqllang.Token token, String text) {
-            addAction(token, new Action(ActionKind.APPEND, text));
-        }
-
-        public void prepend(Liblkqllang.Token token, String text) {
-            addAction(token, new Action(ActionKind.PREPEND, text));
-        }
-
-        /**
-         * Internal method: rewrite a unit by printing all the tokens via the 'write' callback. Apply
-         * the actions at the same time.
-         */
-        private void printAllTokens(Consumer<String> write) {
-            for (var tok = unit.getFirstToken(); !tok.isNone(); tok = tok.next()) {
-                var tokActions = actions.get(getTokenId(tok));
-                if (tokActions != null) {
-                    var replaceActions = tokActions
-                        .stream()
-                        .filter(c -> c.kind == ActionKind.REPLACE)
-                        .toList();
-                    assert replaceActions.size() <= 1 : "Only one replace action per token";
-
-                    var prependActions = tokActions
-                        .stream()
-                        .filter(c -> c.kind == ActionKind.PREPEND)
-                        .toList();
-
-                    var appendActions = tokActions
-                        .stream()
-                        .filter(c -> c.kind == ActionKind.APPEND)
-                        .toList();
-
-                    for (var action : prependActions) {
-                        write.accept(action.text);
-                    }
-
-                    if (!replaceActions.isEmpty()) {
-                        write.accept(replaceActions.get(0).text);
-                    } else {
-                        write.accept(tok.getText());
-                    }
-
-                    for (var action : appendActions) {
-                        write.accept(action.text);
-                    }
-                } else {
-                    write.accept(tok.getText());
-                }
-            }
-        }
-
-        /**
-         * Rewrite a unit by printing all the tokens, either to stdout, or to the
-         * original file, depending on the value of the '-i' command line flag.
-         */
-        public void printAllTokens(boolean inPlace) {
-            try {
-                if (inPlace) {
-                    var writer = new PrintWriter(unit.getFileName(), StandardCharsets.UTF_8);
-                    printAllTokens(writer::print);
-                    writer.close();
-                } else {
-                    printAllTokens(System.out::print);
-                }
-            } catch (Exception e) {
-                throw new RuntimeException(e);
-            }
-        }
-
-        public String toString() {
-            var writer = new StringWriter();
-            printAllTokens(writer::write);
-            return writer.toString();
-        }
-    }
-
-    /**
-     * Helper to create a unique Id for a token. TODO: This method exists only because the
-     * 'hashCode' method on Tokens is wrong. This should be fixed at the Langkit level. See
-     * eng/libadalang/langkit#857.
-     *
-     * @return the id as a string
-     */
-    private static String getTokenId(Liblkqllang.Token token) {
-        return (
-            token.unit.getFileName() +
-            (token.isTrivia() ? "trivia " + token.triviaIndex : token.tokenIndex) +
-            token.kind
-        );
-    }
+    /** Take a parsing tree root and return its source after refactoring actions. */
+    String apply(Liblkqllang.AnalysisUnit unit);
 
     public static boolean isWhitespace(Liblkqllang.Token token) {
         return token.kind == Liblkqllang.TokenKind.LKQL_WHITESPACE;
@@ -198,6 +45,4 @@ public static Stream<Liblkqllang.LkqlNode> stream(Liblkqllang.LkqlNode root) {
     public static Stream<Liblkqllang.Token> streamFrom(Liblkqllang.Token start) {
         return Stream.iterate(start, t -> !t.isNone(), t -> t.next());
     }
-
-    void applyRefactor(State state);
 }