Jak naprawić ostrzeżenia / błędy zgłaszane przez raport przeglądu technicznego Magento Marketplace?

25

W nowym Magento Marketplace przesłane rozszerzenie przechodzi kilka stanów sprawdzania poprawności w celu zatwierdzenia i udostępnienia za pośrednictwem Marketplace.

Jednym z nich jest przegląd techniczny, z którego można uzyskać raport techniczny podobny do następującego:

Raport techniczny dotyczący rynku

Jak widać, ponad 200 ostrzeżeń trochę mnie przeraziło, czy istnieje jakiś zasób, który pomoże naprawić każde ostrzeżenie oprócz listy dostępnej w dokumentacji: http://docs.magento.com/marketplace/user_guide/extensions/ review-code-validation.html ?

Raphael at Digital Pianism
źródło
Czuję, że ktoś powinien zasugerować tutaj używając PHP CodeSniffer i niezależnie od tego, jaki standard używa Magento 2 ... PSR-2? Pewno!?
Robbie Averill,
Wykorzystuje zarówno PSR-1, jak i PSR-2.
Manish
@raphael, proszę zobaczyć to, magento.stackexchange.com/questions/192506/…
abhishek
@raphael pls zobacz to: magento.stackexchange.com/questions/71399/...
abhishek

Odpowiedzi:

31

Po godzinie przeglądania raportu wymyśliłem następującą listę, która może być pomocna dla wszystkich, o których myślę.

Postaram się go aktualizować, gdy tylko znajdę więcej ostrzeżeń / błędów:

Ostrzeżenia

Linia przekracza 80 znaków; zawiera X znaków

Lub

Linia przekracza maksymalny limit 100 znaków; zawiera X znaków

Te są te, które widziałem najbardziej, są oczywiste, dobrą praktyką jest utrzymywanie małych linii kodowania, aby zachować czysty i czytelny kod.

Nie znaleziono spacji po przecinku w wywołaniu funkcji

Wywołałeś funkcję, która odbiera parametry i nie dodałeś spacji po przecinku. Przykład: strrchr($bla,".")powinien byćstrrchr($bla, ".")

Oczekiwano \ "while (...) {\ n \"; znaleziono \ "podczas gdy (...) \ n {\ n \"

Oczekiwany \ "foreach (...) {\ n \"; znaleziono \ "foreach (...) \ n {\ n \"

Oczekiwany \ "if (...) {\ n \"; znaleziono \ "jeśli (...) \ n {\ n \"

Oczekiwany \ "} else {\ n \"; znaleziono \ "} \ n else {\ n \"

Oznacza to, że zwróciłeś wiersz przed nawiasem otwierającym tych instrukcji PHP.

Przykład złej składni z instrukcją if / else:

if (true)
{
}
else
{
}

Powinno być

if (true) {
} else {
}

Nawias zamykający i nawias otwierający deklaracji funkcji wieloliniowej muszą znajdować się w tym samym wierszu

Najczęściej dzieje się to w konstruktorze, w którym deklarujesz coś takiego:

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

Powinno być:

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

Znak końca linii jest nieprawidłowy; oczekiwano \ "\ n \", ale znaleziono \ "\ r \ n \"

Zdarza się najczęściej na początku pliku, jest to spowodowane sposobem, w jaki IDE koduje znak powrotu.

Zmienna \ „twoja_zmienna” nie ma prawidłowego formatu wielbłąda

Każda zmienna musi używać formatu wielbłąda, tak też $your_variablepowinno być$yourVariable

Zmienna \ „one2Three \” zawiera liczby, ale jest to odradzane

Unikaj używania liczb w swoich zmiennych

Wbudowane struktury kontrolne są niedozwolone

Nie powinieneś używać wbudowanych struktur kontrolnych, takich jak:

else $test = true;

Powinieneś użyć:

else {
    $test = true;
}

Nawias otwierający klasy musi znajdować się w wierszu po definicji

Podczas deklarowania klasy zwróciłeś wiersz:

class Test
{

Powinieneś trzymać nawias otwierający w tej samej linii:

class Test {

Zmienna członka prywatnego \ „twoja Zmienna \” musi zawierać wiodący znak podkreślenia

Chroniona zmienna członkowska \ "twoja Zmienna \" musi zawierać wiodący znak podkreślenia

Powinieneś dodać wiodący znak podkreślenia do chronionych i prywatnych zmiennych członka: $_yourVariable

W przeciwieństwie do tych dwóch, jeśli dodasz podkreślenie do swojej zmiennej publicznej, możesz uzyskać:

Zmienna członka publicznego \ "_ twoja Zmienna \" nie może zawierać wiodącego podkreślenia.

Parametr metody $ bla nigdy nie jest używany

Przekazałeś parametr do metody, ale nigdy go nie używasz.

Deklaracja funkcji wieloliniowej niepoprawnie wcięta; oczekiwano 8 spacji, ale znaleziono X

Dodano zbyt dużo wcięcia do parametrów deklaracji funkcji:

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

Powinno być:

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

Wykryto możliwe bezużyteczne przesłonięcie metody

Przesłaniasz metodę bez dodawania modyfikacji, przykład:

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

Model metody LSD load () wykryty w pętli

Używasz load()metody wewnątrz pętli, która nie jest zalecana i należy jej unikać.

Najprawdopodobniej twój kod wygląda następująco:

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

Jeśli ładujesz model w pętli, to jest naprawdę dość źle pod względem wydajności. Jeśli potrzebujesz tylko pobrać kilka atrybutów, powinieneś użyć kolekcji.

Cyklomatyczna złożoność funkcji (X) przekracza 10; rozważ refaktoryzację funkcji

Jeśli nie znasz się na złożoności cyklicznej, sugeruję przeczytanie tego postu: https://pdepend.org/documentation/software-metrics/cyclomatic-complexity.html . To ostrzeżenie zasadniczo oznacza, że ​​w Twojej funkcji jest zbyt wiele pętli i warunków.

Bezpośrednie tworzenie instancji obiektów jest odradzane w Magento 2

Jest to spowodowane tym, że tworzysz obiekt bezpośrednio, wywołując klasę, na przykład:

new \Zend_Filter_LocalizedToNormalized

Należy użyć wstrzykiwania zależności lub w ostateczności, menedżera obiektów.

Komentarze odnoszą się do zadania TODO

Jeden z twoich komentarzy zawiera następującą @TODOflagę.

Unikaj instrukcji IF, które zawsze są prawdziwe lub fałszywe

Utworzyłeś warunek, który wydaje się zawsze prawdziwy lub fałszywy.

Na przykład:

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

Błędy

Przestrzeń nazw dla klasy \ „Class \” nie jest określona.

Brakuje use Path\To\Class;oświadczenia na początku zajęć.

Raphael at Digital Pianism
źródło
1
Jeśli dobrze to zrozumiałem, te zalecenia odnoszą się do wszystkich rozszerzeń M2 (nawet do użytku osobistego)?
Siarhey Uchukhlebau
@SiarheyUchukhlebau tak, to jest raport techniczny, który otrzymujesz po przesłaniu rozszerzenia na Magento Marketplace
Raphael at Digital Pianism
1
You should keep the opening brace on the same line:czy nie jest na odwrót?
Nawiasem
@ClaudiuCreanga Myślę, że masz rację, pozwól mi dwukrotnie sprawdzić;)
Raphael w Digital Pianism
Czy można przeforsować rozszerzenie, które ma komunikat ostrzegawczy przekraczający 10 000? Czy odrzucają wszystko, co zawiera ostrzeżenia?
Roland Soós
9

Używanie Codesniffer z zestawem reguł MEQP1 lub MEQP2 (w zależności od wersji Magento) da ci wyobrażenie o zestawie reguł Magento: https://github.com/magento/marketplace-eqp/tree/master/

Ten zestaw reguł i ten uruchomiony w procesie przesyłania Marketplace NIE zawsze są idealnie zsynchronizowane (choć oczywiście jest to idealny), więc możesz zostać odrzucony za błędy w CodeCffer, nawet jeśli przejdzie najnowszą wersję na Github.

Niektóre z najczęstszych błędów „dotkliwość-10” (jedyne błędy, za które rozszerzenie zostanie odrzucone), a także ich wymienione rekomendacje, obejmują:

Znacznik zamykający jest niedozwolony na końcu pliku PHP

Zalecenie: Usuń tag zamykający PHP.

Zakazuje się przekazywania połączeń według czasu odniesienia

Zalecenie: Przeczytaj dokumentację dotyczącą referencji w PHP 5 i przebuduj kod. Odnośniki: http://php.net/manual/en/language.references.pass.php

Wykryto bezpośrednie użycie $ _ENV Superglobal.

Wykryto bezpośrednie użycie $ _GET Superglobal.

Wykryto bezpośrednie użycie $ _POST Superglobal.

Wykryto bezpośrednie użycie $ _REQUEST Superglobal.

Wykryto bezpośrednie użycie $ _SESSION Superglobal.

Wykryto bezpośrednie użycie $ GLOBALS Superglobal.

Zalecenie: Użyj odpowiednich obiektów opakowania w celu uzyskania plików cookie, danych sesji lub żądania.

Funkcja set_magic_quotes_runtime () została uznana za przestarzałą

Zalecenie: Nie należy używać przestarzałych funkcji, ponieważ można je w dowolnym momencie usunąć z przyszłej wersji. [Prawdopodobnie ogólny błąd dla wszystkich wycofań]

Identyczny operator === nie jest używany do testowania wartości zwracanej funkcji strpos

Identyczny operator === nie jest używany do testowania wartości zwracanej funkcji stripos

Zalecenie: Użyj operatora === do przetestowania wartości zwracanej tej funkcji.

Niepoprawne użycie stałej ciągu cudzysłowu. Cudzysłowy powinny zawsze znajdować się w ciągach znaków.

Zalecenie: [brak oddzielnej rekomendacji. Wyobrażam sobie, że to ma zapobiec egzekucji poprzez cytaty.]

Brak metody ACL _isAllowed () w klasie [ClassName].

Zalecenie: bardzo ostrożnie zarządzaj ustawieniami, zarządzaniem i obsługą uprawnień. Zasób ACL należy zdefiniować w pliku adminhtml.xml dla każdego kontrolera adminhtml i zaimplementować metodę _isAllowed ().

Przestrzeń nazw dla klasy [ExceptionClassName] nie jest określona.

Zalecenie: Określ przestrzeń nazw wyjątków.

Błąd składni PHP: Usunięto przekazanie odwołania według czasu wywołania

Zalecenie: Napraw błąd składniowy. [Ten towarzyszy powyższemu. Wyobrażam sobie, że podobny błąd ogólny występuje dla wszystkich innych błędów składniowych PHP]

Możliwe naruszenie projektu Magento 2. Wykryto typową konstrukcję Magento 1.

Zalecenie: [Nie zawiera żadnych zaleceń, ale opisuje kod, w którym wykrywane jest użycie klas takich jak Mage :: blah lub Mage_blah_blah :: blah - są to klasy, które istnieją tylko w Magento 1 i nie będą działać w Magento 2. Dobrym pomysłem jest poszukaj wyrażenia regularnego w rozszerzeniu M2, Mage(\b|_)aby wstępnie sprawdzić użycie M1.]

zasób jest słowem zastrzeżonym w PHP 7.

Zalecenie: [Brak oddzielnej rekomendacji. Po prostu zmiana nazwy słowa na coś innego powinna działać. Wyobrażam sobie, że ten błąd występuje dla wszystkich zastrzeżonych słów.]

Otwierający znacznik PHP musi być pierwszą zawartością w pliku

Zalecenie: Usuń wszystkie znaki przed tagiem otwierającym PHP.

Odradza się stosowanie konstrukcji języka die.

Odradza się stosowanie konstrukcji języka wyjściowego.

Zalecenie: należy użyć metody obiektu odpowiedzi setBody ().

Używanie konstrukcji języka echa jest odradzane.

Odradza się stosowanie konstrukcji języka drukowanego.

Zalecenie: Należy zmienić architekturę rozszerzenia, aby uniknąć użycia echa, nagłówka itp. W klasach, należy rozważyć użycie metody setBody () obiektu odpowiedzi.

Nie zaleca się używania eval ()

Zalecenie: Unikaj używania eval ().


W przeciwieństwie do tych błędów, które powodują odrzucenie twojego rozszerzenia, ostrzeżenia są obecnie wymienione jedynie jako uprzejmość, aby pomóc poprawić kod twojego rozszerzenia. NIE zostaniesz odrzucony z przeglądu technicznego pod kątem ostrzeżeń, jakkolwiek jest ich wiele.

Oczywiście ta reguła może zostać w przyszłości zaostrzona, a zestaw reguł CodeNeffer jest stale sprawdzany, więc sprawdzenie, ile ostrzeżeń można rozwiązać, jest zawsze dobrym planem. Ostrzeżenia mogą również wskazywać na problemy systemowe z bazą kodu.


Niektóre powody odrzucenia przeglądu technicznego nie pojawiają się obecnie w raporcie online i są podane tylko w wiadomości e-mail.

Wykryte naruszenia kopiuj-wklej i wykryte złośliwe oprogramowanie będą wyświetlać tylko wiadomości otrzymane w wiadomości e-mail z informacją, że Twoje rozszerzenie nie zostało zaakceptowane, więc przeczytaj je uważnie .

Archiwum tych e-maili nie jest obecnie widoczne z portalu dla programistów, więc jeśli usuniesz je bez czytania lub zrzucisz do śmieci, to znikną.

Recenzenci poziomu 1 Magento czasami umieszczają dodatkowe informacje w tym e-mailu, albo tylko pomocne rzeczy, o których myśleli, że powinieneś chcieć wiedzieć, na przykład „ten klucz tablicy„ serwer ”prawdopodobnie powinien być„ serwer ”, lub uzasadnienie ich odrzucenia i sugestii jak szybko go rozwiązać, na przykład: „Skopiowałeś cały plik podstawowy Magento i właśnie zmieniłeś ścieżkę klasy: możesz zastąpić to ustawieniem preferencji klasy.” lub „Skopiowałeś cały plik podstawowy Magento, aby zmienić kilka funkcje publiczne: możesz użyć do tego wtyczek. ”

Jeśli ich nie przeczytasz i po prostu spojrzysz na raport Codeniffer, możesz spróbować rozwiązać nieprawidłowe problemy.


Pamiętaj, że unescaped output detectedNIE należy dodawać wiadomości za pomocą @escapeNotVerifiedlub @noEscape. Prawdopodobnie zostanie to niedozwolone w przyszłych wersjach Magento. Zamiast tego użyj jednej z następujących opcji:

  • Dowolny ciąg statyczny w pojedynczych cudzysłowach.
  • Ciąg statyczny w podwójnych cudzysłowach, bez zmiennych wbudowanych.
  • [zalecane] Wartość uciec z jednym ze sposobów ucieczki z \Magento\Framework\View\Element\AbstractBlock( escapeHtml(), escapeUrl(), escapeQuote(), escapeXssInUrl()).
  • Wartość przypisana do typu liczbowego (przynajmniej bool i int, może inne?)
  • Każde wywołanie metody zawierające słowo „html” w nazwie, np printBannerHtml(). Nie nadużywaj tego! Upewnij się, że twoja blahHtml()metoda naprawdę poprawnie unika wszystkich zmiennych.
Dewi Morgan
źródło
moje rozszerzenie wyświetla ostrzeżenie, ale żadne z powyższych nie jest możliwe, aby moje rozszerzenie zostało odrzucone z powodu ostrzeżenia?
Sanjay Gohil,
Napisałem to jakiś czas temu - od tego czasu mogą być dodawane nowe. Po zalogowaniu się na konto programisty kliknij rozszerzenie i przejrzyj dziennik błędów, jaki jest wyświetlany komunikat i na jakim poziomie są to błędy? Jeśli nie są one dotkliwe 10, możesz zostać odrzucony z innego powodu. Co mówi wiadomość e-mail o odrzuceniu?
Dewi Morgan
6

Błąd:

Wykryto nieokreślony wynik

Błąd w pliku .phtml

<ul class="form-list" id="payment_form_<?php echo $code ?>" style="display:none;">

Krzyczysz:

<ul class="form-list" id="payment_form_<?php /* @noEscape */ echo $code ?>" style="display:none;">

Zapoznaj się z tematem Zabezpieczenia XSS dla http://devdocs.magento.com/guides/v2.0/frontend-dev-guide/templates/template-security.html#escape-functions-for-templates

Sankar_k
źródło
Jest to wyjątkowo zły styl programowania. Proszę nie nadużywać poleceń @noEscapei @escapeNotValidatedw ten sposób: jeśli to zrobisz, prawdopodobnie zostaną one wycofane, a następnie odrzucone przez system MEQP. U dołu mojej odpowiedzi znajduje się wiele lepszych sposobów na ucieczkę danych.
Dewi Morgan
1
@Dewi Morgan: Dziękujemy za cenne informacje.
Sankar_k