Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ you can swap one file's data without affecting others.
**Problem**: Analysis functions directly call:
- `Log_.warning` - logging
- `EmitJson` - JSON output
- `WriteDeadAnnotations` - file I/O
- ~~`WriteDeadAnnotations` - file I/O~~ (removed - added complexity with little value)
- Direct mutation of result data structures

**Impact**: Can't get analysis results as data. Can't test without capturing I/O. Can't reuse analysis logic for different output formats.
Expand Down Expand Up @@ -440,19 +440,14 @@ This enables parallelization, caching, and incremental recomputation.

**Estimated effort**: Medium (many logging call sites, but mechanical)

### Task 9: Separate annotation computation from file writing (P5)
### Task 9: ~~Separate annotation computation from file writing (P5)~~ REMOVED

**Value**: Can compute what to write without actually writing. Testable.
**Status**: Removed ✅ - `WriteDeadAnnotations` feature was deleted entirely.

**Changes**:
- [ ] `WriteDeadAnnotations`: Split into pure `compute_annotations` and impure `write_to_files`
- [ ] Pure function takes deadness results, returns `(filepath * line_annotation list) list`
- [ ] Impure function takes that list and does file I/O
- [ ] Remove file I/O from analysis path

**Test**: Compute annotations, verify correct without touching filesystem.

**Estimated effort**: Small (single module)
The `-write` flag that auto-inserted `@dead` annotations into source files was removed
as it added significant complexity (global state, file I/O during analysis, extra types)
for a rarely-used feature. Users who want to suppress dead code warnings can manually
add `@dead` annotations.

### Task 10: Verify zero `DceConfig.current()` calls in analysis code

Expand Down
3 changes: 1 addition & 2 deletions analysis/reanalyze/src/CollectAnnotations.ml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ let processAttributes ~state ~config ~doGenType ~name ~pos attributes =
doGenType
&& getPayloadFun Annotation.tagIsOneOfTheGenTypeAnnotations <> None
then FileAnnotations.annotate_gentype state pos;
if getPayload WriteDeadAnnotations.deadAnnotation <> None then
FileAnnotations.annotate_dead state pos;
if getPayload "dead" <> None then FileAnnotations.annotate_dead state pos;
let nameIsInLiveNamesOrPaths () =
config.DceConfig.cli.live_names |> List.mem name
||
Expand Down
13 changes: 1 addition & 12 deletions analysis/reanalyze/src/Common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ module Cli = struct

let experimental = ref false
let json = ref false
let write = ref false

(* names to be considered live values *)
let liveNames = ref ([] : string list)
Expand Down Expand Up @@ -174,8 +173,6 @@ type decl = {
mutable report: bool;
}

type line = {mutable declarations: decl list; original: string}

module ExnSet = Set.Make (Exn)

type missingThrowInfo = {
Expand All @@ -202,21 +199,13 @@ type deadWarning =
| WarningDeadValueWithSideEffects
| IncorrectDeadAnnotation

type lineAnnotation = (decl * line) option

type description =
| Circular of {message: string}
| ExceptionAnalysis of {message: string}
| ExceptionAnalysisMissing of missingThrowInfo
| DeadModule of {message: string}
| DeadOptional of {deadOptional: deadOptional; message: string}
| DeadWarning of {
deadWarning: deadWarning;
path: string;
message: string;
shouldWriteLineAnnotation: bool;
lineAnnotation: lineAnnotation;
}
| DeadWarning of {deadWarning: deadWarning; path: string; message: string}
| Termination of {termination: termination; message: string}

type issue = {
Expand Down
2 changes: 0 additions & 2 deletions analysis/reanalyze/src/DceConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ type cli_config = {
debug: bool;
ci: bool;
json: bool;
write: bool;
live_names: string list;
live_paths: string list;
exclude_paths: string list;
Expand All @@ -25,7 +24,6 @@ let current () =
debug = !Common.Cli.debug;
ci = !Common.Cli.ci;
json = !Common.Cli.json;
write = !Common.Cli.write;
live_names = !Common.Cli.liveNames;
live_paths = !Common.Cli.livePaths;
exclude_paths = !Common.Cli.excludePaths;
Expand Down
28 changes: 4 additions & 24 deletions analysis/reanalyze/src/DeadCommon.ml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ end
let declGetLoc decl =
let loc_start =
let offset =
WriteDeadAnnotations.offsetOfPosAdjustment decl.posAdjustment
match decl.posAdjustment with
| FirstVariant | Nothing -> 0
| OtherVariant -> 2
in
let cnumWithOffset = decl.posStart.pos_cnum + offset in
if cnumWithOffset < decl.posEnd.pos_cnum then
Expand Down Expand Up @@ -159,33 +161,11 @@ let addValueDeclaration ~config ~decls ~file ?(isToplevel = true)

let emitWarning ~config ~decl ~message deadWarning =
let loc = decl |> declGetLoc in
let isToplevelValueWithSideEffects decl =
match decl.declKind with
| Value {isToplevel; sideEffects} -> isToplevel && sideEffects
| _ -> false
in
let shouldWriteLineAnnotation =
(not (isToplevelValueWithSideEffects decl))
&& Suppress.filter decl.pos
&& deadWarning <> IncorrectDeadAnnotation
in
let lineAnnotation =
if shouldWriteLineAnnotation then
WriteDeadAnnotations.addLineAnnotation ~config ~decl
else None
in
decl.path
|> Path.toModuleName ~isType:(decl.declKind |> DeclKind.isType)
|> DeadModules.checkModuleDead ~config ~fileName:decl.pos.pos_fname;
Log_.warning ~loc
(DeadWarning
{
deadWarning;
path = Path.withoutHead decl.path;
message;
lineAnnotation;
shouldWriteLineAnnotation;
})
(DeadWarning {deadWarning; path = Path.withoutHead decl.path; message})

module Decl = struct
let isValue decl =
Expand Down
8 changes: 6 additions & 2 deletions analysis/reanalyze/src/DeadType.ml
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,12 @@ let addDeclaration ~config ~decls ~file ~(typeId : Ident.t)
in
let posAdjustment =
(* In Res the variant loc can include the | and spaces after it *)
if WriteDeadAnnotations.posLanguage cd_loc.loc_start = Res then
if i = 0 then FirstVariant else OtherVariant
let isRes =
let fname = cd_loc.loc_start.pos_fname in
Filename.check_suffix fname ".res"
|| Filename.check_suffix fname ".resi"
in
if isRes then if i = 0 then FirstVariant else OtherVariant
else Nothing
in
Ident.name cd_id |> Name.create
Expand Down
10 changes: 3 additions & 7 deletions analysis/reanalyze/src/Log_.ml
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,8 @@ let missingRaiseInfoToText {missingAnnotations; locFull} =
~text:(Format.asprintf "@throws(%s)\\n" missingTxt)
else ""

let logAdditionalInfo ~config ~(description : description) =
let logAdditionalInfo ~(description : description) =
match description with
| DeadWarning {lineAnnotation; shouldWriteLineAnnotation} ->
if shouldWriteLineAnnotation then
WriteDeadAnnotations.lineAnnotationToString ~config lineAnnotation
else ""
| ExceptionAnalysisMissing missingRaiseInfo ->
missingRaiseInfoToText missingRaiseInfo
| _ -> ""
Expand Down Expand Up @@ -187,7 +183,7 @@ let logIssue ~config ~(issue : issue) =
~range:(startLine, startCharacter, endLine, endCharacter)
~message)
()
(logAdditionalInfo ~config ~description:issue.description)
(logAdditionalInfo ~description:issue.description)
(if config.DceConfig.cli.json then EmitJson.emitClose () else "")
else
let color =
Expand All @@ -197,7 +193,7 @@ let logIssue ~config ~(issue : issue) =
in
asprintf "@. %a@. %a@. %s%s@." color issue.name Loc.print issue.loc
(descriptionToMessage issue.description)
(logAdditionalInfo ~config ~description:issue.description)
(logAdditionalInfo ~description:issue.description)

module Stats = struct
let issues = ref []
Expand Down
6 changes: 1 addition & 5 deletions analysis/reanalyze/src/Reanalyze.ml
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ let runAnalysis ~dce_config ~cmtRoot =
let refs = References.freeze_builder refs_builder in
let file_deps = FileDeps.freeze_builder file_deps_builder in
DeadCommon.reportDead ~annotations ~decls ~refs ~file_deps
~config:dce_config ~checkOptionalArg:DeadOptionalArgs.check;
WriteDeadAnnotations.write ~config:dce_config);
~config:dce_config ~checkOptionalArg:DeadOptionalArgs.check);
if dce_config.DceConfig.run.exception_ then
Exception.Checks.doChecks ~config:dce_config;
if dce_config.DceConfig.run.termination && dce_config.DceConfig.cli.debug then
Expand Down Expand Up @@ -271,9 +270,6 @@ let cli () =
specified)" );
("-version", Unit versionAndExit, "Show version information and exit");
("--version", Unit versionAndExit, "Show version information and exit");
( "-write",
Set Common.Cli.write,
"Write @dead annotations directly in the source files" );
]
in
Arg.parse speclist print_endline usage;
Expand Down
155 changes: 0 additions & 155 deletions analysis/reanalyze/src/WriteDeadAnnotations.ml

This file was deleted.

Loading