Merge pull request #48 from spoofax-shell/resolve-fixmes-todos

[RDY] Resolve FIXMEs and TODOs

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/ConsoleReplModule.java

@@ -1,6 +1,5 @@
 package org.metaborg.spoofax.shell.client;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 
@@ -12,10 +11,8 @@
 import org.metaborg.spoofax.shell.client.console.impl.history.JLine2PersistentInputHistory;
 import org.metaborg.spoofax.shell.commands.IReplCommand;
 
-import com.google.inject.Provides;
 import com.google.inject.Singleton;
 import com.google.inject.multibindings.MapBinder;
-import com.google.inject.name.Named;
 import com.google.inject.name.Names;
 
 /**
@@ -54,24 +51,4 @@ protected void configure() {
         super.configure();
         bindUserInterface();
     }
-
-    /**
-     * TODO: Replace with "CheckedProvides" because IO in Provider method is bad practice, see:
-     * https://github.com/google/guice/wiki/ThrowingProviders.
-     *
-     * @param in
-     *            The {@link InputStream}.
-     * @param out
-     *            The {@link OutputStream}.
-     * @return A {@link jline.console.ConsoleReader} with the given streams.
-     * @throws IOException
-     *             When an IO error occurs upon construction.
-     */
-    @Provides
-    @Singleton
-    protected jline.console.ConsoleReader provideConsoleReader(@Named("in") InputStream in,
-                                                               @Named("out") OutputStream out)
-        throws IOException {
-        return new jline.console.ConsoleReader(in, out);
-    }
 }

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/Main.java

@@ -15,18 +15,30 @@
  * This class launches a {@link ConsoleRepl}, a console based REPL.
  */
 public final class Main {
+    private static final String ERROR = "Invalid commandline parameters: %s%nThe only argument "
+                                        + "accepted is the path to a language implementation "
+                                        + "location, using any filesystem supported by Apache VFS";
 
     private Main() {
     }
 
+    private static StyledText error(String[] args) {
+        StringBuilder invalidArgs = new StringBuilder();
+        for (String arg : args) {
+            invalidArgs.append(arg).append(", ");
+        }
+        // Remove the appended ", " from the string.
+        invalidArgs.delete(invalidArgs.length() - 2, invalidArgs.length());
+        return new StyledText(Color.RED, String.format(ERROR, invalidArgs.toString()));
+    }
+
     /**
      * Instantiates and runs a new {@link ConsoleRepl}.
      *
      * @param args
-     *            The path to a language implementation location, using any URI supported by Apache
-     *            VFS.
+     *            The path to a language implementation location, using any filesystem supported by
+     *            Apache VFS.
      */
-    // TODO: make the argument work again.
     public static void main(String[] args) {
         Injector injector = Guice.createInjector(new ConsoleReplModule());
         IDisplay display = injector.getInstance(IDisplay.class);
@@ -37,6 +49,12 @@ public static void main(String[] args) {
         display.displayStyledText(message);
 
         ConsoleRepl repl = injector.getInstance(ConsoleRepl.class);
+        if (args.length == 1) {
+            repl.runOnce(":load " + args[0]);
+        } else if (args.length > 1) {
+            display.displayStyledText(error(args));
+        }
+
         repl.run();
     }
 }

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/commands/ExitCommand.java

@@ -33,8 +33,6 @@ public String description() {
     @Override
     public IResult execute(String... args) {
         ConsoleRepl repl = replProvider.get();
-        return (visitor) -> {
-            repl.setRunning(false);
-        };
+        return (visitor) -> repl.setRunning(false);
     }
 }

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/impl/ConsoleRepl.java

@@ -52,6 +52,21 @@ public void setRunning(boolean running) {
         this.running = running;
     }
 
+    /**
+     * Evaluate {@code input}, without having to enter the main loop in {@link #run()}. This is
+     * useful to initialize some state before control is handed to the user.
+     *
+     * @param input
+     *            The input to evaluate.
+     */
+    public void runOnce(String input) {
+        try {
+            eval(input).accept(display);
+        } catch (CommandNotFoundException e) {
+            this.display.displayStyledText(new StyledText(Color.RED, e.getMessage()));
+        }
+    }
+
     /**
      * Run {@link IRepl#eval(String)} in a loop, for as long as {@code running} is {@code true}.
      *
@@ -65,11 +80,7 @@ public void run() {
             String input;
             setRunning(true);
             while (running && (input = this.iface.getInput()) != null) {
-                try {
-                    eval(input).accept(display);
-                } catch (CommandNotFoundException e) {
-                    this.display.displayStyledText(new StyledText(Color.RED, e.getMessage()));
-                }
+                runOnce(input);
             }
 
             this.iface.history().persistToDisk();

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/impl/TerminalUserInterface.java

@@ -48,13 +48,10 @@ public class TerminalUserInterface implements IDisplay {
      *            The {@link PrintStream} to write errors to.
      * @param hist
      *            The input history adapter for JLine2.
-     * @throws IOException
-     *             When an IO error occurs.
      */
     @Inject
     public TerminalUserInterface(ConsoleReader reader, @Named("out") OutputStream out,
-                                 @Named("err") OutputStream err, IInputHistory hist)
-        throws IOException {
+                                 @Named("err") OutputStream err, IInputHistory hist) {
         this.reader = reader;
         this.hist = hist;
         reader.setExpandEvents(false);

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/impl/history/JLine2PersistentInputHistory.java

@@ -3,10 +3,13 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Optional;
+import java.util.function.Function;
 
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
 
+import jline.console.history.History;
+
 /**
  * An extension to the adapter for JLine2's History implementation. This adapter implementation
  * maintains two versions of JLine2's History classes: one that is initially used in-memory, and
@@ -49,10 +52,10 @@ public JLine2PersistentInputHistory(@Named("historyPath") String filePath,
      *         used instead.
      */
     @Override
-    protected jline.console.history.History delegate() {
+    protected History delegate() {
         // orElse does not work on a subtype, but it does if you put flatMap in between.
         return delegateFileHist
-            .flatMap((jline.console.history.History fileHist) -> Optional.of(fileHist))
+            .flatMap((Function<History, Optional<History>>) Optional::of)
             .orElse(super.delegate());
     }
 

org.metaborg.spoofax.shell.console/src/main/java/org/metaborg/spoofax/shell/client/console/impl/history/package-info.java

@@ -1,5 +1,5 @@
 /**
  * This package contains implementations (one volatile, one persistent) of
- * {@link org.metaborg.spoofax.shell.client.console.IInputHistory}.
+ * {@link org.metaborg.spoofax.shell.client.IInputHistory}.
  */
 package org.metaborg.spoofax.shell.client.console.impl.history;

org.metaborg.spoofax.shell.console/src/test/java/org/metaborg/spoofax/shell/client/console/impl/ConsoleReplTest.java

@@ -1,6 +1,5 @@
 package org.metaborg.spoofax.shell.client.console.impl;
 
-import static org.junit.Assert.fail;
 import static org.metaborg.spoofax.shell.client.console.impl.TerminalUserInterfaceTest.C_D;
 import static org.metaborg.spoofax.shell.client.console.impl.TerminalUserInterfaceTest.ENTER;
 import static org.mockito.Matchers.anyString;
@@ -91,19 +90,41 @@ private void setUpCtrlD() throws IOException {
         createRepl(new UserInputSimulationModule(in, out), new MockModule(invokerMock));
     }
 
+    /**
+     * Test {@link ConsoleRepl#runOnce(String)}.
+     *
+     * @throws IOException
+     *             Should not happen.
+     * @throws CommandNotFoundException
+     *             Should not happen.
+     */
+    @Test
+    public void testRunOnce() throws IOException, CommandNotFoundException {
+        String fake = ":fakecommand foo";
+        setUp(fake);
+        invokerMock = mock(ICommandInvoker.class, RETURNS_MOCKS);
+
+        // Create a user input simulated ConsoleRepl with the mock invoker.
+        createRepl(new UserInputSimulationModule(in, out), new MockModule(invokerMock));
+
+        repl.runOnce(fake);
+        verify(invokerMock, times(1)).execute(fake);
+    }
+
     /**
      * Test whether the REPl exits when Control-D is pressed.
+     *
+     * @throws IOException
+     *             Should not happen.
+     * @throws CommandNotFoundException
+     *             Should not happen.
      */
     @Test
-    public void testCtrlDDoesExit() {
-        try {
-            setUpCtrlD();
-            repl.run();
-            // Ensure that the command invoker is never called with any command.
-            verify(invokerMock, never()).execute(anyString());
-        } catch (IOException | CommandNotFoundException e) {
-            fail("Should not happen");
-        }
+    public void testCtrlDDoesExit() throws CommandNotFoundException, IOException {
+        setUpCtrlD();
+        repl.run();
+        // Ensure that the command invoker is never called with any command.
+        verify(invokerMock, never()).execute(anyString());
     }
 
     private void setUpExit() throws IOException {
@@ -118,28 +139,29 @@ private void setUpExit() throws IOException {
 
     /**
      * Tests the {@link ExitCommand}.
+     *
+     * @throws IOException
+     *             Should not happen.
+     * @throws CommandNotFoundException
+     *             Should not happen.
      */
     @Test
-    public void testExitCommand() {
-        try {
-            setUpExit();
+    public void testExitCommand() throws IOException, CommandNotFoundException {
+        setUpExit();
 
-            // Stub the invoker so that it returns an exit command which we can spy on.
-            ExitCommand exitCommandMock = spy(new ExitCommand(() -> repl));
-            when(invokerMock.commandFromName("exit")).thenReturn(exitCommandMock);
+        // Stub the invoker so that it returns an exit command which we can spy on.
+        ExitCommand exitCommandMock = spy(new ExitCommand(() -> repl));
+        when(invokerMock.commandFromName("exit")).thenReturn(exitCommandMock);
 
-            repl.run();
+        repl.run();
 
-            // Ensure that the command was given to the invoker just once.
-            verify(invokerMock, times(1)).execute(":exit");
+        // Ensure that the command was given to the invoker just once.
+        verify(invokerMock, times(1)).execute(":exit");
 
-            // Ensure that exitCommand was executed once.
-            verify(exitCommandMock, times(1)).execute();
+        // Ensure that exitCommand was executed once.
+        verify(exitCommandMock, times(1)).execute();
 
-            // Verify that the Editor was not asked for input after the exit command was executed.
-            verify(ifaceSpy, times(1)).getInput();
-        } catch (IOException | CommandNotFoundException e) {
-            fail("Should not happen");
-        }
+        // Verify that the Editor was not asked for input after the exit command was executed.
+        verify(ifaceSpy, times(1)).getInput();
     }
 }

org.metaborg.spoofax.shell.console/src/test/java/org/metaborg/spoofax/shell/client/console/impl/TerminalUserInterfaceTest.java

@@ -11,7 +11,6 @@
 import java.io.OutputStream;
 
 import org.junit.Test;
-import org.metaborg.spoofax.shell.client.console.impl.TerminalUserInterface;
 import org.metaborg.spoofax.shell.output.StyledText;
 
 import com.google.inject.Guice;
@@ -37,7 +36,6 @@ public class TerminalUserInterfaceTest {
     /* >>> */ + "qwerty" + ENTER
     /* ... */ + ENTER;
     private TerminalUserInterface ui;
-    private ByteArrayInputStream in;
     private ByteArrayOutputStream out;
 
     /**
@@ -49,7 +47,7 @@ public class TerminalUserInterfaceTest {
      *             When an IO error occurs upon construction of the {@link ConsoleReader}.
      */
     public void setUp(String inputString) throws IOException {
-        in = new ByteArrayInputStream(inputString.getBytes("UTF-8"));
+        ByteArrayInputStream in = new ByteArrayInputStream(inputString.getBytes("UTF-8"));
         out = new ByteArrayOutputStream();
 
         Injector injector = Guice.createInjector(new UserInputSimulationModule(in, out));

org.metaborg.spoofax.shell.core/src/main/java/org/metaborg/spoofax/shell/ReplModule.java

@@ -9,7 +9,9 @@
 import org.metaborg.core.resource.IResourceService;
 import org.metaborg.core.resource.ResourceService;
 import org.metaborg.spoofax.core.SpoofaxModule;
+import org.metaborg.spoofax.shell.client.IDisplay;
 import org.metaborg.spoofax.shell.client.IRepl;
+import org.metaborg.spoofax.shell.commands.DefaultCommand;
 import org.metaborg.spoofax.shell.commands.HelpCommand;
 import org.metaborg.spoofax.shell.commands.IReplCommand;
 import org.metaborg.spoofax.shell.commands.LanguageCommand;
@@ -61,7 +63,8 @@ public abstract class ReplModule extends SpoofaxModule {
     protected void bindCommands(MapBinder<String, IReplCommand> commandBinder) {
         commandBinder.addBinding("help").to(HelpCommand.class);
         commandBinder.addBinding("load").to(LanguageCommand.class);
-
+        bind(IReplCommand.class).annotatedWith(Names.named("default_command"))
+        .to(DefaultCommand.class);
         bind(ICommandInvoker.class).to(SpoofaxCommandInvoker.class);
     }
 

org.metaborg.spoofax.shell.core/src/main/java/org/metaborg/spoofax/shell/client/IDisplay.java

@@ -49,7 +49,7 @@ default void visitFailure(FailResult errorResult) {
         StyledText styled = highlightMessagesInSource(sourceText, messages);
 
         String concat =
-            messages.stream().map(message -> message.message()).collect(Collectors.joining("\n"));
+            messages.stream().map(IMessage::message).collect(Collectors.joining("\n"));
         visitMessage(styled.append("\n").append(Color.RED, concat));
     }
 
@@ -64,7 +64,7 @@ default void visitFailure(FailResult errorResult) {
      * @return The highlighted {@link StyledText}
      */
     default StyledText highlightMessagesInSource(String sourceText, List<IMessage> messages) {
-        List<ISourceRegion> regions = messages.stream().map(message -> message.region())
+        List<ISourceRegion> regions = messages.stream().map(IMessage::region)
             .filter(Objects::nonNull).collect(Collectors.toList());
         StyledText styled = new StyledText();
         IStyle style = new Style(Color.RED, null, true, false, false);

org.metaborg.spoofax.shell.core/src/main/java/org/metaborg/spoofax/shell/commands/DefaultCommand.java

@@ -0,0 +1,19 @@
+package org.metaborg.spoofax.shell.commands;
+
+import org.metaborg.spoofax.shell.output.IResult;
+import org.metaborg.spoofax.shell.output.StyledText;
+
+/**
+ * Dummy class returned only when no language is loaded.
+ */
+public class DefaultCommand implements IReplCommand {
+    @Override
+    public String description() {
+        return "No language is loaded yet, type :help for more information.";
+    }
+
+    @Override
+    public IResult execute(String... args) {
+        return (visitor) -> visitor.visitMessage(new StyledText(description()));
+    }
+}

org.metaborg.spoofax.shell.core/src/main/java/org/metaborg/spoofax/shell/commands/HelpCommand.java

@@ -20,7 +20,7 @@ public class HelpCommand implements IReplCommand {
     private final ICommandInvoker invoker;
 
     /**
-     * Instantiates a new HelpCommand.
+     * Instantiates a new {@link HelpCommand}.
      *
      * @param invoker
      *            The {@link ICommandInvoker}.
@@ -41,7 +41,7 @@ public String description() {
      * @return a formatted string
      */
     public String formathelp(Map<String, IReplCommand> commands) {
-        int longestCommand = commands.keySet().stream().mapToInt(a -> a.length()).max().orElse(0);
+        int longestCommand = commands.keySet().stream().mapToInt(String::length).max().orElse(0);
         String format = "%-" + longestCommand + "s %s";
 
         return commands.keySet().stream().flatMap(name -> {