Skip to content

Commit 4303494

Browse files
lukaszsamsonclaude
andcommitted
fix: address PR review comments (binary unit widths, hint cache key, doc types)
- type_inference: honor `unit` when deciding sub-byte binary segments — the effective bit width is size * unit, so <<1::size(1)-unit(8)>> is a byte-aligned binary, not a bitstring. Also handles the 2*4 size*unit shorthand. Regression tests added. - type_hints: include (sorted) render opts in the type_hint_at cache key so a call with e.g. max_length does not poison later calls expecting the default rendering. Regression test added. - type_presentation_test: async: false — it mutates the global :use_elixir_types app env and could race concurrent modules. - hover docs: variable/attribute/keyword doc typespecs said name: atom() but builders produce strings; keyword_doc kind corrected :attribute -> :keyword. - completion_engine: comment no longer claims rendered type forms are always parseable (open-map markers are not; nil fallback is intentional). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NvwMWQayfdxowBDrnt5iYM
1 parent 702ff31 commit 4303494

7 files changed

Lines changed: 70 additions & 13 deletions

File tree

lib/elixir_sense/core/type_hints.ex

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,10 @@ defmodule ElixirSense.Core.TypeHints do
249249
:skip | {:ok, hint}
250250
def type_hint_at(%Context{} = ctx, position, var_name, opts \\ [])
251251
when is_atom(var_name) do
252-
cached({__MODULE__, ctx.ref, :hint_at, position, var_name}, fn ->
252+
# `opts` affect rendering (e.g. max_length, widen_literals), so they must
253+
# be part of the key — otherwise the first caller's formatting poisons
254+
# later calls with different options.
255+
cached({__MODULE__, ctx.ref, :hint_at, position, var_name, Enum.sort(opts)}, fn ->
253256
compute_hint_at(ctx, position, var_name, opts)
254257
end)
255258
end

lib/elixir_sense/core/type_inference.ex

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -727,22 +727,47 @@ defmodule ElixirSense.Core.TypeInference do
727727
cond do
728728
Enum.any?(parts, &(&1 in [:binary, :bytes, :utf8, :utf16, :utf32, :float])) -> false
729729
Enum.any?(parts, &(&1 in [:bitstring, :bits])) -> true
730-
true -> Enum.any?(parts, &(is_integer(&1) and rem(&1, 8) != 0))
730+
true -> sub_byte_width?(parts)
731731
end
732732
end
733733

734734
# A segment without `::` is the default (integer, 8 bits — byte-aligned).
735735
defp sub_byte_segment?(_other), do: false
736736

737+
# The effective bit width is size * unit (unit defaults to 1 for integer
738+
# segments), so `<<1::size(1)-unit(8)>>` is a byte-aligned 8-bit segment,
739+
# not a sub-byte one. Non-literal sizes yield no `{:size, _}` entry and stay
740+
# conservative (not sub-byte).
741+
defp sub_byte_width?(parts) do
742+
size =
743+
Enum.find_value(parts, fn
744+
{:size, n} -> n
745+
_other -> nil
746+
end)
747+
748+
unit =
749+
Enum.find_value(parts, fn
750+
{:unit, n} -> n
751+
_other -> nil
752+
end) || 1
753+
754+
is_integer(size) and rem(size * unit, 8) != 0
755+
end
756+
737757
defp flatten_segment_spec({:-, _, [left, right]}),
738758
do: flatten_segment_spec(left) ++ flatten_segment_spec(right)
739759

760+
# `size*unit` shorthand (`<<x::2*4>>` is size 2, unit 4).
761+
defp flatten_segment_spec({:*, _, [size, unit]}) when is_integer(size) and is_integer(unit),
762+
do: [{:size, size}, {:unit, unit}]
763+
740764
defp flatten_segment_spec({:*, _, [left, right]}),
741765
do: flatten_segment_spec(left) ++ flatten_segment_spec(right)
742766

743-
defp flatten_segment_spec({:size, _, [n]}) when is_integer(n), do: [n]
767+
defp flatten_segment_spec({:size, _, [n]}) when is_integer(n), do: [{:size, n}]
768+
defp flatten_segment_spec({:unit, _, [n]}) when is_integer(n), do: [{:unit, n}]
744769
defp flatten_segment_spec({name, _, _}) when is_atom(name), do: [name]
745-
defp flatten_segment_spec(n) when is_integer(n), do: [n]
770+
defp flatten_segment_spec(n) when is_integer(n), do: [{:size, n}]
746771
defp flatten_segment_spec(_other), do: []
747772

748773
# The result type of a `for` from its (last-clause) opts. `:reduce` wins over

lib/elixir_sense/providers/completion/completion_engine.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1400,7 +1400,8 @@ defmodule ElixirSense.Providers.Completion.CompletionEngine do
14001400

14011401
# `type_spec` here is a quoted type AST (to_entries renders it with
14021402
# Macro.to_string). TypePresentation renders to text, so parse it back to an
1403-
# AST; the rendered forms are always valid type expressions.
1403+
# AST — best-effort: some rendered forms (e.g. open-map `...` markers) are
1404+
# not parseable, in which case we fall back to nil.
14041405
defp rendered_field_type(value) do
14051406
with {:ok, text} <- TypePresentation.render(value),
14061407
{:ok, ast} <- Code.string_to_quoted(text) do

lib/elixir_sense/providers/hover/docs.ex

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,21 @@ defmodule ElixirSense.Providers.Hover.Docs do
4444
}
4545

4646
@type variable_doc :: %{
47-
name: atom(),
47+
name: String.t(),
4848
kind: :variable,
4949
# Rendered inferred type (best-effort), or nil when nothing useful.
5050
type: String.t() | nil
5151
}
5252

5353
@type attribute_doc :: %{
54-
name: atom(),
54+
name: String.t(),
5555
kind: :attribute,
5656
docs: markdown()
5757
}
5858

5959
@type keyword_doc :: %{
60-
name: atom(),
61-
kind: :attribute,
60+
name: String.t(),
61+
kind: :keyword,
6262
docs: markdown()
6363
}
6464

test/elixir_sense/core/type_hints_test.exs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,24 +514,38 @@ defmodule ElixirSense.Core.TypeHintsTest do
514514
"Flow-sensitivity not observable: both positions returned #{inspect(result_narrow)}"
515515
end
516516

517-
test "result is cached per {ref, :hint_at, position, var_name}" do
517+
test "result is cached per {ref, :hint_at, position, var_name, opts}" do
518518
md = metadata(@flow_code, {5, 9})
519519
pos = {5, 9}
520520
ctx = TypeHints.request_context(md)
521-
cache_key = {TypeHints, ctx.ref, :hint_at, pos, :x}
521+
cache_key = {TypeHints, ctx.ref, :hint_at, pos, :x, []}
522522

523523
assert Process.get(cache_key) == nil
524524
result = TypeHints.type_hint_at(ctx, pos, :x, [])
525525
assert Process.get(cache_key) == result
526526
end
527527

528+
test "different render opts do not poison each other's cache entries" do
529+
md = metadata(@flow_code, {5, 9})
530+
pos = {5, 9}
531+
ctx = TypeHints.request_context(md)
532+
533+
{:ok, default_hint} = TypeHints.type_hint_at(ctx, pos, :x, [])
534+
{:ok, truncated_hint} = TypeHints.type_hint_at(ctx, pos, :x, max_length: 3)
535+
536+
# The truncated call must not return the earlier default rendering...
537+
assert truncated_hint.label != default_hint.label
538+
# ...and the default entry must survive the truncated call unchanged.
539+
assert {:ok, ^default_hint} = TypeHints.type_hint_at(ctx, pos, :x, [])
540+
end
541+
528542
test "discard/1 also erases hint_at cache entries" do
529543
md = metadata(@flow_code, {5, 9})
530544
pos = {5, 9}
531545
ctx = TypeHints.request_context(md)
532546

533547
TypeHints.type_hint_at(ctx, pos, :x, [])
534-
cache_key = {TypeHints, ctx.ref, :hint_at, pos, :x}
548+
cache_key = {TypeHints, ctx.ref, :hint_at, pos, :x, []}
535549
assert Process.get(cache_key) != nil
536550

537551
TypeHints.discard(ctx)

test/elixir_sense/core/type_inference_test.exs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,18 @@ defmodule ElixirSense.Core.TypeInferenceTest do
709709
assert type_of("<<x::binary-size(4)>>") == {:binary, nil}
710710
assert type_of("<<x::utf8>>") == {:binary, nil}
711711
end
712+
713+
test "bitstring vs binary: unit multiplies size for effective bit width" do
714+
# size(1)-unit(8) is 8 bits — byte-aligned binary, not a bitstring
715+
assert type_of("<<1::size(1)-unit(8)>>") == {:binary, nil}
716+
assert type_of("<<x::integer-size(2)-unit(8)>>") == {:binary, nil}
717+
# 2*4 shorthand (size 2, unit 4) is 8 bits — binary
718+
assert type_of("<<1::2*4>>") == {:binary, nil}
719+
# size(3)-unit(2) is 6 bits — sub-byte
720+
assert type_of("<<1::size(3)-unit(2)>>") == :bitstring
721+
# unit without a literal size stays conservative (binary)
722+
assert type_of("<<x::size(n)-unit(8)>>") == {:binary, nil}
723+
end
712724
end
713725

714726
describe "special forms" do

test/elixir_sense/core/type_presentation_test.exs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
defmodule ElixirSense.Core.TypePresentationTest do
2-
use ExUnit.Case, async: true
2+
# async: false — some tests mutate the global :use_elixir_types app env,
3+
# which would race with concurrently-running modules that read the flag.
4+
use ExUnit.Case, async: false
35

46
alias ElixirSense.Core.Binding
57
alias ElixirSense.Core.State.VarInfo

0 commit comments

Comments
 (0)