Об одной интересной ошибке в Lucene.Net


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

    Введение


    Lucene.Net — это портированная с Java на C# популярная библиотека для полнотекстового поиска. Исходный код открыт и доступен на сайте проекта https://lucenenet.apache.org/.

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

    О найденной ошибке


    Есть у нас диагностика V3035 о том, что вместо += можно по ошибке написать =+, где + будет унарным плюсом. Когда я делал её по аналогии с такой же диагностикой V588, предназначенной для языка C++, я думал — разве можно так ошибиться в C#? В C++ ещё ладно — кто-то использует разные текстовые редакторы вместо IDE, в которых можно опечататься и не заметить ошибку. Но набирая текст в Visual Studio, которая автоматически выравнивает код после того, как поставил точку с запятой, как это можно пропустить? Оказывается, что можно. Такую ошибку я обнаружил в Lucene.Net. А интересна она больше потому, что обнаружить её другими способами кроме статического анализа довольно сложно. Рассмотрим код:

    protected virtual void Substitute( StringBuilder buffer )
    {
        substCount = 0;
        for ( int c = 0; c < buffer.Length; c++ ) 
        {
            ....
    
            // Take care that at least one character
            // is left left side from the current one
            if ( c < buffer.Length - 1 ) 
            {
                // Masking several common character combinations
                // with an token
                if ( ( c < buffer.Length - 2 ) && buffer[c] == 's' &&
                    buffer[c + 1] == 'c' && buffer[c + 2] == 'h' )
                {
                    buffer[c] = '$';
                    buffer.Remove(c + 1, 2);
                    substCount =+ 2;
                }
                ....
                else if ( buffer[c] == 's' && buffer[c + 1] == 't' ) 
                {
                    buffer[c] = '!';
                    buffer.Remove(c + 1, 1);
                    substCount++;
                }
                ....
            }
        }
    }

    Есть класс GermanStemmer, который обрезает суффиксы у немецких слов, чтобы выделить общий корень. Работает он следующим образом: сначала метод Substitute заменяет разные хорошие сочетания букв на другие символы, чтобы не спутать их с суффиксом. Заменяются: 'sch' на '$', 'st' на '!' и так далее (это видно из примера кода). Причем количество символов, на которое такими заменами уменьшится длина слова, накапливается в переменной substCount. Дальше метод Strip отрезает лишние суффиксы, и в конце метод Resubstitute выполняет обратную замену: '$' на 'sch', '!' на 'st'. То есть если у нас, например, было слово kapitalistischen (капиталистический), то стеммер отработает следующим образом: kapitalistischen => kapitali!i$en (Substitute) => kapitali!i$ (Strip) => kapitalistisch (Resubstitute).

    Из-за этой опечатки в коде при замене 'sch' на '$' в переменную substCount будет присвоено значение 2, вместо того, чтобы увеличить substCount на 2. И такую ошибку довольно сложно найти другими методами, кроме как статическим анализом. Есть разработчики, которые говорят: зачем мне статический анализатор, если у меня есть юнит-тесты? Так вот, чтобы отловить такую ошибку тестами, нужно тестировать Lucene.Net на немецком тексте, используя GermanStemmer, в тестах должно индексироваться слово, которое содержит в себе сочетание 'sch' и ещё одно сочетание букв, для которого будет выполнена подстановка, причём оно должно присутствовать в слове перед 'sch', чтобы substCount был отличен от нуля к тому моменту, когда выполнится выражение substCount =+ 2. Довольно нетривиальная комбинация для теста, особенно когда не видишь ошибки.

    Заключение


    Юнит-тесты и статический анализ — это не исключающие, а дополняющие друг друга методики разработки программного обеспечения [2]. Предлагаю скачать статический анализатор PVS-Studio, проверить свои проекты и найти ошибки, которые не были обнаружены с помощью юнит-тестов.

    Дополнительные ссылки


    1. Андрей Карпов. Почему в маленьких программах низкая плотность ошибок.
    2. Андрей Карпов. Как статический анализ дополняет TDD.


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Ilya Ivanov. An unusual bug in Lucene.Net.

    Прочитали статью и есть вопрос?
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio, версия 2015. Пожалуйста, ознакомьтесь со списком.
    PVS-Studio
    247,19
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Комментарии 10

      0
      [del]
        +4
        Это все что у вас есть на Lucene.Net?
          0
          Есть ещё пара замечаний. Но на статью найденное не тянет. Зато встретилась эта интересная редкая ошибка и решили про это рассказать.
            0
            Об остальных предупреждениях, на которые стоит посмотреть, я записал им в баг трекер.
            +2
            А зачем вообще нужен унарный плюс, кроме как для единообразия? Это же бесполезный оператор.
              0
              Чтобы в Python, где нет операции инкремента, всё равно можно было писать ++a!
              В Lua, кстати, унарного плюса нет.
                +2
                Зато его можно перегрузить.

                На самом деле, унарный плюс втянут в C++ из C (ради совместимости), а уж оттуда и в C#.
                Пара важных свойств есть (по-крайней мере для C++, но не знаю, передрали ли эту семантику в C#):

                1) он делает из беззнакового целого знаковое, при этом расширяя тип:
                например, если у вас "unsigned short i = 5", то "+i" имеет тип "int"
                2) выражение "+i" нельзя использовать в качестве lvalue
                  0
                  Интересное св-во, не знал, достаточно красиво получается вместо static_cast(), если писать на плюсах.
                  п.с. парсер съедает кавычки у статик каста(
                    0
                    Лучше не использовать для приведения типа. Неочевидно, если встретится в коде. Эта фича досталась из аналогии с унарным минусом.
                    0
                    Класс, спасибо за разъяснение! Может спасти от гневных ругательств на компилятор (в редком случае)!

                Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                Самое читаемое