26.03.2014 3:54 пп

Первые шаги SourceOddity

Хочу рассказать о том, на что я потратил почти все свободное время в начале этой весны. Это статический анализатор кода для Delphi. Назвал я его скромно — SourceOddity. Очень краткий анонс и чуть больше скриншотов доступны по ссылке.

SourceOddity Screenshot

Мотивация достаточно проста.

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

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

Иногда, вовремя обнаружив подобную ситуацию, можно сэкономить много сил и нервов:

SourceOddity Screenshot

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

Статический анализ безусловно полезен.

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

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

Связанные записи:

SourceOddity => FixInsight

Метки: Delphi

39 комментариев

1. Aleksey Timohin

26 Мар 2014 6:48 пп

Я бы сказал, что консольная версия была бы кстати. Чтобы можно было запустить что-то типа:
SpaceOddity.exe /PathToMySources=d:\work\LazyDelphiBuilder [/any_config_or_rules_for_source_oddity=anyconfig.ini]

И получить список результатов анализа. В консоль или файл.
Где PathToMyProject — путь до папки с исходниками
А any_config_or_rules_for_source_oddity — путь до правил анализа для Source Oddity (если что-то такое планируется).

Еще из плюсов — не привязана к версии Delphi, как эксперт.
Может быть с ограничением — выводить не больше 5 предупреждений каждого типа. Для публичной версии. Этого вполне хватит чтобы исправить все ошибки в проекте не покупая лицензии посмотреть на софтину в работе.

2. Роман Янковский

26 Мар 2014 6:58 пп

SpaceOddity.exe

Приятная и неожиданная опечатка. Я не ожидал, что намек на эту песню настолько ясен :)

Консольная утилита конечно нужна. Благо, имея все остальное, сделать ее не очень сложно, а хуже от нее никому не будет.

Еще из плюсов — не привязана к версии Delphi, как эксперт.

Вот это точно, как-то мне даже в голову не пришло. Спасибо за идею.

3. Ильдар Вильданов

26 Мар 2014 7:08 пп

Очень круто. Буду ждать с нетерпением.
В последнее время много статей про анализаторы на хабре, но все для С, C++,
а для delphi пока особого выбора нет среди бесплатных.
p.s. будет ли она OpenSource?

4. IL

26 Мар 2014 10:29 пп

Посмотреть бы список правил, которые программа проверяет.
Ну и хотелось бы видеть проверки CodeVerifier http://www.ivcons.ru/codeverifier/ заодно и тему http://www.sql.ru/forum/297266/peganza-pascal-analyzer-socksoftware-codehealer-ishhu-analogi-i-prodavcov-v-rossii оживить :)
Готов поучаствовать в тестировании на своих и открытых проектах.

5. Роман Янковский

26 Мар 2014 11:23 пп

Ильдар Вильданов, вот opensource врядли. Возможно, некоторые части…

IL, я дам список проверок, которые есть на текущий момент. Этот список медленно, но настойчиво, пополняется, так что он уже завтра может перестать быть полным и актуальным:

Empty EXCEPT block
Это понятно. Ругаемся на пустой блок except.

Empty FINALLY block
То же самое с finally. Странная ситуация.

Assignment right hand side is equal to its left hand side
Правая и левая части присваивания равны (a := a). Неаккуратная копипаста?

Missing INHERITED call in destructor
Нет вызова inherited в деструкторе

Empty THEN block
Пустой блок then. Иногда это осознанный выбор, а иногда вот такая опечатка, например:

if a = b then;
begin
end;

Empty ELSE block
То же самое с else.

THEN statement is equal to ELSE statement
Выражения в then и else совпадают. Неаккуратная копипаста?

if a = b then c else c;

Variable is assigned twice successively
Два присваивания одной переменной подряд. Опечатка?

a := 1;
a := 2;

Unreachable code
Недостижимый код

ProcA;
Exit;
ProcB; // <-- никогда не выполнится

The same value is on the both sides of operator. Consider reviewing it.
Одинаковые операнды по обе стороны оператора. Неаккуратная копипаста или опечатка?

c := a - a;

Object «%s» created in TRY block
Объект создан внутри блока try. Так делать на стоит.

try
  a := TObject.Create;
  a.Method;
finally
  a.Free;
end;

Odd ELSE-IF condition
Странный if else. Неаккуратная копипаста или опечатка?

if a = 1 then ProcA 
else if a = 1 then ProcB;

Unneeded boolean comparison
Сугубо стилевая ругань. Так делать не принято:

if a = True then...

Ну и напоследок еще три сугубо стилевые вещи:
Method «%s» is too long (%d lines)
Too many parameters in «%s» (%d parameters)
Too many variables in «%s» (%d variables)
Слишком длинный метод, слишком много параметров или слишком много переменных.

6. IL

27 Мар 2014 12:02 дп

Уже, однозначно, неплохо! Особенно пустые then; else;
Проверка перекрытия методов во вложенных with и использование в with нескольких идентификаторов http://www.delphifeeds.com/go/s/113867 была бы полезна.
Из 1-й главы книги Ника Х. Coding In Delphi: реагирование на все исключения, использование исключений для проверок типа try StrToInt(str) except … end вместо TryStrToInt(), передачи сообщений внутри программы, когда нет ошибочной ситуации.
Про такие штуки http://www.delphifeeds.com/go/s/114157 даже не заикаюсь, т.к. ничего там не понимаю ;)

7. Aleksey Timohin

27 Мар 2014 12:11 дп

> Объект создан внутри блока try. Так делать на стоит.
Вообще — это наиболее простой (и вполне правильный) способ, в случае когда создаётся несколько объектов. При условии что перед try объект инициализирован.
Пример:

a := nil;
try
  a := TObject.Create;
  a.Method;
finally
  a.Free;
end;

8. Aleksey Timohin

27 Мар 2014 12:14 дп

Как вариант для проверки (warning):
Consider using FreeAndNil instead of Free method.
В идеале, если есть возможность учитывать Scope, то выводить это предупреждение только для объектов объявленных за пределами текущей процедуры/функции.

9. Aleksey Timohin

27 Мар 2014 12:15 дп

Еще идея: ругаться на вызов destroy вне деструктора.

10. Aleksey Timohin

27 Мар 2014 12:17 дп

Еще: Типичнейшие ошибки новичков:
variable := variable.create или variable.create

Вызов методов объекта до создания/инициализации — хотя не представляю можно ли это как-то определить.

11. Николай Зверев

27 Мар 2014 7:30 дп

Круто! :)

> Объект создан внутри блока try
Поддержу Алексея, на это надо ругаться только в том случае, если переменная явно не обнуляется (:= nil, FreeAndNil)

Предложу:
Наверное так же нужны какие-то метки в коде, которые бы говорили анализатору, что вот эта вот «хрень» написана сознательно и не надо на неё ругаться.

Ну и чтобы правила из набора можно было включать/выключать (как хинты и предупреждения компилятора).

Удивлюсь:
По ссылке «Sign Up for Private Beta» нету Delphi 2010 в списке

12. Николай Зверев

27 Мар 2014 7:37 дп

кстати, насчёт try Create Free и FreeAndNil
чтобы сократить число блоков try/finally и повысить читаемость, иногда пишу подобный код:

a := nil;
b := nil;
c := nil;
for i := 0 to Count - 1 do
try
  a := TA.Create;
  b := TB.Create;
  c := TC.Create;
  ...
  а тут может быть и Continue, и Break, и Exit
  ...
finally
  FreeAndNil(c);
  FreeAndNil(b);
  FreeAndNil(a);
end

13. Роман Янковский

27 Мар 2014 9:51 дп

Да, я знаю про эту особенность try/finally, сам так иногда пишу. Пока, если честно, этот нюанс не учитывается, но постараюсь в итоге учесть. Это вполне возможно.

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

За идеи спасибо :)
Сегодня утром добавил еще одно правило — в функции Format количество параметров в строке должно совпадать с реальным количеством передаваемых параметров.

14. Марк Бердников

27 Мар 2014 11:16 дп

Сегодня утром добавил еще одно правило — в функции Format количество параметров в строке должно совпадать с реальным количеством передаваемых параметров.

Лучше, чтобы реальное количество параметров было не меньше, чем требуется. Часто в параметры вбиваю все и копирую, изменяя только строку. Да и тип заодно можно проверять — частая ошибка.

Format('%d: %2:d', [a, b, c, d]);

15. Игорь Шевченко

27 Мар 2014 11:18 дп

А смысл так писать?

foo:=nil;
try
  foo := tfoo.create;
...
finally
  foo.Free;
end;

Честно не понимаю причин.

16. Igor Schevchenko

27 Мар 2014 11:20 дп

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

Копировать плохо в принципе :)

17. Krom

27 Мар 2014 12:36 пп

Здорово. Хочу потестировать на своем проекте!

Еще из пожеланий
— неиспользуемые модули в uses
— забытые -1 в итераторах (for I :=0 to Length(array) do)
— несовпадение объявления и реализации методов (Делфа это допускает, но выглядит как возможная ошибка при обновлении кода)
— двусмысленные переменные в блоках with

18. balmo

27 Мар 2014 3:10 пп

Роман, отличная идея!

Не знаю принципов организации анализатора… Если планируется построение дерева лексем с последующим его анализом (поиск плохих поддеревьев) — было бы просто отлично предоставить сообществу возможность поставлять такие паттерны. В одиночку это очень сложно осилить ИМХО, а сообщество быстро наполнит базу типичными «косяками».

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

Подписываюсь и жду новостей. Если нужна поддержка — адрес в check-in’е…

19. Aleksey Timohin

27 Мар 2014 4:58 пп

> Сегодня утром добавил еще одно правило — в функции Format количество параметров в строке должно совпадать с реальным количеством передаваемых параметров.

Там не всё так просто. Функция формат позволяет указывать индекс параметра в format string. См. параметр index в http://docwiki.embarcadero.com/Libraries/XE5/en/System.SysUtils.Format

А насчёт идеи определения неиспользуемых uses модулей — с этой штукой вообще надо быть очень осторожным. Тогда надо проверять есть ли в модуле initialization/finalization секции — и если есть, то считать модуль априори используемым. А так как initialization/finalization встречается очень часто, то и проку от такой проверки, имхо, нет.

20. Роман Янковский

27 Мар 2014 10:00 пп

Все идеи я запоминаю и буду учитывать. Спасибо!

Добавил ругань на циклы for от 0 до Length(var), т.к. в них вероятно забыт (-1).

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

Если планируется построение дерева лексем с последующим его анализом (поиск плохих поддеревьев) — было бы просто отлично предоставить сообществу возможность поставлять такие паттерны.

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

21. IL

28 Мар 2014 8:04 дп

Роман, а DelphiSpec для написания правил или тестов планируете прикрутить?

22. A

28 Мар 2014 10:40 дп

Проверка правильного преобразования типов была бы кстати. Например, в вызове win api Integer(a) на WParam(a) или LONG_PTR(a).
http://docwiki.embarcadero.com/RADStudio/XE5/en/Converting_32-bit_Delphi_Applications_to_64-bit_Windows

23. Александр Люлин

4 Апр 2014 3:05 дп

Рома, если бы анализатор отлавливал бы Supports («паразитные») к объектам БЕЗ IUnknown, то ЦЕНЫ бы ему не было бы.

Пример тут — http://programmingmindstream.blogspot.ru/2014/04/supports.html

Если такое возможно, то я ПЕРВЫЙ готов его КУПИТЬ.

А вообще говоря тебе надо определяться с «парадигмой».

Либо ты зарабатываешь деньги, тогда начинай продажи.

Либо ты зарабатываешь «славу и авторитет», тогда надо делать OpenSource.

Если вдруг решишь делать OpenSource — «зови меня» :-) Я с УДОВОЛЬСТВИЕМ в этом поучаствую.

Как в плане «расширения» набора «ошибок», так и в части «скрещивания» всего этого с кодогенерацией и тестами.

Я конечно — «зануда», но поверь — всё это — ОЧЕНЬ близкие вещи.

Кстати если ты решишь не делать OpenSource, то предложение «поучаствовать» — всё равно в силе.

Хочется «поучаствовать» в «чём-то нужном», а не «распылять свои силы в своём углу».

Я — идеалист. Я «верю» в силу «сообщества».

А «сообщество» — надо «организовывать».

:-) Прости за занудство.

Ну и как говорил Штрилиц — «запоминается последняя фраза».

Посему — повторю:

Рома, если бы анализатор отлавливал бы Supports («паразитные») к объектам БЕЗ IUnknown, то ЦЕНЫ бы ему не было бы.

Пример тут — http://programmingmindstream.blogspot.ru/2014/04/supports.html

24. Александр Люлин

4 Апр 2014 3:11 дп

Ну и позанудствую ещё…

СНАЧАЛА — «документ» — ИНСТРУКЦИЯ ПРОГРАММИРОВАНИЯ (что можно, а что нельзя), а ПОТОМ — «инструмент» — СТАТИЧЕСКИЙ анализатор. Чтобы было ПОНЯТНО — ЧТО анализатор «анализирует» и валидирует.

СНАЧАЛА «надо убедить людей», что эти «практики валидации» ПРАВИЛЬНЫЕ, а ПОТОМ только ВАЛИДИРОВАТЬ.

Ну ИМХО…

Как в случае «с присно памятным» FreeAndNil.

25. Александр Люлин

9 Апр 2014 2:11 дп

Ром, когда развития ждать? И что с DelphiSpecs?

26. Роман Янковский

9 Апр 2014 10:19 дп

Александр Люлин, так развитие идет, я выпустил несколько билдов беты, она все стабильнее и функциональнее. Ты как-то интереса не проявил, вот я и не дергаю.

С DelphiSpec пока ничего не происходит. Я все-таки один и не могу делать все сразу. Движок для выполнения сценариев там в принципе готов. Я планировал сделать эксперты для IDE для автогенерации шаблонов каких-нибудь, ну и для простых вещей вроде подсветки синтаксиса сценариев, но руки пока не дошли. Однажды я этим займусь. Если есть желание — подключайся, проект опенсорсный и любая помощь только приветствуется :)

27. silver

16 Апр 2014 1:13 дп

я пару буков оставлю — RAII

28. Dmitriy Sinyavskiy

16 Апр 2014 5:23 пп

Я использовал SonarQube Delph Plugin и оно работало для маленьких проектов Delphi XE2, но на большом проекте Delphi 7 ломается и не завершает анализ.
Думаю Вам будет интересно знать, какие проверки выполняет этот плагин
http://snapshots.repository.codehaus.org/org/codehaus/sonar-plugins/sonar-delphi-plugin/0.2-SNAPSHOT/
Например интересная проверка на отсутствие inherited Create;

29. Dmitriy Sinyavskiy

16 Апр 2014 5:27 пп

Хорошо бы выдавать отчет в разных форматах, например EMMA, тогда можно будет эти отчеты в Jenkins CI смотреть.

30. Alex W. Lulin

22 Апр 2014 12:56 дп

«я пару буков оставлю — RAII»

;-)

ВЕСКО :-)

31. Torbins

5 мая 2014 5:34 пп

Насколько продвинутый у вас получился парсер паскаля? Можно с его помощью построить к примеру интелектуальный поисковик, который на запрос «(Class:TSomeClass).Exec» найдет «Executor.Exec», где Executor является наследником TSomeClass? Что то типа такого: https://www.youtube.com/watch?v=mVbDzTM21BQ

32. Роман Янковский

5 мая 2014 7:58 пп

Torbins, в данный момент в парсере нет функционала, чтобы сделать подобное. Но я планирую, что со временем этот функционал будет, лишь бы у меня хватило сил :)

33. Alex W. Lulin

4 Июл 2014 2:51 дп

Как твои успехи, Роман, на данном поприще?

34. Alex W. Lulin

5 Июл 2014 3:33 дп

Понятно.. «никак»…. что я собственно и прогнозировал.. потому что все программисты — «жадины»…

35. Alex W. Lulin

11 Июл 2014 12:33 дп

Парни? Всё затихло? «Повосхищались» и хватит?

Кто-нибудь использует?

36. Николай Зверев

11 Июл 2014 2:02 дп

я использую.. где-то раз в два месяца.
только пришлось почти все проверки отключить :(

37. Alex W. Lulin

30 Сен 2014 1:57 дп

http://fire-monkey.ru/topic/611-besplatnaia-litcenziia-na-castalia/

38. Kazantsev Alexey

20 Ноя 2014 2:48 пп

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

Obj.SomeMethod; // WARNING
Obj.SomeMethod(); // OK

ProcVar; // WARNING
ProcVar(); // OK

39. Николай

20 Ноя 2014 4:24 пп

А заодно и взаимоисключающее, т.е.:
Obj.SomeMethod; // OK
Obj.SomeMethod(); // WARNING

ProcVar; // OK
ProcVar(); // WARNING

Оставьте комментарий

Роман.Янковский.me

Tuo tibi iudicio est utendum

Первые шаги SourceOddity

Связанные записи: