Report invalid input Jass instead of dropping it

de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Jass.g4

@@ -85,8 +85,10 @@ jassTypeDecl: 'type' name=ID 'extends' extended=jassTypeExpr NL;
 
 formalParam: jassTypeExpr name=ID;
 
+// A type is never empty: with an empty alternative `local x = 1` parsed as a local without a type, which the
+// translation cannot handle, and the declaration was dropped without an error.
 jassTypeExpr:
-    | typeName=ID
+      typeName=ID
     | jassTypeExpr 'array'
 ;
 
@@ -126,7 +128,8 @@ exprFuncRef: 'function' (scopeName=ID '.')? funcName=ID;
 
 argumentList: '(' exprList ')';
 
-exprFunctionCall: funcName=ID jassTypeExpr argumentList;
+// Jass has no type arguments, so nothing stands between the name and the arguments.
+exprFunctionCall: funcName=ID argumentList;
 
 exprList : exprs+=jassExpr (',' exprs+=jassExpr)*;
 

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jass/AntlrJassParseTreeTransformer.java

@@ -1,5 +1,6 @@
 package de.peeeq.wurstscript.jass;
 
+import de.peeeq.wurstscript.WLogger;
 import de.peeeq.wurstscript.WurstOperator;
 import de.peeeq.wurstscript.antlr.JassParser;
 import de.peeeq.wurstscript.ast.*;
@@ -33,17 +34,27 @@ public AntlrJassParseTreeTransformer(String file,
 
     public CompilationUnit transform(JassParser.CompilationUnitContext cu) {
         JassToplevelDeclarations jassDecls = Ast.JassToplevelDeclarations();
+        JassParser.JassTopLevelDeclarationContext current = null;
         try {
             for (JassParser.JassTopLevelDeclarationContext decl : cu.decls) {
+                current = decl;
                 jassDecls.add(transformJassToplevelDecl(decl));
             }
 
         } catch (CompileError e) {
             cuErrorHandler.sendError(e);
             e.printStackTrace();
         } catch (NullPointerException e) {
-            e.printStackTrace();
-            // ignore
+            // A part of the tree is missing. The recovery from a syntax error leaves such trees, and the error is
+            // reported already; anything else is a tree the translation does not know. Either way the rest of the
+            // file is not translated, which must never pass without an error. Since the grammar no longer accepts an
+            // empty type, no valid parse is known to get here: the JassSyntaxErrorTests reached it through the
+            // grammar as it was (an untyped local, global or parameter), before the grammar was fixed.
+            WLogger.warning("Error transforming Jass compilation unit " + file, e);
+            Token start = current == null ? cu.start : current.start;
+            cuErrorHandler.sendError(new CompileError(
+                new WPos(file, lineOffsets, start.getStartIndex(), start.getStopIndex() + 1),
+                "Could not translate this declaration, so it and the rest of the file are missing."));
         }
 
         return Ast.CompilationUnit(new CompilationUnitInfo(this.cuErrorHandler), jassDecls, Ast.WPackages());

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/WurstAntlrErrorListener.java

@@ -9,7 +9,6 @@
 import org.antlr.v4.runtime.misc.Interval;
 
 import java.util.function.Supplier;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public final class WurstAntlrErrorListener extends BaseErrorListener {
@@ -103,8 +102,8 @@ public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int
             posStop = Math.max(0, cs.size() - 1);
         }
 
-        Matcher matcher = WS.matcher(cs.getText(new Interval(pos, posStop)));
-        while (pos > 0 && matcher.matches()) {
+        // an error in whitespace (a newline, the end of the file) starts at the code before it
+        while (pos > 0 && WS.matcher(cs.getText(new Interval(pos, posStop))).matches()) {
             pos--;
         }
 

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

@@ -155,6 +155,38 @@ public void theCliBuildFailsOnATypeErrorFoundByItsSingleCheck() throws Exception
         assertNull(request.compiledModel, "nothing is compiled when the check finds errors");
     }
 
+    /**
+     * The map script is the input of a protected map. Jass in it which the parser cannot translate fails the build: a
+     * dropped declaration would ship a map whose script is missing it.
+     */
+    @Test
+    public void theCliBuildFailsOnInvalidJassInTheMapScript() throws Exception {
+        File projectFolder = new File("./temp/testProject_cli_invalid_map_script/");
+        File wurstFolder = new File(projectFolder, "wurst");
+        wurstFolder.mkdirs();
+        Files.writeString(new File(wurstFolder, "Wurst.wurst").toPath(), "package Wurst\n");
+        Files.writeString(new File(wurstFolder, "war3map.j").toPath(),
+            "function main takes nothing returns nothing\nendfunction\n");
+        File configured = new File(projectFolder, "configured.j");
+        Files.writeString(configured.toPath(),
+            "function main takes nothing returns nothing\n"
+                + "    local hero = \"h\"\n"
+                + "    call BJDebugMsg(hero)\n"
+                + "endfunction\n"
+                + "function config takes nothing returns nothing\nendfunction\n");
+
+        CountingModelManager modelManager = new CountingModelManager(projectFolder);
+        modelManager.loadProject();
+        assertFalse(modelManager.hasErrors(), "the project's own script is valid");
+        CapturingCliBuildMap request = new CapturingCliBuildMap(projectFolder);
+
+        RuntimeException e = expectThrows(RuntimeException.class, () -> request.compile(modelManager, configured));
+
+        assertTrue(e.getMessage().contains("missing ID at '='"), e.getMessage());
+        assertTrue(modelManager.hasErrors(), "the invalid local is an error");
+        assertNull(request.compiledModel, "nothing is compiled");
+    }
+
     private static final String NAME_WARNING = "Function names should start with an lower case character.";
     /** Reported by the lexer, when the project is loaded: no check of the model reports it. */
     private static final String INDENTATION_WARNING = "Use an even number of spaces for indentation.";

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

@@ -0,0 +1,127 @@
+package tests.wurstscript.tests;
+
+import de.peeeq.wurstscript.WurstParser;
+import de.peeeq.wurstscript.attributes.CompileError;
+import de.peeeq.wurstscript.attributes.ErrorHandler;
+import de.peeeq.wurstscript.gui.WurstGuiLogger;
+import org.testng.annotations.Test;
+
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+
+/**
+ * Input Jass which is not valid Jass must be a reported error. The fork protects maps whose script is parsed Jass, so a
+ * declaration which the parser accepts but cannot translate, and drops without a word, ships a broken map.
+ */
+public class JassSyntaxErrorTests extends WurstScriptTest {
+
+    private void assertJassError(String expectedError, String... lines) {
+        test().expectError(expectedError).compilationUnits(compilationUnit("u.j", lines));
+    }
+
+    @Test
+    public void untypedLocal() {
+        assertJassError("line 3: missing ID at '='",
+            "native Print takes string msg returns nothing",
+            "function main takes nothing returns nothing",
+            "    local hero = \"h\"",
+            "    call Print(hero)",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    @Test
+    public void untypedArrayLocal() {
+        assertJassError("extraneous input 'array' expecting ID",
+            "function main takes nothing returns nothing",
+            "    local array a",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    @Test
+    public void untypedGlobal() {
+        assertJassError("line 2: missing ID at '='",
+            "globals",
+            "    hero = 1",
+            "endglobals",
+            "function main takes nothing returns nothing",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    @Test
+    public void untypedParameter() {
+        assertJassError("missing ID at 'returns'",
+            "native Print takes msg returns nothing",
+            "function main takes nothing returns nothing",
+            "    call Print(\"h\")",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    @Test
+    public void missingReturnType() {
+        assertJassError("mismatched input '\\n' expecting {'nothing', ID}",
+            "function f takes nothing returns",
+            "endfunction",
+            "function main takes nothing returns nothing",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    @Test
+    public void wordBetweenFunctionNameAndArguments() {
+        assertJassError("extraneous input 'integer' expecting '('",
+            "native Print takes string msg returns nothing",
+            "function main takes nothing returns nothing",
+            "    call Print integer(\"h\")",
+            "endfunction",
+            "function config takes nothing returns nothing",
+            "endfunction");
+    }
+
+    /** More syntax errors than the parser reports: the file is given up, and what it reported fails the compilation. */
+    @Test
+    public void manySyntaxErrors() {
+        List<String> lines = new ArrayList<>();
+        lines.add("function main takes nothing returns nothing");
+        for (int i = 0; i < 40; i++) {
+            lines.add("    call");
+        }
+        lines.add("endfunction");
+        lines.add("function config takes nothing returns nothing");
+        lines.add("endfunction");
+        assertJassError("line 2: mismatched input '\\n' expecting ID", lines.toArray(new String[0]));
+    }
+
+    private static List<CompileError> parseJass(String source) {
+        WurstGuiLogger gui = new WurstGuiLogger();
+        new WurstParser(new ErrorHandler(gui), gui).parseJass(new StringReader(source), "u.j", false);
+        return gui.getErrorList();
+    }
+
+    /**
+     * An error at the end of the file is reported at the code before it, not at the first character of the file: the
+     * start of the error is moved back over the whitespace it is in, and only over that.
+     */
+    @Test
+    public void anErrorAtTheEndOfTheFileIsReportedAtTheLastCode() {
+        List<CompileError> errors = parseJass(
+            "native A takes nothing returns nothing\n"
+                + "function f takes nothing returns nothing\n"
+                + "    call A()\n"
+                + "\n\n   \n");
+        assertFalse(errors.isEmpty(), "the missing endfunction is reported");
+        assertEquals(errors.get(0).getSource().getLine(), 3, errors.get(0).toString());
+    }
+}