ミスを少なくする明日からのCustom Lint Rules

  • 18
    いいね
  • 0
    コメント

社会の荒波に旅立ってから早半年が経過した研修の果にです。
Custom Lint Rulesについて書きます🙇


Custom Lint Rulesの作り方や説明などはhotchemiさんのCustom Lint Rulesが非常に参考になるので御覧ください。
今回は実際に作ってみてハマった点と、既存のGoogleのCodeを読んで参考になった部分を紹介します。

作ったもの

背景

Databindingを使ったNumberPickerでの実装に若干ハマったことがきっかけでした。
NumberPickerではありがたいことにvalueに対し2-way bindingが提供されています。
そのため以下のような実装をしました

<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto">

    <data>

        <variable
            name="hoge"
            type="Hoge" />
    </data>

    <FrameLayout
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <NumberPicker
            android:id="@+id/hoge_number_picker"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:descendantFocusability="blocksDescendants"
            android:value="@={hoge.value}"
            app:maxValue="@{hoge.maxValue}" />

    </FrameLayout>

</layout>

一見普通にNumberPickerに対しmaxValue, valueがセットされるように見えます。
ただ、実際は望んだ動きをしませんでした。

作成されたBindingは以下のようになっていました。

        // batch finished
        if ((dirtyFlags & 0x19L) != 0) {
            // api target 1

            android.databinding.adapters.NumberPickerBindingAdapter.setValue(this.hogeNumberPicker, value);
        }
        if ((dirtyFlags & 0x11L) != 0) {
            // api target 11
            if(getBuildSdkInt() >= 11) {
                this.hogeNumberPicker.setMaxValue(maxValue);
            }
        }

...!!!
setValue -> setMaxValueの順番で呼ばれている!!
といったことが起きたわけです。
NumberPicker自身が最大値を知らない状態でいきなり「3スクロール目!」と言われても困ってしまいますよね。
ということで、表示がきちんとされなくて困ります。

解決策

  • ViewModelで対応する
  • NumberPickerを継承したCustomViewを作る

上記の2パターンがすぐ思いつくかと思います。
今回はなんとなく NumberPickerを継承したCustomViewを作る の方で実装を進めました。

class DatabindingableNumberPicker : NumberPicker {
    constructor(context: Context) : super(context)
    constructor(context: Context, attributeSet: AttributeSet) : super(context, attributeSet)
    constructor(context: Context, attributeSet: AttributeSet, defStyleAttr: Int) : super(context, attributeSet, defStyleAttr)

    private var isCallSetMaxValueBeforeSetValue = false
    private var tempValue = 0

    override fun setValue(value: Int) {
        if (!isCallSetMaxValueBeforeSetValue) {
            tempValue = value
            isCallSetMaxValueBeforeSetValue = true
        } else {
            super.setValue(value)
        }
    }

    override fun setMaxValue(maxValue: Int) {
        super.setMaxValue(maxValue)
        if (isCallSetMaxValueBeforeSetValue) {
            value = tempValue
        } else {
            isCallSetMaxValueBeforeSetValue = true
        }
    }
}

上記のCustomViewを下記のように実装すれば

<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto">

    <data>

        <variable
            name="hoge"
            type="Hoge" />
    </data>

    <FrameLayout
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <DatabindingableNumberPicker
            android:id="@+id/hoge_number_picker"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:descendantFocusability="blocksDescendants"
            android:value="@={hoge.value}"
            app:maxValue="@{hoge.maxValue}" />

    </FrameLayout>

</layout>

Viewの名前の変更だけでXML側の基本的な実装を変えずに使うことが出来ます。
これでハッピーです。


Screen Shot 2016-12-03 at 1.26.41 AM.png

そう、これは全てを解決してないんです。
結局新しく作ったViewを使うのを忘れてしまっては意味がありません。
そこでCustom Lint Rulesが頭をよぎり、作成に至ったというわけです。

Detectorの作成

今回のCustomLint Rulesの方針は NumberPickerに"android:value"属性がある場合は警告を出す ということで進めました。

public class NotUsageDatabindingableNumberPickerDetector extends LayoutDetector {

    public static final Issue ISSUE = Issue.create(
            "NotUsageDatabindingableNumberPicker",
            "Using NumberPicker!!!!",
            "When using NumberPicker with `Databinding`, you must use `DatabindingableNumberPicker`.",
            Category.MESSAGES,
            10,
            Severity.ERROR,
            new Implementation(
                    NotUsageDatabindingableNumberPickerDetector.class,
                    Scope.RESOURCE_FILE_SCOPE));

    public NotUsageDatabindingableNumberPickerDetector() {
    }

    @Override
    public Speed getSpeed() {
        return Speed.FAST;
    }

    public static final String NUMBER_PICKER = "NumberPicker";

    @Override
    public Collection<String> getApplicableElements() {
        return Collections.singletonList(NUMBER_PICKER);
    }

    @Override
    public void visitElement(@NonNull XmlContext context, @NonNull Element element) {
        if (!element.hasAttribute(PREFIX_ANDROID + ATTR_VALUE)) {
            return;
        }
        final Attr attributeNode = element.getAttributeNode(PREFIX_ANDROID + ATTR_VALUE);
        final String attributeValue = attributeNode.getValue();
        final boolean isUseDatabinding = attributeValue.startsWith("@={") && attributeValue.endsWith("}");
        if (isUseDatabinding) {
            context.report(
                    ISSUE,
                    attributeNode,
                    context.getLocation(attributeNode),
                    "This NumberPicker should be a DatabindingableNumberPicker");
        }
    }
}

基本的に説明することはありませんが、若干止まった部分はElement#hasAttributeATTR_VALUEだけを渡していたんですが、ダメでした...

DetectorTestの作成

Custom Lint Rulesを作るにあたって、きちんと動くことを保証する大事な部分です。
先にTestを書いてからDetectorの作成をしたほうが本当に効率が良いです。

DetectorのTestはLintDetectorTestが提供されているのでそちらを使うと簡単にTestを書くことが出来ます。

public class NotUsageDatabindingableNumberPickerDetectorTest extends LintDetectorTest {

    private static String NO_WARNING = "No warnings.";

    @Override
    protected Detector getDetector() {
        return new NotUsageDatabindingableNumberPickerDetector();
    }

    @Override
    protected List<Issue> getIssues() {
        return Collections.singletonList(NotUsageDatabindingableNumberPickerDetector.ISSUE);
    }

    public void testUseNumberPickerWhenNotUseDatabinding() throws Exception {
        final String result = lintProject(xml("res/layout/test.xml", TestData.USE_NUMBER_PICKER_WHEN_NOT_USE_DATABINDING));

        assertEquals(NO_WARNING, result);
    }

    public void testUseDatabindingableNumberPickerWhenUseDatabinding() throws Exception {
        final String result = lintProject(xml("res/layout/test.xml", TestData.USE_DATABINDINGABLE_NUMBER_PICKER_WHEN_USE_DATABINDING));

        assertEquals(NO_WARNING, result);
    }

    public void testUseNumberPickerWhenUseDatabinding() throws Exception {
        final String result = lintProject(xml("res/layout/test.xml", TestData.USE_NUMBER_PICKER_WHEN_USE_DATABINDING));

        assertEquals("" +
                        "res/layout/test.xml:19: Error: This NumberPicker should be a DatabindingableNumberPicker [NotUsageDatabindingableNumberPicker]\n" +
                        "            android:value=\"@={hgoe}\" />\n" +
                        "            ~~~~~~~~~~~~~~~~~~~~~~~~\n" +
                        "1 errors, 0 warnings\n",
                result);
    }
}

TestData.java
Testを作るに当たって幾つかハマった点があるのでそちらを紹介します。

まずはじめに、
xml("res/layout/test.xml",TestData.USE_DATABINDINGABLE_NUMBER_PICKER_WHEN_USE_DATABINDING)
の部分。
xml(String to, String source)toにはLayout Resourcesであることをきちんと書かないと認識してくれません。
具体的には"res/leyout/hoge.xml"のように"res/layout"が非常に大事です。
自分はこれで30分消耗しました。

続いてはsourceの部分。

    static String USE_NUMBER_PICKER_WHEN_USE_DATABINDING = "" +
            "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
            "<layout>\n" +
            "\n" +
            "    <data>\n" +
            "\n" +
            "        <variable\n" +
            "            name=\"hgoe\"\n" +
            "            type=\"int\" />\n" +
            "    </data>\n" +
            "\n" +
            "    <LinearLayout xmlns:android=\"http://schemas.android.com/apk/res/android\"\n" +
            "        android:layout_width=\"match_parent\"\n" +
            "        android:layout_height=\"match_parent\"\n" +
            "        android:orientation=\"vertical\">\n" +
            "\n" +
            "        <NumberPicker\n" +
            "            android:layout_width=\"wrap_content\"\n" +
            "            android:layout_height=\"wrap_content\"\n" +
            "            android:value=\"@={hgoe}\" />\n" +
            "\n" +
            "    </LinearLayout>\n" +
            "\n" +
            "</layout>";

きちんとビルドが通るようなlayout xmlでないといけません。
適当に書いても行けるだろうと考えて、適当に書いたら辛くなります。
これで30分消耗しました...

後は細かい設定

CIのTaskに追加したり、GithubのPR Templateを更新するなどして、定期的に
./gradlew check
を走らせるようにしましょう。

今回できなかったこと

普段Circle CIを使っているのですが、CIでLint checkをした時に限り、SeverityErrorからWarningに上書きされてしまいます。
ほんとなんでだろう...
CIをLint Errorで落としたいんですが、WarningになってしまうのでCI自体が通ってしまいます。
なのでLint結果のXMLをParseし、PriorityをみてCIを落とそうかなと考えています。

ということで今回作ったCustom Lint Rulesの紹介終わり🙇

参考にしたDetectorのCode

ImageViewにcontentDescriptionがないとlayoutでよく黄色くなっていたアレです。
hintcontentDescription両方同時に指定したらいけなかったんですね、知らなかった。

contentDescription=""
contentDescription="TODO"
TODOもLintに引っかかるるんですね...

一番実装の参考になった。

Attribute取得周り参考になる。

終わりに

Custom Lint Rules良いですね。
どれだけ自己レビューをやっても見つけ出せないバグや、引き継いだ際の仕様報告漏れなど、人為的なミスで多くのバグが生まれます。
できるだけ人間の手を介さないようにCheckしていくというのは非常に良いと思います。

「横展開」がいっぱい出来るようにどんどん作っていきたいですね。

良いサービスは良い開発基盤から。