From fa05ceb2bf967ddad9230e25879b95084c80b0dc Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 30 Nov 2020 06:13:25 -0500 Subject: [PATCH 1/3] enable commend_code_linter and apply it (#640) --- .lintr | 1 - R/get_source_expressions.R | 20 ++++++++++---------- R/utils.R | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/.lintr b/.lintr index 7632a93a7..d561e3c01 100644 --- a/.lintr +++ b/.lintr @@ -1,7 +1,6 @@ linters: with_defaults( # The following TODOs are part of an effort to have {lintr} lint-free (#584) line_length_linter = line_length_linter(120), infix_spaces_linter = NULL, # TODO enable (#594) - commented_code_linter = NULL, # TODO enable (#595) cyclocomp_linter = cyclocomp_linter(29), # TODO reduce to 15 object_name_linter = NULL, # TODO enable (#597) spaces_inside_linter = NULL, # TODO enable (#598) diff --git a/R/get_source_expressions.R b/R/get_source_expressions.R index 97e098de0..b0063d3ba 100644 --- a/R/get_source_expressions.R +++ b/R/get_source_expressions.R @@ -327,16 +327,16 @@ fix_column_numbers <- function(content) { # getParseData() counts 1 tab as a variable number of spaces instead of one: # https://github.com/wch/r-source/blame/e7401b68ab0e032fce3e376aaca9a5431619b2b4/src/main/gram.y#L512 # The number of spaces is so that the code is brought to the next 8-character indentation level e.g: -# "1\t;" -> "1 ;" -# "12\t;" -> "12 ;" -# "123\t;" -> "123 ;" -# "1234\t;" -> "1234 ;" -# "12345\t;" -> "12345 ;" -# "123456\t;" -> "123456 ;" -# "1234567\t;" -> "1234567 ;" -# "12345678\t;" -> "12345678 ;" -# "123456789\t;" -> "123456789 ;" -# "1234567890\t;" -> "1234567890 ;" +# "1\t;" --> "1 ;" +# "12\t;" --> "12 ;" +# "123\t;" --> "123 ;" +# "1234\t;" --> "1234 ;" +# "12345\t;" --> "12345 ;" +# "123456\t;" --> "123456 ;" +# "1234567\t;" --> "1234567 ;" +# "12345678\t;" --> "12345678 ;" +# "123456789\t;" --> "123456789 ;" +# "1234567890\t;" --> "1234567890 ;" fix_tab_indentations <- function(source_file) { pc <- getParseData(source_file) diff --git a/R/utils.R b/R/utils.R index f95702758..607f5ce74 100644 --- a/R/utils.R +++ b/R/utils.R @@ -194,9 +194,9 @@ escape_chars <- c( "\\f" = "\f", # form feed "\\v" = "\v" # vertical tab # dynamically-added: - #"\\'" = "'", # ASCII apostrophe - #"\\\"" = "\"", # ASCII quotation mark - #"\\`" = "`" # ASCII grave accent (backtick) + #"\\'" --> "'", # ASCII apostrophe + #"\\\"" --> "\"", # ASCII quotation mark + #"\\`" --> "`" # ASCII grave accent (backtick) ) unescape <- function(str, q="`") { From 34cda089b1418c11ba23ae618384221fe6bf9d1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 30 Nov 2020 14:11:33 +0000 Subject: [PATCH 2/3] Feature GitHub actions ci (#574) Co-authored-by: AshesITR --- .github/.gitignore | 1 + .github/workflows/R-CMD-check.yaml | 100 +++++++++++++++++++++++++++ .github/workflows/lint.yaml | 47 +++++++++++++ .github/workflows/test-coverage.yaml | 49 +++++++++++++ .travis.yml | 25 ------- NEWS.md | 1 + README.md | 2 +- 7 files changed, 199 insertions(+), 26 deletions(-) create mode 100644 .github/.gitignore create mode 100644 .github/workflows/R-CMD-check.yaml create mode 100644 .github/workflows/lint.yaml create mode 100644 .github/workflows/test-coverage.yaml delete mode 100644 .travis.yml diff --git a/.github/.gitignore b/.github/.gitignore new file mode 100644 index 000000000..2d19fc766 --- /dev/null +++ b/.github/.gitignore @@ -0,0 +1 @@ +*.html diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml new file mode 100644 index 000000000..1440c23ce --- /dev/null +++ b/.github/workflows/R-CMD-check.yaml @@ -0,0 +1,100 @@ +on: + push: + branches: + - main + - master + pull_request: + branches: + - main + - master + +name: R-CMD-check + +jobs: + R-CMD-check: + runs-on: ${{ matrix.config.os }} + + name: ${{ matrix.config.os }} (${{ matrix.config.r }}) + + strategy: + fail-fast: false + matrix: + config: + - {os: macOS-latest, r: 'release'} + - {os: windows-latest, r: 'release'} + - {os: windows-latest, r: '3.6'} + - {os: ubuntu-16.04, r: 'devel', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest", http-user-agent: "R/4.0.0 (ubuntu-16.04) R (4.0.0 x86_64-pc-linux-gnu x86_64 linux-gnu) on GitHub Actions" } + - {os: ubuntu-16.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest"} + - {os: ubuntu-16.04, r: 'oldrel', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest"} + - {os: ubuntu-16.04, r: '3.5', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest"} + - {os: ubuntu-16.04, r: '3.4', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest"} + - {os: ubuntu-16.04, r: '3.3', rspm: "https://packagemanager.rstudio.com/cran/__linux__/xenial/latest"} + + env: + R_REMOTES_NO_ERRORS_FROM_WARNINGS: true + RSPM: ${{ matrix.config.rspm }} + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + + steps: + - uses: actions/checkout@v2 + + - uses: r-lib/actions/setup-r@v1 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config.http-user-agent }} + + - uses: r-lib/actions/setup-pandoc@v1 + + - name: Query dependencies + run: | + install.packages('remotes') + saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) + writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version") + shell: Rscript {0} + + - name: Cache R packages + if: runner.os != 'Windows' + uses: actions/cache@v2 + with: + path: ${{ env.R_LIBS_USER }} + key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }} + restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1- + + - name: Install system dependencies + if: runner.os == 'Linux' + run: | + while read -r cmd + do + eval sudo $cmd + done < <(Rscript -e 'writeLines(remotes::system_requirements("ubuntu", "16.04"))') + + - name: Install dependencies + run: | + remotes::install_deps(dependencies = TRUE) + remotes::install_cran("rcmdcheck") + shell: Rscript {0} + + - name: Session info + run: | + options(width = 100) + pkgs <- installed.packages()[, "Package"] + sessioninfo::session_info(pkgs, include_base = TRUE) + shell: Rscript {0} + + - name: Check + env: + _R_CHECK_CRAN_INCOMING_: false + run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning", check_dir = "check") + shell: Rscript {0} + + - name: Show testthat output + if: always() + run: find check -name 'testthat.Rout*' -exec cat '{}' \; || true + shell: bash + + - name: Upload check results + if: failure() + uses: actions/upload-artifact@main + with: + name: ${{ runner.os }}-r${{ matrix.config.r }}-results + path: check diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml new file mode 100644 index 000000000..f087e311a --- /dev/null +++ b/.github/workflows/lint.yaml @@ -0,0 +1,47 @@ +on: + push: + branches: + - main + - master + - "**" + pull_request: + branches: + - main + - master + +name: lint + +jobs: + lint: + runs-on: macOS-latest + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v2 + + - uses: r-lib/actions/setup-r@v1 + + - name: Query dependencies + run: | + install.packages('remotes') + saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) + writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version") + shell: Rscript {0} + + - name: Cache R packages + uses: actions/cache@v2 + with: + path: ${{ env.R_LIBS_USER }} + key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }} + restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1- + + - name: Install dependencies + run: | + install.packages(c("remotes")) + remotes::install_deps(dependencies = TRUE) + remotes::install_github("jimhester/lintr") + shell: Rscript {0} + + - name: Lint + run: lintr::lint_package() + shell: Rscript {0} diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml new file mode 100644 index 000000000..156b1002a --- /dev/null +++ b/.github/workflows/test-coverage.yaml @@ -0,0 +1,49 @@ +on: + push: + branches: + - main + - master + - '**' + pull_request: + branches: + - main + - master + +name: test-coverage + +jobs: + test-coverage: + runs-on: macOS-latest + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v2 + + - uses: r-lib/actions/setup-r@v1 + + - uses: r-lib/actions/setup-pandoc@v1 + + - name: Query dependencies + run: | + install.packages('remotes') + saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) + writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version") + shell: Rscript {0} + + - name: Cache R packages + uses: actions/cache@v2 + with: + path: ${{ env.R_LIBS_USER }} + key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }} + restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1- + + - name: Install dependencies + run: | + install.packages(c("remotes")) + remotes::install_deps(dependencies = TRUE) + remotes::install_cran("covr") + shell: Rscript {0} + + - name: Test coverage + run: covr::codecov() + shell: Rscript {0} diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 855736b11..000000000 --- a/.travis.yml +++ /dev/null @@ -1,25 +0,0 @@ -# R for travis: see documentation at https://docs.travis-ci.com/user/languages/r - -language: R -sudo: false -cache: packages - -addons: - apt: - packages: - libicu-dev - -r_github_packages: jimhester/covr - -matrix: - include: - - r: devel - - r: release - after_success: - - Rscript -e 'covr::codecov()' - - R CMD INSTALL $PKG_TARBALL - - Rscript -e 'lintr::lint_package()' - - r: oldrel - - r: 3.4 - - r: 3.3 - - r: 3.2 diff --git a/NEWS.md b/NEWS.md index d57965267..aede5c723 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # lintr (development version) +* Switched CI from Travis to GitHub Actions, using the full tidyverse recommended R CMD check. Code coverage and linting are implemented using separate GitHub Actions workflows (#572, @dragosmg) * `save_cache` will now recursively create the cache directory; this avoids errors that could arise if any parent directories do not exist (#60, @dankessler). * `extract_r_source` handles Rmd containing unevaluated code blocks with named format specifiers (#472, @russHyde) diff --git a/README.md b/README.md index f28551ae4..e53e6fe26 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ # lintr -[![Travis-CI Build Status](https://travis-ci.org/jimhester/lintr.svg?branch=master)](https://travis-ci.org/jimhester/lintr) +[![R build status](https://github.com/jimhester/lintr/workflows/R-CMD-check/badge.svg)](https://github.com/jimhester/lintr/actions) [![codecov.io](https://codecov.io/github/jimhester/lintr/coverage.svg?branch=master)](https://codecov.io/github/jimhester/lintr?branch=master) [![CRAN_Status_Badge](https://www.r-pkg.org/badges/version/lintr)](https://cran.r-project.org/package=lintr) [![Join the chat at https://gitter.im/jimhester-lintr/Lobby](https://badges.gitter.im/jimhester-lintr/Lobby.svg)](https://gitter.im/jimhester-lintr/Lobby?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) From b4e3f72d7709788f6578dbc1d493240ee599828c Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 30 Nov 2020 09:43:33 -0500 Subject: [PATCH 3/3] improve spaces_left_parentheses_linter for some edge cases (#533) Co-authored-by: Jim Hester --- R/spaces_left_parentheses_linter.R | 28 +++++++++++++++---- .../test-spaces_left_parentheses_linter.R | 4 +++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/R/spaces_left_parentheses_linter.R b/R/spaces_left_parentheses_linter.R index b19b430f5..a7335ee94 100644 --- a/R/spaces_left_parentheses_linter.R +++ b/R/spaces_left_parentheses_linter.R @@ -7,11 +7,10 @@ spaces_left_parentheses_linter <- function(source_file) { parsed <- source_file$parsed_content[id, ] - terminal_tokens_before <- - source_file$parsed_content$token[ - source_file$parsed_content$line1 == parsed$line1 & - source_file$parsed_content$col1 < parsed$col1 & - source_file$parsed_content$terminal] + terminal_tokens_before <- with( + source_file$parsed_content, + token[line1 == parsed$line1 & col1 < parsed$col1 & terminal] + ) last_type <- tail(terminal_tokens_before, n = 1) is_function <- length(last_type) %!=% 0L && @@ -24,7 +23,24 @@ spaces_left_parentheses_linter <- function(source_file) { before_operator <- substr(line, parsed$col1 - 1L, parsed$col1 - 1L) non_space_before <- re_matches(before_operator, rex(non_space)) - not_exception <- !(before_operator %in% c("!", ":", "[", "(")) + not_exception <- !(before_operator %in% c("!", ":", "[", "(", "^")) + + # exception for unary - and unary +, #508 + before_operator_idx <- with( + source_file$parsed_content, + col1 == parsed$col1 - 1L & col1 == col2 + ) + is_sibling_expr <- if (any(before_operator_idx)) { + with( + source_file$parsed_content, + token == "expr" & parent == parent[before_operator_idx] + ) + } else { + rep(FALSE, nrow(source_file$parsed_content)) + } + not_exception <- not_exception && + !(before_operator %in% c("-", "+") && + nrow(source_file$parsed_content[is_sibling_expr, ]) == 1L) if (non_space_before && not_exception) { Lint( diff --git a/tests/testthat/test-spaces_left_parentheses_linter.R b/tests/testthat/test-spaces_left_parentheses_linter.R index f4aa489b0..0762add5e 100644 --- a/tests/testthat/test-spaces_left_parentheses_linter.R +++ b/tests/testthat/test-spaces_left_parentheses_linter.R @@ -72,4 +72,8 @@ test_that("returns the correct linting", { expect_lint("if (!(foo && bar || baz)) { foo }", NULL, spaces_left_parentheses_linter) + + expect_lint("x^(y + z)", NULL, spaces_left_parentheses_linter) + + expect_lint("a <- -(b)", NULL, spaces_left_parentheses_linter) })