From 1714e98e0e4c36238044030ee0c26bb43ac688a7 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Fri, 8 Sep 2023 17:26:09 +0200 Subject: [PATCH 1/5] reftest: add lint --warn tests --- tests/reftests/lint.test | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test index ffa0c9353cd..a8740adcbbc 100644 --- a/tests/reftests/lint.test +++ b/tests/reftests/lint.test @@ -6,6 +6,64 @@ N0REP0 ### and it also has content ### tar czf an-archive.tgz one-more-file +### :::::::::::::::::::::: +### : Test --warn option : +### :::::::::::::::::::::: +### +opam-version: "2.0" +### opam lint ./lint.opam +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### opam lint ./lint.opam --warn=+25 +${BASEDIR}/lint.opam: Warnings. + warning 25: Missing field 'authors' +### opam lint ./lint.opam --warn=-25 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### opam lint ./lint.opam --warn=+25-25 +${BASEDIR}/lint.opam: Passed. +### opam lint ./lint.opam --warn=+20..50 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' +# Return code 1 # +### opam lint ./lint.opam --warn=-20..40 +${BASEDIR}/lint.opam: Errors. + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### opam lint ./lint.opam --warn=+25+25+30..38 +${BASEDIR}/lint.opam: Warnings. + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' +### opam lint ./lint.opam --warn=+50..38+23+25 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' +# Return code 1 # +### opam lint ./lint.opam --warn=-25-25-30..38 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### :::::::::::::: +### : Test lints : +### :::::::::::::: ### : W20: Field 'opam-version' refers to the patch version of opam, it should be of the form MAJOR.MINOR ### : E21: Field 'opam-version' doesn't match the current version, validation may not be accurate ### From 3f34a6a33a99fe76774a624ddb890a52093a6891 Mon Sep 17 00:00:00 2001 From: Kate Date: Mon, 4 Sep 2023 19:39:29 +0100 Subject: [PATCH 2/5] opam lint: Allow to mark a set of warnings as errors --- master_changes.md | 1 + src/client/opamArg.ml | 22 ++++++++++++++++------ src/client/opamArg.mli | 4 ++-- src/client/opamCommands.ml | 27 ++++++++++++++++----------- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/master_changes.md b/master_changes.md index 65e46b140c3..94cc9b4d50b 100644 --- a/master_changes.md +++ b/master_changes.md @@ -51,6 +51,7 @@ users) ## Lint * Fix extra-files handling when linting packages from repositories, see #5068 [#5639 @rjbou] + * Allow to mark a set of warnings as errors using a new syntax -W @1..9 [#5652 @kit-ty-kate @rjbou - fixes #5651] ## Repository diff --git a/src/client/opamArg.ml b/src/client/opamArg.ml index baadf08e313..ca64f7960c3 100644 --- a/src/client/opamArg.ml +++ b/src/client/opamArg.ml @@ -890,13 +890,13 @@ let variable_bindings = let warn_selector = let parse str = - let sep = Re.(compile (set "+-")) in + let sep = Re.(compile (set "+-@")) in let sel = Re.(compile @@ seq [bos; group (rep1 digit); opt @@ seq [str ".."; group (rep1 digit)]; eos]) in let rec seq i j = - if i = j then [i] + if Int.equal i j then [i] else if i < j then i :: seq (i+1) j else j :: seq (j+1) i in @@ -908,8 +908,13 @@ let warn_selector = try seq i (int_of_string (Re.Group.get g 2)) with Not_found -> [i] in - let enabled = Re.Group.get d 0 = "+" in - let acc = List.fold_left (fun acc n -> (n, enabled) :: acc) acc nums in + let state = match Re.Group.get d 0 with + | "+" -> `Enable + | "-" -> `Disable + | "@" -> `EnableError + | _ -> assert false + in + let acc = List.fold_left (fun acc n -> (n, state) :: acc) acc nums in aux acc r | [] -> acc | _ -> raise Not_found @@ -920,8 +925,13 @@ let warn_selector = in let print ppf warns = pr_str ppf @@ - OpamStd.List.concat_map "" (fun (num,enable) -> - Printf.sprintf "%c%d" (if enable then '+' else '-') num) + OpamStd.List.concat_map "" (fun (num,state) -> + let state = match state with + | `Enable -> '+' + | `Disable -> '-' + | `EnableError -> '@' + in + Printf.sprintf "%c%d" state num) warns in parse, print diff --git a/src/client/opamArg.mli b/src/client/opamArg.mli index b5dbf69b997..b652b36d921 100644 --- a/src/client/opamArg.mli +++ b/src/client/opamArg.mli @@ -281,8 +281,8 @@ val dep_formula: formula Arg.conv (** [var=value,...] argument *) val variable_bindings: (OpamVariable.t * string) list Arg.conv -(** Warnings string ["+3..10-4"] *) -val warn_selector: (int * bool) list Arg.conv +(** Warnings string ["+3..10-4@12"] *) +val warn_selector: (int * [`Enable | `Disable | `EnableError]) list Arg.conv val opamlist_columns: OpamListCommand.output_format list Arg.conv diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index 0bb1133204a..e6df1c7ce2d 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -3780,8 +3780,9 @@ let lint cli = let warnings = mk_opt ~cli cli_original ["warnings";"W"] "WARNS" "Select the warnings to show or hide. $(i,WARNS) should be a \ - concatenation of $(b,+N), $(b,-N), $(b,+N..M), $(b,-N..M) to \ - respectively enable or disable warning or error number $(b,N) or \ + concatenation of $(b,+N), $(b,-N), $(b,@N), $(b,+N..M), \ + $(b,-N..M), $(b,@N..M) to respectively enable, disable or \ + enable-as-error warning or error number $(b,N) or \ all warnings with numbers between $(b,N) and $(b,M) inclusive.\n\ All warnings are enabled by default, unless $(i,WARNS) starts with \ $(b,+), which disables all but the selected ones." @@ -3866,6 +3867,10 @@ let lint cli = | None -> None | Some _ -> Some [] in + let default_warn = match warnings_sel with + | (_, (`Enable | `EnableError)) :: _ -> `Disable + | (_, `Disable) :: _ | [] -> `Enable + in let err,json = List.fold_left (fun (err,json) opam_f -> try @@ -3886,20 +3891,20 @@ let lint cli = None in let enabled = - let default = match warnings_sel with - | (_,true) :: _ -> false - | _ -> true - in let map = List.fold_left - (fun acc (wn, enable) -> OpamStd.IntMap.add wn enable acc) + (fun acc (wn, state) -> OpamStd.IntMap.add wn state acc) OpamStd.IntMap.empty warnings_sel in - fun w -> try OpamStd.IntMap.find w map with Not_found -> default + fun w -> try OpamStd.IntMap.find w map with Not_found -> default_warn in - let warnings = List.filter (fun (n, _, _) -> enabled n) warnings in - let failed = - List.exists (function _,`Error,_ -> true | _ -> false) warnings + let warnings, failed = + List.fold_left (fun (warnings, failed) ((n, state, _) as warn) -> + match enabled n, state with + | `Enable, `Warning -> (warnings @ [warn], failed) + | `Enable, `Error | `EnableError, _ -> (warnings @ [warn], true) + | `Disable, _ -> (warnings, failed) + ) ([], false) warnings in if short then (if warnings <> [] then From a0423ef685af47d3021ba45c015ae12672398dbf Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Fri, 8 Sep 2023 17:27:15 +0200 Subject: [PATCH 3/5] reftest: add lint --warn with enable as error tests --- tests/reftests/lint.test | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test index a8740adcbbc..6ee4630ceb9 100644 --- a/tests/reftests/lint.test +++ b/tests/reftests/lint.test @@ -61,6 +61,35 @@ ${BASEDIR}/lint.opam: Errors. error 57: Synopsis must not be empty warning 68: Missing field 'license' # Return code 1 # +### opam lint ./lint.opam --warn=@0..99 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### opam lint ./lint.opam --warn=@0..99 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' + error 57: Synopsis must not be empty + warning 68: Missing field 'license' +# Return code 1 # +### opam lint ./lint.opam --warn=+25@25 +${BASEDIR}/lint.opam: Errors. + warning 25: Missing field 'authors' +# Return code 1 # +### opam lint ./lint.opam --warn=+20..40@30..40 +${BASEDIR}/lint.opam: Errors. + error 23: Missing field 'maintainer' + warning 25: Missing field 'authors' + warning 35: Missing field 'homepage' + warning 36: Missing field 'bug-reports' +# Return code 1 # ### :::::::::::::: ### : Test lints : ### :::::::::::::: From 218448cf9426787ea28aa9ff21f28efb37b63150 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Fri, 8 Sep 2023 17:24:01 +0200 Subject: [PATCH 4/5] lint: change warnings to errors on enabled-as-errors warnings --- src/client/opamCommands.ml | 18 +++++++++++------- tests/reftests/lint.test | 22 +++++++++++----------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index e6df1c7ce2d..ba2d2102c6c 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -3890,13 +3890,10 @@ let lint cli = stdin_f stdin, None in - let enabled = - let map = - List.fold_left - (fun acc (wn, state) -> OpamStd.IntMap.add wn state acc) - OpamStd.IntMap.empty warnings_sel - in - fun w -> try OpamStd.IntMap.find w map with Not_found -> default_warn + let warnings_sel_map = OpamStd.IntMap.of_list warnings_sel in + let enabled w = + try OpamStd.IntMap.find w warnings_sel_map + with Not_found -> default_warn in let warnings, failed = List.fold_left (fun (warnings, failed) ((n, state, _) as warn) -> @@ -3906,6 +3903,13 @@ let lint cli = | `Disable, _ -> (warnings, failed) ) ([], false) warnings in + let warnings = + List.map (fun ((n, _, s) as warn) -> + match OpamStd.IntMap.find_opt n warnings_sel_map with + | Some `EnableError -> (n, `Error, s) + | Some (`Disable | `Enable) | None -> warn) + warnings + in if short then (if warnings <> [] then msg "%s\n" diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test index 6ee4630ceb9..68d8aa7df02 100644 --- a/tests/reftests/lint.test +++ b/tests/reftests/lint.test @@ -64,31 +64,31 @@ ${BASEDIR}/lint.opam: Errors. ### opam lint ./lint.opam --warn=@0..99 ${BASEDIR}/lint.opam: Errors. error 23: Missing field 'maintainer' - warning 25: Missing field 'authors' - warning 35: Missing field 'homepage' - warning 36: Missing field 'bug-reports' + error 25: Missing field 'authors' + error 35: Missing field 'homepage' + error 36: Missing field 'bug-reports' error 57: Synopsis must not be empty - warning 68: Missing field 'license' + error 68: Missing field 'license' # Return code 1 # ### opam lint ./lint.opam --warn=@0..99 ${BASEDIR}/lint.opam: Errors. error 23: Missing field 'maintainer' - warning 25: Missing field 'authors' - warning 35: Missing field 'homepage' - warning 36: Missing field 'bug-reports' + error 25: Missing field 'authors' + error 35: Missing field 'homepage' + error 36: Missing field 'bug-reports' error 57: Synopsis must not be empty - warning 68: Missing field 'license' + error 68: Missing field 'license' # Return code 1 # ### opam lint ./lint.opam --warn=+25@25 ${BASEDIR}/lint.opam: Errors. - warning 25: Missing field 'authors' + error 25: Missing field 'authors' # Return code 1 # ### opam lint ./lint.opam --warn=+20..40@30..40 ${BASEDIR}/lint.opam: Errors. error 23: Missing field 'maintainer' warning 25: Missing field 'authors' - warning 35: Missing field 'homepage' - warning 36: Missing field 'bug-reports' + error 35: Missing field 'homepage' + error 36: Missing field 'bug-reports' # Return code 1 # ### :::::::::::::: ### : Test lints : From 392be219d52c4613ecf4864747cdcbc6a2929142 Mon Sep 17 00:00:00 2001 From: Kate Date: Mon, 11 Sep 2023 19:44:06 +0100 Subject: [PATCH 5/5] Make opam lint a little faster Co-authored-by: R. Boujbel --- src/client/opamCommands.ml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index ba2d2102c6c..acd038a7478 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -3904,6 +3904,7 @@ let lint cli = ) ([], false) warnings in let warnings = + if warnings_sel = [] then warnings else List.map (fun ((n, _, s) as warn) -> match OpamStd.IntMap.find_opt n warnings_sel_map with | Some `EnableError -> (n, `Error, s)