Skip to content

Commit 56cd48b

Browse files
lukaszsamsonclaude
andcommitted
fix: address PR review comments (env parsing, range clamp, stale docs)
- config.exs: accept the same disable values ("false", "0") for ELIXIR_LS_TYPE_INFERENCE as the runtime override in language_server.ex, so dev/test and release behavior match. - inlay_hints: clamp_range now sets a whole-line sentinel end column on the clamped boundary line instead of reusing the original range's final-line column, which could spuriously cut off hints on that line. - markdown_utils: compute the :"..//" atom once as a module attribute instead of String.to_atom/1 per call. - release_smoke_test: docstrings updated to the current git-pin state (the old text described the absolute path dep as a known blocker and referenced a companion test asserting its presence — both stale). - bump elixir_sense pin to 43034945 (binary unit-width fix, hint cache key fix, doc typespec corrections). Note: the hover heredoc-indentation comment was reviewed and declined — the inner heredoc's closing delimiter dedents its content to column 0, so the interpolated markdown is already flush. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NvwMWQayfdxowBDrnt5iYM
1 parent 9a5d6e3 commit 56cd48b

6 files changed

Lines changed: 36 additions & 30 deletions

File tree

apps/language_server/lib/language_server/markdown_utils.ex

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,11 @@ defmodule ElixirLS.LanguageServer.MarkdownUtils do
433433
# 1.20 emits it unquoted (`:..//`) while 1.16 quotes it (`:"..//"`), so no
434434
# single literal form is `mix format --check-formatted`-clean on both. The
435435
# string form is left untouched by every formatter and yields the same atom.
436-
defp get_module_fun_arity("..///3"), do: {Kernel, String.to_atom("..//"), 3}
436+
# Computed once at compile time (module attribute) to keep the dynamic-atom
437+
# call out of the per-invocation path.
438+
@stepped_range_atom String.to_atom("..//")
439+
440+
defp get_module_fun_arity("..///3"), do: {Kernel, @stepped_range_atom, 3}
437441
defp get_module_fun_arity("../2"), do: {Kernel, :.., 2}
438442
defp get_module_fun_arity("../0"), do: {Kernel, :.., 0}
439443
defp get_module_fun_arity("./2"), do: {Kernel.SpecialForms, :., 2}

apps/language_server/lib/language_server/providers/inlay_hints.ex

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ defmodule ElixirLS.LanguageServer.Providers.InlayHints do
4848
@unrecognized_trust_key_prefix {__MODULE__, :unrecognized_trust}
4949

5050
@max_range_lines 1000
51+
# Whole-line sentinel end column used when clamp_range trims a range: large
52+
# enough to exceed any realistic line length, so the clamped boundary line
53+
# is fully covered.
54+
@max_line_column 1_000_000
5155
@max_hints 1000
5256
@default_max_label_length 60
5357

@@ -821,10 +825,13 @@ defmodule ElixirLS.LanguageServer.Providers.InlayHints do
821825
# Clamp so at most @max_range_lines lines are ever processed: the inclusive
822826
# window sl..el spans `el - sl + 1` lines, so anything with `el - sl >=
823827
# @max_range_lines` (i.e. > @max_range_lines lines) is trimmed to the first
824-
# @max_range_lines lines (sl .. sl + @max_range_lines - 1).
825-
defp clamp_range({{sl, _sc} = start, {el, ec}} = range) do
828+
# @max_range_lines lines (sl .. sl + @max_range_lines - 1). The new end
829+
# column must be a whole-line sentinel, not the original range's `ec` — that
830+
# column belongs to a different (later) line and would spuriously cut off
831+
# hints on the clamped boundary line.
832+
defp clamp_range({{sl, _sc} = start, {el, _ec}} = range) do
826833
if el - sl >= @max_range_lines do
827-
{start, {sl + @max_range_lines - 1, ec}}
834+
{start, {sl + @max_range_lines - 1, @max_line_column}}
828835
else
829836
range
830837
end

apps/language_server/test/release_smoke_test.exs

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ defmodule ElixirLS.LanguageServer.ReleaseSmokeTest do
66
77
@moduletag :release_smoke
88
9-
They are intended to run against a CLEAN checkout with production deps (no
10-
`path:` overrides) before cutting a release. Today most of them are excluded
11-
because the workspace intentionally uses a local `path:` dep for
12-
`elixir_sense` during development.
9+
They are intended to run against a CLEAN checkout with production deps
10+
before cutting a release. The `elixir_sense` dependency is a git pin
11+
(`dep_versions.exs`), and the CI release-gate job plus the always-running
12+
companion test below keep it that way.
1313
1414
## Running at release time
1515
@@ -18,12 +18,10 @@ defmodule ElixirLS.LanguageServer.ReleaseSmokeTest do
1818
## Tests in this module
1919
2020
1. `no_absolute_path_deps` — asserts that no `mix.exs` file in the umbrella
21-
tree contains `path: "/"` (an absolute-path dep). Today this test
22-
DOCUMENTS A KNOWN RELEASE BLOCKER: the `elixir_sense` dep in
23-
`apps/language_server/mix.exs` uses an absolute `path:` pointing to a
24-
local worktree. The always-running companion test
25-
(`path_dep_is_still_present`) asserts that the path dep IS present so the
26-
suite notices when it is removed.
21+
tree contains `path: "/"` (an absolute-path dep). The always-running
22+
companion test (`companion: elixir_sense is a git pin ...`) asserts the
23+
same invariant in normal CI, so a reintroduced local path dep is caught
24+
immediately rather than at release time.
2725
2826
2. `packaged_dep_compile_check` — placeholder for a manual smoke step
2927
(clean checkout, `mix deps.get`, hint round-trip).
@@ -102,17 +100,10 @@ defmodule ElixirLS.LanguageServer.ReleaseSmokeTest do
102100

103101
@doc """
104102
Asserts that no mix.exs in the umbrella contains `path: "/"` (an absolute
105-
path pointing outside the repo).
106-
107-
## Known blocker (as of 2026-06-12)
108-
109-
`apps/language_server/mix.exs` contains:
110-
111-
{:elixir_sense, path: "/Users/lukaszsamson/elixir_sense/.claude/worktrees/..."}
112-
113-
This is a local development override. Before cutting a release, replace it
114-
with the published Hex package reference (or a GitHub ref) and verify that
115-
`mix deps.get` resolves cleanly from a clean checkout.
103+
path pointing outside the repo). The `elixir_sense` dep is a git pin
104+
(`dep_versions.exs`); if a local `path:` override is reintroduced for
105+
development, it must be swapped back before releasing — this test (and the
106+
always-running companion above) will flag it.
116107
"""
117108
test "no_absolute_path_deps: no mix.exs uses an absolute path dep" do
118109
# NOTE: This test is excluded by default (@moduletag :release_smoke).
@@ -144,8 +135,8 @@ defmodule ElixirLS.LanguageServer.ReleaseSmokeTest do
144135
Placeholder for the packaged-dep compile-and-hint smoke test.
145136
146137
This test is intentionally skipped via `@tag :skip`. It documents the
147-
MANUAL STEPS that a release engineer must perform after switching from the
148-
local `path:` dep to the published Hex package.
138+
MANUAL STEPS that a release engineer must perform to verify the git-pinned
139+
(or future Hex-published) `elixir_sense` dependency from a clean checkout.
149140
150141
## Manual steps
151142

config/config.exs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,13 @@ end
2929
# powering inlay hints, hover, and completion). Requires Elixir 1.19+; falls
3030
# back to the custom engine automatically when unavailable. On by default on
3131
# this branch — set ELIXIR_LS_TYPE_INFERENCE=false to disable for A/B testing.
32+
# Keep the accepted disable values in sync with the runtime override in
33+
# apps/language_server/lib/language_server.ex ("false" and "0").
3234
config :elixir_sense,
3335
use_elixir_types:
34-
System.get_env("ELIXIR_LS_TYPE_INFERENCE", "true") |> String.downcase() != "false"
36+
System.get_env("ELIXIR_LS_TYPE_INFERENCE", "true")
37+
|> String.downcase()
38+
|> then(&(&1 not in ["false", "0"]))
3539

3640
# NOTE: the native-typing backend's verbose degradation-log flood on Elixir
3741
# 1.18/1.19 is tamed in apps/language_server/test/test_helper.exs via per-module

dep_versions.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[
2-
elixir_sense: "702ff31a601ddf4f04343db7e244719852586bfe",
2+
elixir_sense: "430349450cdaa2bc243bb5ba2a5948f5e9e0ea80",
33
dialyxir_vendored: "accfec9393079abc4a82b7e79a4997f59f085b67",
44
jason_v: "f1c10fa9c445cb9f300266122ef18671054b2330",
55
erl2ex_vendored: "04f93e55f46d35d0aa3c149616f2c7a6a1ad9311",

mix.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"benchee": {:hex, :benchee, "1.1.0", "f3a43817209a92a1fade36ef36b86e1052627fd8934a8b937ac9ab3a76c43062", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}], "hexpm", "7da57d545003165a012b587077f6ba90b89210fd88074ce3c60ce239eb5e6d93"},
33
"deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
44
"dialyxir_vendored": {:git, "https://github.com/elixir-lsp/dialyxir.git", "accfec9393079abc4a82b7e79a4997f59f085b67", [ref: "accfec9393079abc4a82b7e79a4997f59f085b67"]},
5-
"elixir_sense": {:git, "https://github.com/elixir-lsp/elixir_sense.git", "702ff31a601ddf4f04343db7e244719852586bfe", [ref: "702ff31a601ddf4f04343db7e244719852586bfe"]},
5+
"elixir_sense": {:git, "https://github.com/elixir-lsp/elixir_sense.git", "430349450cdaa2bc243bb5ba2a5948f5e9e0ea80", [ref: "430349450cdaa2bc243bb5ba2a5948f5e9e0ea80"]},
66
"erl2ex_vendored": {:git, "https://github.com/elixir-lsp/erl2ex.git", "04f93e55f46d35d0aa3c149616f2c7a6a1ad9311", [ref: "04f93e55f46d35d0aa3c149616f2c7a6a1ad9311"]},
77
"erlex_vendored": {:git, "https://github.com/elixir-lsp/erlex.git", "50b8307f90451a5d0288fb239fb6405b5ca1f1a4", [ref: "50b8307f90451a5d0288fb239fb6405b5ca1f1a4"]},
88
"jason_v": {:git, "https://github.com/elixir-lsp/jason.git", "f1c10fa9c445cb9f300266122ef18671054b2330", [ref: "f1c10fa9c445cb9f300266122ef18671054b2330"]},

0 commit comments

Comments
 (0)