Give cfproperty and cfparam their real attribute sets

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<String>();
 		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<CFIdentifier, CFExpression> _attributes) {
 		super(t, _attributes);

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<String>();
+		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<CFIdentifier, CFExpression> _attributes) {
 		super(t, _attributes);
-		// validateAttributes(t, validAttributes);
 	}
 
 	@Override

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 */