Add type hinting to JFormRule. Fix an E_STRICT error in the process.

joomla · Apr 17, 2012 · e5c0ec8 · e5c0ec8
1 parent df5218d
commit e5c0ec8
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 33 deletions.
diff --git a/libraries/joomla/form/field.php b/libraries/joomla/form/field.php
@@ -184,7 +184,7 @@ abstract class JFormField
 	/**
 	 * Method to instantiate the form field object.
 	 *
-	 * @param   object  $form  The form to attach to the form field object.
+	 * @param   JForm  $form  The form to attach to the form field object.
 	 *
 	 * @since   11.1
 	 */
@@ -298,10 +298,10 @@ public function setForm(JForm $form)
 	 *
 	 * @since   11.1
 	 */
-	public function setup($element, $value, $group = null)
+	public function setup(SimpleXMLElement $element, $value, $group = null)
 	{
 		// Make sure there is a valid JFormField XML element.
-		if (!($element instanceof SimpleXMLElement) || (string) $element->getName() != 'field')
+		if ((string) $element->getName() != 'field')
 		{
 			return false;
 		}

diff --git a/libraries/joomla/form/rule.php b/libraries/joomla/form/rule.php
@@ -56,7 +56,7 @@ class JFormRule
 	 * @since   11.1
 	 * @throws  UnexpectedValueException if rule is invalid.
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// Check for a valid regex.
 		if (empty($this->regex))

diff --git a/libraries/joomla/form/rules/color.php b/libraries/joomla/form/rules/color.php
@@ -33,7 +33,7 @@ class JFormRuleColor extends JFormRule
 	 *
 	 * @since   11.2
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		$value = trim($value);
 

diff --git a/libraries/joomla/form/rules/email.php b/libraries/joomla/form/rules/email.php
@@ -41,7 +41,7 @@ class JFormRuleEmail extends JFormRule
 	 *
 	 * @since   11.1
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// If the field is empty and not required, the field is valid.
 		$required = ((string) $element['required'] == 'true' || (string) $element['required'] == 'required');

diff --git a/libraries/joomla/form/rules/equals.php b/libraries/joomla/form/rules/equals.php
@@ -35,8 +35,9 @@ class JFormRuleEquals extends JFormRule
 	 *
 	 * @since   11.1
 	 * @throws  InvalidArgumentException
+	 * @throws  UnexpectedValueException
 	 */
-	public function test($element, $value, $group = null, $input = null, JForm $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// Initialize variables.
 		$field = (string) $element['field'];
@@ -47,10 +48,9 @@ public function test($element, $value, $group = null, $input = null, JForm $form
 			throw new UnexpectedValueException(sprintf('$field empty in %s::test', get_class($this)));
 		}
 
-		// Check that a valid JForm object is given for retrieving the validation field value.
-		if (!($form instanceof JForm))
+		if (is_null($form))
 		{
-			throw new InvalidArgumentException(sprintf('%s::test(element, value, group, input, *%s*)', get_class($this), gettype($form)));
+			throw new InvalidArgumentException(sprintf('The value for $form must not be null in %s', get_class($this)));
 		}
 
 		// Test the two values against each other.

diff --git a/libraries/joomla/form/rules/options.php b/libraries/joomla/form/rules/options.php
@@ -33,7 +33,7 @@ class JFormRuleOptions extends JFormRule
 	 *
 	 * @since   11.1
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// Check each value and return true if we get a match
 		foreach ($element->option as $option)

diff --git a/libraries/joomla/form/rules/rules.php b/libraries/joomla/form/rules/rules.php
@@ -33,7 +33,7 @@ class JFormRuleRules extends JFormRule
 	 *
 	 * @since   11.1
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// Get the possible field actions and the ones posted to validate them.
 		$fieldActions = self::getFieldActions($element);
@@ -84,7 +84,7 @@ protected function getValueActions($value)
 	 *
 	 * @since   11.1
 	 */
-	protected function getFieldActions($element)
+	protected function getFieldActions(SimpleXMLElement $element)
 	{
 		// Initialise variables.
 		$actions = array();

diff --git a/libraries/joomla/form/rules/tel.php b/libraries/joomla/form/rules/tel.php
@@ -33,7 +33,7 @@ class JFormRuleTel extends JFormRule
 	 *
 	 * @since   11.1
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// If the field is empty and not required, the field is valid.
 		$required = ((string) $element['required'] == 'true' || (string) $element['required'] == 'required');

diff --git a/libraries/joomla/form/rules/url.php b/libraries/joomla/form/rules/url.php
@@ -35,7 +35,7 @@ class JFormRuleUrl extends JFormRule
 	 * @link    http://www.w3.org/Addressing/URL/url-spec.txt
 	 * @see	    Jstring
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// If the field is empty and not required, the field is valid.
 		$required = ((string) $element['required'] == 'true' || (string) $element['required'] == 'required');

diff --git a/libraries/joomla/form/rules/username.php b/libraries/joomla/form/rules/username.php
@@ -33,7 +33,7 @@ class JFormRuleUsername extends JFormRule
 	 *
 	 * @since   11.1
 	 */
-	public function test($element, $value, $group = null, $input = null, $form = null)
+	public function test(SimpleXMLElement $element, $value, $group = null, JRegistry $input = null, JForm $form = null)
 	{
 		// Get the database object and a new query object.
 		$db = JFactory::getDBO();

diff --git a/tests/suites/unit/joomla/form/JFormFieldTest.php b/tests/suites/unit/joomla/form/JFormFieldTest.php
@@ -11,7 +11,7 @@
 require_once JPATH_PLATFORM.'/joomla/form/field.php';
 
 /**
- * Test class for JForm.
+ * Test class for JFormField.
  *
  * @package		Joomla.UnitTest
  * @subpackage  Form
@@ -41,7 +41,9 @@ function tearDown()
 	}
 
 	/**
-	 * Tests the JForm::__construct method
+	 * Tests the JFormField::__construct method
+	 *
+	 * @covers JFormField::__construct
 	 */
 	public function testConstruct()
 	{
@@ -96,15 +98,17 @@ public function testConstruct()
 	}
 
 	/**
-	 * Tests the JForm::__get method
+	 * Tests the JFormField::__get method
 	 */
 	public function testGet()
 	{
 		// Tested in testSetup.
 	}
 
 	/**
-	 * Tests the JForm::GetId method
+	 * Tests the JFormField::GetId method
+	 *
+	 * @covers JFormField::getId
 	 */
 	public function testGetId()
 	{
@@ -138,15 +142,17 @@ public function testGetId()
 	}
 
 	/**
-	 * Tests the JForm::getInput method
+	 * Tests the JFormField::getInput method
 	 */
 	public function testGetInput()
 	{
 		// Tested in actual field types because this is an abstract method.
 	}
 
 	/**
-	 * Tests the JForm::getLabel method
+	 * Tests the JFormField::getLabel method
+	 *
+	 * @covers JFormField::getLabel
 	 */
 	public function testGetLabel()
 	{
@@ -211,6 +217,8 @@ public function testGetLabel()
 
 	/**
 	 * Tests the JFormField::getTitle method
+	 *
+	 * @covers JFormField::getTitle
 	 */
 	public function testGetTitle()
 	{
@@ -259,7 +267,9 @@ public function testGetTitle()
 	}
 
 	/**
-	 * Tests the JForm::setForm method
+	 * Tests the JFormField::setForm method
+	 *
+	 * @covers JFormField::setForm
 	 */
 	public function testSetForm()
 	{
@@ -277,7 +287,30 @@ public function testSetForm()
 	}
 
 	/**
-	 * Tests the JForm::setup method
+	 * Test an invalid argument for the JFormField::setup method
+	 *
+	 * @covers JFormField::setup
+	 * @expectedException PHPUnit_Framework_Error
+	 */
+	public function testSetupInvalidElement()
+	{
+		$form = new JFormInspector('form1');
+		$field = new JFormFieldInspector($form);
+
+		$wrong = 'wrong';
+		$this->assertThat(
+			$field->setup($wrong, 0),
+			$this->isFalse(),
+			'Line:'.__LINE__.' If not a form object, setup should return false.'
+		);
+
+	}
+
+	/**
+	 * Tests the JFormField::setup method
+	 *
+	 * @covers JFormField::setup
+	 * @covers JFormField::__get
 	 */
 	public function testSetup()
 	{
@@ -291,15 +324,6 @@ public function testSetup()
 
 		$field = new JFormFieldInspector($form);
 
-		// Error handling.
-
-		$wrong = 'wrong';
-		$this->assertThat(
-			$field->setup($wrong, 0),
-			$this->isFalse(),
-			'Line:'.__LINE__.' If not a form object, setup should return false.'
-		);
-
 		// Standard usage.
 
 		$xml = $form->getXML();