From 60775df1a00539de4008bc6b6b6ee8020a3eb80e Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Thu, 3 Aug 2023 11:03:48 +0200 Subject: [PATCH] feature: check package names are valid opam names This adds a `Package_name.Opam_compatible` variant that uses opam conventions. The corresponding parser is used if lang dune >= 3.11. Signed-off-by: Etienne Millon --- doc/changes/check-package-names.md | 2 + src/dune_lang/package_name.ml | 52 +++++++++++++-- src/dune_lang/package_name.mli | 12 ++++ src/dune_rules/package.ml | 13 +++- .../test-cases/package-name-strict.t | 63 +++++++++++++++++++ 5 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 doc/changes/check-package-names.md create mode 100644 test/blackbox-tests/test-cases/package-name-strict.t diff --git a/doc/changes/check-package-names.md b/doc/changes/check-package-names.md new file mode 100644 index 000000000000..c9c8f2f94696 --- /dev/null +++ b/doc/changes/check-package-names.md @@ -0,0 +1,2 @@ +- Ensure that package names in `dune-project` are valid opam package + names. (#8331, @emillon) diff --git a/src/dune_lang/package_name.ml b/src/dune_lang/package_name.ml index a8c4e2cc692f..2f5e6ae95607 100644 --- a/src/dune_lang/package_name.ml +++ b/src/dune_lang/package_name.ml @@ -10,10 +10,52 @@ include ( let description = "package name" let description_of_valid_string = None let hint_valid = None - - let of_string_opt s = - (* DUNE3 verify no dots or spaces *) - if s = "" then None else Some s - ;; + let of_string_opt s = if s = "" then None else Some s end) : Dune_util.Stringlike with type t := t) + +module Opam_compatible = struct + include Dune_util.Stringlike.Make (struct + type t = string + + let module_ = "Package.Name.Strict" + let description = "opam package name" + let to_string s = s + + let description_of_valid_string = + Some + (Pp.textf + "Package names can contain letters, numbers, '-', '_' and '+', and need to \ + contain at least a letter.") + ;; + + let is_letter = function + | 'a' .. 'z' | 'A' .. 'Z' -> true + | _ -> false + ;; + + let is_other_valid_char = function + | '0' .. '9' | '-' | '+' | '_' -> true + | _ -> false + ;; + + let is_valid_char c = is_letter c || is_other_valid_char c + + let is_valid_string s = + let all_chars_valid = String.for_all s ~f:is_valid_char in + let has_one_letter = String.exists s ~f:is_letter in + all_chars_valid && has_one_letter + ;; + + let of_string_opt s = Option.some_if (is_valid_string s) s + + let make_valid s = + let replaced = String.map s ~f:(fun c -> if is_valid_char c then c else '_') in + if is_valid_string replaced then replaced else "p" ^ replaced + ;; + + let hint_valid = Some make_valid + end) + + let to_package_name s = s +end diff --git a/src/dune_lang/package_name.mli b/src/dune_lang/package_name.mli index d2bdd1b8abab..6849029cd58e 100644 --- a/src/dune_lang/package_name.mli +++ b/src/dune_lang/package_name.mli @@ -10,3 +10,15 @@ val hash : t -> int include Comparable_intf.S with type key := t include Dune_sexp.Conv.S with type t := t include Stringlike with type t := t + +module Opam_compatible : sig + (** A variant that enforces opam package name constraints: all characters are + [[a-zA-Z0-9_+-]] with at least a letter. *) + + include Stringlike + + type package_name + + val to_package_name : t -> package_name + end + with type package_name := t diff --git a/src/dune_rules/package.ml b/src/dune_rules/package.ml index 3c496b0538f3..226b4bc22cbd 100644 --- a/src/dune_rules/package.ml +++ b/src/dune_rules/package.ml @@ -26,6 +26,10 @@ module Name = struct module Infix = Comparator.Operators (String) module Map_traversals = Memo.Make_map_traversals (Map) + + let decode_opam_compatible = + Dune_lang.Decoder.map ~f:Opam_compatible.to_package_name Opam_compatible.decode + ;; end module Id = struct @@ -571,6 +575,10 @@ let encode list sexp (string "package" :: fields) ;; +let decode_name ~version = + if version >= (3, 11) then Name.decode_opam_compatible else Name.decode +;; + let decode ~dir = let open Dune_lang.Decoder in let name_map syntax of_list_map to_string name decode print_value error_msg = @@ -586,8 +594,9 @@ let decode ~dir = ] in fields - @@ let+ loc = loc - and+ name = field "name" Name.decode + @@ let* version = Syntax.get_exn Stanza.syntax in + let+ loc = loc + and+ name = field "name" (decode_name ~version) and+ synopsis = field_o "synopsis" string and+ description = field_o "description" string and+ version = diff --git a/test/blackbox-tests/test-cases/package-name-strict.t b/test/blackbox-tests/test-cases/package-name-strict.t new file mode 100644 index 000000000000..4c86126d22a6 --- /dev/null +++ b/test/blackbox-tests/test-cases/package-name-strict.t @@ -0,0 +1,63 @@ +Version check: + + $ cat > dune-project << EOF + > (lang dune 3.10) + > (package + > (name some&name) + > (allow_empty)) + > EOF + $ dune build + +Validation: + + $ test() { + > cat > dune-project << EOF + > (lang dune 3.11) + > (package + > (name $1) + > (allow_empty)) + > EOF + > dune build + > } + + $ test 'some&name' + File "dune-project", line 3, characters 7-16: + 3 | (name some&name) + ^^^^^^^^^ + Error: "some&name" is an invalid opam package name. + Package names can contain letters, numbers, '-', '_' and '+', and need to + contain at least a letter. + Hint: some_name would be a correct opam package name + [1] + +Leading invalid characters are removed: + + $ test '0test' + +When all characters are removed, a valid name is suggested: + + $ test '0' + File "dune-project", line 3, characters 7-8: + 3 | (name 0) + ^ + Error: "0" is an invalid opam package name. + Package names can contain letters, numbers, '-', '_' and '+', and need to + contain at least a letter. + Hint: p0 would be a correct opam package name + [1] + +A package name can start with a number: + + $ test 0install + +But it needs at least a letter: + + $ test 0-9 + File "dune-project", line 3, characters 7-10: + 3 | (name 0-9) + ^^^ + Error: "0-9" is an invalid opam package name. + Package names can contain letters, numbers, '-', '_' and '+', and need to + contain at least a letter. + Hint: p0-9 would be a correct opam package name + [1]