Indention false positive in switch/case/if combination

[aimeos]

The "Generic.Whitespace.ScopeIndent" sniff reports a false indention for switch/case statements that contains an if-condition, e.g.:

    switch($type)
    {
        case 'a':
            if($operator == '~=') {
                $value = true;
                break;
            }
        default:
            $value = false;
    }

Result for line with default and below:

 8 | ERROR | [x] Line indented incorrectly; expected 12 spaces, found 8
 9 | ERROR | [x] Line indented incorrectly; expected at least 16 spaces, found 12

[gsherwood]

I'm trying to replicate this, but I think I might need a larger code snippet.

The test file I'm using is:

<?php
switch($type)
{
    case 'a':
        if($operator == '~=') {
            $value = true;
            break;
        }
    default:
        $value = false;
}

PHPCS test output:

$ phpcs temp.php -p --standard=Generic --sniffs=Generic.WhiteSpace.ScopeIndent
. 1 / 1 (100%)


Time: 39ms; Memory: 4MB

So no errors reported from the sniff.

Do you have a sample file that I can use to replicate the error? It is possible that the sniff has been confused by something higher up in the file and has calculated the indent incorrectly.

[aimeos]

Here's the whole file:

<?php

/**
 * @license LGPLv3, http://opensource.org/licenses/LGPL-3.0
 * @copyright Aimeos (aimeos.org), 2015-2018
 * @package MW
 * @subpackage Common
 */


namespace Aimeos\MW\Criteria\Expression\Sort;


/**
 * SQL implementation for sorting objects.
 *
 * @package MW
 * @subpackage Common
 */
class SQL
	extends \Aimeos\MW\Criteria\Expression\Base
	implements \Aimeos\MW\Criteria\Expression\Sort\Iface
{
	private static $operators = array( '+' => 'ASC', '-' => 'DESC' );
	private $operator;
	private $conn;
	private $name;


	/**
	 * Initializes the object.
	 *
	 * @param \Aimeos\MW\DB\Connection\Iface $conn Database connection object
	 * @param string $operator Sorting operator ("+": ascending, "-": descending)
	 * @param string $name Name of the variable or column to sort
	 */
	public function __construct( \Aimeos\MW\DB\Connection\Iface $conn, $operator, $name )
	{
		if( !isset( self::$operators[$operator] ) ) {
			throw new \Aimeos\MW\Common\Exception( sprintf( 'Invalid operator "%1$s"', $operator ) );
		}

		$this->operator = $operator;
		$this->conn = $conn;
		$this->name = $name;
	}


	/**
	 * Returns the sorting direction operator.
	 *
	 * @return string Sorting direction ("+": ascending, "-": descending)
	 */
	public function getOperator()
	{
		return $this->operator;
	}


	/**
	 * Returns the available operators for the expression.
	 *
	 * @return array List of available operators
	 */
	public static function getOperators()
	{
		return array_keys( self::$operators );
	}


	/**
	 * Returns the name of the variable or column to sort.
	 *
	 * @return string Name of the variable or column to sort
	 */
	public function getName()
	{
		return $this->name;
	}


	/**
	 * Generates a string from the expression objects.
	 *
	 * @param array $types Associative list of variable or column names as keys and their corresponding types
	 * @param array $translations Associative list of variable or column names that should be translated
	 * @param \Aimeos\MW\Criteria\Plugin\Iface[] $plugins Associative list of item names as keys and plugin objects as values
	 * @param array $funcs Associative list of item names and functions modifying the conditions
	 * @return mixed Expression that evaluates to a boolean result
	 */
	public function toSource( array $types, array $translations = [], array $plugins = [], array $funcs = [] )
	{
		$this->setPlugins( $plugins );

		$name = $this->name;

		if( ( $transname = $this->translateName( $name, $translations, $funcs ) ) === '' ) {
			return '';
		}

		if( !isset( $types[$name] ) ) {
			throw new \Aimeos\MW\Common\Exception( sprintf( 'Invalid name "%1$s"', $name ) );
		}

		return $transname . ' ' . self::$operators[$this->operator];
	}


	/**
	 * Escapes the value so it can be inserted into a SQL statement
	 *
	 * @param string $operator Operator used for the expression
	 * @param integer $type Type constant
	 * @param mixed $value Value that the variable or column should be compared to
	 * @return string Escaped value
	 */
	protected function escape( $operator, $type, $value )
	{
		$value = $this->translateValue( $this->getName(), $value );

		switch( $type )
		{
			case \Aimeos\MW\DB\Statement\Base::PARAM_BOOL:
				$value = (bool) $value; break;
			case \Aimeos\MW\DB\Statement\Base::PARAM_INT:
				$value = (int) $value; break;
			case \Aimeos\MW\DB\Statement\Base::PARAM_FLOAT:
				$value = (float) $value; break;
			case \Aimeos\MW\DB\Statement\Base::PARAM_STR:
				if( $operator == '~=' )
				{
					$value = '\'%' . $this->conn->escape( $value ) . '%\'';
					break;
				}
			default: // all other operators: escape in default case
				$value = '\'' . $this->conn->escape( $value ) . '\'';
		}

		return (string) $value;
	}


	/**
	 * Returns the internal type of the function parameter.
	 *
	 * @param string &$item Reference to parameter value (will be updated if necessary)
	 * @return integer Internal parameter type
	 * @throws \Aimeos\MW\Common\Exception If an error occurs
	 */
	protected function getParamType( &$item )
	{
		if( $item[0] == '"' )
		{
			if( ( $item = substr( $item, 1, strlen( $item ) - 2 ) ) === false ) {
				throw new \Aimeos\MW\Common\Exception( sprintf( 'Unable to extract string parameter from >%1$s<', $item ) );
			}

			return \Aimeos\MW\DB\Statement\Base::PARAM_STR;
		}
		else if( strpos( $item, '.' ) !== false )
		{
			return \Aimeos\MW\DB\Statement\Base::PARAM_FLOAT;
		}
		else if( ctype_digit( $item ) !== false )
		{
			return \Aimeos\MW\DB\Statement\Base::PARAM_INT;
		}

		return \Aimeos\MW\DB\Statement\Base::PARAM_STR;
	}
}

We use tabs instead of spaces but it's the same with spaces.

[aimeos]

@gsherwood Could you reproduce the issue?

[gsherwood]

Thanks for providing that code sample. I can reproduce the issue with the following code:

<?php

switch($foo)
{
    case 1:
        $value = 1; break;
    case 2:
        if ($bar) {
            break;
        }
    default:
        $value = 0;
}

[gsherwood]

This issue is caused by the fact that the second case only has a conditional break statement, so the indent is not reset correctly by the time the sniff moves to the default case:

$ phpcs temp.php -p --standard=Generic --sniffs=Generic.WhiteSpace.ScopeIndent --runtime-set scope_indent_debug 1
Start with token 0 on line 1 with indent 0
Closing parenthesis found on line 3
	 * ignoring single-line definition *
Open scope (T_SWITCH) on line 4
	=> indent set to 4 by token 7 (T_OPEN_CURLY_BRACKET)
Open scope (T_CASE) on line 5
	=> indent set to 8 by token 13 (T_COLON)
Close scope (T_CASE) on line 6
	* first token is 10 (T_CASE) on line 5 *
	=> indent set to 4 by token 23 (T_BREAK)
Open scope (T_CASE) on line 7
	=> indent set to 8 by token 30 (T_COLON)
Closing parenthesis found on line 8
	 * ignoring single-line definition *
Open scope (T_IF) on line 8
	=> indent set to 12 by token 39 (T_OPEN_CURLY_BRACKET)
Close scope (T_IF) on line 10
	* first token is 33 (T_IF) on line 8 *
	=> indent set to 8 by token 46 (T_CLOSE_CURLY_BRACKET)
[Line 11] Line indented incorrectly; expected 8 spaces, found 4
	=> Add adjustment of 4 for token 49 (T_DEFAULT) on line 11
Open scope (T_DEFAULT) on line 11
	=> indent set to 12 by token 50 (T_COLON)
Indent adjusted to 12 for T_VARIABLE on line 12
	=> Add adjustment of 4 for token 53 (T_VARIABLE) on line 12
[Line 12] Line indented incorrectly; expected at least 12 spaces, found 8
	=> Add adjustment of 4 for token 53 (T_VARIABLE) on line 12
Close scope (T_SWITCH) on line 13
	* first token is 2 (T_SWITCH) on line 3 *
	=> indent set to 0 by token 60 (T_CLOSE_CURLY_BRACKET)
E 1 / 1 (100%)



FILE: temp.php
----------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------------------
 11 | ERROR | [x] Line indented incorrectly; expected 8 spaces, found 4
 12 | ERROR | [x] Line indented incorrectly; expected at least 12 spaces, found 8
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

Time: 127ms; Memory: 4MB

[gsherwood]

This was caused by a bug in the sniff where the break being on the same line as other code caused the sniff to not close the scope of that first case statement. That then caused the sniff to miss the shared closer in the second case statement.

[gsherwood]

Thanks for reporting this, and thanks for providing more code to help figure it out.