diff --git a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Jass.g4 b/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Jass.g4 index c62e8c182..6238348f0 100644 --- a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Jass.g4 +++ b/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)*; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jass/AntlrJassParseTreeTransformer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jass/AntlrJassParseTreeTransformer.java index 769cd3f8d..b1f691093 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jass/AntlrJassParseTreeTransformer.java +++ b/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,8 +34,10 @@ 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)); } @@ -42,8 +45,16 @@ public CompilationUnit transform(JassParser.CompilationUnitContext cu) { 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()); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/WurstAntlrErrorListener.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/WurstAntlrErrorListener.java index a1b9d1976..e3a0dcfe3 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/WurstAntlrErrorListener.java +++ b/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--; } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java index 6c9c35020..824a56aa0 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java +++ b/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."; diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/JassSyntaxErrorTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/JassSyntaxErrorTests.java new file mode 100644 index 000000000..bcaf503d8 --- /dev/null +++ b/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 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 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 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()); + } +}