diff --git a/games/test/test_case_2a.kif b/games/test/test_case_2a.kif index e4e8144f3..f24a83cd3 100644 --- a/games/test/test_case_2a.kif +++ b/games/test/test_case_2a.kif @@ -30,4 +30,4 @@ (<= terminal (true (state 4))) -(<= (goal you 100)) \ No newline at end of file +(goal you 100) \ No newline at end of file diff --git a/games/test/test_case_2c.kif b/games/test/test_case_2c.kif index 217309494..a4586de12 100644 --- a/games/test/test_case_2c.kif +++ b/games/test/test_case_2c.kif @@ -75,7 +75,7 @@ (not (true (pos ?x ?y))) (not (true (tailpos ?x ?y)))) -(<= (goal snake 100)) +(goal snake 100) (<= terminal (true (step 10))) diff --git a/games/test/test_case_5a.kif b/games/test/test_case_5a.kif index c6f29e093..5816943ca 100644 --- a/games/test/test_case_5a.kif +++ b/games/test/test_case_5a.kif @@ -22,4 +22,4 @@ (<= terminal (true (state 1))) -(<= (goal you 100)) \ No newline at end of file +(goal you 100) \ No newline at end of file diff --git a/games/test/test_case_5c.kif b/games/test/test_case_5c.kif index 0eb7d2677..e6b7be987 100644 --- a/games/test/test_case_5c.kif +++ b/games/test/test_case_5c.kif @@ -36,4 +36,4 @@ (<= terminal (true (state 1))) -(<= (goal you 100)) \ No newline at end of file +(goal you 100) \ No newline at end of file diff --git a/src/org/ggp/base/test/GdlCleanerTests.java b/src/org/ggp/base/test/GdlCleanerTests.java index 48724422b..36093b22f 100644 --- a/src/org/ggp/base/test/GdlCleanerTests.java +++ b/src/org/ggp/base/test/GdlCleanerTests.java @@ -20,9 +20,9 @@ public class GdlCleanerTests { public void testCleanNotDistinct() throws Exception { List description = new TestGameRepository().getGame("test_clean_not_distinct").getRules(); description = GdlCleaner.run(description); - + StaticValidator.validateDescription(description); - + StateMachine sm = new ProverStateMachine(); sm.initialize(description); MachineState state = sm.getInitialState(); @@ -33,5 +33,5 @@ public void testCleanNotDistinct() throws Exception { Assert.assertTrue(sm.isTerminal(state)); Assert.assertEquals(100, sm.getGoal(state, player)); } - + } diff --git a/src/org/ggp/base/test/StaticValidationTests.java b/src/org/ggp/base/test/StaticValidationTests.java index fc1a8ae53..9243c8137 100644 --- a/src/org/ggp/base/test/StaticValidationTests.java +++ b/src/org/ggp/base/test/StaticValidationTests.java @@ -15,27 +15,27 @@ public void testConn4Validation() throws Exception { public void testSimpleMutexValidation() throws Exception { validate("simpleMutex"); } - @Test + @Test(expected=ValidatorException.class) public void test1AValidation() throws Exception { validate("test_case_1a"); - } + } @Test public void test1BValidation() throws Exception { validate("test_case_1b"); - } + } @Test public void test2AValidation() throws Exception { validate("test_case_2a"); - } + } @Test public void test2BValidation() throws Exception { validate("test_case_2b"); - } + } @Test public void test2CValidation() throws Exception { validate("test_case_2c"); - } - @Test + } + @Test(expected=ValidatorException.class) public void test3AValidation() throws Exception { validate("test_case_3a"); } @@ -46,7 +46,7 @@ public void test3BValidation() throws Exception { @Test public void test3CValidation() throws Exception { validate("test_case_3c"); - } + } @Test public void test3DValidation() throws Exception { validate("test_case_3d"); @@ -58,19 +58,19 @@ public void test3EValidation() throws Exception { @Test(expected=ValidatorException.class) public void test3FValidation() throws Exception { validate("test_case_3f"); - } + } @Test public void test4AValidation() throws Exception { validate("test_case_4a"); - } + } @Test public void test5AValidation() throws Exception { validate("test_case_5a"); - } + } @Test public void test5BValidation() throws Exception { validate("test_case_5b"); - } + } @Test public void test5CValidation() throws Exception { validate("test_case_5c"); @@ -87,12 +87,12 @@ public void testFunctionAritiesDiffer() throws Exception { public void testSentenceAritiesDiffer() throws Exception { validate("test_invalid_sentence_arities_differ"); } - + @Test public void testTicTacToeValidation() throws Exception { validate("ticTacToe"); - } - + } + protected void validate(String gameName) throws Exception { new StaticValidator().checkValidity(new TestGameRepository().getGame(gameName)); } diff --git a/src/org/ggp/base/util/gdl/grammar/GdlPool.java b/src/org/ggp/base/util/gdl/grammar/GdlPool.java index 1e8968cc3..07f91f44f 100644 --- a/src/org/ggp/base/util/gdl/grammar/GdlPool.java +++ b/src/org/ggp/base/util/gdl/grammar/GdlPool.java @@ -4,13 +4,13 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.MapMaker; public final class GdlPool @@ -25,8 +25,8 @@ public final class GdlPool private static final ConcurrentMap variablePool = new ConcurrentHashMap(); private static final ConcurrentMap constantPool = new ConcurrentHashMap(); private static final Map constantCases = new TreeMap(String.CASE_INSENSITIVE_ORDER); - private static final Map variableCases = new TreeMap(String.CASE_INSENSITIVE_ORDER); - + private static final Map variableCases = new TreeMap(String.CASE_INSENSITIVE_ORDER); + // Controls whether we normalize the case of incoming constants and variables. public static boolean caseSensitive = true; @@ -37,8 +37,8 @@ public final class GdlPool // as if one had attempted to create the GdlConstant "true", regardless of whether the // game-specific constants are case-sensitive or not. These special keywords are never // sent over the network in PLAY requests and responses, so this should be safe. - public static final HashSet keywords = new HashSet(Arrays.asList( - new String[] {"init","true","next","role","does","goal","legal","terminal","base","input","_"})); + public static final ImmutableSet KEYWORDS = ImmutableSet.of( + "init","true","next","role","does","goal","legal","terminal","base","input","_"); public static final GdlConstant BASE = getConstant("base"); public static final GdlConstant DOES = getConstant("does"); public static final GdlConstant GOAL = getConstant("goal"); @@ -58,10 +58,10 @@ public final class GdlPool /** * Drains the contents of the GdlPool. Useful to control memory usage * once you have finished playing a large game. - * + * * WARNING: Should only be called *between games*. */ - public static void drainPool() { + public static void drainPool() { distinctPool.clear(); functionPool.clear(); notPool.clear(); @@ -69,9 +69,9 @@ public static void drainPool() { propositionPool.clear(); relationPool.clear(); rulePool.clear(); - variablePool.clear(); + variablePool.clear(); variableCases.clear(); - + // When draining the pool between matches, we still need to preserve the keywords // since there are global references to them. For example, the Prover state machine // has a reference to the GdlConstant "true", and that reference must still point @@ -80,25 +80,25 @@ public static void drainPool() { // are set aside and returned to the pool after all of the other constants (which // were game-specific) have been drained. Map keywordConstants = new HashMap(); - for (String keyword : keywords) { + for (String keyword : KEYWORDS) { keywordConstants.put(keyword, GdlPool.getConstant(keyword)); } constantPool.clear(); - constantCases.clear(); + constantCases.clear(); for (Map.Entry keywordEntry : keywordConstants.entrySet()) { constantCases.put(keywordEntry.getKey(), keywordEntry.getKey()); constantPool.put(keywordEntry.getKey(), keywordEntry.getValue()); } } - + /** * If the pool does not have a mapping for the given key, adds a mapping from key to value * to the pool. - * + * * Note that even if you've checked to make sure that the pool doesn't contain the key, * you still shouldn't assume that this method actually inserts the given value, since * this class is accessed by multiple threads simultaneously. - * + * * @return the value mapped to by key in the pool */ private static V addToPool(K key, V value, ConcurrentMap pool) { @@ -111,7 +111,7 @@ private static V addToPool(K key, V value, ConcurrentMap pool) { public static GdlConstant getConstant(String value) { - if (keywords.contains(value.toLowerCase())) { + if (KEYWORDS.contains(value.toLowerCase())) { value = value.toLowerCase(); } if (!caseSensitive) { @@ -121,13 +121,13 @@ public static GdlConstant getConstant(String value) constantCases.put(value, value); } } - - GdlConstant ret = constantPool.get(value); + + GdlConstant ret = constantPool.get(value); if(ret == null) - ret = addToPool(value, new GdlConstant(value), constantPool); + ret = addToPool(value, new GdlConstant(value), constantPool); return ret; } - + public static GdlVariable getVariable(String name) { if (!caseSensitive) { @@ -136,24 +136,24 @@ public static GdlVariable getVariable(String name) } else { variableCases.put(name, name); } - } - - GdlVariable ret = variablePool.get(name); + } + + GdlVariable ret = variablePool.get(name); if(ret == null) ret = addToPool(name, new GdlVariable(name), variablePool); return ret; - } + } public static GdlDistinct getDistinct(GdlTerm arg1, GdlTerm arg2) { ConcurrentMap bucket = distinctPool.get(arg1); if(bucket == null) bucket = addToPool(arg1, new ConcurrentHashMap(), distinctPool); - + GdlDistinct ret = bucket.get(arg2); if(ret == null) ret = addToPool(arg2, new GdlDistinct(arg1, arg2), bucket); - + return ret; } @@ -181,7 +181,7 @@ public static GdlFunction getFunction(GdlConstant name, List body) body = getImmutableCopy(body); ret = addToPool(body, new GdlFunction(name, body), bucket); } - + return ret; } @@ -190,7 +190,7 @@ public static GdlNot getNot(GdlLiteral body) GdlNot ret = notPool.get(body); if(ret == null) ret = addToPool(body, new GdlNot(body), notPool); - + return ret; } @@ -206,7 +206,7 @@ public static GdlOr getOr(List disjuncts) disjuncts = getImmutableCopy(disjuncts); ret = addToPool(disjuncts, new GdlOr(disjuncts), orPool); } - + return ret; } @@ -215,7 +215,7 @@ public static GdlProposition getProposition(GdlConstant name) GdlProposition ret = propositionPool.get(name); if(ret == null) ret = addToPool(name, new GdlProposition(name), propositionPool); - + return ret; } @@ -243,7 +243,7 @@ public static GdlRelation getRelation(GdlConstant name, List body) body = getImmutableCopy(body); ret = addToPool(body, new GdlRelation(name, body), bucket); } - + return ret; } @@ -263,16 +263,16 @@ public static GdlRule getRule(GdlSentence head, List body) ConcurrentMap, GdlRule> bucket = rulePool.get(head); if(bucket == null) bucket = addToPool(head, new ConcurrentHashMap, GdlRule>(), rulePool); - + GdlRule ret = bucket.get(body); if(ret == null) { body = getImmutableCopy(body); ret = addToPool(body, new GdlRule(head, body), bucket); } - + return ret; } - + private static List getImmutableCopy(List list) { return Collections.unmodifiableList(new ArrayList(list)); } @@ -297,7 +297,7 @@ public static Gdl immerse(Gdl foreignGdl) { List rval = new ArrayList(); for(int i=0; i rval = new ArrayList(); for(int i=0; i rval = new ArrayList(); for(int i=0; i rval = new ArrayList(); for(int i=0; i run(List description) { + for (int i = 0; i < MAX_ITERATIONS; i++) { + List newDescription = runOnce(description); + if (newDescription.equals(description)) { + break; + } + description = newDescription; + } + return description; + } + + private static List runOnce(List description) { List newDescription = new ArrayList(); - + //First: Clean up all rules with zero-element bodies for(Gdl gdl : description) { if(gdl instanceof GdlRule) { @@ -39,7 +51,7 @@ public static List run(List description) { newDescription.add(gdl); } } - + //TODO: Add (role ?player) where appropriate, i.e. in rules for //"legal" or "input" where the first argument is an undefined //variable @@ -56,7 +68,7 @@ public static List run(List description) { } } //TODO: Get rid of GdlPropositions in the description - + //Get rid of (not (distinct _ _)) literals in rules //TODO: Expand to functions description = newDescription; diff --git a/src/org/ggp/base/validator/StaticValidator.java b/src/org/ggp/base/validator/StaticValidator.java index 8e158b0c2..808945370 100644 --- a/src/org/ggp/base/validator/StaticValidator.java +++ b/src/org/ggp/base/validator/StaticValidator.java @@ -18,6 +18,8 @@ import org.ggp.base.util.game.Game; import org.ggp.base.util.game.GameRepository; import org.ggp.base.util.game.TestGameRepository; +import org.ggp.base.util.gdl.GdlVisitor; +import org.ggp.base.util.gdl.GdlVisitors; import org.ggp.base.util.gdl.grammar.Gdl; import org.ggp.base.util.gdl.grammar.GdlConstant; import org.ggp.base.util.gdl.grammar.GdlDistinct; @@ -110,22 +112,26 @@ public static void validateDescription(List description) throws ValidatorEx List relations = new ArrayList(); List rules = new ArrayList(); //1) Are all objects in the description rules or relations? - for(Gdl gdl : description) { - if(gdl instanceof GdlRelation) { + for (Gdl gdl : description) { + if (gdl instanceof GdlRelation) { relations.add((GdlRelation) gdl); - } else if(gdl instanceof GdlRule) { + } else if (gdl instanceof GdlRule) { rules.add((GdlRule) gdl); + } else if (gdl instanceof GdlProposition) { + System.out.println("StaticValidator warning: The rules contain the GdlProposition " + gdl + ", which may not be intended"); } else { throw new ValidatorException("The rules include a GDL object of type " + gdl.getClass().getSimpleName() + ". Only GdlRelations and GdlRules are expected. The Gdl object is: " + gdl); } } - //2) Do all negations apply directly to sentences? + //2) Do all objects parsed as functions, relations, and rules have non-zero arity? + verifyNonZeroArities(relations, rules); + //3) Do all negations apply directly to sentences? for(GdlRule rule : rules) { for(GdlLiteral literal : rule.getBody()) { testLiteralForImproperNegation(literal); } } - //3) Are the arities of all relations and all functions fixed? + //4) Are the arities of all relations and all functions fixed? Map sentenceArities = new HashMap(); Map functionArities = new HashMap(); for(GdlRelation relation : relations) { @@ -139,30 +145,66 @@ public static void validateDescription(List description) throws ValidatorEx addFunctionArities(sentence, functionArities); } } - //4) Are the arities of the GDL-defined relations correct? - //5) Do any functions have the names of GDL keywords (likely an error)? + //5) Are the arities of the GDL-defined relations correct? + //6) Do any functions have the names of GDL keywords (likely an error)? testPredefinedArities(sentenceArities, functionArities); - //6) Are all rules safe? + //7) Are all rules safe? for(GdlRule rule : rules) { testRuleSafety(rule); } - //7) Are the rules stratified? (Checked as part of dependency graph generation) + //8) Are the rules stratified? (Checked as part of dependency graph generation) //This dependency graph is actually based on relation constants, not sentence forms (like some of the other tools here) SetMultimap dependencyGraph = getDependencyGraphAndValidateNoNegativeCycles(sentenceArities.keySet(), rules); - //8) We check that all the keywords are related to one another correctly, according to the dependency graph + //9) We check that all the keywords are related to one another correctly, according to the dependency graph checkKeywordLocations(dependencyGraph, sentenceArities.keySet()); - //9) We check the restriction on functions and recursion + //10) We check the restriction on functions and recursion Map> ancestorsGraph = getAncestorsGraph(dependencyGraph, sentenceArities.keySet()); for(GdlRule rule : rules) { checkRecursionFunctionRestriction(rule, ancestorsGraph); } } + private static void verifyNonZeroArities(List relations, + List rules) throws ValidatorException { + GdlVisitor arityCheckingVisitor = new GdlVisitor() { + @Override + public void visitFunction(GdlFunction function) { + if (function.arity() == 0) { + throw new RuntimeException(function + " is written as a zero-arity function; " + + "it should be written as a constant instead. " + + "(Try dropping the parentheses.)"); + } + } + @Override + public void visitRelation(GdlRelation relation) { + if (relation.arity() == 0) { + throw new RuntimeException(relation + " is written as a zero-arity relation; " + + "it should be written as a proposition instead. " + + "(Try dropping the parentheses.)"); + } + } + @Override + public void visitRule(GdlRule rule) { + if (rule.arity() == 0) { + throw new RuntimeException(rule + " is written as a zero-arity rule; " + + "if it's always supposed to be true, it should be written as a " + + "relation instead. Otherwise, check your parentheses."); + } + } + }; + try { + GdlVisitors.visitAll(relations, arityCheckingVisitor); + GdlVisitors.visitAll(rules, arityCheckingVisitor); + } catch (RuntimeException e) { + throw new ValidatorException(e.getMessage()); + } + } + /** * Tests whether the parentheses in a given file match correctly. If the * parentheses are unbalanced, gives the line number of an unmatched @@ -488,17 +530,10 @@ private static void testPredefinedArities( } //Look for function arities with these names - if(functionArities.containsKey(ROLE) - || functionArities.containsKey(TERMINAL) - || functionArities.containsKey(GOAL) - || functionArities.containsKey(LEGAL) - || functionArities.containsKey(DOES) - || functionArities.containsKey(INIT) - || functionArities.containsKey(TRUE) - || functionArities.containsKey(NEXT) - || functionArities.containsKey(BASE) - || functionArities.containsKey(INPUT)) { - throw new ValidatorException("Probable error: Misuse of a keyword as a function"); + for (GdlConstant functionName : functionArities.keySet()) { + if (GdlPool.KEYWORDS.contains(functionName)) { + throw new ValidatorException("The keyword " + functionName + " is being used as a function. It should only be used as the name of a sentence."); + } } }