Correctly detect loop assignment as variable read & make loop variable immutable (#1165)

Author: Frotty

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyPrinter.java

@@ -876,7 +876,7 @@ public static void prettyPrint(LocalVarDef e, Spacer spacer, StringBuilder sb, i
         printCommentsBefore(sb, e, indent);
         printIndent(sb, indent);
         if ((e.getOptTyp() instanceof NoTypeExpr)) {
-            if (e.attrIsConstant()) {
+            if (e.attrIsConstant() && !(e.getParent() instanceof StmtForRange)) {
                 sb.append("let");
             } else if (!(e.getParent() instanceof StmtForRange)) {
                 sb.append("var");

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java

@@ -782,8 +782,8 @@ private WStatement transformForLoop(StmtForLoopContext s) {
 
     private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         WPos source = source(s);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar);
-        loopVar.setInitialExpr(transformExpr(s.start));
+        Expr start = transformExpr(s.start);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
         Expr to = transformExpr(s.end);
         Expr step;
         if (s.step == null) {
@@ -800,17 +800,20 @@ private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         throw error(s, "for range loop not implemented: " + text(s));
     }
 
-    private LocalVarDef transformLocalVarDef(LocalVarDefInlineContext v) {
+    private LocalVarDef transformLocalVarDef(LocalVarDefInlineContext v, OptExpr initialExpr, boolean constant) {
         Modifiers modifiers = Ast.Modifiers();
+        if (constant) {
+            // Range-loop variables behave like let-variables for user code.
+            modifiers.add(Ast.ModConstant(source(v)));
+        }
         OptTypeExpr optTyp = transformOptionalType(v.typeExpr());
         Identifier name = text(v.name);
-        OptExpr initialExpr = Ast.NoExpr();
         return Ast.LocalVarDef(source(v), modifiers, optTyp, name, initialExpr);
     }
 
     private WStatement transformForIteratorLoop(ForIteratorLoopContext s) {
         WPos source = source(s);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, Ast.NoExpr(), false);
         Expr in = transformExpr(s.iteratorExpr);
         WStatements body = transformStatements(s.statementsBlock());
         if (s.iterStyle.getType() == JurstParser.IN) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java

@@ -946,8 +946,8 @@ private WStatement transformForLoop(StmtForLoopContext s) {
 
     private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         WPos source = source(s);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar);
-        loopVar.setInitialExpr(transformExpr(s.start));
+        Expr start = transformExpr(s.start);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
         Expr to = transformExpr(s.end);
         Expr step;
         if (s.step == null) {
@@ -964,17 +964,20 @@ private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         throw error(s, "not implemented: " + text(s));
     }
 
-    private LocalVarDef transformLocalVarDef(LocalVarDefInlineContext v) {
+    private LocalVarDef transformLocalVarDef(LocalVarDefInlineContext v, OptExpr initialExpr, boolean constant) {
         Modifiers modifiers = Ast.Modifiers();
+        if (constant) {
+            // Range-loop variables behave like let-variables for user code.
+            modifiers.add(Ast.ModConstant(source(v)));
+        }
         OptTypeExpr optTyp = transformOptionalType(v.typeExpr());
         Identifier name = text(v.name);
-        OptExpr initialExpr = Ast.NoExpr();
         return Ast.LocalVarDef(source(v), modifiers, optTyp, name, initialExpr);
     }
 
     private WStatement transformForIteratorLoop(ForIteratorLoopContext s) {
         WPos source = source(s);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, Ast.NoExpr(), false);
         Expr in = transformExpr(s.iteratorExpr);
         WStatements body = transformStatements(s.statementsBlock());
         if (s.iterStyle.getType() == WurstParser.IN) {

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/BugTests.java

@@ -898,6 +898,37 @@ public void unreadVarWarning3() { // #380
         );
     }
 
+    @Test
+    public void forRangeStartReadsParameter() {
+        CompilationResult result = test()
+            .setStopOnFirstError(false)
+            .executeProg(false)
+            .lines(
+                "package test",
+                "function foo(int bar)",
+                "    for i = bar to 10",
+                "        skip",
+                "endpackage"
+            );
+
+        Assert.assertTrue(
+            result.getGui().getWarningList().stream()
+                .noneMatch(w -> w.getMessage().contains("The parameter bar is never read")),
+            "Unexpected unused warning for parameter bar: " + result.getGui().getWarningList()
+        );
+    }
+
+    @Test
+    public void forRangeLoopVarIsImmutable() {
+        testAssertErrorsLines(false, "Cannot assign a new value to constant",
+            "package test",
+            "init",
+            "    int stackPointer = 3",
+            "    for i = 0 to stackPointer",
+            "        i--",
+            "endpackage");
+    }
+
 
     @Test
     public void unreadVarWarningArrays() { // #813