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

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

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

• Управление ресурсами.
  

Открытый ресурс после использования должен быть закрыт. Иначе получим утечку ресурсов и связанные с этим проблемы. Многие инструменты статик-анализа умеют делать такую проверку для известных ресурсов. Частые ресурсы — это сокеты, соединения к базам данных, области памяти. Но вы можете добавить проверку для своих, специфичных ресурсов.

• Порядок вызова функций в методе.
  

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

• Соглашение по реализации наследников абстрактных классов.
  

Мы знаем, что обе функции hashCode() и equals() в классах-наследниках всегда надо переопределять. У вас в проекте могут быть свои подобные соглашения. Другой пример из этой категории — проверка, что в переопределенном методе используются ожидаемые вами переменные. Этим можно проверить, что метод используется по назначению.

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

Мы будем проверять, что в Android-приложении используется  один логгер, ru.project.Logger, а также не используется класс android.util.Log. Это необходимо, чтобы в релизном приложении можно было в одном месте выключить вывод в лог чувствительной к безопасности информации.

Чем будем проверять

Плохая новость — популярный FindBugs не развивается с 2015 года. Хорошая — есть SpotBugs: «SpotBugs is the spiritual successor of FindBugs, carrying on from the point where it left off with support of its community».

Как он работает? На вход анализатору отдается директория или выбранный файл c байткодом. Для анализа байткода используется библиотека Apache BCEL. Непосредственно проверки реализованы в наследниках класса Detector. Detector — это visitor, который ‘заходит’ в классы, методы и код методов. Существует множество наследников этого интерфейса в коде FindBugs.

Настройка окружения для разработки плагина FindBugs

1. Выкачиваем код spotbugs.
git clone

2. Собираем проект
./gradlew distZip

3. Распаковываем дистрибутив в директорию /spotbugs-4.0.0-SNAPSHOT
Для разработки плагина используем intellij idea community edition.

4. Создаем новый проект java (gradle) приложения.

5. Настраиваем зависимости. Копируем файл spotbugs.jar в директорию  /libs проекта.

build.gradle  
dependencies {
  compile fileTree(dir: 'libs', include: ['*.jar'])
  compile 'org.apache.bcel:bcel:6.1'
}

Из чего состоит плагин

Файл Findbugs.xml — описание плагина

<FindbugsPlugin xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://findbugs.googlecode.com/svn-history/r13379/trunk/findbugs/etc/findbugsplugin.xsd"
              pluginid="ru.sberbank.android.log.plugin"
              provider="Find Android.Util.Log class"
              defaultenabled="true"
              website="https://find-sec-bugs.github.io">
  <Detector class="ru.sbt.detector.AndroidLogCallDetector"/>
  <BugPattern type="ANDROID_LOG_CALL_DETECTED" abbrev="ANDR_LOG" category="CORRECTNESS"/>
</FindbugsPlugin>

Файл Messages.xml — шаблон сообщения о найденном баге

<MessageCollection>
  <Detector class="ru.sbt.detector.AndroidLogCallDetector">
      <Details>ANDROID_LOG_CALL_DETECTED alala.</Details>
  </Detector>
  <BugPattern type="ANDROID_LOG_CALL_DETECTED">
      <ShortDescription>android.util.Log usage detected !</ShortDescription>
      <LongDescription>android.util.Log usage detected !</LongDescription>
      <Details>
          <![CDATA[
      <p> Please not use android.util.Log ! Use 'ru.sberbank.mobile.core.log.Logger' instead.
    ]]>
      </Details>
  </BugPattern>
  <BugCode abbrev="ANDR_LOG">android.util.Log usage detected !</BugCode>
</MessageCollection>

Код кастомного детектора

Напомним, что детектор работает с байткодом. Для понимания работы детектора приведем часть байткода метода onCreate()  Activity, где вызывается Lod.d(«aaa»,«bbb»);  

javap -c LoginActivity.class

Compiled from "LoginActivity.java"
public class ru.sbrf.testandroidapp.LoginActivity extends android.support.v7.app.AppCompatActivity implements android.app.LoaderManager$LoaderCallbacks<android.database.Cursor> {
 public ru.sbrf.testandroidapp.LoginActivity();
 protected void onCreate(android.os.Bundle);
   Code:
      0: aload_0
      1: aload_1
      2: invokespecial #10                 // Method android/support/v7/app/AppCompatActivity.onCreate:(Landroid/os/Bundle;)V
      5: ldc           #11                 // String aaa
      7: ldc           #12                 // String bbb
      9: invokestatic  #13                 // Method    android/util/Log.d:(Ljava/lang/String;Ljava/lang/String;)I
     12: pop
     13: aload_0
     14: ldc           #15                 // int 2130968603
     16: invokevirtual #16                 // Method setContentView:(I)V

Строчка 9 и будет искать наш детектор. Это вызов статического метода в классе android.util.Log

Код детектора

public class AndroidLogCallDetector extends BytecodeScanningDetector implements StatelessDetector {
  public static final String BUG_TYPE = "ANDROID_LOG_CALL_DETECTED";
  public static final String ANDROID_UTIL_LOG = "android/util/Log";
  public static final String ALLOWED_LOGER_CLASS = "ru.sberbank.mobile.core.log.Logger";

  private final BugReporter bugReporter;
  private String clsName;

  public AndroidLogCallDetector(BugReporter bugReporter) {
      this.bugReporter = bugReporter;
  }

  @Override
  public void visitClassContext(ClassContext classContext) {
      JavaClass cls = classContext.getJavaClass();
      clsName = cls.getClassName();
      //skip allowed logger class
      if (!ALLOWED_LOGER_CLASS.equals(clsName)) {
          super.visitClassContext(classContext);
      }
  }

  @Override
  public void sawOpcode(int seen) {
      if (seen == Const.INVOKESTATIC && ANDROID_UTIL_LOG.equals(getClassConstantOperand())) {
      BugInstance bug = new BugInstance(this, BUG_TYPE, NORMAL_PRIORITY).addClassAndMethod(this)
                  .addSourceLine(this);
          bugReporter.reportBug(bug);
      }
  }
}
  

Собираем плагин:

 ./gradlew jar

Тестирование

Копируем файл плагина detector-1.0-SNAPSHOT.jar в  /spotbugs-4.0.0-SNAPSHOT/plugin. Запускам ui интерфейс spotbugs  /spotbugs-4.0.0-SNAPSHOT/bin/spotbugs. Выбираем директорию с байткодом тестового андроид проекта для проверки. /TestAndroidApp/app/build/intermediates/classes/debug



Как видим, нашлись все вызовы класса android.util.Log. Теперь наш кастомный детектор можно использовать в системе CI. Он будет сообщать разработчику о найденных багах.

Проверка стиля кода

Теперь освободим ревьювера от проверки кода на соответствие code-style. Для этого будем использовать плагин Checkstyle. К нему, кстати, прилагается список всех уже реализованных проверок, находится здесь.  

Настраивается плагин с помощью конфигурационного файла checkstyle.xml. В нем указываются нужные проверки и их параметры. Приведем примеры файла для следующих проверок:

• наличие java doc для public методов,
  
• порядок членов в классе(поля, методы)
  
• порядок модификаторов переменных
  
• наличие магических чисел
  
• обязательность { } в if
  
• дефолтная ветка в switch
  
• парность и ковариантность hashcode и equals
  

 

<i><?</i>xml version="1.0"<i>?></i>
<!DOCTYPE module PUBLIC
  "Check Configuration"
  "http://checkstyle.sourceforge.net/dtds/configuration_1_3.dtd"<i>></i>
<module name="Checker">
  <property name="charset" value="UTF-8" />
  <property name="severity" value="warning" />
  <module name="TreeWalker">
      <module name="EqualsHashCode" />
      <module name="CovariantEquals" />
      <module name="MagicNumber" />
      <module name="DeclarationOrder" />
      <module name="JavadocMethod">
          <property name="scope" value="public" />
          <property name="allowMissingParamTags" value="true" />
          <property name="allowMissingThrowsTags" value="true" />
          <property name="allowMissingReturnTag" value="true" />
          <property name="allowThrowsTagsForSubclasses" value="true" />
      </module>
      <module name="LineLength">
          <property name="max" value="160" />
          <property name="ignorePattern"
              value="^package.*|^import.*|a href|href|http://|https://|ftp://" />
      </module>
      <module name="EmptyBlock">
          <property name="option" value="TEXT" />
          <property name="tokens"
              value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH" />
      </module>
      <module name="NeedBraces" />
      <module name="MissingSwitchDefault" />
      <module name="ModifierOrder" />
      <module name="OverloadMethodsDeclarationOrder" />
  </module>
</module>

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

Отметим,  что Checkstyle работает с исходным кодом проекта, анализ которого сделан с помощью ANTLR. Для реализации проверки надо реализовать visitor, который проходится по синтаксическому дереву исходного кода. В этой статье мы не будем рассматривать создание кастомной проверки code-style.

Добавим плагин checkstyle в android приложение

В build.gradle добавляем строки

apply plugin: 'checkstyle'
task checkstyle(type: Checkstyle) {
  configFile file("$project.rootDir/app/checkstyle.xml")
  source 'src/main/java'
  include '**/*.java'
  exclude '**/gen/**'
  exclude '**/R.java'
  exclude '**/BuildConfig.java'
  classpath = files()
}
 
tasks.withType(Checkstyle) {
  reports {
      xml.enabled false
      html.enabled true
  }
}

Запуск проверки  ./gradlew checkstyle.
Отчет будет в директории /build/reports/checkstyle/checkstyle.html . Вот пример отчета.



А вот код, который проверяли

20     static public double  getCount(){
21       if(true) Log.<i>i</i>("a","b");
22      return 43.2*15.0; 
23    }

Итак, мы освободили ревьюверов от рутинной работы — проверки code-style и проверки проектных соглашений. Теперь у разработчиков будет больше времени для творчества, а код станет более качественным. Надеемся, материал был полезен, будем рады ответить на ваши вопросы в комментариях.