[Test] Unit test for LineLengthSniff

[lenaorobei]

Description

LineLengthUnitTest needs to be covered with unit test.

Acceptance Criteria

1. Test is added to the Magento/Tests/Files directory and extend AbstractSniffUnitTest.
2. Positive as well as negative scenarios are covered by unit test. In other words, LineLengthUnitTest.inc should contain "error" code as well as example of "good" code.

Additional information

See already implemented unit tests under Magento/Tests directory.
Check your code by running

$ bin/phpunit

[mzeis]

Hi! I'd like to work on this.

[mzeis]

As discussed with @lenaorobei, we need to fix #36 so that the unit test can work as expected.

[mzeis]

@lenaorobei With PR #50 (that fixes issue #36), we have a unit test covering the basic sniff.

While writing the test, I found that the code for ignoring "long lines in case they contain strings intended for translation" is broken. The reason is that there is a wrong assumption in \Magento\Sniffs\Files\LineLengthSniff which parameters are passed to the checkLineLength method.

If you compare the Magento method and the method that it is overwriting:

• Magento\Sniffs\Files\LineLengthSniff::checkLineLength($phpcsFile, $stackPtr, $lineContent)
• PHP_CodeSniffer\Standards\Generic\Sniffs\Files\LineLengthSniff::checkLineLength($phpcsFile, $tokens, $stackPtr)

you will notice that the Magento sniff expects the complete content of the line in the last parameter but only the stack pointer to the first token on the next line will be passed. To fix this check, one would have to implement the logic to assemble line and do the check as intended originally.

Should this be done as part of this issue or would you like to create another issue for this? In the latter case, we could either close #6 when #50 is merged or the new issue woulde be a blocker for #6.

[mzeis]

I had some time today, so I went ahead and created an implementation in my fork (compare with develop branch).

This implementation should fix the functionality. However, I found that the regular expression that is in use doesn't always work. An error wouldn't be triggered if "Phrase('xy')" or "__('xy')" was used in another context, for example:

$test = "If a string exceeds 120 characters and a part of the string is Phrase('xy'), this would not be caught by the regular expression because it matches any line containing Phrase('xy') or __('xy').";

But then, the question is if fixing this would be within the scope of this issue as it might be more complex to fix this for all possible contexts.

[mzeis]

@lenaorobei Sorry, I forgot to mention you here. Please could you check my last comment?

[lenaorobei]

@mzeis thanks for the investigation. Could you please create separate issue for this use case?

[mzeis]

@lenaorobei This is the issue for this use case: #75

[lenaorobei]

Done in #74.