Tweak path linting (#832)

* Remove "bad relative path linker". This used a very simple heuristic that had many possible false positives. Fixes #244.

* Only check case of potential paths that contain a `/`. This reduces the number of false positives. Fixes #611.

NEWS.md

@@ -1,5 +1,9 @@
 # rsconnect (development version)
 
+* The built-in linter should have fewer false positives for path problems:
+  the relative path linter has been removed (#244) and the case-sensitive 
+  linter now only checks strings containing a `/` (#611).
+
 * `deployApp()`'s `quarto` argument now takes values `TRUE`, `FALSE` or 
   `NA`. The previous value (a path to a quarto binary) is now deprecated,
   and instead we automatically figure out the packge from `QUARTO_PATH` and

R/linters.R

@@ -82,29 +82,9 @@ addLinter("absolute.paths", linter(
   suggestion = "Paths should be to files within the project directory."
 ))
 
-addLinter("invalid.relative.paths", linter(
-
-  apply = function(content, ...) {
-    content <- stripComments(content)
-    badRelativePaths(content, ...)
-  },
-
-  takes = isRCodeFile,
-
-  message = function(content, lines) {
-    makeLinterMessage("The following lines contain invalid relative paths (resolved outside of project directory)",
-                      content,
-                      lines)
-  },
-
-  suggestion = "Paths should be to files within the project directory."
-
-))
-
 addLinter("filepath.capitalization", linter(
 
   apply = function(content, project, path, files) {
-
     content <- stripComments(content)
 
     # Extract references between bounding (unescaped) quotes.
@@ -169,6 +149,8 @@ addLinter("filepath.capitalization", linter(
       lapply(regex, function(x) {
 
         which(
+          # Only examine paths containing a slash to reduce false positives
+          grepl("/", x, fixed = TRUE) &
           (tolower(x) %in% projectFilesLower) &
             (!(x %in% projectFiles))
         )
@@ -331,31 +313,6 @@ noMatch <- function(x) {
   identical(attr(x, "match.length"), -1L)
 }
 
-badRelativePaths <- function(content, project, path, ...) {
-
-  ## Figure out how deeply the path of the file is nested
-  ## (it is relative to the project root)
-  slashMatches <- gregexpr("/", path)
-  nestLevel <- if (noMatch(slashMatches)) 0 else length(slashMatches[[1]])
-
-  ## Identify occurrences of "../"
-  regexResults <- gregexpr("../", content, fixed = TRUE)
-
-  ## Figure out sequential runs of `../`
-  runs <- lapply(regexResults, function(x) {
-    if (noMatch(x)) return(NULL)
-    rle <- rle(as.integer(x) - seq(0, by = 3, length.out = length(x)))
-    rle$lengths
-  })
-
-  badPaths <- vapply(runs, function(x) {
-    any(x > nestLevel)
-  }, logical(1))
-
-  badPaths
-}
-
-
 transposeList <- function(list) {
   unname(as.list(
     as.data.frame(

tests/testthat/_snaps/lint.md

@@ -7,7 +7,7 @@
       index.Rmd
       ---------
       The following lines contain paths to files not matching in case sensitivity:
-      29: ![](rstudio.svg)    ['rstudio.svg' -> 'RStudio.svg']
+      29: ![](img/rstudio.svg)    ['img/rstudio.svg' -> 'img/RStudio.svg']
       
       Filepaths are case-sensitive on deployment server.
     Code

tests/testthat/_snaps/linters.md

@@ -9,20 +9,14 @@
       The following lines contain absolute paths:
       15: Here's some internal help: [Helpful Link](/Users/MrBurns/)
       
-      The following lines contain invalid relative paths (resolved outside of project directory):
-      16: And another: [Favourite Goats](../../goats.txt)
-      
       --------
       server.R
       --------
       The following lines contain absolute paths:
       15:     otherFile <- read.table("~/.rsconnect-tests/local-file.txt")
       
       The following lines contain paths to files not matching in case sensitivity:
-      31:     file <- read.csv("college.txt") ## bad    ['college.txt' -> 'College.txt']
-      
-      The following lines contain invalid relative paths (resolved outside of project directory):
-      16:     anotherFile <- readLines("../../foo.bar")
+      31:     file <- read.csv("data/college.txt") ## bad    ['data/college.txt' -> 'data/College.txt']
       
       Paths should be to files within the project directory.
       Filepaths are case-sensitive on deployment server.

tests/testthat/shinyapp-with-absolute-paths/server.R

@@ -28,8 +28,8 @@ shinyServer(function(input, output) {
     hist(x, breaks = bins, col = "darkgray", border = "white")
 
     ## read a csv file
-    file <- read.csv("college.txt") ## bad
-    file <- read.csv("College.txt") ## okay
+    file <- read.csv("data/college.txt") ## bad
+    file <- read.csv("data/College.txt") ## okay
 
     ## don't warn about absolute paths that could be URL query paths
     file <- paste("/applcations")

tests/testthat/test-linters.R

@@ -8,35 +8,17 @@ test_that("linter warns about absolute paths and relative paths", {
 
   absPathLintedIndices <- result[["server.R"]]$absolute.paths$indices
   expect_identical(as.numeric(absPathLintedIndices), 15)
-
-  relPathLintedIndices <- result[["server.R"]]$invalid.relative.paths$indices
-  expect_identical(as.numeric(relPathLintedIndices), 16)
 })
 
-test_that("badRelativePaths identifies bad paths correctly", {
-
-  path <- "R/test.R"
-  ok <- "file.path('../inst/include')"
-  expect_false(badRelativePaths(ok, path = path))
-
-  bad <- "file.path('../../elsewhere')"
-  expect_true(badRelativePaths(bad, path = path))
-
-  ok <- "'../foo', '../bar', '../baz'"
-  expect_false(badRelativePaths(ok, path = path))
-
-})
-
-
 test_that("The linter identifies files not matching in case sensitivity", {
-  result <- lint("shinyapp-with-absolute-paths")
+  result <- lint(test_path("shinyapp-with-absolute-paths"))
   server.R <- result[["server.R"]]
   filepath.capitalization <- server.R[["filepath.capitalization"]]
   expect_equal(as.integer(filepath.capitalization$indices), 31)
 })
 
 test_that("The linter identifies files with Markdown links not matching in case sensitivity", {
-  result <- lint("test-rmd-bad-case")
+  result <- lint(test_path("test-rmd-bad-case"))
   index.Rmd <- result[["index.Rmd"]]
   filepath.capitalization <- index.Rmd[["filepath.capitalization"]]
   expect_equal(as.integer(filepath.capitalization$indices), 29)

tests/testthat/test-rmd-bad-case/index.Rmd

@@ -2,15 +2,15 @@
 title: "rmarkdown with bad link case"
 ---
 
-One of the embedded images uses `rstudio.svg` even though the on-disk filename
-is `RStudio.svg`. This will render fine on a Mac, for example, but will err
+One of the embedded images uses `img/rstudio.svg` even though the on-disk filename
+is `img/RStudio.svg`. This will render fine on a Mac, for example, but will err
 when rendered on Linux.
 
 The Mac filesystem is case-preserving, but case-insensitive. The Linux
 filesystem is case-sensitive.
 
 The IDE and `rsconnect::deployDoc` attempt to deploy the as-linked filenames.
-With both `rstudio.svg` and `RStudio.svg` links, the bundle has two copies of
+With both `img/rstudio.svg` and `img/RStudio.svg` links, the bundle has two copies of
 the SVG!
 
 You can use `rsconnect::deployApp` directly to see the lint error in the R
@@ -19,16 +19,16 @@ console:
 ```r
 rsconnect::deployApp(
   appDir = getwd(), 
-  appFiles = c("index.Rmd", "RStudio.svg"))
+  appFiles = c("index.Rmd", "img/RStudio.svg"))
 ```
 
 And now, the links!
 
 This one (line 29) is all lower-case and should trigger the error.
 
-![](rstudio.svg)
+![](img/rstudio.svg)
 
 This one is mixed-case and matches the on-disk filename.
 
-![](RStudio.svg)
+![](img/RStudio.svg)