From f38462407ea735f9fff46bdfa941c5d154264ab4 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 22 Jun 2017 05:07:01 -0700 Subject: [PATCH] [utils] make `read_file` return a `Result.t` Summary: Small API improvement for better error messages. Reviewed By: mbouaziz Differential Revision: D5293334 fbshipit-source-id: ef7f7e2 --- infer/src/backend/InferPrint.re | 4 ++-- infer/src/backend/StatsAggregator.re | 4 ++-- infer/src/backend/infer.ml | 11 ++++++---- infer/src/backend/mergeCapture.ml | 13 +++++++----- infer/src/base/Config.ml | 5 +++-- infer/src/base/Multilinks.re | 6 ++++-- infer/src/base/SourceFile.ml | 31 ++++++++++++++++------------ infer/src/base/Utils.ml | 6 +++--- infer/src/base/Utils.mli | 4 ++-- 9 files changed, 49 insertions(+), 35 deletions(-) diff --git a/infer/src/backend/InferPrint.re b/infer/src/backend/InferPrint.re index 56039b31f..3e37387e3 100644 --- a/infer/src/backend/InferPrint.re +++ b/infer/src/backend/InferPrint.re @@ -1026,7 +1026,7 @@ let pp_summary_by_report_kind let pp_json_report_by_report_kind formats_by_report_kind fname => switch (Utils.read_file fname) { - | Some report_lines => + | Ok report_lines => let pp_json_issues format_list report => { let pp_json_issue (format_kind, outf: Utils.outfile) => switch format_kind { @@ -1048,7 +1048,7 @@ let pp_json_report_by_report_kind formats_by_report_kind fname => | _ => () }; List.iter f::pp_report_by_report_kind formats_by_report_kind - | None => failwithf "Error reading %s. Does the file exist?" fname + | Error error => failwithf "Error reading '%s': %s" fname error }; let pp_lint_issues_by_report_kind formats_by_report_kind error_filter linereader procname error_log => { diff --git a/infer/src/backend/StatsAggregator.re b/infer/src/backend/StatsAggregator.re index ba0d8e3c3..bf4783e83 100644 --- a/infer/src/backend/StatsAggregator.re +++ b/infer/src/backend/StatsAggregator.re @@ -71,8 +71,8 @@ let load_data_from_infer_deps file => { let lines = Utils.read_file file; try ( switch lines { - | Some l => Ok (List.map f::extract_target_and_path l) - | None => raise (Failure ("Error reading '" ^ file ^ "'")) + | Ok l => Ok (List.map f::extract_target_and_path l) + | Error error => raise (Failure (Printf.sprintf "Error reading '%s': %s" file error)) } ) { | Failure msg => Error msg diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index d2f7c44de..172aba1d8 100644 --- a/infer/src/backend/infer.ml +++ b/infer/src/backend/infer.ml @@ -403,12 +403,15 @@ let analyze driver_mode = (** as the Config.fail_on_bug flag mandates, exit with error when an issue is reported *) let fail_on_issue_epilogue () = - let issues_json = DB.Results_dir.(path_to_filename Abs_root ["report.json"]) in - match Utils.read_file (DB.filename_to_string issues_json) with - | Some lines -> + let issues_json = DB.Results_dir.(path_to_filename Abs_root ["report.json"]) + |> DB.filename_to_string in + match Utils.read_file issues_json with + | Ok lines -> let issues = Jsonbug_j.report_of_string @@ String.concat ~sep:"" lines in if issues <> [] then exit Config.fail_on_issue_exit_code - | None -> () + | Error error -> + L.internal_error "Failed to read report file '%s': %s@." issues_json error ; + () let log_infer_args driver_mode = L.environment_info "INFER_ARGS = %s@\n" diff --git a/infer/src/backend/mergeCapture.ml b/infer/src/backend/mergeCapture.ml index 5a2c3a282..e8b7d572e 100644 --- a/infer/src/backend/mergeCapture.ml +++ b/infer/src/backend/mergeCapture.ml @@ -29,9 +29,10 @@ let modified_targets = ref String.Set.empty let modified_file file = match Utils.read_file file with - | Some targets -> + | Ok targets -> modified_targets := List.fold ~f:String.Set.add ~init:String.Set.empty targets - | None -> + | Error error -> + L.user_error "Failed to read modified targets file '%s': %s@." file error ; () type stats = @@ -192,9 +193,11 @@ let process_merge_file deps_file = then slink ~stats ~skiplevels infer_out_src infer_out_dst | _ -> () in - Option.iter - ~f:(fun lines -> List.iter ~f:process_line lines) - (Utils.read_file deps_file); + ( + match Utils.read_file deps_file with + | Ok lines -> List.iter ~f:process_line lines + | Error error -> L.internal_error "Couldn't read deps file '%s': %s" deps_file error + ); create_multilinks (); L.progress "Captured results merged.@."; L.progress "Targets merged: %d@." stats.targets_merged; diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 5025eb09f..3f1a78bc6 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -1509,10 +1509,11 @@ and specs_library = if Filename.is_relative path then failwith ("Failing because path " ^ path ^ " is not absolute") in match Utils.read_file (resolve fname) with - | Some pathlist -> + | Ok pathlist -> List.iter ~f:validate_path pathlist; pathlist - | None -> failwith ("cannot read file " ^ fname ^ " from cwd " ^ (Sys.getcwd ())) + | Error error -> + failwithf "cannot read file '%s' from cwd '%s': %s" fname (Sys.getcwd ()) error in (* Add the newline-separated directories listed in to the list of directories to be searched for .spec files *) diff --git a/infer/src/base/Multilinks.re b/infer/src/base/Multilinks.re index 454bac64d..2c65b5205 100644 --- a/infer/src/base/Multilinks.re +++ b/infer/src/base/Multilinks.re @@ -30,8 +30,10 @@ let reset_cache () => String.Table.clear multilink_files_cache; let read ::dir :option t => { let multilink_fname = Filename.concat dir multilink_file_name; switch (Utils.read_file multilink_fname) { - | None => None - | Some lines => + | Error error => + L.internal_error "Couldn't read multilink file '%s': %s@." multilink_fname error; + None + | Ok lines => let links = create (); List.iter f::(fun line => String.Table.set links key::(Filename.basename line) data::line) lines; diff --git a/infer/src/base/SourceFile.ml b/infer/src/base/SourceFile.ml index 951da068b..500097b90 100644 --- a/infer/src/base/SourceFile.ml +++ b/infer/src/base/SourceFile.ml @@ -10,6 +10,8 @@ open! IStd open! PVariant +module L = Logging + let count_newlines (path: string): int = let f file = In_channel.fold_lines file ~init:0 ~f:(fun i _ -> i + 1) in In_channel.with_file path ~f @@ -134,19 +136,22 @@ let create ?(warn_on_error=true) path = from_abs_path ~warn_on_error path let changed_files_set = - Option.bind Config.changed_files_index Utils.read_file |> - Option.map ~f:( - List.fold - ~f:(fun changed_files line -> - let source_file = create line in - let changed_files' = Set.add source_file changed_files in - (* Add source corresponding to changed header if it exists *) - match of_header source_file with - | Some src -> Set.add src changed_files' - | None -> changed_files' - ) - ~init:Set.empty - ) + match Config.changed_files_index with + | None -> + None + | Some index -> match Utils.read_file index with + | Ok lines -> + Some (List.fold lines ~init:Set.empty ~f:(fun changed_files line -> + let source_file = create line in + let changed_files' = Set.add source_file changed_files in + (* Add source corresponding to changed header if it exists *) + match of_header source_file with + | Some src -> Set.add src changed_files' + | None -> changed_files' + )) + | Error error -> + L.user_error "Error reading the changed files index '%s': %s@." index error ; + None module UNSAFE = struct let from_string str = diff --git a/infer/src/base/Utils.ml b/infer/src/base/Utils.ml index c47876967..880c39853 100644 --- a/infer/src/base/Utils.ml +++ b/infer/src/base/Utils.ml @@ -38,10 +38,10 @@ let read_file fname = with | End_of_file -> cleanup (); - Some (List.rev !res) - | Sys_error _ -> + Ok (List.rev !res) + | Sys_error error -> cleanup (); - None + Error error (** copy a source file, return the number of lines, or None in case of error *) let copy_file fname_from fname_to = diff --git a/infer/src/base/Utils.mli b/infer/src/base/Utils.mli index c1bdc2fc9..0643c79fc 100644 --- a/infer/src/base/Utils.mli +++ b/infer/src/base/Utils.mli @@ -21,8 +21,8 @@ val string_crc_hex32 : string -> string (** copy a source file, return the number of lines, or None in case of error *) val copy_file : string -> string -> int option -(** read a source file and return a list of lines, or None in case of error *) -val read_file : string -> string list option +(** read a source file and return a list of lines *) +val read_file : string -> (string list, string) Result.t (** Convert a filename to an absolute one if it is relative, and normalize "." and ".." *) val filename_to_absolute : root:string -> string -> string