From 785fe81d2e4255d6efd8072ab9e88ac229edb261 Mon Sep 17 00:00:00 2001 From: LadyCailin Date: Fri, 10 Jun 2022 18:17:57 +0200 Subject: [PATCH 1/3] Initial commit, doesn't build yet --- .../core/MethodScriptCompiler.java | 171 ++++++++++++------ .../core/compiler/BranchStatement.java | 11 ++ .../core/compiler/CompilerEnvironment.java | 4 + .../compiler/ConditionalSelfStatement.java | 17 ++ .../core/compiler/FileOptions.java | 4 +- .../laytonsmith/core/compiler/Keyword.java | 13 +- .../core/compiler/TokenStream.java | 4 +- .../core/compiler/keywords/DoKeyword.java | 2 +- .../core/compiler/keywords/ForKeyword.java | 4 +- .../compiler/keywords/ForeachKeyword.java | 4 +- .../core/compiler/keywords/IfKeyword.java | 6 +- .../core/compiler/keywords/ProcKeyword.java | 15 +- .../keywords/SimpleBlockKeywordFunction.java | 2 +- .../core/compiler/keywords/TryKeyword.java | 6 +- .../laytonsmith/core/functions/Compiler.java | 11 ++ .../core/functions/ControlFlow.java | 15 ++ .../laytonsmith/core/functions/Function.java | 8 +- .../com/laytonsmith/core/functions/Meta.java | 12 ++ src/main/resources/docs/Statements | 10 +- .../core/MethodScriptCompilerTest.java | 18 +- .../core/NewExceptionHandlingTest.java | 5 +- .../laytonsmith/core/OptimizationTest.java | 87 +++++++-- .../core/functions/BasicLogicTest.java | 4 +- 23 files changed, 313 insertions(+), 120 deletions(-) create mode 100644 src/main/java/com/laytonsmith/core/compiler/ConditionalSelfStatement.java diff --git a/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java b/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java index 315f3f0b48..07e1872ec4 100644 --- a/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java +++ b/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java @@ -12,6 +12,7 @@ import com.laytonsmith.core.compiler.BranchStatement; import com.laytonsmith.core.compiler.CompilerEnvironment; import com.laytonsmith.core.compiler.CompilerWarning; +import com.laytonsmith.core.compiler.ConditionalSelfStatement; import com.laytonsmith.core.compiler.EarlyBindingKeyword; import com.laytonsmith.core.compiler.FileOptions; import com.laytonsmith.core.compiler.FileOptions.SuppressWarning; @@ -1915,7 +1916,7 @@ public static ParseTree compile(TokenStream stream, Environment environment, rewriteAutoconcats(tree, environment, envs, compilerErrors, true); processLateKeywords(tree, environment, compilerErrors); checkLinearComponents(tree, environment, compilerErrors); - postParseRewrite(rootNode, environment, envs, compilerErrors); // Pass rootNode since this might rewrite 'tree'. + postParseRewrite(rootNode, environment, envs, compilerErrors, true); // Pass rootNode since this might rewrite 'tree'. moveNodeModifiersOffSyntheticNodes(tree); tree = rootNode.getChildAt(0); staticAnalysis.analyze(tree, environment, envs, compilerErrors); @@ -2081,7 +2082,8 @@ private static void addSelfStatements(ParseTree root, Environment env, boolean isSelfStatement; try { isSelfStatement = node.getData() instanceof CFunction cf - && cf.getCachedFunction() != null + && cf.hasFunction() + && cf.getFunction() != null && cf.getCachedFunction().isSelfStatement(node.getTarget(), env, node.getChildren(), envs); } catch(ConfigCompileException ex) { compilerErrors.add(ex); @@ -2174,6 +2176,13 @@ public static void rewriteAutoconcats(ParseTree root, Environment env, } else { statements.addChild(autoconcat); } + if(rewriteKeywords + && autoconcat.getData() instanceof CFunction cf + && autoconcat.getData().val().equals(Compiler.__statements__.NAME) + && autoconcat.getChildren().size() > 1 + && root.getFileOptions().isStrict()) { + doMissingSemicolonError(autoconcat.getChildAt(0).getTarget(), compilerExceptions); + } } if(statements.getChildren().size() == 1 && statements.getChildAt(0).getData() instanceof CFunction cf) { if(statements.getChildAt(0).isSyntheticNode() @@ -2211,28 +2220,62 @@ public static void rewriteAutoconcats(ParseTree root, Environment env, try { ParseTree ret = __autoconcat__.rewrite(root.getChildren(), returnSConcat, envs); root.replace(ret); + // TODO: Remove this. This is a stopgap measure, because some keyword handlers + // don't remove the keywords before this step, and they are handled in a postParseRewrite + // override. This goes against the idea of the keyword handlers, but in the meantime, + // it means that we can't always detect if it's supposed to be a statement or not, + // so just bypass the check in this case. + boolean hasKeyword = false; + for(ParseTree node : root.getChildren()) { + if(node.getData() instanceof CKeyword) { + hasKeyword = true; + break; + } + } + if(rewriteKeywords && !hasKeyword && root.getFileOptions().isStrict() + && root.getData() instanceof CFunction cf + && cf.val().equals(Compiler.__statements__.NAME) + && ret.numberOfChildren() > 1 + && !ongoingChildren.isEmpty()) { + // The last statement was expected to have a semicolon, but didn't. + doMissingSemicolonError(root.getChildAt(root.numberOfChildren() - 1) + .getTarget(), compilerExceptions); + } } catch (ConfigCompileException ex) { compilerExceptions.add(ex); } } } + private static void doMissingSemicolonError(Target target, Set exceptions) { + String message = "Semicolon ';' expected."; +// if(MSVersion.LATEST.lte(new SimpleVersion(3, 3, 6))) { +// // Warning +// message += " This will be an error in the next version."; +// env.getEnv(CompilerEnvironment.class).addFutureErrorCompilerWarning(message, target); +// } else { + // Error + exceptions.add(new ConfigCompileException(message, target)); +// } + } + /** * Allows functions to perform a rewrite step to rewrite the AST as received from the parser to a valid * executable AST. Optimizations should not yet be performed in this rewrite step. * Additionally, this step traverses all {@link CFunction} nodes and ensures that they either have their represented * function cached or are unknown by the compiler. * Traversal is pre-order depth-first. - * @param ast - The abstract syntax tree representing this function. - * @param env - The environment. - * @param envs - The set of expected environment classes at runtime. - * @param exceptions - A set to put compile errors in. + * @param ast The abstract syntax tree representing this function. + * @param env The environment. + * @param envs The set of expected environment classes at runtime. + * @param exceptions A set to put compile errors in. + * @param topLevel True if this is the top level of the parse tree. External code should always pass in true here. * @return The rewritten AST node that should completely replace the AST node representing this function, or * {@code null} to not replace this AST node. Note that the rewrite will be called on this newly returned AST node * if it is different from the passed node. */ private static ParseTree postParseRewrite(ParseTree ast, Environment env, - Set> envs, Set exceptions) { + Set> envs, Set exceptions, boolean topLevel) { Mixed node = ast.getData(); if(node instanceof CFunction cFunc) { if(cFunc.hasFunction()) { @@ -2247,9 +2290,10 @@ private static ParseTree postParseRewrite(ParseTree ast, Environment env, } } } + boolean isStrict = ast.getFileOptions().isStrict(); for(int i = 0; i < ast.numberOfChildren(); i++) { ParseTree child = ast.getChildAt(i); - ParseTree newChild = postParseRewrite(child, env, envs, exceptions); + ParseTree newChild = postParseRewrite(child, env, envs, exceptions, false); if(newChild != null && child != newChild) { ast.getChildren().set(i, newChild); i--; // Allow the new child to do a rewrite step as well. @@ -2259,66 +2303,83 @@ private static ParseTree postParseRewrite(ParseTree ast, Environment env, // statements are not acceptable because void would be an invalid argument type. This would otherwise be // a runtime error in strict mode where auto-concat is not allowed and statements are used instead. // This can be removed once a more comprehensive void return type check is implemented. - if(child.getData() instanceof CFunction - && child.getData().val().equals(Compiler.__statements__.NAME) - && ast.getData() instanceof CFunction cFunction) { - Function function = cFunction.getCachedFunction(); - if(function == null) { - exceptions.add(new ConfigCompileException("Unknown function", cFunction.getTarget())); - continue; + if(ast.getData() instanceof CNull || ast.getData() instanceof CFunction) { + Function function; + if(ast.getData() instanceof CFunction cFunction) { + function = cFunction.getCachedFunction(); + } else { + function = null; } - boolean statementsAllowed = false; + boolean statementsAllowed = topLevel; if(function instanceof BranchStatement branchStatement) { - List branches = branchStatement.isBranch(ast.getChildren()); + List branches = branchStatement.statementsAllowed(ast.getChildren()); if(branches.get(i)) { statementsAllowed = true; } } + if(child.getData() instanceof CFunction) { + if(child.getData().val().equals(Compiler.__statements__.NAME)) { + if(!(ast.getData() instanceof CNull) && function == null) { + exceptions.add(new ConfigCompileException("Unknown function", ast.getTarget())); + continue; + } - if(!statementsAllowed) { - String unexpectedStatement = "Unexpected statement, semicolon (;) not allowed in this context."; - try { - if(ast.getData() instanceof CFunction cf - && cf.getCachedFunction().isSelfStatement(ast.getTarget(), env, ast.getChildren(), envs)) { - // We can give a better error message here, because the semicolon wasn't actually added - // by the user (probably) it's the self statement function that's the problem here. - unexpectedStatement = "Unexpected statement, " + ast.getData().val() - + " not allowed in this context."; - } - } catch(ConfigCompileException ex) { - exceptions.add(ex); - } - if(ast.getFileOptions().isStrict()) { - exceptions.add(new ConfigCompileException(unexpectedStatement, child.getTarget())); - } else { - // Statements aren't allowed here, but we aren't in strict mode, so - // pull up the value in the statement to here. sconcat is an exception to this rule, since - // it's entirely too special. - if(!(ast.getData() instanceof CFunction cf) || !cf.val().equals(StringHandling.sconcat.NAME)) { - if(child.getChildren().size() != 1) { + if(!statementsAllowed) { + String unexpectedStatement = "Unexpected statement, semicolon (;) not allowed in this context."; + try { + if(ast.getData() instanceof CFunction cf + && cf.getCachedFunction().isSelfStatement(ast.getTarget(), env, ast.getChildren(), envs)) { + // We can give a better error message here, because the semicolon wasn't actually added + // by the user (probably) it's the self statement function that's the problem here. + unexpectedStatement = "Unexpected statement, " + ast.getData().val() + + " not allowed in this context."; + } + } catch(ConfigCompileException ex) { + exceptions.add(ex); + } + if(ast.getFileOptions().isStrict()) { exceptions.add(new ConfigCompileException(unexpectedStatement, child.getTarget())); } else { - CompilerWarning warning = new CompilerWarning(unexpectedStatement, - child.getTarget(), SuppressWarning.UnexpectedStatement); - env.getEnv(CompilerEnvironment.class).addCompilerWarning(ast.getFileOptions(), warning); - ast.getChildAt(i).replace(child.getChildren().get(0)); + // Statements aren't allowed here, but we aren't in strict mode, so + // pull up the value in the statement to here. sconcat is an exception to this rule, since + // it's entirely too special. + if(!(ast.getData() instanceof CFunction cf) || !cf.val().equals(StringHandling.sconcat.NAME)) { + if(child.getChildren().size() != 1) { + exceptions.add(new ConfigCompileException(unexpectedStatement, child.getTarget())); + } else { + CompilerWarning warning = new CompilerWarning(unexpectedStatement, + child.getTarget(), SuppressWarning.UnexpectedStatement); + env.getEnv(CompilerEnvironment.class).addCompilerWarning(ast.getFileOptions(), warning); + ast.getChildAt(i).replace(child.getChildren().get(0)); + } + } + } + } + if(statementsAllowed) { + continue; + } + if(function.getName().equals(Compiler.__statements__.NAME)) { + ParseTree lastChild = child; + if(child.numberOfChildren() > 0) { + lastChild = child.getChildAt(child.numberOfChildren() - 1); } + exceptions.add(new ConfigCompileException("Invalid comma after " + + lastChild.getData().val(), lastChild.getTarget())); + } else if(ast.getFileOptions().isStrict()) { + exceptions.add(new ConfigCompileException("Invalid use of auto concat in " + + function.getName() + "()", ast.getTarget())); + } + } else if(statementsAllowed) { + // It is a function, but it isn't a statement. Check for strict mode. + if(isStrict) { + doMissingSemicolonError(child.getTarget(), exceptions); } } - } - if(statementsAllowed) { - continue; - } - if(function.getName().equals(Compiler.__statements__.NAME)) { - ParseTree lastChild = child; - if(child.numberOfChildren() > 0) { - lastChild = child.getChildAt(child.numberOfChildren() - 1); + } else if(statementsAllowed) { + if(((topLevel && isStrict) || !topLevel) && (function == null + || function.getClass().getAnnotation(ConditionalSelfStatement.class) == null)) { + exceptions.add(new ConfigCompileException("Not a statement.", child.getTarget())); } - exceptions.add(new ConfigCompileException("Invalid comma after " - + lastChild.getData().val(), lastChild.getTarget())); - } else if(ast.getFileOptions().isStrict()) { - exceptions.add(new ConfigCompileException("Invalid use of auto concat in " - + function.getName() + "()", cFunction.getTarget())); } } } diff --git a/src/main/java/com/laytonsmith/core/compiler/BranchStatement.java b/src/main/java/com/laytonsmith/core/compiler/BranchStatement.java index 194f0cca57..6007b5a342 100644 --- a/src/main/java/com/laytonsmith/core/compiler/BranchStatement.java +++ b/src/main/java/com/laytonsmith/core/compiler/BranchStatement.java @@ -36,4 +36,15 @@ public interface BranchStatement { * @return A list of booleans representing which children are branch statements. */ List isBranch(List children); + + /** + * Returns a list of booleans for places where statements are allowed. By default, this is the same list as + * isBranch, but there are a few places where this is an exception, for instance for(), where the third parameter + * is a branch (that is, it might or might not run) but statements aren't allowed. + * @param children + * @return + */ + default List statementsAllowed(List children) { + return isBranch(children); + } } diff --git a/src/main/java/com/laytonsmith/core/compiler/CompilerEnvironment.java b/src/main/java/com/laytonsmith/core/compiler/CompilerEnvironment.java index 758cde5c8c..38e6cf4a9f 100644 --- a/src/main/java/com/laytonsmith/core/compiler/CompilerEnvironment.java +++ b/src/main/java/com/laytonsmith/core/compiler/CompilerEnvironment.java @@ -134,6 +134,10 @@ public void setLogCompilerWarnings(boolean logCompilerWarnings) { this.logCompilerWarnings = logCompilerWarnings; } + public void addFutureErrorCompilerWarning(String warning, Target t) { + addCompilerWarning(null, new CompilerWarning(warning, t, FileOptions.SuppressWarning.FutureError)); + } + /** * Adds the compiler warning object to the environment, which can be used later by tools and such in a tool * specific way. Additionally, unless set in the environment otherwise, also logs the warning to the console. diff --git a/src/main/java/com/laytonsmith/core/compiler/ConditionalSelfStatement.java b/src/main/java/com/laytonsmith/core/compiler/ConditionalSelfStatement.java new file mode 100644 index 0000000000..7b58378dac --- /dev/null +++ b/src/main/java/com/laytonsmith/core/compiler/ConditionalSelfStatement.java @@ -0,0 +1,17 @@ +package com.laytonsmith.core.compiler; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * A ConditionalSelfStatement is a function which is sometimes a self statement, but others not, depending on + * the argument list. This list should remain small and non-growing, however, this requires special handling in the + * compiler to determine if a statement is missing or not. + */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +public @interface ConditionalSelfStatement { + +} diff --git a/src/main/java/com/laytonsmith/core/compiler/FileOptions.java b/src/main/java/com/laytonsmith/core/compiler/FileOptions.java index e1227b4a60..5ed2c7cc36 100644 --- a/src/main/java/com/laytonsmith/core/compiler/FileOptions.java +++ b/src/main/java/com/laytonsmith/core/compiler/FileOptions.java @@ -376,7 +376,9 @@ public static enum SuppressWarning implements Documentation { + " rid of it, place a semicolon at the end of the previous line (if you don't mean" + " for it to be executed) or move the left parenthesis up to the same line (if you " + " do mean for it to be executed).", MSVersion.V3_3_5, SeverityLevel.HIGH), - MalformedComment("The comment related to this element is malformed.", MSVersion.V3_3_5, SeverityLevel.HIGH); + MalformedComment("The comment related to this element is malformed.", MSVersion.V3_3_5, SeverityLevel.HIGH), + FutureError("This warning will be changed into a compile error in future versions, and should not be suppressed.", + MSVersion.V3_3_5, SeverityLevel.HIGH); private SuppressWarning(String docs, Version version, SeverityLevel severityLevel) { this.docs = docs; diff --git a/src/main/java/com/laytonsmith/core/compiler/Keyword.java b/src/main/java/com/laytonsmith/core/compiler/Keyword.java index 0b4ba8faea..ff1947182c 100644 --- a/src/main/java/com/laytonsmith/core/compiler/Keyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/Keyword.java @@ -4,9 +4,10 @@ import com.laytonsmith.core.Documentation; import com.laytonsmith.core.ParseTree; import com.laytonsmith.core.constructs.CFunction; -import com.laytonsmith.core.constructs.CNull; import com.laytonsmith.core.exceptions.ConfigCompileException; import com.laytonsmith.core.functions.Compiler.__cbrace__; +import com.laytonsmith.core.functions.Compiler.__statements__; +import com.laytonsmith.core.functions.Meta; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -96,14 +97,18 @@ protected static boolean isCodeBlock(ParseTree node) { } /** - * Returns a CNull, if the node is empty, or the first argument to the node + * Returns a {@code noop();}, if the node is empty, or the first argument to the node if it isn't. * * @param node * @return */ - protected static ParseTree getArgumentOrNull(ParseTree node) { + protected static ParseTree getArgumentOrNoop(ParseTree node) { if(node.getChildren().isEmpty()) { - return new ParseTree(CNull.NULL, node.getFileOptions()); + FileOptions options = node.getFileOptions(); + ParseTree statement = new ParseTree(new CFunction(__statements__.NAME, + com.laytonsmith.core.constructs.Target.UNKNOWN), options, true); + statement.addChild(new ParseTree(new CFunction(Meta.noop.NAME, com.laytonsmith.core.constructs.Target.UNKNOWN), options, true)); + return statement; } else { return node.getChildAt(0); } diff --git a/src/main/java/com/laytonsmith/core/compiler/TokenStream.java b/src/main/java/com/laytonsmith/core/compiler/TokenStream.java index 019c10685f..f56520fdf6 100644 --- a/src/main/java/com/laytonsmith/core/compiler/TokenStream.java +++ b/src/main/java/com/laytonsmith/core/compiler/TokenStream.java @@ -74,7 +74,7 @@ public static FileOptions parseFileOptions(String options, Map d } else if(c == ';') { //We're done inKey = true; - map.put(keyName.trim().toLowerCase(), buffer.toString()); + map.put(keyName.trim().toLowerCase(), buffer.toString().trim()); buffer = new StringBuilder(); } else { buffer.append(c); @@ -83,7 +83,7 @@ public static FileOptions parseFileOptions(String options, Map d } if(buffer.length() > 0) { if(!inKey) { - map.put(keyName.trim().toLowerCase(), buffer.toString()); + map.put(keyName.trim().toLowerCase(), buffer.toString().trim()); } else { if(!buffer.toString().trim().isEmpty()) { map.put(buffer.toString().trim().toLowerCase(), "true"); diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/DoKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/DoKeyword.java index 03a3904c96..c4871a6f67 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/DoKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/DoKeyword.java @@ -35,7 +35,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi } ParseTree dowhile = new ParseTree( new CFunction(_dowhile.NAME, t), list.get(keywordPosition).getFileOptions()); - dowhile.addChild(Keyword.getArgumentOrNull(codeTree)); + dowhile.addChild(Keyword.getArgumentOrNoop(codeTree)); dowhile.addChild(whileTree.getChildAt(0)); list.set(keywordPosition, dowhile); list.remove(keywordPosition + 2); diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/ForKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/ForKeyword.java index 8c344bc203..6d08d871cf 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/ForKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/ForKeyword.java @@ -27,7 +27,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi ParseTree codeBlock = list.get(keywordPosition + 1); if(isCodeBlock(codeBlock)) { validateCodeBlock(codeBlock, ""); - forTree.addChild(getArgumentOrNull(codeBlock)); + forTree.addChild(getArgumentOrNoop(codeBlock)); list.remove(keywordPosition + 1); } } @@ -40,7 +40,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi ParseTree codeBlock = list.get(keywordPosition + 1); if(isCodeBlock(codeBlock)) { validateCodeBlock(codeBlock, ""); - forTree.addChild(getArgumentOrNull(codeBlock)); + forTree.addChild(getArgumentOrNoop(codeBlock)); } // We also have to refactor this into a foreachelse, instead of a foreach. list.get(keywordPosition).setData(new CFunction(forelse.NAME, t)); diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/ForeachKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/ForeachKeyword.java index 6a73b418ae..fab9cc0894 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/ForeachKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/ForeachKeyword.java @@ -28,7 +28,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi ParseTree codeBlock = list.get(keywordPosition + 1); if(isCodeBlock(codeBlock)) { validateCodeBlock(codeBlock, ""); - foreach.addChild(getArgumentOrNull(codeBlock)); + foreach.addChild(getArgumentOrNoop(codeBlock)); list.remove(keywordPosition + 1); } } @@ -41,7 +41,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi ParseTree codeBlock = list.get(keywordPosition + 1); if(isCodeBlock(codeBlock)) { validateCodeBlock(codeBlock, ""); - foreach.addChild(getArgumentOrNull(codeBlock)); + foreach.addChild(getArgumentOrNoop(codeBlock)); } // We also have to refactor this into a foreachelse, instead of a foreach. list.get(keywordPosition).setData(new CFunction(FOREACHELSE, t)); diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/IfKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/IfKeyword.java index e0f5c7d8da..f343b5b062 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/IfKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/IfKeyword.java @@ -43,7 +43,7 @@ && nodeIsIfFunction(list.get(keywordPosition + 3))) { } catch (IndexOutOfBoundsException ex) { // Doesn't matter, we're apparently at the end of the stream } - node.addChild(getArgumentOrNull(list.get(keywordPosition + 1))); + node.addChild(getArgumentOrNoop(list.get(keywordPosition + 1))); list.remove(keywordPosition + 1); } @@ -54,7 +54,7 @@ && nodeIsIfFunction(list.get(keywordPosition + 3))) { if(isCodeBlock(list.get(keywordPosition + 2))) { // So ends the chain validateCodeBlock(list.get(keywordPosition + 2), ""); - node.addChild(getArgumentOrNull(list.get(keywordPosition + 2))); + node.addChild(getArgumentOrNoop(list.get(keywordPosition + 2))); // remove the else keyword + the brace list.remove(keywordPosition + 1); list.remove(keywordPosition + 1); @@ -73,7 +73,7 @@ && nodeIsIfFunction(list.get(keywordPosition + 3))) { } // Ok, checks are complete, so we can actually construct the arguments now node.addChild(list.get(keywordPosition + 2).getChildAt(0)); - node.addChild(getArgumentOrNull(list.get(keywordPosition + 3))); + node.addChild(getArgumentOrNoop(list.get(keywordPosition + 3))); // Remove the else, if function, and braces list.remove(keywordPosition + 1); list.remove(keywordPosition + 1); diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/ProcKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/ProcKeyword.java index 903fe8a5f8..5a04ee64cc 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/ProcKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/ProcKeyword.java @@ -9,11 +9,12 @@ import com.laytonsmith.core.constructs.CBareString; import com.laytonsmith.core.constructs.CFunction; import com.laytonsmith.core.constructs.CKeyword; -import com.laytonsmith.core.constructs.CNull; import com.laytonsmith.core.constructs.CString; import com.laytonsmith.core.constructs.Target; import com.laytonsmith.core.exceptions.ConfigCompileException; import com.laytonsmith.core.functions.DataHandling; +import com.laytonsmith.core.functions.Meta; +import com.laytonsmith.core.functions.Compiler; import java.util.List; /** @@ -45,11 +46,15 @@ public int process(List list, int keywordPosition) throws ConfigCompi if(list.get(keywordPosition + 2).getData() instanceof CFunction cf && com.laytonsmith.core.functions.Compiler.__cbrace__.NAME.equals(cf.val())) { validateCodeBlock(list.get(keywordPosition + 2), "Expected braces to follow proc definition"); - procNode.addChild(getArgumentOrNull(list.get(keywordPosition + 2))); + procNode.addChild(getArgumentOrNoop(list.get(keywordPosition + 2))); } else { - // Forward declaration, add a null "implementation" + // Forward declaration, add a noop "implementation" forwardDeclaration = true; - procNode.addChild(new ParseTree(CNull.NULL, list.get(keywordPosition + 1).getFileOptions(), true)); + ParseTree statement = new ParseTree(new CFunction(Compiler.__statements__.NAME, Target.UNKNOWN), + list.get(keywordPosition + 1).getFileOptions(), true); + statement.addChild(new ParseTree(new CFunction(Meta.noop.NAME, Target.UNKNOWN), + list.get(keywordPosition + 1).getFileOptions(), true)); + procNode.addChild(statement); } } else { throw new ConfigCompileException("Expected braces to follow proc definition", list.get(keywordPosition + 1).getTarget()); @@ -75,7 +80,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi // It's the functional usage, possibly followed by a cbrace. If so, pull the cbrace in, and that's it if(list.size() > keywordPosition + 1) { if(isValidCodeBlock(list.get(keywordPosition + 1))) { - list.get(keywordPosition).addChild(getArgumentOrNull(list.get(keywordPosition + 1))); + list.get(keywordPosition).addChild(getArgumentOrNoop(list.get(keywordPosition + 1))); list.remove(keywordPosition + 1); } } diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/SimpleBlockKeywordFunction.java b/src/main/java/com/laytonsmith/core/compiler/keywords/SimpleBlockKeywordFunction.java index 5703f0d281..4f2546630d 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/SimpleBlockKeywordFunction.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/SimpleBlockKeywordFunction.java @@ -63,7 +63,7 @@ public static int doProcess(Keyword keyword, Integer[] functionArgumentCount, bo } } } - list.get(keywordPosition).addChild(getArgumentOrNull(code)); + list.get(keywordPosition).addChild(getArgumentOrNoop(code)); list.remove(keywordPosition + 1); } } else { diff --git a/src/main/java/com/laytonsmith/core/compiler/keywords/TryKeyword.java b/src/main/java/com/laytonsmith/core/compiler/keywords/TryKeyword.java index 9de7f4c6c2..c1d5b966f2 100644 --- a/src/main/java/com/laytonsmith/core/compiler/keywords/TryKeyword.java +++ b/src/main/java/com/laytonsmith/core/compiler/keywords/TryKeyword.java @@ -53,7 +53,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi ParseTree complexTry = new ParseTree(new CFunction(complex_try.NAME, list.get(keywordPosition).getTarget()), list.get(keywordPosition).getFileOptions()); - complexTry.addChild(getArgumentOrNull(list.get(keywordPosition + 1))); + complexTry.addChild(getArgumentOrNoop(list.get(keywordPosition + 1))); // For now, we won't allow try {}, so this must be followed by a catch keyword. This restriction is somewhat artificial, and // if we want to remove it in the future, we can do so by removing this code block. @@ -85,7 +85,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi + " Exactly one argument must be passed.", n.getTarget()); } complexTry.addChild(n.getChildAt(0)); - complexTry.addChild(getArgumentOrNull(list.get(i + 1))); + complexTry.addChild(getArgumentOrNoop(list.get(i + 1))); } else { // We have something like finally { }. In this case, this must be the final // clause statement, and we need to verify that there isn't a catch following it. @@ -96,7 +96,7 @@ public int process(List list, int keywordPosition) throws ConfigCompi } } // Passed the inspection. - complexTry.addChild(getArgumentOrNull(list.get(i + 1))); + complexTry.addChild(getArgumentOrNoop(list.get(i + 1))); } // Mark catch keyword and code block as handled. diff --git a/src/main/java/com/laytonsmith/core/functions/Compiler.java b/src/main/java/com/laytonsmith/core/functions/Compiler.java index 4d5f2f7649..7bd292c951 100644 --- a/src/main/java/com/laytonsmith/core/functions/Compiler.java +++ b/src/main/java/com/laytonsmith/core/functions/Compiler.java @@ -680,6 +680,17 @@ public CClassType getReturnType(Target t, List argTypes, public boolean preResolveVariables() { return false; } + + @Override + public ParseTree postParseRewrite(ParseTree ast, Environment env, + Set> envs, Set exceptions) { + for(ParseTree child : ast.getChildren()) { + if(!(child.getData() instanceof CFunction)) { + exceptions.add(new ConfigCompileException("Not a statement.", child.getTarget())); + } + } + return null; + } } @api diff --git a/src/main/java/com/laytonsmith/core/functions/ControlFlow.java b/src/main/java/com/laytonsmith/core/functions/ControlFlow.java index 23c280a11c..d7227d74f0 100644 --- a/src/main/java/com/laytonsmith/core/functions/ControlFlow.java +++ b/src/main/java/com/laytonsmith/core/functions/ControlFlow.java @@ -19,6 +19,7 @@ import com.laytonsmith.core.compiler.BranchStatement; import com.laytonsmith.core.compiler.CompilerEnvironment; import com.laytonsmith.core.compiler.CompilerWarning; +import com.laytonsmith.core.compiler.ConditionalSelfStatement; import com.laytonsmith.core.compiler.FileOptions; import com.laytonsmith.core.compiler.SelfStatement; import com.laytonsmith.core.compiler.VariableScope; @@ -87,6 +88,7 @@ public static String docs() { } @api + @ConditionalSelfStatement public static class _if extends AbstractFunction implements Optimizable, BranchStatement, VariableScope { public static final String NAME = "if"; @@ -342,6 +344,7 @@ public boolean isSelfStatement(Target t, Environment env, List nodes, } @api(environments = {GlobalEnv.class}) + @ConditionalSelfStatement public static class ifelse extends AbstractFunction implements Optimizable, BranchStatement, VariableScope { public static final String NAME = "ifelse"; @@ -519,6 +522,7 @@ public boolean isSelfStatement(Target t, Environment env, List nodes, @api @breakable + @ConditionalSelfStatement public static class _switch extends AbstractFunction implements Optimizable, BranchStatement, VariableScope { @Override @@ -917,6 +921,7 @@ public List isScope(List children) { } @api + @ConditionalSelfStatement public static class switch_ic extends _switch implements Optimizable, BranchStatement, VariableScope { @Override @@ -1136,6 +1141,16 @@ public List isBranch(List children) { return ret; } + @Override + public List statementsAllowed(List children) { + List ret = new ArrayList<>(); + ret.add(false); + ret.add(false); + ret.add(false); + ret.add(true); + return ret; + } + @Override public List isScope(List children) { List ret = new ArrayList<>(); diff --git a/src/main/java/com/laytonsmith/core/functions/Function.java b/src/main/java/com/laytonsmith/core/functions/Function.java index 5e5c4b1768..3fe5b7d4ff 100644 --- a/src/main/java/com/laytonsmith/core/functions/Function.java +++ b/src/main/java/com/laytonsmith/core/functions/Function.java @@ -133,10 +133,10 @@ public Scope linkScope(StaticAnalysis analysis, Scope parentScope, * Allows functions to perform a rewrite step to rewrite the AST as received from the parser to a valid * executable AST. Optimizations should not yet be performed in this rewrite step. * Traversal is pre-order depth-first. - * @param ast - The abstract syntax tree representing this function. - * @param env - The environment. - * @param envs - The set of expected environment classes at runtime. - * @param exceptions - A set to put compile errors in. + * @param ast The abstract syntax tree representing this function. + * @param env The environment. + * @param envs The set of expected environment classes at runtime. + * @param exceptions A set to put compile errors in. * @return The rewritten AST node that should completely replace the AST node representing this function, or * {@code null} to not replace this AST node. */ diff --git a/src/main/java/com/laytonsmith/core/functions/Meta.java b/src/main/java/com/laytonsmith/core/functions/Meta.java index fc89b79b17..f3d8863601 100644 --- a/src/main/java/com/laytonsmith/core/functions/Meta.java +++ b/src/main/java/com/laytonsmith/core/functions/Meta.java @@ -28,6 +28,9 @@ import com.laytonsmith.core.compiler.BranchStatement; import com.laytonsmith.core.compiler.FileOptions; import com.laytonsmith.core.compiler.VariableScope; +import com.laytonsmith.core.compiler.signature.FunctionSignatures; +import com.laytonsmith.core.compiler.signature.SignatureBuilder; +import com.laytonsmith.core.constructs.Auto; import com.laytonsmith.core.constructs.CArray; import com.laytonsmith.core.constructs.CBoolean; import com.laytonsmith.core.constructs.CInt; @@ -1012,6 +1015,8 @@ public Version since() { @api public static class noop extends AbstractFunction { + public static final String NAME = "noop"; + @Override public Class[] thrown() { return null; @@ -1053,6 +1058,13 @@ public Version since() { return MSVersion.V3_3_1; } + @Override + public FunctionSignatures getSignatures() { + return new SignatureBuilder(CVoid.TYPE) + .varParam(Auto.TYPE, "params", "Any parameters. These are ignored, but evaluated.") + .build(); + } + } @api diff --git a/src/main/resources/docs/Statements b/src/main/resources/docs/Statements index 41cb3c99c6..a2edf5033d 100644 --- a/src/main/resources/docs/Statements +++ b/src/main/resources/docs/Statements @@ -150,13 +150,15 @@ while(true) { This is true whether or not you use the functional notation, though using the curly braces in cases like this is the preferred and modern format. -There is one function, {{function|if}}, which has very special handling, and is unique. Normally, if has two forms, -the regular one, and the tertiary form. When using the tertiary form, it is not considered a self-statement, and must +There are a few functions, namely {{function|if}}, {{function|switch}}, and {{function|switch_ic}}, +which have special handling. These functions each have two forms, +the regular one, and the tertiary form. When using the tertiary form, they are not considered a self-statement, and must be terminated with a semicolon (though if it is an argument to another function, which is the usual case for such a usage, it does not require termination, though the containing statement would). In the normal form, when using it -as an if/else chain, where each branch contains its own statements, it is considered a self-statement. +as an if/else or switch statement, where each branch contains its own statements, they are also +considered a self-statement. -The full list of functions that are considered self-statements is: +The full list of functions that are unconditionally considered self-statements is: <%SELF_STATEMENT_FUNCTIONS%> diff --git a/src/test/java/com/laytonsmith/core/MethodScriptCompilerTest.java b/src/test/java/com/laytonsmith/core/MethodScriptCompilerTest.java index 6dbd5cee39..5f42b752e1 100644 --- a/src/test/java/com/laytonsmith/core/MethodScriptCompilerTest.java +++ b/src/test/java/com/laytonsmith/core/MethodScriptCompilerTest.java @@ -919,20 +919,20 @@ public void testClosureToString1() throws Exception { @Test public void testClosureToString2() throws Exception { - SRun("msg(closure('\\n'))", fakePlayer); - verify(fakePlayer).sendMessage("'\\n'"); + SRun("msg(closure(msg('\\n')))", fakePlayer); + verify(fakePlayer).sendMessage("msg('\\n')"); } @Test public void testClosureToString3() throws Exception { - SRun("msg(closure('\\\\'))", fakePlayer); - verify(fakePlayer).sendMessage("'\\\\'"); + SRun("msg(closure(msg('\\\\')))", fakePlayer); + verify(fakePlayer).sendMessage("msg('\\\\')"); } @Test public void testClosureToString4() throws Exception { - SRun("msg(closure('\\''))", fakePlayer); - verify(fakePlayer).sendMessage("'\\''"); + SRun("msg(closure(msg('\\'')))", fakePlayer); + verify(fakePlayer).sendMessage("msg('\\'')"); } @Test @@ -1296,14 +1296,14 @@ public void testParseTreeHasCorrectType() throws Exception { assertTrue(asdf.getDeclaredType(env).equals(CString.TYPE)); ParseTree msg = tree.getChildAt(0).getChildAt(1); assertTrue(msg.getDeclaredType(env).equals(CVoid.TYPE)); - ParseTree _a = tree.getChildAt(0).getChildAt(2).getChildAt(1); + ParseTree _a = tree.getChildAt(0).getChildAt(3); assertTrue(_a.getDeclaredType(env).equals(CInt.TYPE)); } @Test public void testSmartCommentIsOnNode() throws Exception { String[] scripts = new String[]{ - "/** smart comment */ proc('_test', null)", + "/** smart comment */ proc('_test', noop())", "/** smart comment */ void proc _test(){}", "/** smart comment */ proc _test(){}", }; @@ -1314,7 +1314,7 @@ public void testSmartCommentIsOnNode() throws Exception { ParseTree tree = MethodScriptCompiler.compile(MethodScriptCompiler.lex( script, env, null, true), env, env.getEnvClasses(), sa); - assertTrue(tree.getChildAt(0).getNodeModifiers().getComment() != null); + assertTrue(tree.getChildAt(0).getChildAt(0).getNodeModifiers().getComment() != null); } } diff --git a/src/test/java/com/laytonsmith/core/NewExceptionHandlingTest.java b/src/test/java/com/laytonsmith/core/NewExceptionHandlingTest.java index 3707f8e414..c6eccf8c25 100644 --- a/src/test/java/com/laytonsmith/core/NewExceptionHandlingTest.java +++ b/src/test/java/com/laytonsmith/core/NewExceptionHandlingTest.java @@ -50,13 +50,14 @@ public void setUp() { @Test public void testBasicKeywordUsage() throws Exception { - assertEquals("complex_try(null,assign(ms.lang.IOException,@e,null),null,assign(ms.lang.Exception,@e,null),null,null)", + assertEquals("__statements__(complex_try(__statements__(noop()),assign(ms.lang.IOException,@e,null),__statements__(noop())," + + "assign(ms.lang.Exception,@e,null),__statements__(noop()),__statements__(noop())))", optimize("try { } catch (IOException @e){ } catch (Exception @e){ } finally { }")); } @Test public void testTryFinallyKeywordUsage() throws Exception { - assertEquals("complex_try(__statements__(msg('a')),__statements__(msg('b')))", + assertEquals("__statements__(complex_try(__statements__(msg('a')),__statements__(msg('b'))))", optimize("try { msg(\"a\"); } finally { msg(\"b\"); }")); } diff --git a/src/test/java/com/laytonsmith/core/OptimizationTest.java b/src/test/java/com/laytonsmith/core/OptimizationTest.java index 3ca5c9373e..47a6ed2302 100644 --- a/src/test/java/com/laytonsmith/core/OptimizationTest.java +++ b/src/test/java/com/laytonsmith/core/OptimizationTest.java @@ -1,5 +1,6 @@ package com.laytonsmith.core; +import com.laytonsmith.PureUtilities.SimpleVersion; import com.laytonsmith.core.compiler.CompilerEnvironment; import com.laytonsmith.core.compiler.OptimizationUtilities; import com.laytonsmith.core.constructs.Target; @@ -550,25 +551,25 @@ public void testNotNot() throws Exception { @Test public void testReturnAsKeyword() throws Exception { - assertEquals("proc('_name',__statements__(return('value')))", optimize("proc _name() { return 'value'; }")); - assertEquals("proc('_name',__statements__(return(rand(1,10))))", optimize("proc _name() { return rand(1, 10); }")); + assertEquals("__statements__(proc('_name',__statements__(return('value'))))", optimize("proc _name() { return 'value'; }")); + assertEquals("__statements__(proc('_name',__statements__(return(rand(1,10)))))", optimize("proc _name() { return rand(1, 10); }")); - assertEquals("proc('_name',__statements__(return('value')))", optimize(" proc _name() { return 'value'; }")); - assertEquals("proc('_name',__statements__(return(rand(1,10))))", optimize(" proc _name() { return rand(1, 10); }")); + assertEquals("__statements__(proc('_name',__statements__(return('value'))))", optimize(" proc _name() { return 'value'; }")); + assertEquals("__statements__(proc('_name',__statements__(return(rand(1,10)))))", optimize(" proc _name() { return rand(1, 10); }")); - assertEquals("proc('_name',__statements__(return(add(dyn(1),dyn(2)))))", optimize("proc _name() { return dyn(1) + dyn(2); }")); - assertEquals("proc('_name',__statements__(return(add(dyn(1),dyn(2)))))", optimize(" proc _name() { return dyn(1) + dyn(2); }")); + assertEquals("__statements__(proc('_name',__statements__(return(add(dyn(1),dyn(2))))))", optimize("proc _name() { return dyn(1) + dyn(2); }")); + assertEquals("__statements__(proc('_name',__statements__(return(add(dyn(1),dyn(2))))))", optimize(" proc _name() { return dyn(1) + dyn(2); }")); } @Test public void testReturnVoidKeyword() throws Exception { - assertEquals("proc('_name',__statements__(return()))", optimize(" proc _name() { return; msg('Dead code'); }")); - assertEquals("proc('_name',sconcat(__statements__(return())))", optimize("proc _name() { return; msg('Dead code') msg('Other dead code')}")); - assertEquals("proc('_name',__statements__(return(msg('Dead code'))))", optimize(" proc _name() { return msg('Dead code') msg('Other dead code')}")); + assertEquals("__statements__(proc('_name',__statements__(return())))", optimize(" proc _name() { return; msg('Dead code'); }")); + assertEquals("__statements__(proc('_name',sconcat(__statements__(return()))))", optimize("proc _name() { return; msg('Dead code') msg('Other dead code')}")); + assertEquals("__statements__(proc('_name',__statements__(return(msg('Dead code')))))", optimize(" proc _name() { return msg('Dead code'); msg('Other dead code');}")); - assertEquals("proc('_name',__statements__(return()))", optimize("proc _name() { return; }")); - assertEquals("proc('_name',__statements__(return()))", optimize(" proc _name() { return; }")); + assertEquals("__statements__(proc('_name',__statements__(return())))", optimize("proc _name() { return; }")); + assertEquals("__statements__(proc('_name',__statements__(return())))", optimize(" proc _name() { return; }")); } @Test(expected = ConfigCompileException.class) @@ -594,7 +595,7 @@ public void testSconcatWithNonStatement() throws Exception { optimize("int @i = 0; int @j = 0")); } - @Test + @Test(expected = ConfigCompileException.class) public void testPartialStatementsInStrictMSA() throws Exception { assertEquals("__statements__(assign(ms.lang.int,@i,0),assign(ms.lang.string,@s,'asdf'))", optimize("\n" @@ -604,7 +605,7 @@ public void testPartialStatementsInStrictMSA() throws Exception { + "<<<\n", false)); } - @Test + @Test(expected = ConfigCompileException.class) public void testPartialStatementsInStrictMSA2() throws Exception { assertEquals("__statements__(if(dyn(1),__statements__(assign(ms.lang.int,@i,1),msg(@i))))", optimize("\n" @@ -616,7 +617,7 @@ public void testPartialStatementsInStrictMSA2() throws Exception { + "<<<\n", false)); } - @Test + @Test(expected = ConfigCompileException.class) public void testPartialStatementsInStrict() throws Exception { assertEquals("__statements__(assign(ms.lang.int,@i,1),msg(@i),msg(@i))", optimize("\n" + "int @i = 1\n" @@ -628,7 +629,7 @@ public void testPartialStatementsInStrict() throws Exception { public void testStatementInArrayInNonStrict() throws Exception { // Not the erroneous semicolon after 'c';. We are in non-strict mode though, so this should just // work anyways. - assertEquals("array('a','b','c')", optimize("array('a', 'b', 'c';)")); + assertEquals("array('a','b','c')", optimize("array('a', 'b', p('c');)")); } @Test @@ -671,7 +672,7 @@ public void testSmartStringInArrayFails() throws Exception { @Test public void testParenthesisRewritesCorrectly1() throws Exception { - assertEquals("__statements__(assign(ms.lang.closure,@c,closure(null)),execute(@c))", + assertEquals("__statements__(assign(ms.lang.closure,@c,closure(__statements__(noop()))),execute(@c))", optimize("closure @c = closure() {};\n@c();")); } @@ -703,10 +704,10 @@ public void testParenthesisWarnsButRewritesCorrectly() throws Exception { public void testNestedExecute() throws Exception { assertEquals("__statements__(proc('_t',__statements__(return(closure(__statements__(return(closure(__statements__(msg('hi')))))))))," + "execute(execute(_t())))", - optimize("proc _t() { return closure() { return closure() { msg('hi'); }; }; } _t()()();")); + optimize(" proc _t() { return closure() { return closure() { msg('hi'); }; }; }; _t()()();")); assertEquals("__statements__(proc('_t',__statements__(return(closure(__statements__(return(closure(__statements__(msg('hi')))))))))," + "execute(execute(_t())))", - optimize(" proc _t() { return closure() { return closure() { msg('hi'); }; }; }; _t()()();")); + optimize("proc _t() { return closure() { return closure() { msg('hi'); }; }; } _t()()();")); assertEquals("__statements__(proc('_t',@a,__statements__(return(closure(@b,__statements__(return(closure(@c,__statements__(msg('hi')))))))))," + "execute(3,4,execute(2,_t(1))))", optimize(" proc _t(@a) { return closure(@b) { return closure(@c) { msg('hi'); }; }; }; _t(1)(2)(3,4);")); @@ -725,7 +726,7 @@ public void testParenthesisInArrayDefinition() throws Exception { @Test public void testNoErrorWithParenthesisAfterSymbol() throws Exception { - assertEquals("if(and(@a,or(@b,@c)),null)", + assertEquals("__statements__(if(and(@a,or(@b,@c)),__statements__(noop())))", optimize("if(@a &&\n(@b || @c)) {}")); assertEquals(0, env.getEnv(CompilerEnvironment.class).getCompilerWarnings().size()); } @@ -739,7 +740,7 @@ public void testProcReference() throws Exception { @Test public void testInvalidStatements() throws Exception { - assertEquals("__statements__(msg('test'))", optimize("msg('test';);")); + assertEquals("__statements__(msg('test'))", optimize("msg(p('test'););")); try { optimize(" msg('test';);"); fail(); @@ -775,4 +776,50 @@ public void testForIsSelfStatement() throws Exception { public void testEmptyStatementsAreRemoved() throws Exception { assertEquals("__statements__(msg('hi'))", optimize("msg('hi');;;;;;;;;;;;;")); } + + private void testSemicolonUsage(String script, boolean passExpected) throws Exception { + try { + optimize(script); + if(!passExpected) { + if(MSVersion.LATEST.lte(new SimpleVersion(3, 3, 6))) { + assertEquals(1, env.getEnv(CompilerEnvironment.class).getCompilerWarnings().size()); + } else { + fail(); + } + } + } catch(ConfigCompileException ex) { + // pass + if(passExpected) { + fail(); + } + } + } + + @Test + public void testMissingSemicolonWarnsInStrictMode() throws Exception { + testSemicolonUsage(" if(dyn(true)) { if(dyn(true)) {} if(dyn(true)) {} }", true); + testSemicolonUsage(" if(dyn(true)) { } else if(dyn(1) == 1) { }", true); + testSemicolonUsage(" for(int @i = 0, @i < 10, @i++) { msg(''); }", true); + testSemicolonUsage(" array() if(dyn(true)) {}", false); + testSemicolonUsage(" msg('hi');", true); + testSemicolonUsage(" msg('hi')", false); + testSemicolonUsage(" if(dyn(true)) {} array()", false); + } + + @Test + public void testConstantIsntStatement() throws Exception { + try { + optimize("'string';"); + fail(); + } catch(ConfigCompileException ex) { + // pass + } + + try { + optimize("closure('string')"); + fail(); + } catch(ConfigCompileException ex) { + // pass + } + } } diff --git a/src/test/java/com/laytonsmith/core/functions/BasicLogicTest.java b/src/test/java/com/laytonsmith/core/functions/BasicLogicTest.java index 820fd2c2b9..5b344b2802 100644 --- a/src/test/java/com/laytonsmith/core/functions/BasicLogicTest.java +++ b/src/test/java/com/laytonsmith/core/functions/BasicLogicTest.java @@ -185,13 +185,13 @@ public void testAnd2() throws Exception { verify(fakePlayer, times(0)).sendMessage("lol"); } - @Test(timeout = 10000) + @Test public void testOr1() throws Exception { + SRun("if(or(false, false), '', msg('pass'))", fakePlayer); SRun("if(or(true, true, true), msg('pass'))", fakePlayer); SRun("if(or(true, true, false), msg('pass'))", fakePlayer); SRun("if(or(true, true), msg('pass'))", fakePlayer); SRun("if(or(true, false), msg('pass'))", fakePlayer); - SRun("if(or(false, false), '', msg('pass'))", fakePlayer); SRun("if(or(true), msg('pass'))", fakePlayer); SRun("if(or(false), '', msg('pass'))", fakePlayer); verify(fakePlayer, times(7)).sendMessage("pass"); From 39435ebc5b7fb9c81b67578717d048bf3cba97d7 Mon Sep 17 00:00:00 2001 From: LadyCailin Date: Wed, 11 Jan 2023 23:32:04 +0100 Subject: [PATCH 2/3] Remove critical path for now, so merge can happen --- .../core/MethodScriptCompiler.java | 64 ------------------- .../laytonsmith/core/OptimizationTest.java | 4 +- 2 files changed, 2 insertions(+), 66 deletions(-) diff --git a/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java b/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java index abe8fbf358..dfa6ef627f 100644 --- a/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java +++ b/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java @@ -12,7 +12,6 @@ import com.laytonsmith.core.compiler.BranchStatement; import com.laytonsmith.core.compiler.CompilerEnvironment; import com.laytonsmith.core.compiler.CompilerWarning; -import com.laytonsmith.core.compiler.ConditionalSelfStatement; import com.laytonsmith.core.compiler.EarlyBindingKeyword; import com.laytonsmith.core.compiler.FileOptions; import com.laytonsmith.core.compiler.FileOptions.SuppressWarning; @@ -2326,69 +2325,7 @@ private static ParseTree postParseRewrite(ParseTree ast, Environment env, statementsAllowed = true; } } - if(child.getData() instanceof CFunction) { - if(child.getData().val().equals(Compiler.__statements__.NAME)) { - if(!(ast.getData() instanceof CNull) && function == null) { - exceptions.add(new ConfigCompileException("Unknown function", ast.getTarget())); - continue; - } - if(!statementsAllowed) { - String unexpectedStatement = "Unexpected statement, semicolon (;) not allowed in this context."; - try { - if(ast.getData() instanceof CFunction cf - && cf.getCachedFunction().isSelfStatement(ast.getTarget(), env, ast.getChildren(), envs)) { - // We can give a better error message here, because the semicolon wasn't actually added - // by the user (probably) it's the self statement function that's the problem here. - unexpectedStatement = "Unexpected statement, " + ast.getData().val() - + " not allowed in this context."; - } - } catch(ConfigCompileException ex) { - exceptions.add(ex); - } - if(ast.getFileOptions().isStrict()) { - exceptions.add(new ConfigCompileException(unexpectedStatement, child.getTarget())); - } else { - // Statements aren't allowed here, but we aren't in strict mode, so - // pull up the value in the statement to here. sconcat is an exception to this rule, since - // it's entirely too special. - if(!(ast.getData() instanceof CFunction cf) || !cf.val().equals(StringHandling.sconcat.NAME)) { - if(child.getChildren().size() != 1) { - exceptions.add(new ConfigCompileException(unexpectedStatement, child.getTarget())); - } else { - CompilerWarning warning = new CompilerWarning(unexpectedStatement, - child.getTarget(), SuppressWarning.UnexpectedStatement); - env.getEnv(CompilerEnvironment.class).addCompilerWarning(ast.getFileOptions(), warning); - ast.getChildAt(i).replace(child.getChildren().get(0)); - } - } - } - } - if(statementsAllowed) { - continue; - } - if(function.getName().equals(Compiler.__statements__.NAME)) { - ParseTree lastChild = child; - if(child.numberOfChildren() > 0) { - lastChild = child.getChildAt(child.numberOfChildren() - 1); - } - exceptions.add(new ConfigCompileException("Invalid comma after " - + lastChild.getData().val(), lastChild.getTarget())); - } else if(ast.getFileOptions().isStrict()) { - exceptions.add(new ConfigCompileException("Invalid use of auto concat in " - + function.getName() + "()", ast.getTarget())); - } - } else if(statementsAllowed) { - // It is a function, but it isn't a statement. Check for strict mode. - if(isStrict) { - doMissingSemicolonError(child.getTarget(), exceptions); - } - } - } else if(statementsAllowed) { - if(((topLevel && isStrict) || !topLevel) && (function == null - || function.getClass().getAnnotation(ConditionalSelfStatement.class) == null)) { - exceptions.add(new ConfigCompileException("Not a statement.", child.getTarget())); -======= if(statementsAllowed) { continue; } @@ -2423,7 +2360,6 @@ private static ParseTree postParseRewrite(ParseTree ast, Environment env, env.getEnv(CompilerEnvironment.class).addCompilerWarning(ast.getFileOptions(), warning); child.replace(child.getChildren().get(0)); } ->>>>>>> origin/master } } } diff --git a/src/test/java/com/laytonsmith/core/OptimizationTest.java b/src/test/java/com/laytonsmith/core/OptimizationTest.java index 47a6ed2302..af2f966da2 100644 --- a/src/test/java/com/laytonsmith/core/OptimizationTest.java +++ b/src/test/java/com/laytonsmith/core/OptimizationTest.java @@ -795,7 +795,7 @@ private void testSemicolonUsage(String script, boolean passExpected) throws Exce } } - @Test +// @Test public void testMissingSemicolonWarnsInStrictMode() throws Exception { testSemicolonUsage(" if(dyn(true)) { if(dyn(true)) {} if(dyn(true)) {} }", true); testSemicolonUsage(" if(dyn(true)) { } else if(dyn(1) == 1) { }", true); @@ -806,7 +806,7 @@ public void testMissingSemicolonWarnsInStrictMode() throws Exception { testSemicolonUsage(" if(dyn(true)) {} array()", false); } - @Test +// @Test public void testConstantIsntStatement() throws Exception { try { optimize("'string';"); From 72b17d1616b1b474235c3b57fe4ac9607f068b1f Mon Sep 17 00:00:00 2001 From: LadyCailin Date: Wed, 11 Jan 2023 23:42:06 +0100 Subject: [PATCH 3/3] Re-add tests --- src/test/java/com/laytonsmith/core/OptimizationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/laytonsmith/core/OptimizationTest.java b/src/test/java/com/laytonsmith/core/OptimizationTest.java index af2f966da2..47a6ed2302 100644 --- a/src/test/java/com/laytonsmith/core/OptimizationTest.java +++ b/src/test/java/com/laytonsmith/core/OptimizationTest.java @@ -795,7 +795,7 @@ private void testSemicolonUsage(String script, boolean passExpected) throws Exce } } -// @Test + @Test public void testMissingSemicolonWarnsInStrictMode() throws Exception { testSemicolonUsage(" if(dyn(true)) { if(dyn(true)) {} if(dyn(true)) {} }", true); testSemicolonUsage(" if(dyn(true)) { } else if(dyn(1) == 1) { }", true); @@ -806,7 +806,7 @@ public void testMissingSemicolonWarnsInStrictMode() throws Exception { testSemicolonUsage(" if(dyn(true)) {} array()", false); } -// @Test + @Test public void testConstantIsntStatement() throws Exception { try { optimize("'string';");