Write a linter for R-side things in CRAN_release

Open
#4,190 5 comments 2 reactions 1 assignee View on GitHub

@MichaelChirico is already working on this.

Since Jan 22, 2020.

Assessment

This issue has not been assessed yet.

Description

code-quality internals

A few things in CRAN_release would be better served by a linting script rather than relying on regex (accuracy can also be improved):

At a glance I think these can be migrated there:

# Ensure all .Call's first argument are unquoted.
grep "[.]Call(\"" ./R/*.R

# No T or F symbols in tests.Rraw. 24 valid F (quoted, column name or in data) and 1 valid T at the time of writing
grep -n "[^A-Za-z0-9]T[^A-Za-z0-9]" ./inst/tests/tests.Rraw
grep -n "[^A-Za-z0-9]F[^A-Za-z0-9]" ./inst/tests/tests.Rraw

# All integers internally should have L suffix to avoid lots of one-item coercions
# Where 0 numeric is intended we should perhaps use 0.0 for clarity and make the grep easier
# 1) tolerance=0 usages in setops.R are valid numeric 0, as are anything in strings
# 2) leave the rollends default using roll>=0 though; comments in PR #3803
grep -Enr "^[^#]*(?:\[|==|>|<|>=|<=|,|\(|\+)\s*[-]?[0-9]+[^0-9L:.e]" R | grep -Ev "stop|warning|tolerance"

# Never use ifelse. fifelse for vectors when necessary (nothing yet)
 grep -Enr "\bifelse" R

# No system.time in main tests.Rraw. Timings should be in benchmark.Rraw
grep -n "system[.]time" ./inst/tests/tests.Rraw

Can also probably handle the rest of the stuff in our mini Style Guide:

Coding Style
A few minor points of style that you should adhere to in your PR:

Use = for assignment (not <-); see here, here, here and here
Spacing
2-space indentation
No trailing white space
In tests use long lines and vertical alignment to easily spot small differences
Argument spacing style: fun(arg1=value1, arg2=val2, ...)
Add a whitespace between if and opening bracket, also before opening curly bracket: if (condition) {
Use L suffix for integer; i.e. x[1L] not x[1] (to save coercion)
Use error(fmt, arg1, arg2, ...) not error(paste(...)) or error(sprintf(...)) as per Writing R Extensions#1.7 point 2
Dominant language
R
Stars
3.9k
Forks
1.1k
Avg merge
14h 4m
Merged PRs (30d)
4

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Rdatatable/data.table

All issues in Rdatatable/data.table

Similar issues

More R issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.