Closed dabaer closed 1 year ago
Interestingly only the Lasagna test failed on my docker install due to differences in how the map was sorted in expected_analysis.json
, but on CI it fails on even more.
We can't rely on the ordering of maps in the json, so I wonder if instead we should be doing the comparison on the term instead of the string.
%{a: 1, b: 2} == %{b: 2, a: 1}
true
==
compares maps regardless of the key order, so I will look at doing that sort of comparison in the tests instead.
@angelikatyborska I'm a bit stumped on the last 3 failing tests, I'm assuming adjustment of the expected comments here wouldn't be wise as it's not a simple object key ordering change like the other ones.
Something underlying has caused the german-sysadmin
test to not match the exemplar, and emit module-not-formatted
comments in tests it didn't before.
Okay I finally paid attention to the changes you made in the main elixir
repo regarding charlists and applied those changes to the heredocs of the test files in the analyzer.
The last roadblock here is that the analyzer extensions for german_sysadmin
are flagging our newfangled charlists as having used String in the code.
Curiously, if I use Code.string_to_quoted
and manually analyze the AST, I see the references to :sigil_c
but not to String. I'm not quite sure why this assertion is flagging these tests for this usage.
I was slightly off on thinking the comments were returned by assert_no_call "doesn't use any string functions"
and discovered they are actually returned by assert_no_call "doesn't create binaries from character codes"
after dissecting the analyzer extensions.
I'm experimenting with replacing this extension with a check_source
that performs a postwalk on the AST to determine if :<<>>
is being called outside of a :sigil_c
, hopefully this is the right path here.
To be merged at the same time as