Merge pull request #1853 from lf-lang/reaction-syntax

More natural syntax for reaction declarations

cli/lfc/src/test/resources/org/lflang/cli/issue490.stderr

@@ -6,6 +6,14 @@ lfc: error: Name of main reactor must match the file name (or be omitted).
   |              ^ Name of main reactor must match the file name (or be omitted).
   |
 5 |   state liss(2, 3);
+lfc: error: The Python target does not support reaction declarations. Please specify a reaction body.
+--> %%%PATH.lf%%%:6:3
+  |
+5 |   state liss(2, 3);
+6 |   reaction (startup) {
+  |   ^^^^^^^^^^^^^^^^^^^^ The Python target does not support reaction declarations. Please specify a reaction body.
+  |
+7 |     print(self.liss)
 lfc: error: no viable alternative at input '{'
 --> %%%PATH.lf%%%:6:22
   |

core/src/main/java/org/lflang/LinguaFranca.xtext

@@ -200,10 +200,11 @@ Action:
 Reaction:
     (attributes+=Attribute)*
     (('reaction') | mutation ?= 'mutation')
+    (name=ID)?
     ('(' (triggers+=TriggerRef (',' triggers+=TriggerRef)*)? ')')
-    (sources+=VarRef (',' sources+=VarRef)*)?
+    ( => sources+=VarRef (',' sources+=VarRef)*)?
     ('->' effects+=VarRefOrModeTransition (',' effects+=VarRefOrModeTransition)*)?
-    ((('named' name=ID)? code=Code) | 'named' name=ID)(stp=STP)?(deadline=Deadline)?
+    (code=Code)? (stp=STP)? (deadline=Deadline)? (delimited?=';')?
     ;
 
 TriggerRef:
@@ -511,7 +512,7 @@ Token:
     'mutable' | 'input' | 'output' | 'timer' | 'action' | 'reaction' |
     'startup' | 'shutdown' | 'after' | 'deadline' | 'mutation' | 'preamble' |
     'new' | 'federated' | 'at' | 'as' | 'from' | 'widthof' | 'const' | 'method' |
-    'interleaved' | 'mode' | 'initial' | 'reset' | 'history' | 'watchdog' | 'named' |
+    'interleaved' | 'mode' | 'initial' | 'reset' | 'history' | 'watchdog' |
 
     // Other terminals
     NEGINT | TRUE | FALSE |

core/src/main/java/org/lflang/Target.java

@@ -490,6 +490,15 @@ public boolean supportsParameterizedWidths() {
     return true;
   }
 
+  /**
+   * Return true of reaction declarations (i.e., reactions without inlined code) are supported by
+   * this target.
+   */
+  public boolean supportsReactionDeclarations() {
+    if (this.equals(Target.C)) return true;
+    else return false;
+  }
+
   /**
    * Return true if this code for this target should be built using Docker if Docker is used.
    *

core/src/main/java/org/lflang/ast/ToLf.java

@@ -618,6 +618,7 @@ public MalleableString caseReaction(Reaction object) {
     } else {
       msb.append("reaction");
     }
+    if (object.getName() != null) msb.append(" ").append(object.getName());
     msb.append(list(true, object.getTriggers()));
     msb.append(list(", ", " ", "", true, false, object.getSources()));
     if (!object.getEffects().isEmpty()) {
@@ -639,7 +640,6 @@ public MalleableString caseReaction(Reaction object) {
                               : doSwitch(varRef))
                   .collect(new Joiner(", ")));
     }
-    if (object.getName() != null) msb.append(" named ").append(object.getName());
     if (object.getCode() != null) msb.append(" ").append(doSwitch(object.getCode()));
     if (object.getStp() != null) msb.append(" ").append(doSwitch(object.getStp()));
     if (object.getDeadline() != null) msb.append(" ").append(doSwitch(object.getDeadline()));

core/src/main/java/org/lflang/validation/LFValidator.java

@@ -24,6 +24,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  ***************/
+
 package org.lflang.validation;
 
 import static org.lflang.ast.ASTUtils.inferPortWidth;
@@ -42,6 +43,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.eclipse.emf.common.util.EList;
@@ -706,12 +708,35 @@ public void checkReaction(Reaction reaction) {
     if (reaction.getTriggers() == null || reaction.getTriggers().size() == 0) {
       warning("Reaction has no trigger.", Literals.REACTION__TRIGGERS);
     }
-    HashSet<Variable> triggers = new HashSet<>();
+
+    if (reaction.getCode() == null) {
+      if (!this.target.supportsReactionDeclarations()) {
+        error(
+            "The "
+                + this.target
+                + " target does not support reaction declarations. Please specify a reaction body.",
+            Literals.REACTION__CODE);
+        return;
+      }
+      if (reaction.getDeadline() == null && reaction.getStp() == null) {
+        var text = NodeModelUtils.findActualNodeFor(reaction).getText();
+        var matcher = Pattern.compile("\\)\\s*[\\n\\r]+(.*[\\n\\r])*.*->").matcher(text);
+        if (matcher.find()) {
+          error(
+              "A connection statement may have been unintentionally parsed as the sources and"
+                  + " effects of a reaction declaration. To correct this, add a semicolon at the"
+                  + " end of the reaction declaration. To instead silence this message, remove any"
+                  + " newlines between the reaction triggers and sources.",
+              Literals.REACTION__CODE);
+        }
+      }
+    }
+    HashSet<VarRef> triggers = new HashSet<>();
     // Make sure input triggers have no container and output sources do.
     for (TriggerRef trigger : reaction.getTriggers()) {
       if (trigger instanceof VarRef) {
         VarRef triggerVarRef = (VarRef) trigger;
-        triggers.add(triggerVarRef.getVariable());
+        triggers.add(triggerVarRef);
         if (triggerVarRef instanceof Input) {
           if (triggerVarRef.getContainer() != null) {
             error(
@@ -735,7 +760,14 @@ public void checkReaction(Reaction reaction) {
     // Make sure input sources have no container and output sources do.
     // Also check that a source is not already listed as a trigger.
     for (VarRef source : reaction.getSources()) {
-      if (triggers.contains(source.getVariable())) {
+      var duplicate =
+          triggers.stream()
+              .anyMatch(
+                  t -> {
+                    return t.getVariable().equals(source.getVariable())
+                        && t.getContainer().equals(source.getContainer());
+                  });
+      if (duplicate) {
         error(
             String.format(
                 "Source is already listed as a trigger: %s", source.getVariable().getName()),
@@ -1889,8 +1921,12 @@ private boolean sameType(Type type1, Type type2) {
     // Most common case first.
     if (type1.getId() != null) {
       if (type1.getStars() != null) {
-        if (type2.getStars() == null) return false;
-        if (type1.getStars().size() != type2.getStars().size()) return false;
+        if (type2.getStars() == null) {
+          return false;
+        }
+        if (type1.getStars().size() != type2.getStars().size()) {
+          return false;
+        }
       }
       return (type1.getId().equals(type2.getId()));
     }
@@ -1899,7 +1935,9 @@ private boolean sameType(Type type1, Type type2) {
     // (time?='time' (arraySpec=ArraySpec)?) | ((id=(DottedName) (stars+='*')* ('<'
     // typeParms+=TypeParm (',' typeParms+=TypeParm)* '>')? (arraySpec=ArraySpec)?) | code=Code);
     if (type1.isTime()) {
-      if (!type2.isTime()) return false;
+      if (!type2.isTime()) {
+        return false;
+      }
       // Ignore the arraySpec because that is checked when connection
       // is checked for balance.
       return true;

core/src/test/java/org/lflang/tests/compiler/LinguaFrancaValidationTest.java

@@ -176,6 +176,52 @@ public void disallowReactorCalledPreamble() throws Exception {
         "Reactor cannot be named 'Preamble'");
   }
 
+  @Test
+  public void requireSemicolonIfAmbiguous() throws Exception {
+    String testCase =
+        """
+            target C
+
+            reactor Foo {
+              output out: int
+              input inp: int
+              reaction(inp) -> out {==}
+            }
+
+            main reactor {
+              f1 = new Foo()
+              f2 = new Foo()
+              f3 = new Foo()
+
+              reaction increment(f1.out)
+              f2.out -> f3.inp
+            }
+
+            """;
+    validator.assertError(
+        parseWithoutError(testCase),
+        LfPackage.eINSTANCE.getReaction(),
+        null,
+        "A connection statement may have been unintentionally parsed");
+  }
+
+  @Test
+  public void noSemicolonIfNotAmbiguous() throws Exception {
+    String testCase =
+        """
+            target C
+
+            main reactor {
+              timer t(0)
+
+              reaction increment(t)
+              reaction multiply(t)
+            }
+
+            """;
+    validator.assertNoErrors(parseWithoutError(testCase));
+  }
+
   /** Ensure that "__" is not allowed at the start of an input name. */
   @Test
   public void disallowUnderscoreInputs() throws Exception {

test/C/src/no_inlining/BankMultiportToReactionNoInlining.lf

@@ -20,7 +20,7 @@ main reactor {
 
   s = new[2] DoubleCount()
 
-  reaction(s.out) named check
+  reaction check(s.out)
 
   reaction(shutdown) {=
     if (!self->received) {

test/C/src/no_inlining/BankToReactionNoInlining.lf

@@ -12,5 +12,5 @@ main reactor {
 
   s = new[2] Count()
 
-  reaction(s.out) named check
+  reaction check(s.out)
 }

test/C/src/no_inlining/Count.lf

@@ -8,9 +8,9 @@ main reactor Count {
 
   state count: int
 
-  reaction(t) named increment
+  reaction increment(t)
 
-  reaction(t) named check_done
+  reaction check_done(t)
 
   reaction(shutdown) {= printf("%s", "shutting down\n"); =}
 }

test/C/src/no_inlining/CountHierarchy.lf

@@ -15,9 +15,9 @@ main reactor {
 
   state count: int
 
-  reaction(t.out) named increment
+  reaction increment(t.out)
 
-  reaction(t.out) named check_done
+  reaction check_done(t.out)
 
   reaction(shutdown) {= printf("%s", "shutting down\n"); =}
 }

test/C/src/no_inlining/IntPrint.lf

@@ -6,14 +6,14 @@ target C {
 reactor Print {
   output out: int
 
-  reaction(startup) -> out named sender
+  reaction sender(startup) -> out
 }
 
 // expected parameter is for testing.
 reactor Check(expected: int = 42) {
   input in: int
 
-  reaction(in) named receiver
+  reaction receiver(in)
 }
 
 main reactor {

test/C/src/no_inlining/MultiportToReactionNoInlining.lf

@@ -23,7 +23,7 @@ main reactor {
   state s: int = 6
   b = new Source(width = 4)
 
-  reaction(b.out) named check
+  reaction check(b.out)
 
   reaction(shutdown) {=
     if (self->s <= 6) {