Use stricter regexes for package and file name checks (#742)

Closes #741
r-lib · May 9, 2019 · d234bdd · d234bdd
1 parent 0cc83b5
commit d234bdd
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 4 deletions.
diff --git a/R/description.R b/R/description.R
@@ -98,7 +98,7 @@ build_description_list <- function(fields = list()) {
 }
 
 check_package_name <- function(name) {
-  if (!valid_name(name)) {
+  if (!valid_package_name(name)) {
     ui_stop(c(
       "{ui_value(name)} is not a valid package name. It should:",
       "* Contain only ASCII letters, numbers, and '.'",
@@ -110,8 +110,8 @@ check_package_name <- function(name) {
 
 }
 
-valid_name <- function(x) {
-  grepl("^[[:alpha:]][[:alnum:].]+$", x) && !grepl("\\.$", x)
+valid_package_name <- function(x) {
+  grepl("^[a-zA-Z][a-zA-Z0-9.]+$", x) && !grepl("\\.$", x)
 }
 
 tidy_desc <- function(desc) {

diff --git a/R/r.R b/R/r.R
@@ -30,7 +30,7 @@ check_file_name <- function(name) {
 }
 
 valid_file_name <- function(x) {
-  grepl("^[[:alnum:]_-]+$", x)
+  grepl("^[a-zA-Z0-9._-]+$", x)
 }
 
 get_active_r_file <- function(path = "R") {

diff --git a/tests/testthat/test-helpers.R b/tests/testthat/test-helpers.R
@@ -81,3 +81,31 @@ test_that("use_description_field() ignores whitespace", {
   use_description_field(name = "foo", value = "bar")
   expect_identical(c(foo = "\n bar"), desc::desc_get("foo", pkg))
 })
+
+test_that("valid_package_name() enforces valid package names", {
+  # Contain only ASCII letters, numbers, and '.'
+  # Have at least two characters
+  # Start with a letter
+  # Not end with '.'
+
+  expect_true(valid_package_name("aa"))
+  expect_true(valid_package_name("a7"))
+  expect_true(valid_package_name("a.2"))
+
+  expect_false(valid_package_name("a"))
+  expect_false(valid_package_name("a-2"))
+  expect_false(valid_package_name("2fa"))
+  expect_false(valid_package_name(".fa"))
+  expect_false(valid_package_name("aa\u00C0")) # \u00C0 is a-grave
+  expect_false(valid_package_name("a3."))
+})
+
+test_that("valid_file_name() enforces valid file names", {
+  # Contain only ASCII letters, numbers, '-', and '_'
+  expect_true(valid_file_name("aa.R"))
+  expect_true(valid_file_name("a7.R"))
+  expect_true(valid_file_name("a-2.R"))
+  expect_true(valid_file_name("a_2.R"))
+  expect_false(valid_file_name("aa\u00C0.R")) # \u00C0 is a-grave
+  expect_false(valid_file_name("a?3.R"))
+})