Skip to content

Add lint CI workflow and clean up package lints - #1117

Open
mattfidler wants to merge 1 commit into
mainfrom
lint-ci
Open

Add lint CI workflow and clean up package lints#1117
mattfidler wants to merge 1 commit into
mainfrom
lint-ci

Conversation

@mattfidler

Copy link
Copy Markdown
Member

Summary

  • Adds .github/workflows/lint.yaml: runs lintr::lint_package() on pushes to main and on pull requests, failing the job on any lint (LINTR_ERROR_ON_LINT=true). Matches the setup style of R-CMD-check.yaml (installs the package plus the nlmixr2 GitHub dependencies so object_usage-style linters could be enabled later).
  • Brings the package to zero lints so the gate is green from the first run (verified locally with lintr::lint_package()).

Lint configuration (.lintr)

The package had ~11,000 lints, most of which cannot be mechanically fixed (3,548 are object names that would require renaming exported API). Following the existing precedent in .lintr of disabling high-count linters with their violation counts:

  • The gate covers package code in R/; tests/, inst/, vignettes/, and generated files (RcppExports.R, rxrandomui.R, parseFuns.R, rxSyntaxFunctions.R, rxode2_md5.R, dfIni.R) are excluded for now.
  • Disabled with counts, to be ratcheted down later: return_linter (533; codebase style is explicit return()), indentation (292), commented_code (147), line_length (112), object_usage (NSE/symengine false positives), brace (78).
  • Name regexes now accept the conventions actually in use: dotted internals (.var.name, ..lhs env-protocol names), snake_case S3 methods for other packages' generics (as_tibble.rxEt), and initial-caps names (ggproto objects, RxODE, math notation like .J).
  • cyclocomp(160) and object_length(45) caps with targeted # nolint tags on the handful of existing outliers.

Fixes applied (R/, 44 files)

All position-driven from lintr output (never blind regex over strings), each file parse-verified before writing:

  • removed trailing whitespace / trailing blank lines
  • split compound a; b statements onto separate lines
  • fixed spaces inside/before parentheses
  • expanded T/F to TRUE/FALSE
  • converted = and right-assign (->) statements to <-

Bug fix found by the linter

attr(rxUiGet.thetaUpper, "desc") -> "thetaUpper" (note ->) assigned the attribute's value (NULL) to a stray thetaUpper variable instead of setting the desc attribute; every sibling uses <-. Fixed and noted in NEWS.md.

Testing

  • lintr::lint_package(): 0 lints
  • Package compiles and loads; devtools::test(filter='basic'): 49 pass / 0 fail / 0 warn

🤖 Generated with Claude Code

Adds .github/workflows/lint.yaml running lintr::lint_package() with
LINTR_ERROR_ON_LINT=true on pushes to main and pull requests.

The lint gate covers package code in R/; tests, inst, vignettes, and
generated files are excluded for now.  High-violation style linters are
disabled in .lintr with counts so they can be ratcheted down later.

Fixes applied across R/: trailing whitespace, compound semicolons split
into statements, spaces inside/before parentheses, T/F expanded to
TRUE/FALSE, = and -> assignments converted to <-, and targeted nolint
tags for deliberate names (ggplot2 conventions, C-name mirrors,
checkmate-style .var.name arguments) and known-complex functions.

Also fixes a real bug found by the linter: attr(rxUiGet.thetaUpper,
"desc") used -> instead of <-, so the desc attribute was never set and
a stray thetaUpper variable was created in the namespace.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant