Skip to content
This repository has been archived by the owner on Jul 17, 2023. It is now read-only.

Стайлгайд для докблоков #24

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* [Как быть хорошим куратором?](docs/development/curators.md)
* [Полезные сайты для PHP-разработчиков](docs/development/links.md)
* [Полезные видео для PHP-разработчиков](docs/development/videos.md)
* [Стандарты кодинга](docs/development/code-style.md)

## Менеджерам

Expand Down
4 changes: 4 additions & 0 deletions docs/development/code-style/code-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Кодстайл

## PHP
- [Докблоки](docs/development/code-style/php/docblocks.md)
83 changes: 83 additions & 0 deletions docs/development/code-style/php/docblocks-motivation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Почему докблоки не нужны
Copy link
Author

@velosipedist velosipedist May 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Возможно, текст лишний, это скорее для того чтобы не переспрашивали 10 раз


## Если коротко

- минимизируется типичная тавтология (class-class, param-param)
- код пишется и — что важнее — читается быстрее
- пулл-реквесты делаются существенно короче, иной раз вдвое и больше
- не нужно сопровождать устаревающие комментарии и хинты, экономится время
- мотивирует соблюдать SRP, удаляя симфонические/бандловые аннотации везде, где возможно, с перенесением в раздельные конфиги
- в частичном, а не 100%-м покрытии класса доктайпами нет ничего страшного / противоестественного. Это всего лишь непривычно;
- как утверждает Р. Мартин в "Чистом коде" - обилие комментариев вырабатывает привычку их избегать при быстром чтении кода. И тогда те блоки комментариев, где действительно содержится ценная информация - легко теряются и перестают работать / обновляться.


## Мы не пишем библиотеки

В Симфони-гайде полагается документировать все классы, объекты и их члены.

Однако:
- сам документ, как и PSRы, составлялся во времена PHP < 7, когда не было другого способа полностью документировать возвращаемые типы и аргументы;
- обширная документация имеет куда большую важность для многократно используемой библиотеки (фреймворки, бандлы)
- в архитектуру, в интересах BC, нечасто вносятся серъезные изменения, комментарии имеют меньше шансов устареть по недосмотру
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может лучше избегать ненужных аббревиатур как ВС, например?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему эта ненужная? Мне казалось, она достаточно прижилась.

- зато очень важно указать в каждом классе лицензию, версию, автора, его email ~и кличку любимой собаки~

Тогда как:
- в частных, не-коробочных проектах код носит «штучный» характер, который предполагает куда меньше вариантов (пере)использования;
- архитектура в них меняется намного чаще, от фичи к фиче, от релиза к релизу;
- потому и коментарии устаревают на практике чаще — и тайпхинты, и пояснения;
- даже если и подписывать с каждым коммитом им автора(ов) — оценить авторство, кроме коллег, будет практически некому

## Мы пишем на PHP 7.1+

Поэтому у нас уже есть практически всё, чего можно добиться комментариями.
В то же время именно «живые» тайпхинты:
- если упущены при рефакторинге, то с большой вероятностью ломают код, поэтому защищены от устаревания
- позволяют читать код метода подряд, слева направо, не прыгая со строки на строку


## В классах документировать, как правило, нечего

В большинстве классов используются тавтологические конструкции вида:
```php
/**
* Class SomeClass
*/
class SomeClass
```

Каковые конструкции:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может Такие конструкции?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это моя старообрядческая манера выражаться :)
Но раз есть риск непрочтения в надлежащем смысле, заменю-с.

- не добавляют никакой информации к `class SomeClass`;
- требуют поддержания в актуальном состоянии (имена классов устаревают, переносятся вместе с копипастой);
- в результате устаревания комментария мы получаем не просто тавтологию, а сломанную тавтологию:
```php
/**
* Class RefactoredOrCopyPastedClass
*/
class SomeClass
```

## Автоматизация несовершенна

Докблоки удобно генерить с помощью IDE / консольных утилит.

Однако, при недостаточном контроле эти генерированные доки могут скрывать / искажать информацию о параметрах, но выглядеть при этом валидно.

В лучшем случае, без ручного вмешательства они ничего не добавляют к семантике, например, возвращаемых массивов:

```php

/**
* @return array
**/
function foo(): array {
return ['hey','it','is','string','list'];
}

```

А если ручное вмешательство всё же необходимо — времени такая автоматизация не столько экономит, сколько отнимает.

### `@inheritdoc` не несет существенной пользы
Только - при чтении PR людьми, незнакомыми с архитектурой.

В остальном - IDE всё показывает и без `@inheritdoc`. Кроме случая, если метод унаследован от суперкласса, который, в свою очередь, наследует из интерфейса.
86 changes: 86 additions & 0 deletions docs/development/code-style/php/docblocks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Докблоки

## Всегда можно писать

- `@deprecated`, `@throws`
- Symfony-аннотации для контроллеров, сущностей и пр.
- `@var` в свойствах класса - только в "вертикальной" форме:

```php
/**
* @var Foo $foo Vertical example - OK
**/
private $foo;

/** @var Bar $bar Horizontal example - NOT OK */
private $bar;
```

## Никогда нельзя писать

В классе - «волшебные» `@method` и `@property`.

?`@inheritDoc`?


# Rule of thumb

> Писать докблоки **обязательно** только тогда, когда они привносят хоть что-либо полезное в код и нет иного способа это сделать.

В остальных случаях докблоки не нужны. [Почему?](docs/development/code-style/php/docblocks-motivation.md)

Если докблок использован, то он должен иметь полную форму - `@param`, `@returns` и пр., по ситуации.

Далее описаны случаи, когда именно докблоки что-либо привносят.


## Класс / Интерфейс / Трейт

Документировать, когда:
- информацию о его предназначении не выразить в имени класса ± неймспейса, или нет на это времени;
- нужно указать примеры использования класса, а времени писать тесты — нет;

## Метод / Конструктор

Документировать метод нужно, если:
- хотя бы один из `@param` может быть массивом / итератором элементов строго определенного типа (`string[]`, ` Collection|SomeClass[]`)
- `@returns` если возращаемый методом тип может быть таким же массивом/итератором

## Инлайновые `@var`

Следует избегать, где позволяют возможности IDE / документирования иными способами (описаны ниже)

Да, это несколько более хлопотно, зато защищено от устаревания (=> косяков при рефакторинге).

## Решения для замены инлайновых `@var`

#### Сервисы, извлекаемые в контроллере

```php
/** @var $myService MyService */
$myService = $this->get(MyService::class);
$repository = $this->getDoctrine()->getRepository($name);
```

Можно [инжектить в конструктор](https://symfony.com/doc/3.3/service_container.html#services-constructor-injection) до Symfony 3.3 и [хинтить прямо в метод](https://symfony.com/doc/3.3/controller.html#fetching-services-as-controller-arguments), начиная с 3.3.

Заодно меньше проблем с `private: true` по умолчанию и рефакторинге именованных сервисов.

#### Обход циклом данных из итератора

```php
foreach($service->iterateStuff() as $thing) {
/** @var Foo $thing */
$thing->fooMethod();
}
```

Это как раз сигнал, что `iterateStuff`
- либо совсем не задокументирован
- либо покрыт только автогенерацией
- либо дока к нему устарела

Во всех случаях достаточно документировать сам `iterateStuff` хинтом `@return Foo[]`.

Если же это результат работы внешней библиотеки — завернуть в декоратор / адаптер и пр.