Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic.WhiteSpaceScopeIndent closure argument indenting incorrect with multi-line strings #1637

Closed
turbo8p opened this issue Sep 1, 2017 · 2 comments
Milestone

Comments

@turbo8p
Copy link

turbo8p commented Sep 1, 2017

Hello,
I'm using Sniffer to lint the test code which is using Codeception\Specify in the code.

Problem

When doing this:

class Dummy extends Unit
{
    use Specify;

    public function foo()
    {
        $this->specify('some
        long description', function () {
            echo 'hello';
        });
    }
}

After running phpcbf

class Dummy extends Unit
{
    use Specify;

    public function foo()
    {
        $this->specify('some
        long description', function () {
            echo 'hello';
}); // <----- wrong indent
    }
}

What I expected

Sniff should not adjust the indent of the first code.

Solution ?

Should we include T_CONSTANT_ENCAPSED_STRING when finding the first token position of the line ?

// ScopeIndentSniff.php, Line 589

if (isset($tokens[$scopeCloser]['scope_condition']) === true) {
        $first = $phpcsFile->findFirstOnLine([T_WHITESPACE, T_CONSTANT_ENCAPSED_STRING], $tokens[$scopeCloser]['scope_condition'], true);

        $currentIndent = ($tokens[$first]['column'] - 1);
// ...
@gsherwood
Copy link
Member

Shorter code to replicate:

<?php
public function foo()
{
    $foo('some
    long description', function () {
    });
}

Scope indent debug output:

Start with token 0 on line 1 with indent 0
Closing parenthesis found on line 2
	 * ignoring single-line definition *
Open scope (T_FUNCTION) on line 3
	=> indent set to 4 by token 9 (T_OPEN_CURLY_BRACKET)
Open closure on line 5
	=> indent set to 4 by token 23 (T_OPEN_CURLY_BRACKET)
Close scope (T_CLOSURE) on line 6
	=> indent set to 0 by token 26 (T_CLOSE_CURLY_BRACKET)
[Line 6] Line indented incorrectly; expected 0 spaces, found 4
	=> Add adjustment of -4 for token 26 (T_CLOSE_CURLY_BRACKET) on line 6
Close closure on line 6
	* token has nested parenthesis 13 on line 4 *
	* token is inside condition 3 (T_FUNCTION) on line 2 *
	* using parenthesis *
	* previous token is T_VARIABLE on line 4 *
	* first token on line 4 is 12 (T_VARIABLE) *
	=> indent set to 4 by token 12 (T_VARIABLE)
Closing parenthesis found on line 6
	* token is inside condition 3 (T_FUNCTION) on line 2 *
	* using condition *
	=> checking indent of 4; main indent set to 4 by token 3 (T_FUNCTION)
Close scope (T_FUNCTION) on line 7
	=> indent set to 0 by token 30 (T_CLOSE_CURLY_BRACKET)

Problem is here:

Open closure on line 5
	=> indent set to 4 by token 23 (T_OPEN_CURLY_BRACKET)

The indent should be set to 8, but the first non-whitespace token on the line is a multi-string at column 0, so it can't be used to determine indent in this case. The indent checks needs to go back another line to find the true indent.

@gsherwood gsherwood changed the title ScopeIndent working incorrect for anonymous function Generic.WhiteSpaceScopeIndent closure argument indenting incorrect with multi-line strings Sep 1, 2017
gsherwood added a commit that referenced this issue Sep 1, 2017
@gsherwood gsherwood added this to the 3.1.0 milestone Sep 1, 2017
@gsherwood
Copy link
Member

I've committed a fix for this issue, which required checks for multi-line strings when opening and closing closures. Thanks for reporting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants