Хочу рассказать о том, на что я потратил почти все свободное время в начале этой весны. Это статический анализатор кода для Delphi. Назвал я его скромно — SourceOddity. Очень краткий анонс и чуть больше скриншотов доступны по ссылке.
Мотивация достаточно проста.
Все мы люди и совершаем ошибки. Более того — мы работаем с другими людьми, которые тоже совершают ошибки. Нередко эти ошибки достаточно типичны, но искать их и исправлять от этого не проще. Почему бы не автоматизировать этот процесс? Всю работу за нас статический анализатор конечно не сделает, но почему бы не отдать ему право искать в коде какие-то типичные ситуации, которые потенциально могут вызвать проблемы?
Какие-то ситуации в коде встречаются часто, какие-то реже, какие-то совсем редко. И вот здесь проявляется преимущество статического анализа — бездушной машине можно поручить регулярный поиск даже очень редких случаев. Машина безмолвна, она не будет против. И чем больше будет таких правил, тем больше пользы принесет проверка.
Иногда, вовремя обнаружив подобную ситуацию, можно сэкономить много сил и нервов:
Разработчик поторопился, поленился, скопировал условие и забыл исправить. Проблему обнаружили только тестировщики или того хуже — пользователи. Все это потеря времени, денег и лояльности пользователей.
Статический анализ безусловно полезен.
Я осознаю, что статические анализаторы изобрел не я и SourceOddity не первая ласточка, но я думаю, что у меня есть некоторые идеи и опыт, которые позволят сделать более пригодный к повседневному использованию инструмент.
Первая публичная версия будет доступна в ближайшие недели, а пока я хотел бы обсудить эту тему и, может быть, услышать идеи — какие проблемные места в коде вы хотели бы автоматически определять.
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 пп
Приятная и неожиданная опечатка. Я не ожидал, что намек на эту песню настолько ясен :)
Консольная утилита конечно нужна. Благо, имея все остальное, сделать ее не очень сложно, а хуже от нее никому не будет.
Вот это точно, как-то мне даже в голову не пришло. Спасибо за идею.
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. Иногда это осознанный выбор, а иногда вот такая опечатка, например:
ifa = bthen;beginend;Empty ELSE block
То же самое с else.
THEN statement is equal to ELSE statement
Выражения в then и else совпадают. Неаккуратная копипаста?
ifa = bthencelsec;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. Так делать на стоит.
trya := TObject.Create;a.Method;finallya.Free;end;Odd ELSE-IF condition
Странный if else. Неаккуратная копипаста или опечатка?
ifa =1thenProcAelseifa =1thenProcB;Unneeded boolean comparison
Сугубо стилевая ругань. Так делать не принято:
ifa =Truethen...Ну и напоследок еще три сугубо стилевые вещи:
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;trya := TObject.Create;a.Method;finallya.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;fori :=0toCount -1dotrya := TA.Create;b := TB.Create;c := TC.Create;...а тут может быть и Continue, и Break, и Exit...finallyFreeAndNil(c);FreeAndNil(b);FreeAndNil(a);end13. Роман Янковский
27 Мар 2014 9:51 дп
Да, я знаю про эту особенность try/finally, сам так иногда пишу. Пока, если честно, этот нюанс не учитывается, но постараюсь в итоге учесть. Это вполне возможно.
Правила уже сейчас можно включать/выключать. Выключение их на отдельных местах кода безусловно запланировано, но надо подумать как это сделать так, чтоб это было удобно и код визуально не засоряло.
За идеи спасибо :)
Сегодня утром добавил еще одно правило — в функции Format количество параметров в строке должно совпадать с реальным количеством передаваемых параметров.
14.
Марк Бердников
27 Мар 2014 11:16 дп
Лучше, чтобы реальное количество параметров было не меньше, чем требуется. Часто в параметры вбиваю все и копирую, изменяя только строку. Да и тип заодно можно проверять — частая ошибка.
Format('%d: %2:d', [a, b, c, d]);15. Игорь Шевченко
27 Мар 2014 11:18 дп
А смысл так писать?
foo:=nil;tryfoo := tfoo.create;...finallyfoo.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