I'm using the code review feature as I work on developing a module. It seems to indicate that a __toString method implementation is invalid for a class because it is not a PHP magic method. But my review of PHP indicates it is. The code in question looks like this:

class MediaSharedShelfSearchFilter {
  public $type;
  public $field;
  public $comparison;
  public $value;

  // SharedShelf values allowed for use in a filter construction.
  protected $typeValues = array(
    'string',
    'numeric',
    'date',
    'thumbnail',
    'profile',
  );
  protected $comparisonValues = array('eq', 'lt', 'gt');

  /**
   * Constructor for an instance.
   * 
   * @param string $field
   *   The name of the field.
   * @param string $value
   *   The value for the field.
   * @param string $comparison
   *   A comparison operator.
   * @param string $type
   *   The type of the field.
   */
  public function __construct($field, $value, $comparison = 'eq', $type = 'string') {
    // Make sure only a valid type value is set.
    if (in_array($type, $this->typeValues)) {
      $this->type = $type;
    }
    else {
      // Default to string if an invalid value is passed.
      $this->type = $this->typeValues[0];
    }
    $this->field = $field;
    // Make sure only a valid comparison value is set.
    if (in_array($comparison, $this->comparisonValues)) {
      $this->comparison = $comparison;
    }
    else {
      // Default to eq if an invalid value is passed.
      $this->comparison = $this->comparisonValues[0];
    }
    // Make sure the value is converted to a string for numbers because the
    // SharedShelf filter will fail otherwise.
    $this->value = strval($value);
  }

  /**
   * String representation of the object.
   * 
   * @return string
   *   JSON encoded string representation of the object. 
   */
  public function __toString() {
    return json_encode($this);
  }

  /**
   * Checks to make sure that an object is valid.
   * 
   * @return bool
   *   Returns TRUE if object is valid. 
   */
  public function exists() {
    if (($this->field == '') or ($this->value == '')) {
      return FALSE;
    }
    return TRUE;
  }
}
CommentFileSizeAuthor
#1 magictostring-1916238-2.patch880 bytescthos

Comments

cthos’s picture

Status: Active » Needs review
StatusFileSize
new880 bytes

This is because of a small deviation from what exists in the existing PEAR conventions. Whereas the PEAR basis for this check has the following:

$magicPart = strtolower(substr($methodName, 2));

Coder's version is thus:

$magicPart = substr($methodName, 2);

That causes magic __toString to be unrecognized, because $this->magicMethods only contains all lower cased method names.

This is consistent with PHP's case-insensitivity when it comes to method names, so I believe the strtolower check needs to be preserved here.

Patch attached!

jlk4p’s picture

I just applied this patch and I no longer get the false error. Thanks for fixing this!

klausi’s picture

Component: Coder Review » Coder Sniffer
klausi’s picture

Status: Needs review » Fixed

Committed patch from #1 and a test case in good.php, thanks for reporting!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.