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

feature: check package names are valid opam names #8331

Merged
merged 1 commit into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions doc/changes/check-package-names.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Ensure that package names in `dune-project` are valid opam package
names. (#8331, @emillon)
52 changes: 47 additions & 5 deletions src/dune_lang/package_name.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 12 additions & 0 deletions src/dune_lang/package_name.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 11 additions & 2 deletions src/dune_rules/package.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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 =
Expand Down
63 changes: 63 additions & 0 deletions test/blackbox-tests/test-cases/package-name-strict.t
Original file line number Diff line number Diff line change
@@ -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]