Magento 2 – How to Fix Warnings and Errors from Marketplace Technical Review

extensionsmagento2marketplace

With the new Magento Marketplace, a submitted extension goes through several validation states in order to be approved and available via the Marketplace.

One of them is the technical review, from which you can get a technical report like the following:

Marketplace Technical Report

As you can see, 200+ warnings kinda scared the hell out of me, is there any resource to help fixing every warning apart from the list available in the docs: http://docs.magento.com/marketplace/user_guide/extensions/review-code-validation.html?

Best Answer

After an hour going through the report I came up with the following list, it could be helpful for everyone I think.

I will try to keep it updated as soon as I find more warnings/errors:

Warnings

Line exceeds 80 characters; contains X characters

Or

Line exceeds maximum limit of 100 characters; contains X characters

Those one are the one I've seen the most, they are self explanatory, it is a good practice to keep coding lines small in order to keep a clean and readable code.

No space found after comma in function call

You have called a function that receives parameters and didn't add a space after the comma. Example: strrchr($bla,".") should be strrchr($bla, ".")

Expected \"while (...) {\n\"; found \"while (...)\n {\n\"

Expected \"foreach (...) {\n\"; found \"foreach (...)\n {\n\"

Expected \"if (...) {\n\"; found \"if (...)\n {\n\"

Expected \"} else {\n\"; found \"}\n else {\n\"

That means you have returned a line before the opening bracket of those PHP statements.

Example of a bad syntax with an if/else statement:

if (true)
{
}
else
{
}

Should be

if (true) {
} else {
}

The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line

Most of the time, it happens in the constructor where you declare something like this:

public function __construct(
    ProductFactory $productFactory,
    Test $test
)
{
}

Whereas it should be:

public function __construct(
    ProductFactory $productFactory,
    Test $test
) {
}

End of line character is invalid; expected \"\n\" but found \"\r\n\"

Happens most of the time at the beginning of the file, it's caused by the way your IDE encodes the return character.

Variable \"your_variable\" is not in valid camel caps format

Every variable must use the camel caps format, so $your_variable should be $yourVariable

Variable \"one2Three\" contains numbers but this is discouraged

Avoid using numbers in your variables

Inline control structures are not allowed

You should not be using inline control structures such as:

else $test = true;

You should use:

else {
    $test = true;
}

Opening brace of a class must be on the line after the definition

You have returned a line when declaring a class:

class Test
{

You should keep the opening brace on the same line:

class Test {

Private member variable \"yourVariable\" must contain a leading underscore

Protected member variable \"yourVariable\" must contain a leading underscore

You should add a leading underscore to your protected and private member variables: $_yourVariable

As opposite of those two, if you add an underscore in your public variable you can get:

Public member variable \"_yourVariable\" must not contain a leading underscore.

The method parameter $bla is never used

You have passed a parameter to a method but you never use it.

Multi-line function declaration not indented correctly; expected 8 spaces but found X

You have added too much indentation to your function declaration parameters:

public function __construct(ProductRepository $productRepository,
                            ListsInterface $listsInterface,
                            Data $helper
) {

Should be:

public function __construct(ProductRepository $productRepository,
    ListsInterface $listsInterface,
    Data $helper
) {

Possible useless method overriding detected

You're overriding a method without adding modifications, example:

public function __construct(Context $context) {
    parent::__construct($context);
}

Model LSD method load() detected in loop

You're using the load() method inside a loop which in not recommended and has to be avoided.

Most likely your code looks like this:

foreach(...) {
    $model->load();
}

If you're loading a model in a loop it's indeed pretty bad in terms of performance. If you only need to retrieve a few attributes, you should use collections instead.

Function's cyclomatic complexity (X) exceeeds 10; consider refactoring the function

If you're not familiar with cyclomatic complexity I suggest you have a read at this post: https://pdepend.org/documentation/software-metrics/cyclomatic-complexity.html . This warning basically means that there is too many loops and conditions in your function.

Direct object instantiation is discourage in Magento 2

It is caused by the fact that you're instantiating an object directly by calling the class, for example:

new \Zend_Filter_LocalizedToNormalized

You should use dependency injection or a last resort, the object manager.

Comments refer to a TODO task

One of your comment contains the following @TODO flag.

Avoid IF statements that are always true or false

You have create a condition that seems to always be true or false.

For example:

$variable = "6";
...
// More code that doesn't change $variable
...
if ($variable)

Errors

Namespace for \"Class\" class is not specified.

You are missing the use Path\To\Class; statement at the beginning of your class.

Related Topic