Skip to content

Commit 076bd4a

Browse files
cristianoccknitt
authored andcommitted
Reanalyze: link type re-export labels (#8217)
* Reanalyze: link type re-export labels Track manifest type paths on label declarations so re-exported record/variant labels can be linked to their manifest counterparts during type label dependency wiring. Adds a regression test for `type y = x = {...}` field liveness. Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> * Changelog: mention reanalyze type re-export label linking Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> * Reanalyze: suppress dead warnings on re-exported labels When a type is re-exported via `type y = x = {...}`, the re-exported record/variant labels are restated but not independently actionable. Suppress dead-type warning emission for those labels while keeping per-label liveness and linking intact. Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> * Reanalyze: link re-export manifests across modules Track manifest type paths (not just local type ids) so `type y = M.x = ...` and other cross-module type equations correctly link label/case liveness. Extend deadcode tests to cover variants and cross-file record re-exports. Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> --------- Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> # Conflicts: # tests/analysis_tests/tests-reanalyze/deadcode/expected/deadcode.txt
1 parent d8b903a commit 076bd4a

9 files changed

Lines changed: 552 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#### :bug: Bug fix
2727

2828
- Reanalyze: fix reactive/server stale results when cross-file references change without changing dead declarations (non-transitive mode). https://github.com/rescript-lang/rescript/pull/8173
29+
- Reanalyze: link record/variant label liveness across type re-exports (`type y = x = {...}`). https://github.com/rescript-lang/rescript/pull/8217
2930
- Add duplicate package detection to rewatch. https://github.com/rescript-lang/rescript/pull/8180
3031
- Rewatch: do not warn about "reanalyze" config field. https://github.com/rescript-lang/rescript/pull/8181
3132
- Fix error when importing CommonJS runtime modules with `require()`. https://github.com/rescript-lang/rescript/pull/8194

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ module FileContext = struct
44
(** Get module name as Name.t tagged with interface/implementation info *)
55
let module_name_tagged file =
66
file.module_name |> Name.create ~isInterface:file.is_interface
7+
8+
let isInterface (file : t) = file.is_interface
79
end
810

911
(* Adapted from https://github.com/LexiFi/dead_code_analyzer *)
@@ -82,7 +84,7 @@ let addValueReference ~config ~refs ~file_deps ~(binding : Location.t)
8284

8385
let addDeclaration_ ~config ~decls ~(file : FileContext.t) ?posEnd ?posStart
8486
~declKind ~path ~(loc : Location.t) ?(posAdjustment = Decl.Nothing)
85-
~moduleLoc (name : Name.t) =
87+
?manifestTypePath ~moduleLoc (name : Name.t) =
8688
let pos = loc.loc_start in
8789
let posStart =
8890
match posStart with
@@ -110,6 +112,7 @@ let addDeclaration_ ~config ~decls ~(file : FileContext.t) ?posEnd ?posStart
110112
moduleLoc;
111113
posAdjustment;
112114
path = name :: path;
115+
manifestTypePath;
113116
pos;
114117
posEnd;
115118
posStart;
@@ -182,6 +185,16 @@ let reportDeclaration ~config ~hasRefBelow ?checkModuleDead ?shouldReport
182185
| Some f -> f decl
183186
| None -> decl.report
184187
in
188+
(* For type re-exports (type y = x = {...}), the re-exported record/variant
189+
labels are restated but not independently actionable. Avoid duplicate/noisy
190+
warnings by suppressing reporting for the re-exported copy. *)
191+
let should_report =
192+
should_report
193+
&&
194+
match (decl.declKind, decl.manifestTypePath) with
195+
| (RecordLabel | VariantCase), Some _ -> false
196+
| _ -> true
197+
in
185198
if not should_report then []
186199
else
187200
let deadWarning, message =

analysis/reanalyze/src/DeadType.ml

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@ let extendTypeDependencies ~config ~refs (loc1 : Location.t) (loc2 : Location.t)
1919
addTypeReference ~config ~refs ~posFrom ~posTo)
2020

2121
let addDeclaration ~config ~decls ~file ~(modulePath : ModulePath.t)
22-
~(typeId : Ident.t) ~(typeKind : Types.type_kind) =
23-
let pathToType =
24-
(typeId |> Ident.name |> Name.create)
25-
:: (modulePath.path @ [FileContext.module_name_tagged file])
26-
in
22+
~(typeId : Ident.t) ~(typeKind : Types.type_kind)
23+
~(manifestTypePath : DcePath.t option) =
24+
let moduleContext = modulePath.path @ [FileContext.module_name_tagged file] in
25+
let pathToType = (typeId |> Ident.name |> Name.create) :: moduleContext in
2726
let processTypeLabel ?(posAdjustment = Decl.Nothing) typeLabelName ~declKind
2827
~(loc : Location.t) =
2928
addDeclaration_ ~config ~decls ~file ~declKind ~path:pathToType ~loc
30-
~moduleLoc:modulePath.loc ~posAdjustment typeLabelName
29+
?manifestTypePath ~moduleLoc:modulePath.loc ~posAdjustment typeLabelName
3130
in
3231
match typeKind with
3332
| Type_record (l, _) ->
@@ -162,4 +161,59 @@ let process_type_label_dependencies ~config ~decls ~refs =
162161
if not Config.reportTypesDeadOnlyInInterface then
163162
extendTypeDependencies ~config ~refs loc loc1))
164163
| _ -> ())
165-
decls
164+
decls;
165+
166+
(* Link fields of re-exported types (type y = x = {...}) to original type fields.
167+
We store the manifest type path on the label declarations themselves, and
168+
derive the set of re-export relationships here. To preserve stable output
169+
ordering, we process types bottom-to-top (by their first label position)
170+
and fields top-to-bottom (by their label position). *)
171+
let compare_pos (p1 : Lexing.position) (p2 : Lexing.position) =
172+
match compare p1.Lexing.pos_fname p2.Lexing.pos_fname with
173+
| 0 -> compare p1.Lexing.pos_cnum p2.Lexing.pos_cnum
174+
| c -> c
175+
in
176+
(* currentTypePath -> (rep_pos, manifestTypePath, (pos, fieldName, currentLoc) list) *)
177+
let groups :
178+
( DcePath.t,
179+
Lexing.position
180+
* DcePath.t
181+
* (Lexing.position * Name.t * Location.t) list )
182+
Hashtbl.t =
183+
Hashtbl.create 32
184+
in
185+
Declarations.iter
186+
(fun _pos decl ->
187+
match (decl.Decl.declKind, decl.manifestTypePath, decl.path) with
188+
| ( (RecordLabel | VariantCase),
189+
Some manifestTypePath,
190+
fieldName :: currentTypePath ) -> (
191+
let item = (decl.pos, fieldName, decl_raw_loc decl) in
192+
match Hashtbl.find_opt groups currentTypePath with
193+
| None ->
194+
Hashtbl.replace groups currentTypePath
195+
(decl.pos, manifestTypePath, [item])
196+
| Some (rep_pos, mtp0, items) ->
197+
(* manifestTypePath should be stable for a given currentTypePath *)
198+
let rep_pos =
199+
if compare_pos decl.pos rep_pos < 0 then decl.pos else rep_pos
200+
in
201+
Hashtbl.replace groups currentTypePath (rep_pos, mtp0, item :: items))
202+
| _ -> ())
203+
decls;
204+
205+
groups |> Hashtbl.to_seq |> List.of_seq
206+
|> List.map (fun (currentTypePath, (rep_pos, manifestTypePath, items)) ->
207+
(rep_pos, currentTypePath, manifestTypePath, items))
208+
(* Later (lower) types first *)
209+
|> List.fast_sort (fun (p1, _, _, _) (p2, _, _, _) -> compare_pos p2 p1)
210+
|> List.iter (fun (_rep_pos, _currentTypePath, manifestTypePath, items) ->
211+
items
212+
|> List.fast_sort (fun (p1, _, _) (p2, _, _) -> compare_pos p1 p2)
213+
|> List.iter (fun (_pos, fieldName, currentLoc) ->
214+
let manifestFieldPath = fieldName :: manifestTypePath in
215+
match find_one manifestFieldPath with
216+
| None -> ()
217+
| Some manifestLoc ->
218+
extendTypeDependencies ~config ~refs currentLoc manifestLoc;
219+
extendTypeDependencies ~config ~refs manifestLoc currentLoc))

analysis/reanalyze/src/DeadValue.ml

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,26 @@ let rec processSignatureItem ~config ~decls ~file ~doTypes ~doValues ~moduleLoc
247247
match si with
248248
| Sig_type (id, t, _) when doTypes ->
249249
if !Config.analyzeTypes then
250+
(* Extract manifest type path for type re-exports (type y = x = {...}).
251+
Use full Path.t so cross-module re-exports work (Path.Pdot, aliases, etc.). *)
252+
let manifestTypePath =
253+
match t.type_manifest with
254+
| Some {desc = Tconstr (path, _, _)} -> (
255+
let p = path |> DcePath.fromPathT in
256+
match p with
257+
| [typeName] ->
258+
let moduleContext =
259+
modulePath.path @ [FileContext.module_name_tagged file]
260+
in
261+
Some (typeName :: moduleContext)
262+
| _ ->
263+
Some
264+
(if FileContext.isInterface file then DcePath.moduleToInterface p
265+
else DcePath.moduleToImplementation p))
266+
| _ -> None
267+
in
250268
DeadType.addDeclaration ~config ~decls ~file ~modulePath ~typeId:id
251-
~typeKind:t.type_kind
269+
~typeKind:t.type_kind ~manifestTypePath
252270
| Sig_value (id, {Types.val_loc = loc; val_kind = kind; val_type})
253271
when doValues ->
254272
if not loc.Location.loc_ghost then
@@ -361,9 +379,29 @@ let traverseStructure ~config ~decls ~refs ~file_deps ~cross_file ~file ~doTypes
361379
typeDeclarations
362380
|> List.iter
363381
(fun (typeDeclaration : Typedtree.type_declaration) ->
382+
(* Extract manifest type path for type re-exports (type y = x = {...}). *)
383+
let manifestTypePath =
384+
match typeDeclaration.typ_manifest with
385+
| Some {ctyp_desc = Ttyp_constr (path, _, _)} -> (
386+
let p = path |> DcePath.fromPathT in
387+
match p with
388+
| [typeName] ->
389+
let moduleContext =
390+
modulePath.path
391+
@ [FileContext.module_name_tagged file]
392+
in
393+
Some (typeName :: moduleContext)
394+
| _ ->
395+
Some
396+
(if FileContext.isInterface file then
397+
DcePath.moduleToInterface p
398+
else DcePath.moduleToImplementation p))
399+
| _ -> None
400+
in
364401
DeadType.addDeclaration ~config ~decls ~file
365402
~modulePath ~typeId:typeDeclaration.typ_id
366-
~typeKind:typeDeclaration.typ_type.type_kind);
403+
~typeKind:typeDeclaration.typ_type.type_kind
404+
~manifestTypePath);
367405
None
368406
| Tstr_include {incl_mod; incl_type} ->
369407
(match incl_mod.mod_desc with

analysis/reanalyze/src/Decl.ml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ type t = {
3131
moduleLoc: Location.t;
3232
posAdjustment: posAdjustment;
3333
path: DcePath.t;
34+
(** For type re-exports (e.g. [type y = x = {...}]), record/variant label
35+
declarations belonging to the re-exporting type can carry the manifest
36+
type path so [DeadType.process_type_label_dependencies] can link fields
37+
without needing the typed tree. *)
38+
manifestTypePath: DcePath.t option;
3439
pos: Lexing.position;
3540
posEnd: Lexing.position;
3641
posStart: Lexing.position;

0 commit comments

Comments
 (0)