Skip to content
Permalink
Browse files
Allow keywords to be part of other syntax
Allow keywords to be part of other syntax than their keyword processing code is written for. An example is code `array(static: 1)`, which generates a compile error when attempting to process the keyword `static`, while it is actually part of valid `array()` syntax.
Fixes #1220.
  • Loading branch information
Pieter12345 committed Jul 21, 2020
1 parent f44e6f5 commit f496a780a6124471809fe17efe6162348cf7adc3
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 27 deletions.
@@ -1825,7 +1825,7 @@ public static ParseTree compile(TokenStream stream, Environment environment,
// Process the AST.
Stack<List<Procedure>> procs = new Stack<>();
procs.add(new ArrayList<>());
processKeywords(tree, compilerErrors);
processKeywords(tree, environment, compilerErrors);
rewriteAutoconcats(tree, environment, envs, compilerErrors);
checkLinearComponents(tree, environment, compilerErrors);
postParseRewrite(rootNode, environment, envs, compilerErrors); // Pass rootNode since this might rewrite 'tree'.
@@ -1838,6 +1838,9 @@ public static ParseTree compile(TokenStream stream, Environment environment,
checkFunctionsExist(tree, compilerErrors, envs);
checkLabels(tree, compilerErrors);
checkBreaks(tree, compilerErrors);
if(staticAnalysis == null) {
checkUnhandledCompilerConstructs(tree, environment, compilerErrors);
}
if(!compilerErrors.isEmpty()) {
if(compilerErrors.size() == 1) {
// Just throw the one CCE
@@ -2670,34 +2673,63 @@ private static boolean eliminateDeadCode(ParseTree tree, Environment env, Set<Cl
*
* @param tree
*/
private static void processKeywords(ParseTree tree, Set<ConfigCompileException> compileErrors) {
private static void processKeywords(ParseTree tree, Environment env, Set<ConfigCompileException> compileErrors) {
// Keyword processing
List<ParseTree> children = tree.getChildren();
for(int i = 0; i < children.size(); i++) {
ParseTree node = children.get(i);
// Keywords can be standalone, or a function can double as a keyword. So we have to check for both
// conditions.
processKeywords(node, compileErrors);
if(node.getData() instanceof CKeyword
|| (node.getData() instanceof CLabel && ((CLabel) node.getData()).cVal() instanceof CKeyword)
|| (node.getData() instanceof CFunction && KeywordList.getKeywordByName(node.getData().val()) != null)) {
processKeywords(node, env, compileErrors);
Mixed m = node.getData();
if(m instanceof CKeyword
|| (m instanceof CLabel && ((CLabel) m).cVal() instanceof CKeyword)
|| (m instanceof CFunction && KeywordList.getKeywordByName(m.val()) != null)) {
// This looks a bit confusing, but is fairly straightforward. We want to process the child elements of all
// remaining nodes, so that subchildren that need processing will be finished, and our current tree level will
// be able to independently process it. We don't want to process THIS level though, just the children of this level.
for(int j = i + 1; j < children.size(); j++) {
processKeywords(children.get(j), compileErrors);
processKeywords(children.get(j), env, compileErrors);
}
// Now that all the children of the rest of the chain are processed, we can do the processing of this level.
try {
i = KeywordList.getKeywordByName(node.getData().val()).process(children, i);
i = KeywordList.getKeywordByName(m.val()).process(children, i);
} catch (ConfigCompileException ex) {
compileErrors.add(ex);
// Keyword processing failed, but the keyword might be part of some other syntax where it's valid.
// Store the compile error so that it can be thrown after all if the keyword won't be handled.
env.getEnv(CompilerEnvironment.class).potentialKeywordCompileErrors.put(m.getTarget(), ex);
}
}
}

}

/**
* Generates compile errors for unhandled compiler constructs that should not be present in the final AST,
* such as {@link CKeyword}.
* This is purely validation and should be called on the final AST.
* @param tree - The final abstract syntax tree.
* @param env - The environment.
* @param compilerErrors - A set to put compile errors in.
* @deprecated This is handled in {@link StaticAnalysis} and will no longer be useful when static analysis is
* permanently enabled.
*/
@Deprecated
private static void checkUnhandledCompilerConstructs(ParseTree tree,
Environment env, Set<ConfigCompileException> compilerErrors) {
for(ParseTree node : tree.getAllNodes()) {
Mixed m = node.getData();

// Create compile error for unexpected keywords.
if(m instanceof CKeyword) {
ConfigCompileException ex =
env.getEnv(CompilerEnvironment.class).potentialKeywordCompileErrors.get(m.getTarget());
compilerErrors.add(ex != null ? ex
: new ConfigCompileException("Unexpected keyword: " + m.val(), m.getTarget()));
}
}
}

/**
* Shorthand for lexing, compiling, and executing a script.
*
@@ -8,6 +8,7 @@
import com.laytonsmith.core.constructs.Target;
import com.laytonsmith.core.environments.Environment;
import com.laytonsmith.core.environments.Environment.EnvironmentImpl;
import com.laytonsmith.core.exceptions.ConfigCompileException;
import com.laytonsmith.core.objects.ObjectDefinitionTable;
import java.util.ArrayList;
import java.util.HashMap;
@@ -54,6 +55,13 @@ public class CompilerEnvironment implements Environment.EnvironmentImpl {

private final List<CompilerWarning> compilerWarnings = new ArrayList<>();

/**
* When keyword processing causes a compile error, it is stored by Target in this map. If the keyword is still
* present after parsing is fully completed, we know that the keyword is not part of some other syntax and the
* exception from this map can be thrown after all.
*/
public final Map<Target, ConfigCompileException> potentialKeywordCompileErrors = new HashMap<>();

private boolean logCompilerWarnings = true;

//TODO: Need to figure out how to do known procs.
@@ -13,6 +13,7 @@

import com.laytonsmith.core.ParseTree;
import com.laytonsmith.core.Static;
import com.laytonsmith.core.compiler.CompilerEnvironment;
import com.laytonsmith.core.constructs.CClassType;
import com.laytonsmith.core.constructs.CFunction;
import com.laytonsmith.core.constructs.CKeyword;
@@ -393,7 +394,12 @@ public CClassType typecheck(ParseTree ast, Environment env, Set<ConfigCompileExc
} else if(node instanceof Variable) {
return CString.TYPE; // $vars can only be strings.
} else if(node instanceof CKeyword) {
exceptions.add(new ConfigCompileException("Unexpected keyword: " + node.val(), node.getTarget()));

// Use the more specific compile error caused during keyword processing if available.
ConfigCompileException ex =
env.getEnv(CompilerEnvironment.class).potentialKeywordCompileErrors.get(node.getTarget());
exceptions.add(ex != null ? ex
: new ConfigCompileException("Unexpected keyword: " + node.val(), node.getTarget()));
return CClassType.AUTO;
} else if(node instanceof CLabel) {
exceptions.add(new ConfigCompileException(
@@ -8,6 +8,7 @@
import com.laytonsmith.core.environments.CommandHelperEnvironment;
import com.laytonsmith.core.environments.Environment;
import com.laytonsmith.core.exceptions.CRE.CREFormatException;
import com.laytonsmith.core.exceptions.AbstractCompileException;
import com.laytonsmith.core.exceptions.ConfigCompileException;
import com.laytonsmith.core.exceptions.ConfigCompileGroupException;
import com.laytonsmith.core.exceptions.ConfigRuntimeException;
@@ -272,15 +273,15 @@ public void testExecute9() throws Exception {
verify(fakePlayer).sendMessage("hello");
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testExecute10() throws Exception {
String script
= "msg('hello') /* This is a comment too invalid()'\"'' function\n"
+ "yup, still a comment. yay() This will fail though, because the comment is unended.";
MethodScriptCompiler.execute(MethodScriptCompiler.compile(MethodScriptCompiler.lex(script, null, null, true), null, envs), env, null, null);
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testExecute11() throws Exception {
String script
= "msg('hello') 'unended string";
@@ -714,7 +715,7 @@ public void testParenthesisAfterQuotedString() throws Exception {
assertEquals("2 + 2 is 4", SRun("'2 + 2 is' (2 + 2)", fakePlayer));
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testCompileErrorOfStaticConstructOptimization() throws Exception {
MethodScriptCompiler.compile(MethodScriptCompiler.lex("2 / 0", null, null, true), null, envs);
}
@@ -773,7 +774,7 @@ public void testExtraParenthesis() throws Exception {
}
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testSpuriousSymbols() throws Exception {
SRun("2 +", fakePlayer);
}
@@ -811,12 +812,12 @@ public void testBraceElseIfElseWithElseCondTrue() throws Exception {
verify(fakePlayer).sendMessage("success!");
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testFailureOfBraces() throws Exception {
SRun("and(1){ 1 }", fakePlayer);
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testInnerElseInElseIf() throws Exception {
SRun("if(true){"
+ "msg('fail')"
@@ -5,8 +5,7 @@
import com.laytonsmith.core.constructs.Target;
import com.laytonsmith.core.environments.Environment;
import com.laytonsmith.core.exceptions.CRE.CRECastException;
import com.laytonsmith.core.exceptions.ConfigCompileException;
import com.laytonsmith.core.exceptions.ConfigCompileGroupException;
import com.laytonsmith.core.exceptions.AbstractCompileException;
import com.laytonsmith.core.functions.Exceptions;
import com.laytonsmith.testing.StaticTest;
import org.junit.BeforeClass;
@@ -174,7 +173,7 @@ public void testHiddenThrowSetsOffLog() throws Exception {
verify(log).Log(eq(MSLog.Tags.RUNTIME), eq(LogLevel.WARNING), anyString(), any(Target.class));
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testDuplicateExceptionTypeThrowsException() throws Exception {
SRun("try { } catch (CastException @e){ } catch (CastException @e){ }", fakePlayer);
}
@@ -200,37 +199,37 @@ public void testNestedTryWorks() throws Exception {
verify(fakePlayer).sendMessage("run");
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testFinallyMustBeLast() throws Exception {
SRun("try { } finally { } catch (Exception @e){ }", fakePlayer);
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testFinallyErrors() throws Exception {
SRun("finally { }", fakePlayer);
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testCatchErrors() throws Exception {
SRun("catch (Exception @e) { }", fakePlayer);
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testCatchErrors2() throws Exception {
SRun("catch { }", fakePlayer);
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testCatchOnlyAllows1Parameter1() throws Exception {
SRun("try { } catch (Exception @e, IOException @b) { }", fakePlayer);
}

@Test(expected = ConfigCompileGroupException.class)
@Test(expected = AbstractCompileException.class)
public void testCatchOnlyAllows1Parameter2() throws Exception {
SRun("catch (){ }", fakePlayer);
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testTryAloneFails() throws Exception {
SRun("try { }", fakePlayer);
}
@@ -292,7 +291,7 @@ public void testCausedBy() throws Exception {
+ "}");
}

@Test(expected = ConfigCompileException.class)
@Test(expected = AbstractCompileException.class)
public void testUnknownExceptionType() throws Exception {
SRun("try { } catch (NoTaReAlExCePtIoNtYpE @e){ }", fakePlayer);
}

0 comments on commit f496a78

Please sign in to comment.