Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opam lint: Allow to mark a set of warnings as errors #5652

Merged
merged 5 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 16 additions & 6 deletions src/client/opamArg.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/client/opamArg.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
41 changes: 25 additions & 16 deletions src/client/opamCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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
Expand All @@ -3885,21 +3890,25 @@ let lint cli =
stdin_f stdin,
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)
OpamStd.IntMap.empty warnings_sel
in
fun w -> try OpamStd.IntMap.find w map with Not_found -> default
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) ->
match enabled n, state with
| `Enable, `Warning -> (warnings @ [warn], failed)
| `Enable, `Error | `EnableError, _ -> (warnings @ [warn], true)
| `Disable, _ -> (warnings, failed)
) ([], false) warnings
in
let warnings = List.filter (fun (n, _, _) -> enabled n) warnings in
let failed =
List.exists (function _,`Error,_ -> true | _ -> false) warnings
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
kit-ty-kate marked this conversation as resolved.
Show resolved Hide resolved
in
if short then
(if warnings <> [] then
Expand Down
87 changes: 87 additions & 0 deletions tests/reftests/lint.test
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,93 @@ N0REP0
### <one-more-file>
and it also has content
### tar czf an-archive.tgz one-more-file
### ::::::::::::::::::::::
### : Test --warn option :
### ::::::::::::::::::::::
### <lint.opam>
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 #
### opam lint ./lint.opam --warn=@0..99
${BASEDIR}/lint.opam: Errors.
error 23: Missing field 'maintainer'
error 25: Missing field 'authors'
error 35: Missing field 'homepage'
error 36: Missing field 'bug-reports'
error 57: Synopsis must not be empty
error 68: Missing field 'license'
# Return code 1 #
### opam lint ./lint.opam --warn=@0..99
${BASEDIR}/lint.opam: Errors.
error 23: Missing field 'maintainer'
error 25: Missing field 'authors'
error 35: Missing field 'homepage'
error 36: Missing field 'bug-reports'
error 57: Synopsis must not be empty
error 68: Missing field 'license'
# Return code 1 #
### opam lint ./lint.opam --warn=+25@25
${BASEDIR}/lint.opam: Errors.
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'
error 35: Missing field 'homepage'
error 36: Missing field 'bug-reports'
# 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
### <lint.opam>
Expand Down