Merge pull request #132 from metaborg/aml-release

Bugfixes in Command-line SPT `transform` expectation evaluation.
metaborg · Feb 9, 2024 · f0d6bd1 · f0d6bd1
2 parents 3b1dadc + 140c5ef
commit f0d6bd1
Show file tree

Hide file tree

Showing 21 changed files with 108 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,15 @@ All notable changes to this project are documented in this file, based on [Keep
 
 
 ## [Unreleased]
+### Changed
+- Command Line Interface now returns a non-zero error code when the underlying command fails.
+- Command Line Interface executing tests now prints the number of passed and failed test cases.
+- SPT `transform` expectations support commands using the enclosing file and/or enclosing project.
+
+### Fixed
+- Termination issues in Code Completion
+- Serialization issues of meta-language configuration objects
+- Region arguments to SPT `transform` expectations now correspond to the imploder attachments.
 
 ## [0.19.7] - 2024-02-09
 ### Changed

diff --git a/core/spoofax.cli/src/main/java/mb/spoofax/cli/CommandRunner.java b/core/spoofax.cli/src/main/java/mb/spoofax/cli/CommandRunner.java
@@ -1,6 +1,7 @@
 package mb.spoofax.cli;
 
 import mb.common.message.KeyedMessages;
+import mb.pie.api.ExecException;
 import mb.pie.api.MixedSession;
 import mb.pie.api.Task;
 import mb.resource.ReadableResource;
@@ -53,7 +54,7 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException
         rawArgsBuilder.setArg(paramId, (Serializable)value);
     }
 
-    @Override public @Nullable Object call() throws Exception {
+    @Override public @Nullable Object call() throws SpoofaxCliException, InterruptedException, ExecException {
         final RawArgs rawArgs = rawArgsBuilder.build(context);
         final A args = commandDef.fromRawArgs(rawArgs);
         final Task<CommandFeedback> task = commandDef.createTask(args);
@@ -67,11 +68,12 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException
 
         final KeyedMessages keyedMessages = feedback.getMessages();
         if(!keyedMessages.isEmpty()) {
-            System.out.println("The following messages were produced by command '" + commandDef.getDisplayName() + "':\n" + keyedMessages.toString());
+            System.out.println("The following messages were produced by command '" + commandDef.getDisplayName() + "':\n" + keyedMessages);
         }
 
+        boolean commandFailed = exception != null || keyedMessages.containsErrorOrHigher();
         for(ShowFeedback showFeedback : feedback.getShowFeedbacks()) {
-            showFeedback.caseOf()
+            commandFailed |= showFeedback.caseOf()
                 .showFile((file, region) -> {
                     try {
                         final ReadableResource resource = resourceService.getReadableResource(file);
@@ -85,23 +87,28 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException
                         System.err.println("An exception occurred while showing file '" + file + "':");
                         e.printStackTrace(System.err);
                     }
-                    return Optional.empty();
+                    return false;
                 })
                 .showText((text, name, region) -> {
                     if(printFeedbackNames && !name.isEmpty()) {
                         System.out.println(name + ":");
                         System.out.println();
                     }
                     System.out.println(text);
-                    return Optional.empty();
+                    return false;
                 })
                 .showTestResults(((testResults, region) -> {
                     StringBuilder builder = new StringBuilder();
                     testResults.addToStringBuilder(builder);
                     System.out.print(builder);
-                    return Optional.empty();
+                    return testResults.numFailed > 0;
                 }));
         }
+
+        if(commandFailed) {
+            // Exception is processed and turned into an exit code by picocli.
+            throw new SpoofaxCliException("Command '" + commandDef.getDisplayName() + "' failed (see messages above).", exception);
+        }
         return null;
     }
 }
diff --git a/core/spoofax.cli/src/main/java/mb/spoofax/cli/SpoofaxCliException.java b/core/spoofax.cli/src/main/java/mb/spoofax/cli/SpoofaxCliException.java
@@ -0,0 +1,14 @@
+package mb.spoofax.cli;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+public class SpoofaxCliException extends Exception {
+
+    public SpoofaxCliException(String message) {
+        super(message);
+    }
+
+    public SpoofaxCliException(String message, @Nullable Throwable cause) {
+        super(message, cause);
+    }
+}
diff --git a/core/spoofax.compiler/src/main/java/mb/spoofax/compiler/adapter/data/CliParamRepr.java b/core/spoofax.compiler/src/main/java/mb/spoofax/compiler/adapter/data/CliParamRepr.java
@@ -146,7 +146,7 @@ public String toJavaCode() {
     }
 
 
-    public static class Option {
+    public static class Option implements Serializable {
         public final ListView<String> names;
         public final boolean negatable;
         public final @Nullable String label;
@@ -187,7 +187,7 @@ public Option(ListView<String> names, boolean negatable, @Nullable String label,
         }
     }
 
-    public static class Positional {
+    public static class Positional implements Serializable {
         public final int index;
         public final @Nullable String label;
         public final @Nullable String description;

diff --git a/core/spoofax.compiler/src/main/java/mb/spoofax/compiler/adapter/data/SeparatorRepr.java b/core/spoofax.compiler/src/main/java/mb/spoofax/compiler/adapter/data/SeparatorRepr.java
@@ -2,10 +2,11 @@
 
 import org.immutables.value.Value;
 
+import java.io.Serializable;
 import java.util.Optional;
 
 @Value.Immutable
-public interface SeparatorRepr {
+public interface SeparatorRepr extends Serializable  {
     class Builder extends ImmutableSeparatorRepr.Builder {}
 
     static Builder builder() {

diff --git a/core/spoofax.core/src/main/java/mb/spoofax/core/language/testrunner/TestResults.java b/core/spoofax.core/src/main/java/mb/spoofax/core/language/testrunner/TestResults.java
@@ -39,6 +39,11 @@ public void addToStringBuilder(StringBuilder builder) {
         for (TestSuiteResult suite : suites) {
             suite.addToStringBuilder(builder);
         }
+        builder
+            .append(numPassed)
+            .append(" test cases passed, ")
+            .append(numFailed)
+            .append(" test cases failed.");
     }
 
     @Override

diff --git a/...letion/src/main/java/mb/statix/codecompletion/strategies/runtime/ExpandQueryStrategy.java b/...letion/src/main/java/mb/statix/codecompletion/strategies/runtime/ExpandQueryStrategy.java
@@ -148,7 +148,12 @@ public static Seq<CCSolverState> eval(
         if(!unifier.isGround(query.scopeTerm())) {
             // Delay
             final Delay delay = Delay.ofVars(unifier.getVars(query.scopeTerm()));
-            return Seq.of(input.withoutSelected().withDelay(query, delay));
+            return Seq.of(input
+                .withoutSelected()
+                // remove query from active constraint set, as it is delayed now
+                .withUpdatedConstraints(Collections.emptySet(), Collections.singleton(query))
+                .withDelay(query, delay)
+            );
         }
         @Nullable final Scope scope = Scope.matcher().match(query.scopeTerm(), unifier).orElse(null);
         assert scope != null;
@@ -353,7 +358,12 @@ public F1<Scope, Env<Scope, ITerm, ITerm>> caseCompiledQuery(CCompiledQuery q) {
         } catch(IncompleteException e) {
             // Delay
             final Delay delay = Delay.ofVars(unifier.getVars(query.scopeTerm()));
-            return Collections.singletonList(input.withoutSelected().withDelay(query, delay));
+            return Collections.singletonList(input
+                .withoutSelected()
+                // remove query from active constraint set, as it is delayed now
+                .withUpdatedConstraints(Collections.emptySet(), Collections.singleton(query))
+                .withDelay(query, delay)
+            );
         } catch(ResolutionException e) {
             throw new RuntimeException("Unexpected ResolutionException: " + e.getMessage(), e);
         } catch(InterruptedException e) {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/CheckCountExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/CheckCountExpectation.java
@@ -10,12 +10,14 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spt.lut.LanguageUnderTestProvider;
 import mb.spt.model.LanguageUnderTest;
 import mb.spt.model.SelectionReference;
 import mb.spt.model.TestCase;
 import mb.spt.model.TestExpectation;
 import mb.spt.util.SptMessageRemap;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.util.ArrayList;
 import java.util.stream.Collectors;
@@ -77,6 +79,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/CheckPatternExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/CheckPatternExpectation.java
@@ -10,12 +10,14 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spt.lut.LanguageUnderTestProvider;
 import mb.spt.model.LanguageUnderTest;
 import mb.spt.model.SelectionReference;
 import mb.spt.model.TestCase;
 import mb.spt.model.TestExpectation;
 import mb.spt.util.SptMessageRemap;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.util.ArrayList;
 import java.util.stream.Collectors;
@@ -39,6 +41,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseExpectation.java
@@ -9,6 +9,7 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.language.LanguageInstance;
 import mb.spt.api.parse.ParseResult;
 import mb.spt.api.parse.TestableParse;
@@ -17,6 +18,7 @@
 import mb.spt.model.TestCase;
 import mb.spt.model.TestExpectation;
 import mb.spt.util.SptMessageRemap;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class ParseExpectation implements TestExpectation {
     public enum Recovery {
@@ -49,6 +51,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseToAtermExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseToAtermExpectation.java
@@ -10,16 +10,19 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.language.LanguageInstance;
 import mb.spt.lut.LanguageUnderTestProvider;
 import mb.spt.model.LanguageUnderTest;
 import mb.spt.model.TestCase;
 import mb.spt.api.parse.TestableParse;
 import mb.spt.model.TestExpectation;
 import mb.spt.util.SptAtermMatcher;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.spoofax.interpreter.terms.IStrategoTerm;
 import org.spoofax.terms.TermFactory;
 
+
 public class ParseToAtermExpectation implements TestExpectation {
     private final IStrategoTerm expectedMatch;
     private final Region sourceRegion;
@@ -35,6 +38,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseToFragmentExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ParseToFragmentExpectation.java
@@ -11,6 +11,7 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.CoordinateRequirement;
 import mb.spoofax.core.language.LanguageInstance;
 import mb.spt.api.parse.TestableParse;
@@ -38,6 +39,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ResolveExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/ResolveExpectation.java
@@ -10,6 +10,7 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.language.LanguageInstance;
 import mb.spt.api.resolve.TestableResolve;
 import mb.spt.lut.LanguageUnderTestProvider;
@@ -18,6 +19,7 @@
 import mb.spt.model.TestCase;
 import mb.spt.model.TestExpectation;
 import mb.spt.util.SptSelectionUtil;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class ResolveExpectation implements TestExpectation {
     public final SelectionReference fromTerm;
@@ -44,6 +46,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/RunStrategoExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/RunStrategoExpectation.java
@@ -11,6 +11,7 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.language.LanguageInstance;
 import mb.spt.api.analyze.StrategoRunArgument;
 import mb.spt.api.stratego.TestableStratego;
@@ -21,6 +22,7 @@
 import mb.spt.model.TestCase;
 import mb.spt.model.TestExpectation;
 import mb.stratego.common.StrategoException;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.spoofax.interpreter.terms.IStrategoAppl;
 import org.spoofax.interpreter.terms.IStrategoInt;
 import org.spoofax.interpreter.terms.IStrategoString;
@@ -62,6 +64,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/TransformExpectation.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/TransformExpectation.java
@@ -9,6 +9,7 @@
 import mb.pie.api.Session;
 import mb.pie.api.exec.CancelToken;
 import mb.resource.ResourceKey;
+import mb.resource.hierarchical.ResourcePath;
 import mb.spoofax.core.language.command.CommandDef;
 import mb.spoofax.core.language.command.CommandFeedback;
 import mb.spt.lut.LanguageUnderTestProvider;
@@ -37,6 +38,7 @@ public KeyedMessages evaluate(
         LanguageUnderTest languageUnderTest,
         Session languageUnderTestSession,
         LanguageUnderTestProvider languageUnderTestProvider,
+        @Nullable ResourcePath rootDirectoryHint,
         ExecContext context,
         CancelToken cancel
     ) throws InterruptedException {
@@ -55,7 +57,7 @@ public KeyedMessages evaluate(
         final @Nullable Region selectionRegion = TransformExpectationUtil.getSelection(testCase, selectionReference);
 
         final @Nullable CommandFeedback feedback = TransformExpectationUtil.runCommand(testCase.resource, commandDef,
-            languageUnderTest, languageUnderTestSession, messagesBuilder, file, sourceRegion, selectionRegion);
+            languageUnderTest, languageUnderTestSession, messagesBuilder, file, rootDirectoryHint, sourceRegion, selectionRegion);
         if(feedback == null) {
             return messagesBuilder.build(file);
         }

diff --git a/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/TransformExpectationUtil.java b/lwb/metalang/spt/spt/src/main/java/mb/spt/expectation/TransformExpectationUtil.java
@@ -49,13 +49,17 @@ public class TransformExpectationUtil {
         Session languageUnderTestSession,
         KeyedMessagesBuilder messagesBuilder,
         ResourceKey failMessageFile,
+        @Nullable ResourcePath rootDirectoryHint,
         Region fileMessageRegion,
         @Nullable Region selection
     ) throws InterruptedException {
         try {
-            final CommandContext commandContext = CommandContext.ofReadableResource(resource, selection);
+            // Using `ofFile` is required to make `transform` test expectations work.
+            // Otherwise, the argument provider `Context(File)` will not pick up this resource.
+            final CommandContext commandContext = CommandContext.ofFile(resource, selection);
             commandContext.setEnclosing(EnclosingCommandContextType.Directory, CommandContext.ofDirectory(resource));
-            commandContext.setEnclosing(EnclosingCommandContextType.Project, CommandContext.ofProject(resource));
+            // If no root directory hint is given, use the resource itself as the project context.
+            commandContext.setEnclosing(EnclosingCommandContextType.Project, CommandContext.ofProject(rootDirectoryHint != null ? rootDirectoryHint : resource));
             final Task<CommandFeedback> task = commandDef.createTask(CommandExecutionType.ManualOnce, commandContext, new ArgConverters(languageUnderTest.getResourceServiceComponent().getResourceService()));
             return languageUnderTestSession.require(task);
         } catch(ExecException | ArgumentBuilderException e) {
@@ -82,7 +86,9 @@ public static boolean isSelectionValid(TestCase testCase, Option<SelectionRefere
         if(selectionReference.isNone()) {
             return null;
         } else {
-            final ListView<Region> availableSelections = testCase.testFragment.getInFragmentSelections();
+            // `getSelections()` returns the mapped regions, as the object language parser sees them.
+            // This corresponds to the regions Stratego strategies such as `origin-offset` etc. return.
+            final ListView<Region> availableSelections = testCase.testFragment.getSelections();
             return availableSelections.get(selectionReference.get().selection - 1);
         }
     }