About Teletype
Join
RNDSOFT Technology Blog
@rnds
rails
July 30

Awesome code review on Rails

В данной статье расскажем, как автоматизировать код ревью для проекта на Ruby on Rails.

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

Но есть и негативные факторы, такие как трата ценного времени на написание рутинных комментариев типа: "Тут оставил лишний пробел, а тут не соблюдены договоренности по коду".

Есть много инструментов, которые могут помочь автоматизировать проведение код ревью и поднять его на более высокий уровень.

В качестве основы нашего решения мы взяли gem pronto, он умеет запускать разные проверки и автоматически отправляет комментарии к MR относительно только измененных строк кода.

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

Добавляем гем в проект

gem 'pronto', require: false

Для проверки измененного кода нам нужно в вашей ветке запустить команду

pronto run -c origin/main

Pronto получит изменения вашей ветки относительно ветки main через команду git diff и для измененных файлов запустит проверки и отфильтрует только те замечания, которые относятся к измененным строкам.

Нам нужно заранее добавить эти проверки в проект, делается это через установку дополнительных гемов для pronto, наиболее полный список возможный гемов можно найти тут.

Для отправки замечаний в Merge Request потребуется запустить другую команду, параметры которой будут зависеть от веб-платформы управления вашим репозиторием. Вот пример команды для gitlab

PRONTO_GITLAB_API_PRIVATE_TOKEN=token pronto run -f gitlab -c origin/main

token - это access token пользователя в gitlab, от которого будут создаваться замечания.

Подробнее о том, как создать токен https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#create-a-personal-access-token

Для CI/CD можно завести отдельного пользователя и добавить его в проект.

Добавляем статические анализаторы кода (a.k.a. linter)

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

Для Ruby

Для статического анализа кода на ruby традиционно используют rubocop, поэтому добавляем его в Gemfile, если еще нет

gem 'rubocop', require: false

Вы можете настроить для себя набор правил, которые вам подходят, или те, от которых вы хотите отказаться. Делается это в файле .rubocop.yml

Например, отключим все правила для файлов в папке db, config и script

AllCops:
  Exclude:
    - 'db/**/*'
    - 'config/**/*'
    - 'script/**/*'

Подробнее о настройке правил можно посмотреть в документации https://docs.rubocop.org/rubocop/1.65/configuration.html

Для проверки кода мы запускаем команду

rubocop

Чтобы pronto умел запускал эту проверку, нам понадобится установить

gem 'pronto-rubocop', require: false

Для Ruby On Rails приложения

Brakeman

Полезный сканер безопасности, который предоставляет простой в чтении отчет об оценке уязвимости для приложений на Ruby на Rails

gem 'brakeman', require: false

Для проверки кода мы запускаем команду

brakeman

Чтобы pronto умел запускал эту проверку, нам понадобится установить

gem 'pronto-brakeman', require: false

Подробнее про brakeman  https://brakemanscanner.org/docs/quickstart/

Code smell

Термин “запах кода” (code smell) некоторое время назад был введен Кентом Беком (отцом экстремального программирования), по сути, запах кода - это ключевые признаки, говорящие о необходимости рефакторинга. Для ruby есть гем reek, позволяющий найти такие признаки

gem 'reek', require: false

Для проверки кода мы запускаем команду

reek

И чтобы вывести найденные признаки в Merge request, нам понадобится

gem 'pronto-reek', require: false

Подробнее про reek https://github.com/troessner/reek

Следим за покрытием кода тестами

Тут нам понадобится сначала настроить SimpleCov — это инструмент анализа покрытия кода для Ruby. Про расширенные настройки тут https://github.com/simplecov-ruby/simplecov

Теперь возьмем gem undercover, он умеет предупреждать о методах, классах и блоках, которые были изменены, но код остался непокрытым тестами. Он делает это путем анализа данных из git diffs и отчетов о покрытии SimpleCov.

Еще нам понадобится simplecov-lcov, чтобы конвертировать отчеты о покрытии в формат lcov

gem 'simplecov'
gem 'simplecov-lcov'
gem 'undercover'

Загрузите и запустите SimpleCov в самом начале файла с тестами test/test_helper.rb (или spec_helper.rb, rails_helper, в зависимости от того, какой вы предпочитаете фреймворк для тестирования)

require 'simplecov'
require 'simplecov-lcov'
SimpleCov::Formatter::LcovFormatter.config.report_with_single_file = true
SimpleCov.formatter = SimpleCov::Formatter::LcovFormatter
SimpleCov.start do
  add_filter(/^\/spec\//) # For RSpec
  add_filter(/^\/test\//) # For Minitest
end
require 'undercover'

Для проверки кода непокрытым тестами, нам сначала надо запустить тесты, и после используем команду

undercover

Чтобы pronto умел запускать эту проверку, нам понадобится установить

gem 'pronto-undercover', require: false

Следует учитывать, что при запуске pronto run, нам нужно иметь готовый отчет о покрытии. Это накладывает некоторое ограничение, например, в CI/CD мы должны позаботиться, чтобы тесты запускались раньше, и файл с отчетом о покрытии тестов coverage/lcov/app.lcov был доступен до запуска pronto.

Для javascript

Если вы в проекте используете javascript, то автоматизировать проверку вам поможет пакет eslint, добавляем его к зависимостям проекта в файл package.json.

У eslint есть множество плагинов, которые расширяют набор базовых правил. Например, мы дополнительно используем следующие пакеты:

eslint-config-airbnb-base, который, на наш взгляд, добавляет много адекватных правил
eslint-plugin-vue так как мы в проекте используем vue, а там есть свои специфические правила
eslint-plugin-cypress так как мы пишем e2e тесты на фреймворке cypress

Т.к. у eslint-config-airbnb-base еще нет поддержки 9 версии eslint, то мы используем версию eslint@8.57.0.

Вот список зависимостей в package.json на момент написания статьи

"devDependencies": {
    …
    "eslint": "^8.57.0",
    "eslint-config-airbnb-base": "^15.0.0",
    "eslint-plugin-cypress": "^3.3.0",
    "eslint-plugin-import": "^2.29.1",
    "eslint-plugin-vue": "^9.27.0",
    "eslint-webpack-plugin": "^4.2.0",
    "globals": "^15.8.0",
    …
}

Файл с настройками .eslintrc.json

{
  "env": {
    "browser": true,
    "cypress/globals": true
  },


  "extends": [
    "airbnb-base",
    "plugin:vue/recommended",
    "plugin:cypress/recommended"
  ],


  "parser": "vue-eslint-parser",
  "parserOptions": {
    "ecmaVersion": 14,
    "sourceType": "module"
  },


  "plugins": [
    "vue",
    "cypress"
  ],


  "ignorePatterns": [
    "cypress/support/*.js",
    "cypress.config.js"
  ],


  "rules": {
    "no-plusplus": "off",
    "semi": "off",
    "eqeqeq": "off",
    "max-len": [2, 120, 2, {"ignoreUrls": true}],
    "vue/script-indent": ["error", 2, { "baseIndent": 1 }],
    "comma-dangle": ["error", "never"],
    "space-before-function-paren": ["error", "always"],
    "no-console": ["error", { "allow": ["warn", "error"] }],
    "prefer-const": "off",
    "no-underscore-dangle": "off",
    "no-multi-assign": "off",
    "no-param-reassign": "off",
    "vue/component-definition-name-casing": "off",
    "vue/max-attributes-per-line": ["error", {
      "singleline": 3,
      "multiline": { "max": 1 }
    }],
  },


  "overrides": [
    {
      "files": ["*.vue"],
      "rules": {
        "indent": "off"
      }
    },
    {
      "files": ["*_spec.js", "*.cy.js"],
      "rules": {
        "import/no-extraneous-dependencies": "off",
        "max-len": "off",
        "prefer-arrow-callback": "off",
        "func-names": "off",
        "newline-per-chained-call": "off"
      }
    }
  ],


  "globals": {
    "Routes": "readonly",
    "Turbo": "readonly",
    "I18n": "readonly",
    "toastr": "readonly"
  }
}

Подробнее про настройки eslint можно прочитать тут https://eslint.org/docs/latest/use/getting-started

Для проверки кода мы запускаем команду

yarn run eslint --ext .vue,.js app/javascript

Чтобы pronto умел запускал эту проверку, нам понадобится установить
gem 'pronto-eslint_npm', require: false

К сожалению, gem более не поддерживается, но есть рабочий форк, который выполняет свою работу, т.е. просто запускает скрипт проверки, который установлен, через NPM и парсит вывод консоли, после чего передает его в pronto для анализа строк. Поэтому можно забрать его себе в проект и немного поправить настройки.

Для этого создаем файл .pronto_eslint_npm.yml

cmd_line_opts: '--ext .js'
files_to_lint: '\.js#x27;
eslint_executable: 'yarn --silent eslint'

Теперь замечания по eslint выводятся в комментарии к MR.

Настраиваемый линтер на основе регулярных выражений

Иногда в команде возникают договоренности по коду и проверку некоторых из них можно легко автоматизировать, если написать регулярное выражение. Для этого нам понадобится гем goodcheck

gem ‘goodcheck’, require: false

Инициализируем

goodcheck init

Теперь в файл goodcheck.yml вносим свои проверки на основе регулярных выражений

rules:
  - id: com.sample.no_blink
    pattern: <blink
    message: |
      Stop using <blink> tag.

Подробнее про настройки https://sider.github.io/goodcheck/

Для проверки кода мы запускаем команду 

goodcheck check

И получаем замечания вида
index.html:50:5: Stop using <blink> tag.

Чтобы pronto умел запускать эту проверку, нам понадобится установить
gem 'pronto-goodcheck', require: false

Рекомендации по настойке Gitlab CI/CD

Теперь настроим наш gitlab так, чтобы после открытия merge request и его изменения запускалась проверка pronto, а полученные замечания создавались как threads(дискуссии) к нашему merge request. Далее предполагается, что у вас уже есть настроенный .gitlab-ci.yml и необходимо добавить запуск pronto.

Сначала заведите отдельного пользователя, от имени которого будет создавать замечания к MR, мы его назвали bot.

Потом создаем personal access token для пользователя bot, идем в настройки проекта Settings -> CI/CD -> Variables и создаем переменную BOT_TOKEN, в значение пишем полученный токен.

Внесем изменения в .gitlab-ci.yml, добавим еще одну работу

tests:pronto:
  stage: tests # секция в которой запускаются тесты
  needs: ['tests:units'] # необходимо указать зависимость от тестов, т.к. нужен отчет о покрытии
  variables:
    PRONTO_GITLAB_API_ENDPOINT: "https://ваш-gitlab-хост/api/v4"
    PRONTO_GITLAB_SLUG: ${CI_PROJECT_ID}
    PRONTO_GITLAB_API_PRIVATE_TOKEN: ${BOT_TOKEN}
  script:
    - git remote prune origin
    - git fetch --unshallow
    - bundle install
    - yarn install --check-files
    - bundle exec pronto run -f gitlab_mr -c origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME
  rules:
    - if: $CI_PIPELINE_SOURCE == 'merge_request_event' # запускать только на Merge requests

Для секции с тестами нужно положить файлы coverage/* в артефакты, чтобы pronto-undercover мог использовать их для анализа покрытия тестами

tests:units:
  …
  artifacts:
    when: always
    name: "test"
    paths:
      - coverage/*

Теперь при создании merge request мы в веб интерфейсе можем видеть замечания от наших линтеров по измененным строкам кода.

И для полного счастья запрещаем сливать MR, если есть хоть одна открытая дискуссия.

Для этого ставим галочку в настройках проекта
Settings -> Merge requests -> All threads must be resolved

Заключение

В результате мы автоматизировали большую часть рутинных правил оформления кода и, благодаря pronto, можем постепенно приводить код в проекте к единому стилю оформления. И больше внимания уделять проверке кода, ведь самое важное в код ревью — это проверять код.

Дмитрий Аникин
July 30, 12:00
2 reactions
0 views