Поддержка стандартов командного кодирования

Поддержка стандартов командного кодирования: создание правил Lint.

В предыдущем посте мы узнали, как создать простую проверку, используя уже существующие средства тестирования языка. Мы также упомянули о существовании инструментов Lint: в VA Smalltalk этот инструмент называется SmallLint, а в Pharo он называется Quality Assistant. Оба инструмента - следуя философии открытой экосистемы - позволяют легко расширять предоставляемые правила.

Одним из преимуществ автоматизации стандартов командного кодирования является то, что, когда приходит новый член команды, каждый раз, когда единица работы закрывается, эти проверки запускаются, и это помогает новичку адаптироваться к особенностям команды. В этом смысле важно, чтобы каждая реализованная проверка или правило сопровождалась обоснованием, объясняющим, почему команда считает это хорошей или плохой практикой.

Есть несколько примеров этой идеи в дикой природе. Например, веб-фреймворк Seaside поставляется с собственной программой проверки под названием Slime, которая расширяет правила SmallLint.

Давайте теперь посмотрим на некоторые правила, которые добавила наша команда, и их обоснование.

В этих примерах будет использоваться синтаксис Rewrite Engine, поэтому минимальное понимание этого является плюсом. Если вы этого не знаете: вкратце `@variable соответствует практически любому выражению в дереве синтаксического анализа.

Первые правила, которые мы увидим, имеют такое же обоснование: Не рекомендуется возвращать (или назначать) nil в результате отправки сообщения. Потому что, как правило, это вызывает множество неподдерживаемых нулевых проверок отправителей (или красивых ошибок во время выполнения: P).

Использует #ifTrue: или #ifFalse: по отдельности

Обоснование здесь состоит в том, чтобы избегать использования ifTrue: или ifFalse: по отдельности и возврата, присвоения переменной или передачи в качестве параметра результата, потому что в случае, если блок не оценивается, результатом будет nil.

ParseTreeLintRule class>>usesIfTrueOrIfFalseIndividually
  ^self createParseTreeRule: #(
    ‘^`@object ifTrue: ``@block’ 
    ‘^`@object ifFalse: ``@block’
    ‘`var := ``@object ifTrue: ``@block’ 
    ‘`var := ``@object ifFalse: ``@block’
    ‘``@object `message: (``@anotherObject ifTrue: ``@block)’
    ‘``@object `message: (``@anotherObject ifFalse: ``@block)’
    ) name: ‘Uses #ifTrue: or #ifFalse: individually’

Каждая строка внутри массива представляет собой выражение сопоставления дерева синтаксического анализа.

Использует #ifNotNil: индивидуально

Обоснование здесь аналогично предыдущему случаю. Поскольку в случае, если получатель nil, блок не оценивается, и результат будет nil.

ParseTreeLintRule class>>usesIfTrueOrIfFalseIndividually
^self createParseTreeRule: #(
    ‘^`@object ifNotNil: ``@block’ 
    ‘`var := ``@object ifNotNil: ``@block’ 
    ‘``@object `message: (``@anotherObject ifNotNil: ``@block)’
    ) name: ‘Uses #ifNotNil: individually’

Возвращает или присваивает #detect: ifFound: result

Этот метод является расширением, которое мы сделали для иерархии Коллекции (Pharo теперь включает его в библиотеку базовых классов), чтобы помочь, когда вы хотите что-то сделать с результатом обнаружения: но на всякий случай, если найден соответствующий объект.

Обоснование здесь аналогично предыдущему случаю (обратите внимание, что в Pharo он вернет коллекцию, а не nil, как это делает наша исходная реализация). В любом случае, если вы отправляете это сообщение, вы не должны использовать результат.

ParseTreeLintRule class>>returnsOrAssignsDetectIfFound
^self createParseTreeRule: #(
  '^`@receiver detect: `@detectBlock ifFound: `@foundBlock'
  '`var := `@receiver detect: `@detectBlock ifFound: `@foundBlock'
  ) name: ‘Return or Assigns #detect:ifFound:’

Правила выявления намерений

Следующие кейсы не обнаруживают ошибок, но улучшают читаемость результирующего кода.

Использует #detect: ifNone: вместо #anySatisfy: / #noneSatisfy:

Это правило выявляет некоторые ошибки новичков, в основном из-за того, что они пропустили какое-то сообщение в Collection API, которое лучше подходит для ситуации.

ParseTreeLintRule class>>returnsOrAssignsDetectIfFound
^self createParseTreeRule: #(
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) isNil'
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) = nil'
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) == nil'
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) notNil'
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) ~= nil'
 '(``@col detect: [:`e| |`@temps| ``@.exp] ifNone: [nil]) ~~ nil' ) 
name: ‘Uses #detect:ifNone: instead of #anySatisfy:/#noneSatisfy:’

Когда мы обнаруживаем что-то подобное при проверке кода, по возможности мы добавляем новое правило, предлагающее замену кода, чтобы это никогда не повторилось.

Назначение и тестирование

Мы обнаружили, что иногда трудно понять намерение кода, когда объект присваивается некоторой переменной, а затем сразу же отправляется сообщение о том же выражении. Поэтому для ясности мы предпочитаем сначала выполнить присвоение и отправить сообщение в следующем операторе.

ParseTreeLintRule class>>assignmentAndTesting
^self  createParseTreeRule: #(‘( `var := `@object ) `message’)
       name: ‘Assigns and tests an object’

Использует 1 / объект вместо #reciprocal

Это кажется лишним, но в Меркапе мы разрабатываем финансовое программное обеспечение, и у нас есть много математических абстракций, помимо чисел, таких как цены, котировки, денежные меры и т. Д. Поэтому проще отправить ответное сообщение математическому объекту чтобы получить обратную валюту, например, вместо того, чтобы делать 1 / (20.45 ARS/USD), мы делаем (20.45 ARS/USD) reciprocal, чтобы получить обратную котировку валюты. Когда у вас длинная финансовая формула, это обеспечивает лучшую читаемость.

ParseTreeLintRule class>>assignmentAndTesting
^self  createParseTreeRule: #('1 / `@object') 
       name: 'Uses 1 / object instead of #reciprocal'

Ошибки и возможные ошибки

Использует каскадирование при создании Array / OrderedCollection. Удалите лишний «;»

Это правило возникло из-за неудачного опыта преобразования некоторого #add: cascade в более простой метод send. Например токарная обработка:

OrderedCollection new 
  add:1;
  add:2;
  add:3;
  yourself

OrderedCollection with: 1 with: 2 with: 3

Часто случалось, что мы немного искали #add: затем заменяли его на #with:, забывая удалить ; и замечая его только тогда, когда позже какой-то тест терпит неудачу.

ParseTreeLintRule class>>avoidCascadingAnArrayCreation
^self createParseTreeRule: #(
  ‘Array with: `@firstObject ; with: `@secondObject’
  ‘Array with: `@firstObject ; with: `@secondObject ; with: `@thirdObject’
  ‘Array with: `@firstObject with: `@secondObject ; with: `@thirdObject’
  ‘Array with: `@firstObject ; with: `@secondObject with: `@thirdObject’
  ‘Array with: `@firstObject ; with: `@secondObject ; with: `@thirdObject ; with: `@fourthObject’
  ‘Array with: `@firstObject with: `@secondObject ; with: `@thirdObject ; with: `@fourthObject’
  ‘Array with: `@firstObject with: `@secondObject with: `@thirdObject ; with: `@fourthObject’
  ‘Array with: `@firstObject with: `@secondObject ; with: `@thirdObject with: `@fourthObject’
  ‘Array with: `@firstObject ; with: `@secondObject with: `@thirdObject ; with: `@fourthObject’
  ‘Array with: `@firstObject ; with: `@secondObject ; with: `@thirdObject with: `@fourthObject’
  ‘Array with: `@firstObject ; with: `@secondObject with: `@thirdObject with: `@fourthObject’
)
name: ‘Uses cascading in Array/OrderedCollection creation. Remove the extra “;”’

В предыдущем примере используется массив, но исходное правило немного сложнее, включая другие коллекции. Я упростил его здесь по дидактическим причинам.

Класс реализует #doesNotUnderstand: но пропускает #respondsTo:

Это правило покрывает возможную ошибку и проверяет, что если класс реализует #doesNotUnderstand:, вероятно, ему также необходимо реализовать #respondsTo:, потому что эти методы должны работать в тандеме. Мы не используем его часто, но были случаи, когда мы пропускали #respondsTo: реализацию и замечали ее слишком поздно.

BlockLintRule>>implementorsOfDoesNotUnderstandMustImplementRespondsTo
^self new 
  resultClass: ClassEnvironment;
  name: ‘Class implements #doesNotUnderstand: but misses #respondsTo:’;
  classBlock: [:context :result | | currentClass |
    currentClass := context selectedClass.
    ((currentClass includesSelector: #doesNotUnderstand:)
     and: [(currentClass includesSelector: #respondsTo:) not])
        ifTrue: [result addClass: currentClass]];
  yourself

Спасибо за чтение и не стесняйтесь делиться своими собственными правилами или практиками. Если вы еще не автоматизировали эти правила, начните прямо сейчас !.

Поддержка стандартов командного кодирования