Skip to content

build: adopt Arity and enforce R formatting in CI - #1946

Merged
eitsupi merged 4 commits into
REditorSupport:mainfrom
eitsupi:chore/arity-r-format-lint
Nov 14, 2026
Merged

eitsupi merged 4 commits into
REditorSupport:mainfrom
eitsupi:chore/arity-r-format-lint

Conversation

@eitsupi

@eitsupi eitsupi commented Nov 14, 2026 •

Copy link
Copy Markdown
Member

Summary

TypeScript in this repository is formatted and linted with oxfmt and oxlint, but R code had no formatter, and the only linter was lintr, which takes about 14 seconds locally and needs an R installation with lintr.

This PR adds Arity (formatter and linter for R, shipped as a prebuilt binary through npm) as a dev dependency, so pnpm install is all a contributor needs:

  • pnpm run format:r / pnpm run format:r:check: format R files (about 0.2 s for the whole repository)
  • pnpm run lint:r: lint R files (about 0.3 s)

Changes

  • Dependency and config: arity-cli pinned to 0.29.0 (formatter output depends on the version, same as oxfmt), plus arity.toml.
  • Style: all R files are formatted with line width 100 (the same as the TypeScript sources) and the default indent width of 2. R/ used four spaces and sess/ used two; the four-space setting only existed because indentation_linter(4) was added to keep lintr 3.1 quiet about the existing code (Fix indentation_linter message #1433).
  • lintr: the lintr job still runs, but line_length_linter and indentation_linter are turned off in both .lintr files. Layout is now owned by the formatter. The formatter and those two linters disagree in a few places (multi-line if conditions, long string literals); see the report below.
  • Lint fixes: 12 safe autofixes from arity lint --fix (while (TRUE) → repeat, any(!x) → !all(x), !a == b → a != b, fixed = TRUE for literal patterns).
  • CI: new format and lint-arity jobs in r.yml run next to the lintr job, which is renamed from lint to lint-lintr and otherwise unchanged.
  • Docs: CONTRIBUTING.md describes the new scripts.

Notes for reviewers

  • The formatting commit is mechanical. I compared parse(keep.source = FALSE) of every file before and after: the parsed code is identical except for braces around single-expression bodies (Arity writes function(x) {\n x + 1\n} as function(x) x + 1 when it fits on one line).
  • sess/DESCRIPTION is excluded from formatting (description = false) because Arity reorders its fields and build scripts rewrite that file.
  • These lint rules are ignored for now:
    • undefined-symbol, unused-binding: tinytest files and source()d helpers cannot be resolved statically (lintr's object_usage_linter is off for the same reason).
    • coalesce: suggests %||%, which needs R 4.4.
    • internal-function: ::: is deliberate here.
    • roxygen-param, roxygen-return, shadowed-builtin, string-boundary, unnecessary-nesting: 23 existing findings that are left for a follow-up.
  • The commits are split into config, formatting, lint fixes, and CI. After the squash merge, the merge commit could be listed in a .git-blame-ignore-revs file.

Why Arity and not Air + Jarl

The only reason to prefer Arity is that it installs through npm, which fits how this repository already manages its tooling. Air (formatter) and Jarl (linter) are more widely used, so I compared them on this code base to check that Arity is not a clearly worse choice. It is not: the formatter is close to Air, and the linter has different strengths from Jarl.

Evaluation report: Arity 0.29.0 vs Air 0.13.0 + Jarl 0.7.1 (and feedback for both projects)

Setup

  • 56 R files (about 11.4k lines) from R/, sess/, and scripts/ at 80b16f6
  • Arity 0.29.0 (npm arity-cli), Air 0.13.0, Jarl 0.7.1, lintr 3.4.0, R 4.5.2
  • For the formatter comparison, both tools were configured to match the previous lintr settings (R/: width 120, indent 4; sess/: width 100, indent 2) so that the existing lintr configuration could act as a referee.

Formatter: Arity vs Air

Arity Air
Changed lines (diff +/- lines) 8,257 7,929
Files whose parsed code changed (ignoring single-expression braces) 0 of 56 0 of 56
lintr findings after formatting (previous .lintr) 10 10, at the same places
Idempotent on this code no, 1 file needs a second pass yes
Time for all files 0.2 s 0.15 s
  • Same conflicts with lintr. Both outputs produce the same 10 lintr findings: 8 from indentation_linter on multi-line conditions inside if (, where lintr wants a hanging indent, and 2 from line_length_linter on string literals that cannot be split.
  • Arity output is almost a fixed point for Air. Running Air on Arity's output changes 25 of 13,092 lines (0.2%). Those are Air breaking after %in% in "x" %in% y && ... chains, and Air always adding braces to if (cond) return(x).
  • Stable across versions. Arity 0.19.1 and 0.29.0 (10 releases apart) produce identical output for this repository.

Style differences from Air (about 1,100 lines when going from Air's output to Arity's):

  1. Arity does not keep existing line breaks. Air keeps an argument list expanded when the first argument starts on a new line; Arity collapses it whenever it fits.
  2. Arity removes braces from single-expression function bodies that fit on one line. Nine top-level named functions became one-liners here. There is no option to turn this off.
  3. Arity also formats roxygen comments (roxygen = false disables it) and DESCRIPTION (description = false).

Linter: Arity vs Jarl

Arity Jarl
lintr rules in our .lintr with a lint counterpart 4 (equals-na, seq, true-false-symbol, vector-logic) 7 (those plus assignment, quotes, pipe_consistency)
Findings with defaults 844 340
Largest group undefined-symbol: 759 undesirable_operator: 320
Safe autofixes applied 12, all correct 12, one changes behaviour
Time for all files 0.3 s 0.13 s
  • Neither tool replaces lintr for the rules we have enabled, and both need configuration here. Layout rules are left to a formatter by both.
  • Arity noise: 749 of the 759 undefined-symbol findings are tinytest's expect_* functions in sess/inst/tinytest/; the rest are helpers loaded with source(). unused-binding (27) flags functions that are used from other files.
  • Jarl noise: undesirable_operator is 283 ::: and 37 <<-. Most ::: are the sess tests calling sess internals; Arity recognises the package from DESCRIPTION and only reports ::: into other packages (6).
  • With those groups turned off, Arity reports 58 findings. None is a runtime bug. Both tools found the same unused helper (namespace_has in sess/R/rstudioapi.R).

Feedback for Arity

  1. Non-idempotent formatting. The first pass expands the call, the second pass collapses it again; arity format --verify also reports it. Minimal input:

    saved <- lapply(fields, function(name) {
      list(exists = exists(name, envir = state, inherits = FALSE),
           value = if (exists(name, envir = state, inherits = FALSE)) get(name, state) else NULL)
    })

    First pass:

    saved <- lapply(
      fields,
      function(name) {
        list(
          ...
        )
      }
    )

    Second pass:

    saved <- lapply(fields, function(name) {
      list(
        ...
      )
    })
  2. Config discovery follows the working directory, not the file. The README says arity.toml is "discovered by walking up from each file's directory", but arity format R run from the repository root does not read R/arity.toml. Only a config in the working directory or --config is used.

  3. No per-file or per-directory rule selection. ignore is global and exclude skips whole files, so undefined-symbol cannot be turned off for test directories only. A way to declare names attached by a test framework (tinytest, testthat) would remove most of our findings.

  4. seq misses seq(length(x)), which lintr and Jarl report.

  5. Layout depends on the input in one case. x <- if (a) b else c stays on one line if it was written on one line and stays expanded if it was written expanded, so the output is not fully determined by the syntax tree.

  6. --output json prints nothing when there are no findings, rather than [].

  7. An option to keep braces on single-expression function bodies would make adoption easier for projects coming from Air.

Feedback for Jarl

  1. A safe fix for class_equals changes behaviour. In sess/R/handlers.R:

    - if (isS4(object) || !identical(class(object), "data.frame")) return(FALSE)
    + if (isS4(object) || !inherits(object, "data.frame")) return(FALSE)

    The original deliberately accepts only objects whose class is exactly "data.frame". For an object with class(object) <- c("custom", "data.frame"), the original returns FALSE at this guard and the fixed code continues past it. identical(class(x), "y") is an exact-class test and not equivalent to inherits(), so this fix should not be marked safe.

  2. implicit_assignment reports assignments inside on.exit() (7 findings, such as on.exit(state$con <- old, add = TRUE)). Moving the assignment out of the call, as suggested, would change when it runs. on.exit could be skipped by default.

  3. undesirable_operator reports ::: into the package's own namespace in its own tests (277 of 283 here). Skipping pkg::: when pkg is the package being linted would remove most of the default noise for packages.

  4. pipe_consistency is silent without a minimum R version. When the rule is selected explicitly but no R version is known, it reports nothing and gives no hint why; --min-r-version 4.2.0 makes it report.

Limits of this comparison

  • One code base, one platform (Linux x64). Installation on other platforms was not tested.
  • The repository's test suites were not used to compare the tools; the equivalence claim rests on the parse comparison.
  • Rule coverage was compared against the lintr rules this repository enables, not against each tool's full rule list.

@eitsupi eitsupi changed the title chore: format and lint R code with Arity build: adopt Arity and enforce R formatting in CI Nov 14, 2026
Add arity-cli as a pinned dev dependency with format:r, format:r:check and
lint:r scripts. Layout is now owned by the formatter, so drop the line
length and indentation linters from the lintr configuration.
Mechanical change from `pnpm run format:r`. The parsed code is unchanged
apart from braces around single-expression bodies.
Apply the safe autofixes (repeat, outer-negation, comparison-negation,
fixed-regex).
Add format and lint-arity jobs next to the lintr job, which is renamed to
lint-lintr, and document the new scripts for contributors.
@eitsupi
eitsupi force-pushed the chore/arity-r-format-lint branch from 9af58b5 to 3c54a6e Compare November 14, 2026 07:05
@eitsupi
eitsupi marked this pull request as ready for review November 14, 2026 07:07
@eitsupi

eitsupi commented Nov 14, 2026

Copy link
Copy Markdown
Member Author

@etiennebacher @jolars The difference between Jarl and Arity, as described in the PR body, might be interesting.

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 3c54a6e6f732104ca8b56619771c9be342c6fdff. No blocking findings.

I independently compared the formatting commit across all 58 R files: the parsed expressions match after normalizing single-expression braces. I also inspected the separate autofix commit and the dependency, configuration, and workflow changes. The checked-in tree passes both an uncached formatting check and arity format --verify, so the non-idempotence noted in the evaluation does not prevent this head from satisfying CI.

Local validation on macOS:

  • Frozen-lockfile installation of Arity 0.29.0; R format, verification, and Arity lint checks.
  • Retained lintr checks for both R/ and sess/.
  • Extension build and TypeScript test compilation.
  • Source identity/bootstrap, manual attach, tool library/consent, jgd preparation, and rstudioapi preparation tests.
  • The PR's bundled sess installed into a temporary library: all 1,341 package test results passed.

All 13 hosted checks are also successful, including Node and extension tests on Linux, macOS, and Windows.

@eitsupi
eitsupi merged commit ada9a6c into REditorSupport:main Nov 14, 2026
13 checks passed
@eitsupi
eitsupi deleted the chore/arity-r-format-lint branch November 14, 2026 09:01
@jolars

jolars commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Thanks for the feedback! I'll take a look at the idempotency issue and other issues.

jolars added a commit to jolars/arity that referenced this pull request Nov 14, 2026
Expose the braced fallback through `Ir::FlatChoice` so a trailing function
can hug the call prefix on the first formatting pass. Keep leading
arguments flat during hug measurement and cover conditional bodies across
line widths, indentation sizes, lambdas, and comments.

Refs REditorSupport/vscode-R#1946
jolars added a commit to jolars/arity that referenced this pull request Nov 14, 2026
@jolars

jolars commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

An option to keep braces on single-expression function bodies would make adoption easier for projects coming from Air.

I think I am actually going to change the current behavior, even without a knob, to always brace (and expand), because this has been chafing at me quite a bit recently. Is this what you want, or are you looking to still keep short functions on one line, but with braces?

@eitsupi

eitsupi commented Nov 14, 2026

Copy link
Copy Markdown
Member Author

Thanks, I think the change is fine.

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.

3 participants