Skip to content

Preliminary Elixir support#5

Open
UncleGrumpy wants to merge 3 commits into
mainfrom
elixir-support
Open

Preliminary Elixir support#5
UncleGrumpy wants to merge 3 commits into
mainfrom
elixir-support

Conversation

@UncleGrumpy
Copy link
Copy Markdown
Owner

@UncleGrumpy UncleGrumpy commented May 21, 2026

Add support for Elixir MFA in update, support, and query commands.
Expand overall test coverage.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Elixir query parsing (dotted/module shorthand, optional arity), Elixir module-name utilities, .ex scanner to populate supported-functions data, CLI flags (--ex/--erl) and help/README updates, build/install tweaks, and expanded EUnit tests.

Changes

Elixir Support for AtomVM Spectrometer

Layer / File(s) Summary
Supported Elixir functions dataset
priv/supported_functions.data
Large contiguous addition of Elixir.* entries and a new {unknown, [...]} entry mapping functions to {function,arity,platforms,since}.
Module-name utilities & TEMP fallback
src/spectrometer_utils.erl
Adds is_elixir_module_name/1, normalize_module_name/1 and /2 (exported) and refines system_temp_dir/0 TEMP/TEMPDIR fallback behavior.
Elixir library scanning and parsing
src/spectrometer_updater.erl
Adds scan_elixir_libs/3, find_ex_files/1-2, parse_elixir_file/4, helpers to extract public def exports (skip defp), arity extraction, top-level comma counting, and widens NIF regex capture.
Query parsing, filtering, and formatting
src/spectrometer_atomvm.erl
parse_query_string/1 accepts Erlang colon and Elixir dotted syntax with optional arity; report_supported/1 reads filter and uses filter_modules_by_type/2; per-function output consolidated into format_function_line/5.
CLI args, help, README and small defaults
src/atomvm_spectrometer.erl, src/spectrometer_help.erl, README.md
--module input normalized via utilities, new --erl/--ex flags added, help and README updated with Elixir query formats/examples; ecosystem default-options helper renamed.
Build hooks and installer behavior
rebar.config.script, release/install.in
Adds rebar3_uncovered to check profile, runs escriptize for {tar,...}, and changes installer to allow --force path that uninstalls and removes existing install dir before reinstall.
Tests: new and expanded EUnit coverage
test/* (many files)
Adds/extends tests for CLI flags, Elixir query parsing, utils behaviors, updater scanning and .ex parsing, reporter CSV/report map handling, scanner error cases, OTP cache lifecycle, and network-gated clone tests.

Sequence Diagram (high-level Elixir scan + query flow):

sequenceDiagram
  participant CLI
  participant parse_query_string
  participant normalize_module_name
  participant scan_atomvm_repo
  participant scan_elixir_libs
  participant parse_elixir_file
  participant supported_data
  CLI->>parse_query_string: receive query (Elixir.GPIO.digital_read/1)
  parse_query_string->>normalize_module_name: normalize module token
  CLI->>scan_atomvm_repo: run updater scan
  scan_atomvm_repo->>scan_elixir_libs: discover libs/exavmlib
  scan_elixir_libs->>parse_elixir_file: read .ex, extract def exports
  parse_elixir_file->>supported_data: record {Module,Fun,Arity}
  parse_query_string->>supported_data: lookup matches
  supported_data->>CLI: return filtered/formatted lines
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hop through .ex and .erl with glee,
Dotted names and arities greet me,
I sniff each def and count the args,
I sort Elixir from Erlang tags,
A rabbit cheers this tidy spree.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Preliminary Elixir support' accurately summarizes the main objective of the PR, which is to add Elixir MFA support across multiple commands and subsystems.
Description check ✅ Passed The description 'Add support for Elixir MFA in update, support, and query commands. Expand overall test coverage.' directly aligns with the changeset, clearly describing the primary changes across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch elixir-support

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/spectrometer_atomvm.erl`:
- Around line 547-549: The bug: when Arity is the atom all you call
format_function_line(Fun, [], ...) which renders a blank "/" arity; change the
handling so the arity all is preserved and rendered explicitly (e.g. "all" or
"*" ) instead of converting to []. Update format_function_line so there is an
explicit clause for format_function_line(Fun, all, Platform, Since) (or detect
is_atom(Arity) and Arity == all) and ensure the downstream arity-formatting
logic prints a clear wildcard token (reference format_function_line and any
helper that formats the arity used by it); apply the same change to the other
occurrence around the 560-564 region so "all" never becomes an empty arity
string.
- Around line 335-337: Update the invalid-format error string to mention the
supported dot syntax; replace the tuple {error, "Invalid format. Use
Module:Function or Module:Function/Arity"} (the branch handling DotPos) with a
message that includes both Module:Function[/Arity] and Module.Function[/Arity]
so users see the dot syntax is supported.

In `@src/spectrometer_help.erl`:
- Line 189: Update the help text for the "--ex" flag to accurately describe its
behavior: it filters by Elixir module type rather than rewriting names. Replace
the string "Show only Elixir functions (strip Elixir. prefix)" with a concise,
correct description such as "Show only Elixir modules" or "Filter output to
Elixir modules (does not rename or strip module prefixes)"; locate and edit the
literal string in spectrometer_help.erl (the line containing the "--ex"
description) and ensure the new wording does not promise name normalization.

In `@src/spectrometer_updater.erl`:
- Around line 1088-1105: parse_elixir_file/4 currently attributes all def
exports to the file basename; instead parse module scopes from the source and
assign each export to its enclosing module/impl. Change the export discovery so
that find_elixir_exports (or a new find_elixir_exports_with_modules) scans Lines
for "defmodule" and "defimpl" blocks, tracks the active module/implementation
name (handling nested modules and multiple module blocks in one file), and
returns exported funs as tuples {ModuleNameAtom, FunName, Arity}; then in
parse_elixir_file/4 build Key = {ModuleAtom, FunName, Arity} from the parsed
ModuleNameAtom (not filename) and store maps:put(Key, {Platforms, Since}, A).
Ensure module name strings are converted to "Elixir.<Name>" atoms the same way
ModAtom was before and preserve existing behavior if no module is found.

In `@src/spectrometer_utils.erl`:
- Around line 406-434: Change detection and normalization so we no longer infer
"Elixir." from capitalization: update is_elixir_module_name/1 to only return
true when the atom/string starts with the literal "Elixir." (remove the
leading-uppercase branch), and stop having normalize_module_name/1 add the
"Elixir." prefix based on a regex match of an uppercase first letter; instead
introduce normalize_module_name/2 (normalize_module_name(Str, ElixirFlag)) where
ElixirFlag (provided by the caller who already knows they used Elixir dot
syntax) controls whether to prepend "Elixir." and make the existing
normalize_module_name/1 delegate to normalize_module_name/2 with ElixirFlag =
false so lowercase names like "json" or "Elixir.json" are handled correctly and
only explicit Elixir input gets prefixed.

In `@test/spectrometer_elixir_tests.erl`:
- Around line 81-94: Tests call spectrometer_atomvm:format_function_line but
only ensure it doesn't crash; update each test
(format_function_line_integer_arity_test, format_function_line_all_arities_test,
format_function_line_arities_list_test) to capture the return value and assert
expected output: call spectrometer_atomvm:format_function_line(...) -> Result
and use e.g. ?assertEqual(ExpectedString, Result) or appropriate assertion to
verify the formatted string contains the function name, the arity or arities,
the platform list, and the version tag (e.g. <<"v0.5.0">> / <<"v0.6.0">>), and
add separate assertions for different arity representations (integer, all, list)
to lock expected formatting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6893d9bf-cf9d-43c7-918b-6cba77f50b6a

📥 Commits

Reviewing files that changed from the base of the PR and between af8eab1 and 8e2567b.

📒 Files selected for processing (11)
  • README.md
  • priv/supported_functions.data
  • rebar.config.script
  • release/install.in
  • src/atomvm_spectrometer.erl
  • src/spectrometer_atomvm.erl
  • src/spectrometer_help.erl
  • src/spectrometer_updater.erl
  • src/spectrometer_utils.erl
  • test/atomvm_spectrometer_tests.erl
  • test/spectrometer_elixir_tests.erl

Comment thread src/spectrometer_atomvm.erl
Comment thread src/spectrometer_atomvm.erl
Comment thread src/spectrometer_help.erl Outdated
Comment thread src/spectrometer_updater.erl
Comment thread src/spectrometer_utils.erl
Comment thread test/spectrometer_elixir_tests.erl Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
release/install.in (1)

117-117: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Ensure destination parent exists before copy.

Without creating ${dest_dir} (or at least ${prefix}/lib), fresh installs can fail on prefixes missing a lib directory.

Suggested fix
+echo_run mkdir -p "${dest_dir}"
 echo_run cp -fr "${tmp_dir}" "${dest_dir}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@release/install.in` at line 117, Before the cp command (echo_run cp -fr
"${tmp_dir}" "${dest_dir}"), ensure the destination parent directory exists by
creating it with mkdir -p; for example, run mkdir -p "${dest_dir}" (or mkdir -p
"${prefix}/lib" if you prefer to ensure the common parent) so the subsequent
echo_run cp -fr "${tmp_dir}" "${dest_dir}" cannot fail on fresh prefixes missing
a lib directory.
src/spectrometer_atomvm.erl (1)

295-297: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update query usage text to include dot syntax.

Parser accepts Module.Function[/Arity], but usage still only advertises colon format.

Suggested fix
-            io:format(
-                standard_error, "Usage: query Module:Function[/Arity]\n", []
-            ),
+            io:format(
+                standard_error,
+                "Usage: query Module:Function[/Arity] | Module.Function[/Arity]\n",
+                []
+            ),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spectrometer_atomvm.erl` around lines 295 - 297, Update the usage message
printed by the io:format call in spectrometer_atomvm.erl to reflect that the
parser accepts both colon and dot syntax; change the string currently showing
"Module:Function[/Arity]" to include the dot form (for example
"Module:Function[/Arity] or Module.Function[/Arity]") so callers see both
accepted formats when the usage text is printed by the function that does
io:format(standard_error, ...).
♻️ Duplicate comments (1)
test/spectrometer_elixir_tests.erl (1)

87-113: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Formatter tests still don't validate formatted output.

These tests only assert ok, so formatting regressions can still pass undetected. Please assert emitted content (function name/arity/platform/since) rather than just non-crash behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/spectrometer_elixir_tests.erl` around lines 87 - 113, The tests
currently only assert ok and don't verify the formatted output; update each test
(format_function_line_integer_arity_test, format_function_line_all_arities_test,
format_function_line_arities_list_test) to capture the string emitted by
spectrometer_atomvm:format_function_line and assert it equals the expected
formatted line containing the function name (test_fun), the correct arity
rendering ("1", "*" for all, "1,2,3" for lists), the platform(s) (e.g., esp32,
stm32) and the "since" version (e.g., "v0.5.0"/"v0.6.0"); use an output-capture
helper (EUnit capture_io or equivalent) around the call to format_function_line
and replace the ?assertEqual(ok, ...) with an assertion that the captured string
matches the exact expected output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@release/install.in`:
- Around line 98-99: The current install script only runs rm -rf
"${prefix}/lib/atomvm_spectrometer" if sh
"${prefix}/lib/atomvm_spectrometer/uninstall.sh" fails, which can leave stale
files when uninstall.sh exits successfully but doesn't remove everything; change
the sequence so the uninstall script is executed and then always force-remove
the directory (i.e., run the uninstall.sh and subsequently run rm -rf
"${prefix}/lib/atomvm_spectrometer" regardless of uninstall.sh exit status) to
guarantee a clean reinstall.

In `@src/atomvm_spectrometer.erl`:
- Around line 372-375: The parse_supported_args clauses currently insert a
string key "filter" into the options map, violating the atom-keyed opts_map()
contract; change both clauses to update the map with an atom key instead, e.g.
replace Opts#{"filter" => erlang_only} and Opts#{"filter" => elixir_only} with
Opts#{filter => erlang_only} and Opts#{filter => elixir_only} in the
parse_supported_args function so the map uses the atom filter key consistently
with opts_map().

In `@src/spectrometer_atomvm.erl`:
- Line 453: Replace the string-key lookup for the filter option with an atom-key
lookup: change the maps:get("filter", Opts, undefined) call that assigns Filter
to use the atom key (maps:get(filter, Opts, undefined)) so the Opts map stays
consistent with atom-keyed options and the parser-side storage; update the
assignment to Filter accordingly.

In `@src/spectrometer_updater.erl`:
- Around line 1241-1252: count_arity/1 currently strips whitespace and splits on
all commas, which over-counts when commas appear inside nested (), [], or {};
replace this by implementing a scanner (e.g., count_top_level_args/3) that
iterates the CleanArgs string, tracks nesting depth for '()', '[]', and '{}' and
only treats commas at depth 0 as argument separators, returning 0 for empty
strings and otherwise 1 + top-level-comma-count; update count_arity/1 to call
this new function and return the correct arity.

In `@test/spectrometer_otp_tests.erl`:
- Around line 99-101: The cleanup callbacks currently return a zero-arity fun
instead of performing cleanup (e.g., fun(TempDir) -> fun() ->
spectrometer_utils:purge_dir(TempDir) end end), so temp dirs are never removed;
fix by removing the nested fun wrapper and invoke
spectrometer_utils:purge_dir(TempDir) directly in the callback (replace the
outer fun to call spectrometer_utils:purge_dir/1 rather than returning another
fun), and apply the same change to all similar instances (the blocks around
lines referencing spectrometer_utils:purge_dir in this file).

In `@test/spectrometer_reporter_tests.erl`:
- Around line 211-214: The CSV line-count assertion is nondeterministic because
binary:split(Content, <<"\n">>, [global]) can produce a trailing empty element
if Content ends with a newline; update the test to remove empty trailing entries
after splitting (e.g. call binary:split(Content, <<"\n">>, [global, trim]) or
filter out <<>> entries from Lines) before asserting length(Lines) equals 3 so
the assertion is stable; change the code around the variable Lines (and the
binary:split call that uses Content) accordingly.

---

Outside diff comments:
In `@release/install.in`:
- Line 117: Before the cp command (echo_run cp -fr "${tmp_dir}" "${dest_dir}"),
ensure the destination parent directory exists by creating it with mkdir -p; for
example, run mkdir -p "${dest_dir}" (or mkdir -p "${prefix}/lib" if you prefer
to ensure the common parent) so the subsequent echo_run cp -fr "${tmp_dir}"
"${dest_dir}" cannot fail on fresh prefixes missing a lib directory.

In `@src/spectrometer_atomvm.erl`:
- Around line 295-297: Update the usage message printed by the io:format call in
spectrometer_atomvm.erl to reflect that the parser accepts both colon and dot
syntax; change the string currently showing "Module:Function[/Arity]" to include
the dot form (for example "Module:Function[/Arity] or Module.Function[/Arity]")
so callers see both accepted formats when the usage text is printed by the
function that does io:format(standard_error, ...).

---

Duplicate comments:
In `@test/spectrometer_elixir_tests.erl`:
- Around line 87-113: The tests currently only assert ok and don't verify the
formatted output; update each test (format_function_line_integer_arity_test,
format_function_line_all_arities_test, format_function_line_arities_list_test)
to capture the string emitted by spectrometer_atomvm:format_function_line and
assert it equals the expected formatted line containing the function name
(test_fun), the correct arity rendering ("1", "*" for all, "1,2,3" for lists),
the platform(s) (e.g., esp32, stm32) and the "since" version (e.g.,
"v0.5.0"/"v0.6.0"); use an output-capture helper (EUnit capture_io or
equivalent) around the call to format_function_line and replace the
?assertEqual(ok, ...) with an assertion that the captured string matches the
exact expected output.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e458c6f8-98c7-47d2-a0b2-39810fbf6f57

📥 Commits

Reviewing files that changed from the base of the PR and between 8e2567b and e057a16.

📒 Files selected for processing (16)
  • README.md
  • priv/supported_functions.data
  • rebar.config.script
  • release/install.in
  • src/atomvm_spectrometer.erl
  • src/spectrometer_atomvm.erl
  • src/spectrometer_help.erl
  • src/spectrometer_updater.erl
  • src/spectrometer_utils.erl
  • test/atomvm_spectrometer_tests.erl
  • test/spectrometer_elixir_tests.erl
  • test/spectrometer_otp_tests.erl
  • test/spectrometer_reporter_tests.erl
  • test/spectrometer_scanner_tests.erl
  • test/spectrometer_updater_tests.erl
  • test/spectrometer_utils_tests.erl
✅ Files skipped from review due to trivial changes (3)
  • README.md
  • src/spectrometer_help.erl
  • priv/supported_functions.data

Comment thread release/install.in Outdated
Comment thread src/atomvm_spectrometer.erl Outdated
Comment thread src/spectrometer_atomvm.erl Outdated
Comment thread src/spectrometer_updater.erl
Comment thread src/spectrometer_updater.erl Outdated
Comment thread test/spectrometer_otp_tests.erl
Comment thread test/spectrometer_reporter_tests.erl Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/spectrometer_atomvm.erl (1)

54-55: ⚡ Quick win

Hide the test-only helper from xref.

The pipeline is already warning that format_function_line_string/4 is an unused export. If this is only for tests, gate the export behind a test macro or add a local xref ignore so code-quality stays clean.

Also applies to: 583-624

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spectrometer_atomvm.erl` around lines 54 - 55, The export
format_function_line_string/4 is only used in tests and is causing xref
warnings—wrap its export in a test guard (e.g. -ifdef(TEST) ... -endif) so it’s
only exported during tests, and apply the same pattern to other test-only
helpers in the block around lines 583-624; locate the export for
format_function_line_string/4 and the other test helpers and move each
-export(...) into an -ifdef(TEST) / -endif pair (or alternatively remove the
export and add an -ifdef(TEST) export declaration) so the functions remain
available to tests but are hidden from xref in normal builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@release/install.in`:
- Line 110: Remove the stray leading pipe character from the comment line that
currently reads "|# Extract the embedded tar.gz" so it becomes a normal shell
comment "# Extract the embedded tar.gz" (open release/install.in, find the exact
string "|# Extract the embedded tar.gz" and delete the leading "|" to fix the sh
syntax error).

In `@src/spectrometer_updater.erl`:
- Around line 1257-1266: The clause patterns in count_top_level_commas/3 are
using one-character strings (e.g. "(") but Char is an integer codepoint; change
the case patterns to Erlang character literals (use $() for "(", $[ for "[", ${
for "{", $) for ")", $] for "]", $} for "}", and $, for ",") so delimiters and
the top-level comma clause (when Depth == 0) will match the integer Char values
correctly.
- Around line 1126-1130: The defimpl handling currently only uses the protocol
name from find_elixir_module_def/1 and then builds the module atom in
find_elixir_exports/4 as list_to_atom("Elixir." ++ ModName), which omits the
for: target (so e.g. Enumerable, for: List becomes Elixir.Enumerable instead of
Elixir.Enumerable.List); update the logic so the parsed defimpl record from
find_elixir_module_def/1 carries the target (for) identifier and
find_elixir_exports/4 composes the full module atom by appending the target
(e.g. "Elixir." ++ ProtocolName ++ "." ++ TargetName) before calling
list_to_atom, ensuring defimpl entries map to Elixir.Protocol.Target (also
adjust the related branch at the second occurrence around the
find_elixir_exports/4 usage to use the same composed ModAtom).

In `@test/atomvm_spectrometer_tests.erl`:
- Around line 458-472: The tests parse_supported_args_erl_test_ and
parse_supported_args_ex_test_ are using a string key for maps:get("filter",
Opts) but atomvm_spectrometer:parse_args returns a map with an atom key :filter;
update both assertions to use the atom key (e.g., maps:get(filter, Opts) or
Opts# maps access) so they compare erlang_only and elixir_only against the
atom-keyed value returned by parse_args.

In `@test/spectrometer_elixir_tests.erl`:
- Around line 87-106: Tests call
spectrometer_atomvm:format_function_line_string/4 which returns iodata;
normalize Output with iolist_to_binary/1 (e.g. OutputBin =
iolist_to_binary(spectrometer_atomvm:format_function_line_string(...))) and
compare against a binary expected (<<"  test_fun/1  (all since: v0.5.0)\n">>,
etc.) in format_function_line_integer_arity_test,
format_function_line_all_arities_test and format_function_line_arities_list_test
so assertions don't fail across CI matrices.

In `@test/spectrometer_updater_tests.erl`:
- Around line 972-979: The test fails because count_arity/1 in module
spectrometer_updater treats the whole argument string as one token when spaces
are present; change count_arity/1 to return 0 for the empty string, otherwise
split the input on commas, trim whitespace from each token, ignore empty tokens
and return the count of non-empty tokens (so inputs like "x, y" and "  x  ,  y 
" yield 2). Locate and update the spectrometer_updater:count_arity/1
implementation referenced by the failing test in spectrometer_updater_tests to
use comma-based splitting and token trimming before counting.

---

Nitpick comments:
In `@src/spectrometer_atomvm.erl`:
- Around line 54-55: The export format_function_line_string/4 is only used in
tests and is causing xref warnings—wrap its export in a test guard (e.g.
-ifdef(TEST) ... -endif) so it’s only exported during tests, and apply the same
pattern to other test-only helpers in the block around lines 583-624; locate the
export for format_function_line_string/4 and the other test helpers and move
each -export(...) into an -ifdef(TEST) / -endif pair (or alternatively remove
the export and add an -ifdef(TEST) export declaration) so the functions remain
available to tests but are hidden from xref in normal builds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e5da10b-6765-4c15-8720-716027ade152

📥 Commits

Reviewing files that changed from the base of the PR and between e057a16 and 27e2dba.

📒 Files selected for processing (16)
  • README.md
  • priv/supported_functions.data
  • rebar.config.script
  • release/install.in
  • src/atomvm_spectrometer.erl
  • src/spectrometer_atomvm.erl
  • src/spectrometer_help.erl
  • src/spectrometer_updater.erl
  • src/spectrometer_utils.erl
  • test/atomvm_spectrometer_tests.erl
  • test/spectrometer_elixir_tests.erl
  • test/spectrometer_otp_tests.erl
  • test/spectrometer_reporter_tests.erl
  • test/spectrometer_scanner_tests.erl
  • test/spectrometer_updater_tests.erl
  • test/spectrometer_utils_tests.erl
✅ Files skipped from review due to trivial changes (2)
  • README.md
  • src/spectrometer_help.erl

Comment thread release/install.in Outdated
Comment thread src/spectrometer_updater.erl Outdated
Comment on lines +1257 to +1266
count_top_level_commas([Char | Rest], Depth, Count) ->
case Char of
"(" -> count_top_level_commas(Rest, Depth + 1, Count);
"[" -> count_top_level_commas(Rest, Depth + 1, Count);
"{" -> count_top_level_commas(Rest, Depth + 1, Count);
")" -> count_top_level_commas(Rest, Depth - 1, Count);
"]" -> count_top_level_commas(Rest, Depth - 1, Count);
"}" -> count_top_level_commas(Rest, Depth - 1, Count);
"," when Depth == 0 -> count_top_level_commas(Rest, Depth, Count + 1);
_ -> count_top_level_commas(Rest, Depth, Count)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and print the exact implementation around the reported lines
FILE="src/spectrometer_updater.erl"
echo "== file =="
echo "$FILE"
echo "== lines 1240-1290 =="
nl -ba "$FILE" | sed -n '1240,1295p'

# Also find the definition header to confirm arity and any callers/assumptions
echo "== rg: definition of count_top_level_commas/3 =="
rg -n "count_top_level_commas/3|count_top_level_commas\(" "$FILE"

# Extract any surrounding functions that call count_top_level_commas/3 (small neighborhood)
echo "== search callers (limited context) =="
rg -n "count_top_level_commas\(" "$FILE" | head -n 50

Repository: UncleGrumpy/atomvm_spectrometer

Length of output: 198


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/spectrometer_updater.erl"
echo "== lines 1235-1275 =="
awk 'NR>=1235 && NR<=1275 {printf "%d:%s\n", NR, $0}' "$FILE"

echo "== function headers (maybe multiple arities) =="
rg -n "count_top_level_commas\s*\(" "$FILE"

echo "== show exact case clause block containing the delimiter patterns =="
rg -n "case Char of" "$FILE" -n || true
# Print a small region around the first occurrence
line=$(rg -n "case Char of" "$FILE" | head -n1 | cut -d: -f1)
if [ -n "${line:-}" ]; then
  start=$((line-5))
  end=$((line+25))
  echo "== around case Char of (lines $start-$end) =="
  awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%d:%s\n", NR, $0}' "$FILE"
fi

echo "== search for how Char is produced/pattern-matched (call sites) =="
# Look for where count_top_level_commas/3 is called and what the first argument is
rg -n "count_top_level_commas\s*\(" "$FILE"

Repository: UncleGrumpy/atomvm_spectrometer

Length of output: 4206


Fix delimiter matching in count_top_level_commas/3 (use Erlang character literals)

Char is an integer codepoint from [Char | Rest] over CleanArgs (built with {return, list}), but the case patterns use one-character strings ("(", "[", etc.), which won’t match; delimiters are therefore never recognized.

Suggested fix
 count_top_level_commas([Char | Rest], Depth, Count) ->
     case Char of
-        "(" -> count_top_level_commas(Rest, Depth + 1, Count);
-        "[" -> count_top_level_commas(Rest, Depth + 1, Count);
-        "{" -> count_top_level_commas(Rest, Depth + 1, Count);
-        ")" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "]" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "}" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "," when Depth == 0 -> count_top_level_commas(Rest, Depth, Count + 1);
+        $( -> count_top_level_commas(Rest, Depth + 1, Count);
+        $[ -> count_top_level_commas(Rest, Depth + 1, Count);
+        ${ -> count_top_level_commas(Rest, Depth + 1, Count);
+        $) -> count_top_level_commas(Rest, Depth - 1, Count);
+        $] -> count_top_level_commas(Rest, Depth - 1, Count);
+        $} -> count_top_level_commas(Rest, Depth - 1, Count);
+        $, when Depth == 0 -> count_top_level_commas(Rest, Depth, Count + 1);
         _ -> count_top_level_commas(Rest, Depth, Count)
     end;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spectrometer_updater.erl` around lines 1257 - 1266, The clause patterns
in count_top_level_commas/3 are using one-character strings (e.g. "(") but Char
is an integer codepoint; change the case patterns to Erlang character literals
(use $() for "(", $[ for "[", ${ for "{", $) for ")", $] for "]", $} for "}",
and $, for ",") so delimiters and the top-level comma clause (when Depth == 0)
will match the integer Char values correctly.

Comment thread test/atomvm_spectrometer_tests.erl
Comment thread test/spectrometer_elixir_tests.erl Outdated
Comment on lines +87 to +106
format_function_line_integer_arity_test() ->
% Test that format_function_line_string handles integer arity correctly
Output = spectrometer_atomvm:format_function_line_string(
test_fun, 1, all, <<"v0.5.0">>
),
?assertEqual(" test_fun/1 (all since: v0.5.0)\n", Output).

format_function_line_all_arities_test() ->
% Test that "all" arity renders as "*" instead of empty string
Output = spectrometer_atomvm:format_function_line_string(
test_fun, all, [esp32], <<"v0.5.0">>
),
?assertEqual(" test_fun/* (esp32 since: v0.5.0)\n", Output).

format_function_line_arities_list_test() ->
% Test that list of arities renders correctly
Output = spectrometer_atomvm:format_function_line_string(
test_fun, [1, 2, 3], [stm32], <<"v0.6.0">>
),
?assertEqual(" test_fun/1/2/3 (stm32 since: v0.6.0)\n", Output).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize iolist output before equality assertions in format-line tests.

format_function_line_string/4 returns iodata in current behavior, so direct string equality is failing across CI matrices.

Suggested fix
-    Output = spectrometer_atomvm:format_function_line_string(
+    Output = iolist_to_binary(spectrometer_atomvm:format_function_line_string(
         test_fun, 1, all, <<"v0.5.0">>
-    ),
-    ?assertEqual("  test_fun/1  (all since: v0.5.0)\n", Output).
+    )),
+    ?assertEqual(<<"  test_fun/1  (all since: v0.5.0)\n">>, Output).

-    Output = spectrometer_atomvm:format_function_line_string(
+    Output = iolist_to_binary(spectrometer_atomvm:format_function_line_string(
         test_fun, all, [esp32], <<"v0.5.0">>
-    ),
-    ?assertEqual("  test_fun/*  (esp32 since: v0.5.0)\n", Output).
+    )),
+    ?assertEqual(<<"  test_fun/*  (esp32 since: v0.5.0)\n">>, Output).

-    Output = spectrometer_atomvm:format_function_line_string(
+    Output = iolist_to_binary(spectrometer_atomvm:format_function_line_string(
         test_fun, [1, 2, 3], [stm32], <<"v0.6.0">>
-    ),
-    ?assertEqual("  test_fun/1/2/3  (stm32 since: v0.6.0)\n", Output).
+    )),
+    ?assertEqual(<<"  test_fun/1/2/3  (stm32 since: v0.6.0)\n">>, Output).
🧰 Tools
🪛 GitHub Actions: Build and Test / 0_build-and-test (28).txt

[error] 92-92: Test failure: spectrometer_elixir_tests:format_function_line_integer_arity_test/0 expected " test_fun/1 (all since: v0.5.0)\n" but got list output


[error] 99-99: Test failure: spectrometer_elixir_tests:format_function_line_all_arities_test/0 expected " test_fun/* (esp32 since: v0.5.0)\n" but got list output


[error] 106-106: Test failure: spectrometer_elixir_tests:format_function_line_arities_list_test/0 expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" but got list output

🪛 GitHub Actions: Build and Test / 1_build-and-test (29, 3.27.0).txt

[error] 92-92: spectrometer_elixir_tests: format_function_line_integer_arity_test failed: expected output string for "test_fun/1" but got an iolist/list of integers


[error] 99-99: spectrometer_elixir_tests: format_function_line_all_arities_test failed: expected output string for "test_fun/*" but got an iolist/list of integers


[error] 106-106: spectrometer_elixir_tests: format_function_line_arities_list_test failed: expected output string for "test_fun/1/2/3" but got an iolist/list of integers

🪛 GitHub Actions: Build and Test / 2_build-and-test (27).txt

[error] 92-92: spectrometer_elixir_tests:format_function_line_integer_arity_test/0: expected " test_fun/1 (all since: v0.5.0)\n" but got output as list/char tokens.


[error] 99-99: spectrometer_elixir_tests:format_function_line_all_arities_test/0: expected " test_fun/* (esp32 since: v0.5.0)\n" but got output as list/char tokens.


[error] 106-106: spectrometer_elixir_tests:format_function_line_arities_list_test/0: expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" but got output as list/char tokens.

🪛 GitHub Actions: Build and Test / 3_build-and-test (master).txt

[error] 92-92: EUnit failure: format_function_line_integer_arity_test. Expected " test_fun/1 (all since: v0.5.0)\n", got list output


[error] 99-99: EUnit failure: format_function_line_all_arities_test. Expected " test_fun/* (esp32 since: v0.5.0)\n", got list output


[error] 106-106: EUnit failure: format_function_line_arities_list_test. Expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n", got list output

🪛 GitHub Actions: Build and Test / build-and-test (27)

[error] 92-92: Test failure: format_function_line_integer_arity_test expected string " test_fun/1 (all since: v0.5.0)\n" but got a list with incorrect tokenization (output contained [32,32,"test_fun",...])


[error] 99-99: Test failure: format_function_line_all_arities_test expected " test_fun/* (esp32 since: v0.5.0)\n" but got tokenized list output ("esp32" and "v0.5.0" fragments present)


[error] 106-106: Test failure: format_function_line_arities_list_test expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" but got tokenized list output ("1/2/3", "stm32", "v0.6.0" present)

🪛 GitHub Actions: Build and Test / build-and-test (28)

[error] 92-92: Test failure: format_function_line_integer_arity_test. Expected " test_fun/1 (all since: v0.5.0)\n" but got a different representation (charlist-like output)


[error] 99-99: Test failure: format_function_line_all_arities_test. Expected " test_fun/* (esp32 since: v0.5.0)\n" but got a different representation (charlist-like output)


[error] 106-106: Test failure: format_function_line_arities_list_test. Expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" but got a different representation (charlist-like output)

🪛 GitHub Actions: Build and Test / build-and-test (29, 3.27.0)

[error] 92-92: Test failure: spectrometer_elixir_tests:format_function_line_integer_arity_test/0. Expected " test_fun/1 (all since: v0.5.0)\n" but got a charlist representation ("test_fun", "1", "all", "v0.5.0").


[error] 99-99: Test failure: spectrometer_elixir_tests:format_function_line_all_arities_test/0. Expected " test_fun/* (esp32 since: v0.5.0)\n" but got a charlist representation ("test_fun", "*", "esp32", "v0.5.0").


[error] 106-106: Test failure: spectrometer_elixir_tests:format_function_line_arities_list_test/0. Expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" but got a charlist representation ("test_fun", "1/2/3", "stm32", "v0.6.0").

🪛 GitHub Actions: Build and Test / build-and-test (master)

[error] 92-92: spectrometer_elixir_tests: format_function_line_integer_arity_test failed: expected " test_fun/1 (all since: v0.5.0)\n" got different output


[error] 99-99: spectrometer_elixir_tests: format_function_line_all_arities_test failed: expected " test_fun/* (esp32 since: v0.5.0)\n" got different output


[error] 106-106: spectrometer_elixir_tests: format_function_line_arities_list_test failed: expected " test_fun/1/2/3 (stm32 since: v0.6.0)\n" got different output

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/spectrometer_elixir_tests.erl` around lines 87 - 106, Tests call
spectrometer_atomvm:format_function_line_string/4 which returns iodata;
normalize Output with iolist_to_binary/1 (e.g. OutputBin =
iolist_to_binary(spectrometer_atomvm:format_function_line_string(...))) and
compare against a binary expected (<<"  test_fun/1  (all since: v0.5.0)\n">>,
etc.) in format_function_line_integer_arity_test,
format_function_line_all_arities_test and format_function_line_arities_list_test
so assertions don't fail across CI matrices.

Comment on lines +972 to +979
count_arity_test_() ->
{"counts function arity from argument string", fun() ->
?assertEqual(0, spectrometer_updater:count_arity("")),
?assertEqual(1, spectrometer_updater:count_arity("x")),
?assertEqual(2, spectrometer_updater:count_arity("x, y")),
?assertEqual(3, spectrometer_updater:count_arity("x, y, z")),
?assertEqual(2, spectrometer_updater:count_arity(" x , y "))
end}.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

count_arity/1 currently fails for spaced argument lists and is red in CI.

The new test is valid for Elixir-style signatures ("x, y"), but implementation returns 1. Please update count_arity/1 to split on commas and trim tokens before counting.

🧰 Tools
🪛 GitHub Actions: Build and Test / 0_build-and-test (28).txt

[error] 976-976: Test failure: spectrometer_updater_tests:-count_arity_test_/0-fun-5-/0 counts function arity from argument string. expected: 2, got: 1

🪛 GitHub Actions: Build and Test / 1_build-and-test (29, 3.27.0).txt

[error] 976-976: spectrometer_updater_tests: -count_arity_test failed: expected 2 from count_arity("x, y"), got 1

🪛 GitHub Actions: Build and Test / 2_build-and-test (27).txt

[error] 976-976: spectrometer_updater_tests:-count_arity_test_/0-fun-5-/0: count_arity("x, y") expected 2, got 1.

🪛 GitHub Actions: Build and Test / 3_build-and-test (master).txt

[error] 976-976: EUnit failure: counts function arity from argument string. ?assertEqual(2, spectrometer_updater:count_arity("x, y")) failed (expected 2, got 1)

🪛 GitHub Actions: Build and Test / build-and-test (27)

[error] 976-976: Test failure: count_arity("x, y") expected 2, got 1

🪛 GitHub Actions: Build and Test / build-and-test (28)

[error] 976-976: Test failure: counts function arity from argument string. ?assertEqual(2, spectrometer_updater:count_arity("x, y")) expected 2, got 1

🪛 GitHub Actions: Build and Test / build-and-test (29, 3.27.0)

[error] 976-976: Test failure: spectrometer_updater_tests:-count_arity_test_/0 counts function arity from argument string. Expected 2, got 1 for count_arity("x, y").

🪛 GitHub Actions: Build and Test / build-and-test (master)

[error] 976-976: spectrometer_updater_tests: count_arity test failed: expected 2 got 1 for count_arity("x, y")

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/spectrometer_updater_tests.erl` around lines 972 - 979, The test fails
because count_arity/1 in module spectrometer_updater treats the whole argument
string as one token when spaces are present; change count_arity/1 to return 0
for the empty string, otherwise split the input on commas, trim whitespace from
each token, ignore empty tokens and return the count of non-empty tokens (so
inputs like "x, y" and "  x  ,  y  " yield 2). Locate and update the
spectrometer_updater:count_arity/1 implementation referenced by the failing test
in spectrometer_updater_tests to use comma-based splitting and token trimming
before counting.

UncleGrumpy and others added 3 commits May 22, 2026 00:46
Fix bug in install.in that can lead to broken installs.
Fix bug in `rebar3 as prod tar` where the escript may be omitted.
Fix bug when getting temp dir from environment.

Signed-off-by: Winford <winford@object.stream>
Signed-off-by: Winford <winford@object.stream>
Add support for Elixir MFA in update, support, and query commands.

Signed-off-by: Winford <winford@object.stream>
@UncleGrumpy UncleGrumpy changed the title Elixir support Preliminary Elixir support May 22, 2026
@UncleGrumpy
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
src/spectrometer_updater.erl (2)

1278-1287: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use Erlang character literals in the comma scanner.

Char here is an integer codepoint from a charlist, but the case arms are matching one-character strings. None of the delimiter branches will ever fire, so multi-argument Elixir defs are recorded with the wrong arity.

🛠️ Proposed fix
 count_top_level_commas([Char | Rest], Depth, Count) ->
     case Char of
-        "(" -> count_top_level_commas(Rest, Depth + 1, Count);
-        "[" -> count_top_level_commas(Rest, Depth + 1, Count);
-        "{" -> count_top_level_commas(Rest, Depth + 1, Count);
-        ")" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "]" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "}" -> count_top_level_commas(Rest, Depth - 1, Count);
-        "," when Depth == 0 -> count_top_level_commas(Rest, Depth, Count + 1);
+        $( -> count_top_level_commas(Rest, Depth + 1, Count);
+        $[ -> count_top_level_commas(Rest, Depth + 1, Count);
+        ${ -> count_top_level_commas(Rest, Depth + 1, Count);
+        $) -> count_top_level_commas(Rest, Depth - 1, Count);
+        $] -> count_top_level_commas(Rest, Depth - 1, Count);
+        $} -> count_top_level_commas(Rest, Depth - 1, Count);
+        $, when Depth == 0 -> count_top_level_commas(Rest, Depth, Count + 1);
         _ -> count_top_level_commas(Rest, Depth, Count)
     end;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spectrometer_updater.erl` around lines 1278 - 1287, The case in
count_top_level_commas is matching one-character strings but Char is an integer
codepoint; change all delimiter and comma patterns in count_top_level_commas to
Erlang character literals (e.g., use $\(, $[, ${, $), $], $}, $,) so the
branches (parentheses/brackets/braces and top-level comma) actually match;
update the guard for the comma branch to use Depth == 0 as-is and ensure the
function still recurses on Rest for all branches (keeping the same call sites:
count_top_level_commas(Rest, Depth + 1, Count), count_top_level_commas(Rest,
Depth - 1, Count), count_top_level_commas(Rest, Depth, Count + 1), and default).

1183-1223: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Module scope still collapses on any standalone end.

find_elixir_module_def/1 returns end_defmodule for every bare end, so function/case/if endings still pop ModuleStack too. The new {unknown, [...]} entry checked into priv/supported_functions.data is the downstream symptom: module context is still being lost while scanning.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spectrometer_updater.erl` around lines 1183 - 1223,
find_elixir_module_def/1 currently returns end_defmodule for any standalone
"end", which causes ModuleStack to pop module entries incorrectly (producing
{unknown,...} in priv/supported_functions.data); change the function so a plain
"end" returns a generic end marker (e.g. {end_block} or just end) instead of
end_defmodule, and only return end_defmodule when you can match it to a
previously opened defmodule/defimpl (or include the matching opener name in the
return so the caller can decide). Update the caller logic that manages
ModuleStack to pop module entries only when the returned token explicitly
matches a module/impl opener from find_elixir_module_def/1, leaving other ends
to close inner constructs (functions/cases) without affecting module context.
test/spectrometer_updater_tests.erl (1)

957-957: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix EUnit teardown callbacks and align count_arity/1 test expectations.

  • In test/spectrometer_updater_tests.erl at lines 957 and 1179, the EUnit cleanup callback returns fun() -> spectrometer_utils:purge_dir(Dir) end (nested fun) instead of executing spectrometer_utils:purge_dir(Dir), so teardown won’t run.
  • In test/spectrometer_updater_tests.erl, count_arity_test_ expects arity 1 for "x, y" / "x, y, z" (lines 976-977), but src/spectrometer_updater.erl:count_arity/1 returns top_level_commas + 1, which yields 2 and 3 respectively.
Suggested fix
-            fun(Dir) -> fun() -> spectrometer_utils:purge_dir(Dir) end end, fun(
+            fun(Dir) -> spectrometer_utils:purge_dir(Dir) end, fun(
                 Dir
             ) ->
...
-            fun(Dir) -> fun() -> spectrometer_utils:purge_dir(Dir) end end, fun(
+            fun(Dir) -> spectrometer_utils:purge_dir(Dir) end, fun(
                 Dir
             ) ->
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/spectrometer_updater_tests.erl` at line 957, The teardown callback
currently returns a nested fun instead of executing the purge, and the tests
assert wrong arities; change the cleanup callback from returning fun() ->
spectrometer_utils:purge_dir(Dir) end (the extra nesting) to a callback that
actually calls purge (i.e., fun(Dir) -> spectrometer_utils:purge_dir(Dir) end or
return fun() -> spectrometer_utils:purge_dir(Dir) end directly from the setup as
EUnit expects), and update the expectations in count_arity_test_ to match
spectrometer_updater:count_arity/1 (expect 2 for "x, y" and 3 for "x, y, z").
Reference spectrometer_utils:purge_dir, count_arity/1, and count_arity_test_
when making these edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/spectrometer_reporter_tests.erl`:
- Around line 171-178: The test currently asserts that Content contains
<<"header">>, which is not a real CSV header and causes false failures; update
the assertion to check for the actual CSV header token used by the reporter
(instead of <<"header">>)—for example call the reporter's header provider (e.g.
spectrometer_reporter:csv_header() or the module/function that generates the CSV
header) and assert that Content contains that returned binary (using
binary:match(Content, ExpectedHeader) =/= nomatch) so the test validates the
real header string rather than the literal <<"header">>.

---

Duplicate comments:
In `@src/spectrometer_updater.erl`:
- Around line 1278-1287: The case in count_top_level_commas is matching
one-character strings but Char is an integer codepoint; change all delimiter and
comma patterns in count_top_level_commas to Erlang character literals (e.g., use
$\(, $[, ${, $), $], $}, $,) so the branches (parentheses/brackets/braces and
top-level comma) actually match; update the guard for the comma branch to use
Depth == 0 as-is and ensure the function still recurses on Rest for all branches
(keeping the same call sites: count_top_level_commas(Rest, Depth + 1, Count),
count_top_level_commas(Rest, Depth - 1, Count), count_top_level_commas(Rest,
Depth, Count + 1), and default).
- Around line 1183-1223: find_elixir_module_def/1 currently returns
end_defmodule for any standalone "end", which causes ModuleStack to pop module
entries incorrectly (producing {unknown,...} in priv/supported_functions.data);
change the function so a plain "end" returns a generic end marker (e.g.
{end_block} or just end) instead of end_defmodule, and only return end_defmodule
when you can match it to a previously opened defmodule/defimpl (or include the
matching opener name in the return so the caller can decide). Update the caller
logic that manages ModuleStack to pop module entries only when the returned
token explicitly matches a module/impl opener from find_elixir_module_def/1,
leaving other ends to close inner constructs (functions/cases) without affecting
module context.

In `@test/spectrometer_updater_tests.erl`:
- Line 957: The teardown callback currently returns a nested fun instead of
executing the purge, and the tests assert wrong arities; change the cleanup
callback from returning fun() -> spectrometer_utils:purge_dir(Dir) end (the
extra nesting) to a callback that actually calls purge (i.e., fun(Dir) ->
spectrometer_utils:purge_dir(Dir) end or return fun() ->
spectrometer_utils:purge_dir(Dir) end directly from the setup as EUnit expects),
and update the expectations in count_arity_test_ to match
spectrometer_updater:count_arity/1 (expect 2 for "x, y" and 3 for "x, y, z").
Reference spectrometer_utils:purge_dir, count_arity/1, and count_arity_test_
when making these edits.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47603002-1b8c-4594-8257-cae0ef8739dd

📥 Commits

Reviewing files that changed from the base of the PR and between 27e2dba and a362298.

📒 Files selected for processing (16)
  • README.md
  • priv/supported_functions.data
  • rebar.config.script
  • release/install.in
  • src/atomvm_spectrometer.erl
  • src/spectrometer_atomvm.erl
  • src/spectrometer_help.erl
  • src/spectrometer_updater.erl
  • src/spectrometer_utils.erl
  • test/atomvm_spectrometer_tests.erl
  • test/spectrometer_elixir_tests.erl
  • test/spectrometer_otp_tests.erl
  • test/spectrometer_reporter_tests.erl
  • test/spectrometer_scanner_tests.erl
  • test/spectrometer_updater_tests.erl
  • test/spectrometer_utils_tests.erl
✅ Files skipped from review due to trivial changes (2)
  • src/spectrometer_help.erl
  • README.md

Comment on lines 171 to 178
?assert(
binary:match(Content, <<"lists,map,2">>) =/= nomatch
),
?assert(
binary:match(Content, <<"io,format,2">>) =/= nomatch
)
),
?assert(binary:match(Content, <<"header">>) =/= nomatch)
end)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Incorrect CSV header assertion literal.

Line 177 checks for <<"header">>, which is not a real CSV header token and can fail even when output is correct.

Suggested fix
-                    ?assert(binary:match(Content, <<"header">>) =/= nomatch)
+                    ?assert(
+                        binary:match(Content, <<"module,function,arity">>) =/= nomatch
+                    )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/spectrometer_reporter_tests.erl` around lines 171 - 178, The test
currently asserts that Content contains <<"header">>, which is not a real CSV
header and causes false failures; update the assertion to check for the actual
CSV header token used by the reporter (instead of <<"header">>)—for example call
the reporter's header provider (e.g. spectrometer_reporter:csv_header() or the
module/function that generates the CSV header) and assert that Content contains
that returned binary (using binary:match(Content, ExpectedHeader) =/= nomatch)
so the test validates the real header string rather than the literal
<<"header">>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant