diff --git a/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFParamStatement.java b/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFParamStatement.java index ea66ef1..c7cbe9c 100644 --- a/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFParamStatement.java +++ b/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFParamStatement.java @@ -17,21 +17,28 @@ public class CFParamStatement extends CFParsedAttributeStatement { private CFExpression paramType; private CFExpression defaultExpression; + /* + * cfparam's attributes, per the shipped dictionaries, which agree across cf11 and lucee5. + * MAXLENGTH was missing, so `param name="x" maxlength="5";` counted as invalid. + */ static { validAttributes = new HashSet(); validAttributes.add("DEFAULT"); - validAttributes.add("TYPE"); - validAttributes.add("NAME"); validAttributes.add("MAX"); + validAttributes.add("MAXLENGTH"); validAttributes.add("MIN"); + validAttributes.add("NAME"); validAttributes.add("PATTERN"); + validAttributes.add("TYPE"); } /** * The visitor fills _attributes after construction, so this used to call validateAttributes on a - * map that was always empty -- the validation never ran. Wiring it up is a behaviour change - * rather than a bug fix: validateAttributes throws, parseScript does not catch it, and one - * unrecognised attribute would abort the whole file instead of reporting an error. See #39. + * map that was always empty -- the validation never ran. It stays uncalled: validateAttributes + * throws, parseScript does not catch it, and one unrecognised attribute would abort the whole + * file instead of reporting an error on the statement. The set above is pinned to dictionaries + * that ship with the parser and will fall behind whatever the next CF release adds, so that + * failure would land on valid code. See #39. */ public CFParamStatement(org.antlr.v4.runtime.Token t, Map _attributes) { super(t, _attributes); diff --git a/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFPropertyStatement.java b/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFPropertyStatement.java index dcf74db..a790cbf 100644 --- a/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFPropertyStatement.java +++ b/cfml.parsing/src/main/java/cfml/parsing/cfscript/script/CFPropertyStatement.java @@ -16,19 +16,91 @@ public class CFPropertyStatement extends CFParsedAttributeStatement { private CFExpression propertyName; private CFExpression propertyType; + /* + * cfproperty's attributes, taken from the shipped dictionaries, which agree on all 65 across + * cf11 and lucee5. This set previously held cfparam's six -- name, type, default, max, min, + * pattern -- which is not a subset of anything cfproperty accepts beyond the first three. Every + * ORM and bean property in real code carries getter, setter, persistent, fieldtype or ormtype, + * and getValidAttributes() is public, so consumers asking cfparser what a property may carry + * were told cfparam's answer. See #39. + */ static { validAttributes = new HashSet(); + validAttributes.add("CACHEUSE"); + validAttributes.add("CASCADE"); + validAttributes.add("CFC"); + validAttributes.add("COLUMN"); + validAttributes.add("CONSTRAINED"); + validAttributes.add("DBDEFAULT"); validAttributes.add("DEFAULT"); - validAttributes.add("TYPE"); + validAttributes.add("DISPLAYNAME"); + validAttributes.add("ELEMENTCOLUMN"); + validAttributes.add("ELEMENTTYPE"); + validAttributes.add("FETCH"); + validAttributes.add("FIELDTYPE"); + validAttributes.add("FKCOLUMN"); + validAttributes.add("FORMULA"); + validAttributes.add("GENERATED"); + validAttributes.add("GENERATOR"); + validAttributes.add("GETTER"); + validAttributes.add("HINT"); + validAttributes.add("INDEX"); + validAttributes.add("INDEXABLE"); + validAttributes.add("INDEXBOOST"); + validAttributes.add("INDEXFIELDNAME"); + validAttributes.add("INDEXLANGUAGE"); + validAttributes.add("INDEXSTORE"); + validAttributes.add("INDEXTOKENIZE"); + validAttributes.add("INSERT"); + validAttributes.add("INVERSE"); + validAttributes.add("INVERSEJOINCOLUMN"); + validAttributes.add("JOINCOLUMN"); + validAttributes.add("LAZY"); + validAttributes.add("LENGTH"); + validAttributes.add("LINKCATALOG"); + validAttributes.add("LINKSCHEMA"); + validAttributes.add("LINKTABLE"); + validAttributes.add("MAPPEDBY"); + validAttributes.add("MISSINGROWIGNORED"); validAttributes.add("NAME"); - validAttributes.add("MAX"); - validAttributes.add("MIN"); - validAttributes.add("PATTERN"); + validAttributes.add("NOTNULL"); + validAttributes.add("OPTIMISTICLOCK"); + validAttributes.add("ORDERBY"); + validAttributes.add("ORMTYPE"); + validAttributes.add("PARAMS"); + validAttributes.add("PERSISTENT"); + validAttributes.add("PRECISION"); + validAttributes.add("REMOTINGFETCH"); + validAttributes.add("REQUIRED"); + validAttributes.add("SCALE"); + validAttributes.add("SELECTKEY"); + validAttributes.add("SEQUENCE"); + validAttributes.add("SERIALIZABLE"); + validAttributes.add("SETTER"); + validAttributes.add("SINGULARNAME"); + validAttributes.add("SOURCE"); + validAttributes.add("SQLTYPE"); + validAttributes.add("STRUCTKEYCOLUMN"); + validAttributes.add("STRUCTKEYTYPE"); + validAttributes.add("TABLE"); + validAttributes.add("TYPE"); + validAttributes.add("UNIQUE"); + validAttributes.add("UNIQUEKEY"); + validAttributes.add("UNSAVEDVALUE"); + validAttributes.add("UPDATE"); + validAttributes.add("VALIDATE"); + validAttributes.add("VALIDATEPARAMS"); + validAttributes.add("WHERE"); } - + + /* + * validateAttributes is deliberately not called here. It throws, and parseScript does not catch, + * so one attribute this set does not know would abort the whole file rather than report an error + * on the statement. The set above is pinned to dictionaries that ship with the parser and will + * fall behind whatever the next CF release adds, so that failure would land on valid code. + */ public CFPropertyStatement(org.antlr.v4.runtime.Token t, Map _attributes) { super(t, _attributes); - // validateAttributes(t, validAttributes); } @Override diff --git a/cfml.parsing/src/test/java/cfml/parsing/TestCFMLFunctionStatement.java b/cfml.parsing/src/test/java/cfml/parsing/TestCFMLFunctionStatement.java index 2d37d71..42dfbad 100644 --- a/cfml.parsing/src/test/java/cfml/parsing/TestCFMLFunctionStatement.java +++ b/cfml.parsing/src/test/java/cfml/parsing/TestCFMLFunctionStatement.java @@ -12,6 +12,7 @@ import cfml.parsing.cfscript.script.CFIncludeStatement; import cfml.parsing.cfscript.script.CFMLFunctionStatement; import cfml.parsing.cfscript.script.CFParamStatement; +import cfml.parsing.cfscript.script.CFPropertyStatement; import cfml.parsing.cfscript.script.CFScriptStatement; import cfml.parsing.reporting.ParseException; @@ -205,6 +206,57 @@ public void testValidateAttributesStillRejectsUnknownOnes() { } } + /** + * maxlength is a cfparam attribute the allowed set did not list, so a valid param counted as + * invalid. getValidAttributes() is public, so the wrong answer was reachable from outside. + */ + @Test + public void testParamAllowsMaxLength() { + CFScriptStatement scriptStatement = parseScript("param name=\"x\" maxlength=\"5\";"); + CFParamStatement paramStatement = (CFParamStatement) scriptStatement; + paramStatement.validateAttributes(paramStatement.getToken(), CFParamStatement.getValidAttributes()); + } + + /** + * cfproperty's allowed set held cfparam's six attributes. Every one of these is a real + * cfproperty attribute and every one of them was rejected -- getter and setter are on more or + * less every accessor-generating CFC, and the ORM ones on every persistent entity. + */ + @Test + public void testPropertyAllowsItsOwnAttributes() { + CFScriptStatement scriptStatement = parseScript( + "property name=\"email\" type=\"string\" getter=\"true\" setter=\"false\" " + + "required=\"true\" persistent=\"true\" fieldtype=\"column\" ormtype=\"string\" " + + "hint=\"the address\";"); + CFPropertyStatement propertyStatement = (CFPropertyStatement) scriptStatement; + assertEquals(9, propertyStatement.getAttributes().size()); + propertyStatement.validateAttributes(propertyStatement.getToken(), CFPropertyStatement.getValidAttributes()); + } + + /** + * The two sets are genuinely different tags and must not drift back into being copies. + */ + @Test + public void testPropertyAndParamSetsAreNotTheSame() { + assertEquals(7, CFParamStatement.getValidAttributes().size()); + assertEquals(65, CFPropertyStatement.getValidAttributes().size()); + assertFalse("property must not simply carry param's attributes", + CFPropertyStatement.getValidAttributes().equals(CFParamStatement.getValidAttributes())); + } + + @Test + public void testPropertyStillRejectsUnknownAttributes() { + CFScriptStatement scriptStatement = parseScript("property name=\"x\" bogus=\"y\";"); + CFPropertyStatement propertyStatement = (CFPropertyStatement) scriptStatement; + try { + propertyStatement.validateAttributes(propertyStatement.getToken(), + CFPropertyStatement.getValidAttributes()); + fail("bogus should not have validated"); + } catch (ParseException expected) { + assertTrue(expected.getMessage(), expected.getMessage().contains("bogus")); + } + } + @Test public void testTransactionStatement() { /* need to check if this is valid in OBD/ACF */