Skip to content

Commit

Permalink
[flow] Kill support for traces (user-visible part)
Browse files Browse the repository at this point in the history
Summary:
The traces feature is not that useful nowadays given that we have use_op that makes the error much more understandable. Althrough our errors right now is not perfect, what's currently in the traces are completely non-comprehensible. Instead of dragging on, let's just delete this feature.

This diff starts from the user visible part. We will stop tracking it in errors, and remove the config options. `Context.max_trace_depth` is hardcoded to 0 for now (value when the flag is off), which will be cleaned up in a later diff.

Changelog: [breaking] Support for `--traces` and the corresponding flowconfig has been removed.

Reviewed By: panagosg7

Differential Revision: D56735978

fbshipit-source-id: 0baf317b39ed5893724c3a1118b2fdfdf2fb3afb
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Apr 30, 2024
1 parent bd4e3ed commit b356743
Show file tree
Hide file tree
Showing 38 changed files with 38 additions and 1,014 deletions.
5 changes: 0 additions & 5 deletions src/commands/commandUtils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,6 @@ module Options_flags = struct
slow_to_check_logging: Slow_to_check_logging.t;
strip_root: bool;
temp_dir: string option;
traces: int option;
verbose: Verbose.t option;
wait_for_recheck: bool option;
include_suppressions: bool;
Expand Down Expand Up @@ -943,7 +942,6 @@ let options_flags =
profile
all
wait_for_recheck
traces
no_flowlib
munge_underscore_members
max_workers
Expand Down Expand Up @@ -971,7 +969,6 @@ let options_flags =
profile;
all;
wait_for_recheck;
traces;
no_flowlib;
munge_underscore_members;
max_workers;
Expand Down Expand Up @@ -1002,7 +999,6 @@ let options_flags =
(optional bool)
~doc:
"If true, always wait for rechecks to finish before serving commands (default: false)"
|> flag "--traces" (optional int) ~doc:"Outline an error path up to a specified level"
|> flag "--no-flowlib" truthy ~doc:"Do not include embedded declarations"
|> flag
"--munge-underscore-members"
Expand Down Expand Up @@ -1350,7 +1346,6 @@ let make_options
opt_casting_syntax =
Base.Option.value (FlowConfig.casting_syntax flowconfig) ~default:Options.CastingSyntax.Both;
opt_wait_for_recheck;
opt_traces = Base.Option.value options_flags.traces ~default:(FlowConfig.traces flowconfig);
opt_quiet = options_flags.Options_flags.quiet;
opt_module_name_mappers = FlowConfig.module_name_mappers flowconfig;
opt_modules_are_use_strict = FlowConfig.modules_are_use_strict flowconfig;
Expand Down
5 changes: 0 additions & 5 deletions src/commands/config/flowConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ module Opts = struct
strict_es6_import_export: bool;
strict_es6_import_export_excludes: string list;
suppress_types: SSet.t;
traces: int;
ts_syntax: bool;
one_sided_type_guards: bool;
use_mixed_in_catch_variables: bool option;
Expand Down Expand Up @@ -262,7 +261,6 @@ module Opts = struct
strict_es6_import_export = false;
strict_es6_import_export_excludes = [];
suppress_types = SSet.empty |> SSet.add "$FlowFixMe";
traces = 0;
ts_syntax = false;
one_sided_type_guards = false;
use_mixed_in_catch_variables = None;
Expand Down Expand Up @@ -1003,7 +1001,6 @@ module Opts = struct
("sharedmemory.hash_table_pow", shm_hash_table_pow_parser);
("sharedmemory.heap_size", uint (fun opts shm_heap_size -> Ok { opts with shm_heap_size }));
("suppress_type", suppress_types_parser);
("traces", uint (fun opts v -> Ok { opts with traces = v }));
("types_first.max_files_checked_per_worker", max_files_checked_per_worker_parser);
("types_first.max_seconds_for_check_per_worker", max_seconds_for_check_per_worker_parser);
("use_mixed_in_catch_variables", use_mixed_in_catch_variables_parser);
Expand Down Expand Up @@ -1728,8 +1725,6 @@ let strict_mode c = c.strict_mode

let suppress_types c = c.options.Opts.suppress_types

let traces c = c.options.Opts.traces

let ts_syntax c = c.options.Opts.ts_syntax

let one_sided_type_guards c = c.options.Opts.one_sided_type_guards
Expand Down
2 changes: 0 additions & 2 deletions src/commands/config/flowConfig.mli
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ val strict_mode : config -> StrictModeSettings.t

val suppress_types : config -> SSet.t

val traces : config -> int

val ts_syntax : config -> bool

val one_sided_type_guards : config -> bool
Expand Down
19 changes: 4 additions & 15 deletions src/common/errors/flow_errors_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -781,42 +781,31 @@ let infos_to_messages infos = Base.List.(infos >>= info_to_messages)

let mk_error
?(kind = InferError)
?(trace_infos : 'loc info list option)
?(root : ('loc * 'loc Friendly.message) option)
?(frames : 'loc Friendly.message list option)
?(explanations : 'loc Friendly.message list option)
(loc : 'loc)
(error_code : Error_codes.error_code option)
(message : 'loc Friendly.message) : 'loc printable_error =
Friendly.(
let trace = Base.Option.value_map trace_infos ~default:[] ~f:infos_to_messages in
Friendly.
( kind,
trace,
[],
{
loc;
root = Base.Option.map root ~f:(fun (root_loc, root_message) -> { root_loc; root_message });
code = error_code;
message = Normal { message; frames; explanations };
}
)
)

let mk_speculation_error
?(kind = InferError)
?trace_infos
~loc
~root
~frames
~explanations
~error_code
speculation_errors =
?(kind = InferError) ~loc ~root ~frames ~explanations ~error_code speculation_errors =
Friendly.(
let trace = Base.Option.value_map trace_infos ~default:[] ~f:infos_to_messages in
let branches =
Base.List.map ~f:(fun (score, (_, _, error)) -> (score, error)) speculation_errors
in
( kind,
trace,
[],
{
loc;
root = Base.Option.map root ~f:(fun (root_loc, root_message) -> { root_loc; root_message });
Expand Down
2 changes: 0 additions & 2 deletions src/common/errors/flow_errors_utils.mli
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ type 'loc printable_error

val mk_error :
?kind:error_kind ->
?trace_infos:Loc.t info list ->
?root:Loc.t * Loc.t Friendly.message ->
?frames:Loc.t Friendly.message list ->
?explanations:Loc.t Friendly.message list ->
Expand All @@ -97,7 +96,6 @@ val mk_error :

val mk_speculation_error :
?kind:error_kind ->
?trace_infos:Loc.t info list ->
loc:Loc.t ->
root:(Loc.t * Loc.t Friendly.message) option ->
frames:Loc.t Friendly.message list ->
Expand Down
3 changes: 0 additions & 3 deletions src/common/options.ml
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ type t = {
opt_strip_root: bool;
opt_suppress_types: SSet.t;
opt_temp_dir: string;
opt_traces: int;
opt_ts_syntax: bool;
opt_one_sided_type_guards: bool;
opt_use_mixed_in_catch_variables: bool;
Expand Down Expand Up @@ -288,8 +287,6 @@ let max_literal_length opts = opts.opt_max_literal_length

let max_seconds_for_check_per_worker opts = opts.opt_max_seconds_for_check_per_worker

let max_trace_depth opts = opts.opt_traces

let max_workers opts = opts.opt_max_workers

let merge_timeout opts = opts.opt_merge_timeout
Expand Down
2 changes: 0 additions & 2 deletions src/common/test_utils/test_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ let make_options_flags
?(quiet = false)
?(strip_root = false)
?temp_dir
?traces
?verbose
?wait_for_recheck
?(include_suppressions = false)
Expand All @@ -51,7 +50,6 @@ let make_options_flags
quiet;
strip_root;
temp_dir;
traces;
verbose;
slow_to_check_logging = Slow_to_check_logging.default;
wait_for_recheck;
Expand Down
3 changes: 1 addition & 2 deletions src/flow_dot_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let loc_of_aloc = ALoc.to_loc_exn

let error_of_parse_error source_file (loc, err) =
Error_message.EParseError (ALoc.of_loc loc, err)
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file
|> Flow_error.error_of_msg ~source_file
|> Flow_intermediate_error.make_intermediate_error ~loc_of_aloc
|> Flow_intermediate_error.to_printable_error ~loc_of_aloc ~strip_root:None

Expand Down Expand Up @@ -140,7 +140,6 @@ let stub_metadata ~root ~checked =
file_options = Files.default_options;
ignore_non_literal_requires = false;
max_literal_length = 100;
max_trace_depth = 0;
max_workers = 0;
missing_module_generators = [];
namespaces = true;
Expand Down
6 changes: 3 additions & 3 deletions src/server/error_collator/errorCollator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ let add_suppression_warnings root checked unused warnings =
if not (CheckedSet.mem_dependency source_file checked) then
let err =
let msg = Error_message.EUnusedSuppression loc in
Flow_error.error_of_msg ~trace_reasons:[] ~source_file msg
Flow_error.error_of_msg ~source_file msg
|> Flow_intermediate_error.make_intermediate_error ~loc_of_aloc:Fun.id
|> Flow_intermediate_error.to_printable_error ~loc_of_aloc:Fun.id ~strip_root:(Some root)
in
Expand All @@ -55,7 +55,7 @@ let add_suppression_warnings root checked unused warnings =
in
let err =
Error_message.ECodelessSuppression (loc, code)
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file
|> Flow_error.error_of_msg ~source_file
|> Flow_intermediate_error.make_intermediate_error ~loc_of_aloc:Fun.id
|> Flow_intermediate_error.to_printable_error ~loc_of_aloc:Fun.id ~strip_root:(Some root)
in
Expand All @@ -74,7 +74,7 @@ let collate_duplicate_providers ~update root =
let conflict = Loc.{ source = Some duplicate; start = pos; _end = pos } in
let err =
Error_message.EDuplicateModuleProvider { module_name; provider; conflict }
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file:duplicate
|> Flow_error.error_of_msg ~source_file:duplicate
|> Flow_intermediate_error.make_intermediate_error ~loc_of_aloc:Fun.id
|> Flow_intermediate_error.to_printable_error ~loc_of_aloc:Fun.id ~strip_root:(Some root)
in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ let stub_metadata ~root ~checked =
file_options;
ignore_non_literal_requires = false;
max_literal_length = 100;
max_trace_depth = 0;
max_workers = 0;
missing_module_generators = [];
namespaces = false;
Expand Down
9 changes: 4 additions & 5 deletions src/services/inference/inference_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,23 @@ let error_of_docblock_error ~source_file (loc, err) =
Flow_intermediate_error_types.DisallowedSupportsPlatform
)
in
Flow_error.error_of_msg ~trace_reasons:[] ~source_file flow_err
Flow_error.error_of_msg ~source_file flow_err

let set_of_docblock_errors ~source_file =
Base.List.fold_left
~f:(fun acc err -> Flow_error.ErrorSet.add (error_of_docblock_error ~source_file err) acc)
~init:Flow_error.ErrorSet.empty

let error_of_parse_error ~source_file (loc, err) =
Error_message.EParseError (ALoc.of_loc loc, err)
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file
Error_message.EParseError (ALoc.of_loc loc, err) |> Flow_error.error_of_msg ~source_file

let set_of_parse_error ~source_file =
error_of_parse_error ~source_file %> Flow_error.ErrorSet.singleton

let error_of_parse_exception ~source_file exn =
let file_loc = Loc.{ none with source = Some source_file } |> ALoc.of_loc in
Error_message.EInternal (file_loc, Error_message.ParseJobException exn)
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file
|> Flow_error.error_of_msg ~source_file

let set_of_parse_exception ~source_file =
error_of_parse_exception ~source_file %> Flow_error.ErrorSet.singleton
Expand All @@ -57,7 +56,7 @@ let error_of_file_sig_tolerable_error ~source_file err =
let (SignatureVerificationError sve) = err in
Error_message.ESignatureVerification (Signature_error.map ALoc.of_loc sve)
in
Flow_error.error_of_msg ~trace_reasons:[] ~source_file flow_err
Flow_error.error_of_msg ~source_file flow_err

let set_of_file_sig_tolerable_errors ~source_file =
Base.List.map ~f:(error_of_file_sig_tolerable_error ~source_file) %> Flow_error.ErrorSet.of_list
2 changes: 1 addition & 1 deletion src/services/inference/types_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ let resolve_requires ~transaction ~reader ~options ~profiling ~workers ~parsed ~

let error_set_of_internal_error file (loc, internal_error) =
Error_message.EInternal (loc, internal_error)
|> Flow_error.error_of_msg ~trace_reasons:[] ~source_file:file
|> Flow_error.error_of_msg ~source_file:file
|> Flow_error.ErrorSet.singleton

let calc_deps ~options ~profiling ~components to_merge =
Expand Down
1 change: 0 additions & 1 deletion src/typing/__tests__/type_hint_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ let metadata =
file_options = Files.default_options;
ignore_non_literal_requires = false;
max_literal_length = 100;
max_trace_depth = 0;
max_workers = 0;
missing_module_generators = [];
namespaces = false;
Expand Down
1 change: 0 additions & 1 deletion src/typing/__tests__/typed_ast_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ let metadata =
file_options = Files.default_options;
ignore_non_literal_requires = false;
max_literal_length = 100;
max_trace_depth = 0;
max_workers = 0;
missing_module_generators = [];
namespaces = false;
Expand Down
6 changes: 2 additions & 4 deletions src/typing/context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ type metadata = {
file_options: Files.options;
ignore_non_literal_requires: bool;
max_literal_length: int;
max_trace_depth: int;
max_workers: int;
missing_module_generators: (Str.regexp * string) list;
namespaces: bool;
Expand Down Expand Up @@ -273,7 +272,6 @@ let metadata_of_options options =
file_options = Options.file_options options;
ignore_non_literal_requires = Options.should_ignore_non_literal_requires options;
max_literal_length = Options.max_literal_length options;
max_trace_depth = Options.max_trace_depth options;
max_workers = Options.max_workers options;
missing_module_generators = Options.missing_module_generators options;
namespaces = Options.namespaces options;
Expand Down Expand Up @@ -552,7 +550,7 @@ let include_suppressions cx = cx.metadata.include_suppressions

let severity_cover cx = cx.ccx.severity_cover

let max_trace_depth cx = cx.metadata.max_trace_depth
let max_trace_depth _ = 0

let property_maps cx = cx.ccx.sig_cx.property_maps

Expand Down Expand Up @@ -1013,7 +1011,7 @@ let find_root_id cx id = Type.Constraint.find_root_id cx.ccx.sig_cx.graph id

let on_cyclic_tvar_error cx reason =
let msg = Error_message.(ETrivialRecursiveDefinition (Reason.loc_of_reason reason, reason)) in
let error = Flow_error.error_of_msg ~trace_reasons:[] ~source_file:cx.file msg in
let error = Flow_error.error_of_msg ~source_file:cx.file msg in
if is_verbose cx then
Utils_js.prerr_endlinef
"\nCyclic type: %s"
Expand Down
1 change: 0 additions & 1 deletion src/typing/context.mli
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ type metadata = {
file_options: Files.options;
ignore_non_literal_requires: bool;
max_literal_length: int;
max_trace_depth: int;
max_workers: int;
missing_module_generators: (Str.regexp * string) list;
namespaces: bool;
Expand Down
16 changes: 4 additions & 12 deletions src/typing/errors/flow_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type 'loc t = {
loc: 'loc option;
msg: 'loc Error_message.t';
source_file: File_key.t;
trace_reasons: 'loc Reason.virtual_reason list;
}

let loc_of_error { loc; _ } = loc
Expand All @@ -24,15 +23,8 @@ let code_of_error err = msg_of_error err |> Error_message.error_code_of_message

let source_file { source_file; _ } = source_file

let trace_reasons { trace_reasons; _ } = trace_reasons

let map_loc_of_error f { loc; msg; source_file; trace_reasons } =
{
loc = Base.Option.map ~f loc;
msg = map_loc_of_error_message f msg;
source_file;
trace_reasons = Base.List.map ~f:(Reason.map_reason_locs f) trace_reasons;
}
let map_loc_of_error f { loc; msg; source_file } =
{ loc = Base.Option.map ~f loc; msg = map_loc_of_error_message f msg; source_file }

let kind_of_error err = msg_of_error err |> kind_of_msg

Expand All @@ -53,5 +45,5 @@ let ordered_reasons ((rl, ru) as reasons) =
else
reasons

let error_of_msg ~trace_reasons ~source_file (msg : 'loc Error_message.t') : 'loc t =
{ loc = loc_of_msg msg; msg; source_file; trace_reasons }
let error_of_msg ~source_file (msg : 'loc Error_message.t') : 'loc t =
{ loc = loc_of_msg msg; msg; source_file }
8 changes: 1 addition & 7 deletions src/typing/errors/flow_error.mli
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,9 @@ val code_of_error : 'loc t -> Error_codes.error_code option

val source_file : 'loc t -> File_key.t

val trace_reasons : 'loc t -> 'loc Reason.virtual_reason list

val kind_of_error : 'loc t -> Flow_errors_utils.error_kind

val error_of_msg :
trace_reasons:'loc Reason.virtual_reason list ->
source_file:File_key.t ->
'loc Error_message.t' ->
'loc t
val error_of_msg : source_file:File_key.t -> 'loc Error_message.t' -> 'loc t

val ordered_reasons : Reason.t * Reason.t -> Reason.t * Reason.t

Expand Down

0 comments on commit b356743

Please sign in to comment.