Ameba — это линтер для Crystal.
Недавно я написал немного кода для Crystal, так что давайте посмотрим, что из этого получится.
Я запущу его как в настройках по умолчанию, так и с --all. Надеюсь, что при настройках по умолчанию будет мало ложных срабатываний.
Проблема шебанга
ameba игнорировала скрипты Crystal, начинающиеся с #!/usr/bin/env crystal — она просматривала только файлы с расширением .cr. Это не основной вариант использования Crystal, но вполне допустимый.
Расширение VSCode в Crystal имеет ту же проблему.
Проблема может быть решена с помощью конфигурационного файла .ameba.yml.
crystal-z3 и Lint/UselessAssign.
В настройках по умолчанию он находит одну проблему:
$ ameba
Inspecting 25 files
......................F..
src/z3/api.cr:89:7
[W] Lint/UselessAssign: Useless assignment to variable `var`
> var = LibZ3.mk_const(Context, name_sym, sort)
^-^
Finished in 223.71 milliseconds
25 inspected, 1 failure
Он не показывает никакого контекста, но это из этого метода:
def mk_const(name, sort)
name_sym = LibZ3.mk_string_symbol(Context, name)
var = LibZ3.mk_const(Context, name_sym, sort)
end
Это определенно реально.
crystal-z3 с --all и Lint/ComparisonToBoolean
Это порождает огромное количество проблем Lint/ComparisonToBoolean, некоторые примеры здесь:
spec/bool_spec.cr:10:6 [Correctable]
[W] Lint/ComparisonToBoolean: Comparison to a boolean is pointless
> [a == true, b == true, c == (a & b)].should have_solution({c => true})
^--------^
spec/model_spec.cr:15:19 [Correctable]
[W] Lint/ComparisonToBoolean: Comparison to a boolean is pointless
> solver.assert a == true
^-------^
[W] Lint/ComparisonToBoolean: Comparison to a boolean is pointless
> raise Z3::Exception.new("Cannot evaluate") unless result == true
^------------^
Ну, crystal-z3 переопределяет Z3::BoolExpr#==, так что семантически это правило не работает, но я не могу винить ameba за то, что она не поддерживает такой необычный код.
Я не уверен, что это хорошее правило в целом. Пока x может быть чем угодно, кроме булевой, x == true — это совершенно не то же самое, что x.
Thue Interptetter и Performance/CompactAfterMap.
Есть две проблемы, обнаруженные с интерпретатором Thue из этой серии:
./episode-65-crystal-thue-randomized-finite-automaton/thue_rfa.cr:162:27
[C] Performance/CompactAfterMap: Use `compact_map {...}` instead of `map {...}.compact`
> next_tries = active.map{|t| t[char]}.compact
^-----------------------^
compact_map — достойное предложение, так как эти объединенные общие операции обычно более производительны, но подобные методы продолжают добавляться в каждую минорную версию каждого языка, трудно запомнить, в каком языке какие комбинации есть.
./episode-65-crystal-thue-randomized-finite-automaton/thue_rfa.cr:191:13
[W] Lint/UselessAssign: Useless assignment to variable `line_no`
> line, line_no = lines.shift
^-----^
Мне это не нравится. Бесполезное присвоение одной переменной может быть удалено, так что это имеет смысл, но при деструктуризации массива альтернативой является уродливый код, как в одном из примеров:
line, _ = lines.shift
line, _line_no = lines.shift
line = lines.shift[0]
Я бы предпочел придерживаться оригинала.
Но это не специфично для ameba, в большинстве линтеров проверка «Бесполезное присваивание переменной» не имеет исключения для множественного присваивания.
Для примечания, если бы это был именованный кортеж, мы могли бы сделать некий эквивалент (JavaScript здесь):
let {line} = lines.shift()
Для деструктуризации хэша / именованного кортежа / объекта нет причин включать эти дополнения.
Thue Interpretter и --all
Поскольку мы не используем --all, ложные срабатывания вполне ожидаемы. Вот один пример, таких случаев больше:
./episode-65-crystal-thue-randomized-finite-automaton/thue_rfa.cr:221:5 [Correctable]
[C] Style/GuardClause: Use a guard clause (`return unless debug`) instead of wrapping the code inside a conditional expression.
> if debug
^^
Таким образом, linter хочет, чтобы мы заменили это:
def run(debug)
@state = @initial
if debug
@rules.each do |rule|
STDERR.puts rule
end
end
while match = @rfa.not_nil!.random_match(@state)
rule = match[:rule]
idx = match[:idx]
if debug
STDERR.puts "Applying rule #{rule} at #{idx} to #{@state.inspect}"
end
@state = rule.apply(@state, idx)
end
if debug
STDERR.puts "No more matches. Final state: #{@state.inspect}"
end
end
этим:
def run(debug)
@state = @initial
if debug
@rules.each do |rule|
STDERR.puts rule
end
end
while match = @rfa.not_nil!.random_match(@state)
rule = match[:rule]
idx = match[:idx]
if debug
STDERR.puts "Applying rule #{rule} at #{idx} to #{@state.inspect}"
end
@state = rule.apply(@state, idx)
end
return unless debug
STDERR.puts "No more matches. Final state: #{@state.inspect}"
end
Это не будет улучшением. Охранные клаузулы в начале метода или цикла — это обычная схема, но ставить их после основной части — это странно.
Приключения с открытым исходным кодом и Style/VerboseBlock
Чтобы заставить ameba увидеть все файлы, мне нужно запустить его со странным интерфейсом:
$ ameba `git grep -l '#!/usr/bin/env crystal'` `git ls "*.cr"`
Вот некоторые предложения:
episode-65/minesweeper.cr:40:25 [Correctable]
[C] Style/VerboseBlock: Use short block notation instead: `map(&.ite(1, 0))`
> neighbourhood(x, y).map{|v| v.ite(1, 0)}.reduce{|a,b| a+b}
^------------------^
Это код Crystal, который нельзя напрямую реализовать в Ruby. Это разумное предложение.
episode-70/aquarium:17:25 [Correctable]
[C] Style/VerboseBlock: Use short block notation instead: `map(&.[2..].chars)`
> @board = lines[2..].map{|l| l[2..].chars}
^-------------------^
Кажется, что это слишком, но, возможно, я смогу к этому привыкнуть.
episode-72/dominosa:65:23
[W] Lint/UnusedArgument: Unused argument `type`. If it's necessary, use `_` as an argument name to indicate that it won't be used.
> @dominos.each do |type, vars|
^
Здесь можно использовать .each_value, но, как и в случае с множественным присваиванием, мне не нравится предложение @dominos.each do |_type, vars| или @dominos.each do |_, vars|.
episode-68/switches:50:67 [Correctable]
[C] Performance/ChainedCallWithNoBang: Use bang method variant `sort!` after chained `select` call
> puts @nodes.select{|n| model[@switches[n]].to_s == "true" }.sort.join(" ")
^--^
Я понимаю, почему это имеет смысл для производительности, но .sort! в середине цепочки — это так странно, что я бы предпочел этого не делать.
С --all не было никаких новых типов предложений.
Стоит ли использовать ameba?
В целом, он кажется приличным линтером, и настройки по умолчанию не создают слишком много ложных срабатываний.
Он не делает никакого глубокого анализа. Например, я знаю, что у меня были некоторые проверки if, которые всегда true из-за проблемы с типом (Z3 выдает LBool enum, а не Bool, поэтому результат всегда true), но ameba никогда не обнаруживал этого, так как это не видно при чисто синтаксической проверке.
Далее
Ладно, на сегодня Кристалла действительно достаточно. В следующем эпизоде мы попробуем другую технологию, как и обещали.