Skip to content

feat(r): add R-specific code smell detectors#450

Merged
peteromallet merged 3 commits intopeteromallet:mainfrom
sims1253:feat/r-code-smell-detectors
Mar 19, 2026
Merged

feat(r): add R-specific code smell detectors#450
peteromallet merged 3 commits intopeteromallet:mainfrom
sims1253:feat/r-code-smell-detectors

Conversation

@sims1253
Copy link
Copy Markdown
Contributor

Summary

Adds 10 R-specific code smell checks to detect common R anti-patterns:

Smell Description
setwd() Changes global working directory
<<- Global assignment operator
attach() Attaches data to search path
rm(list=ls()) Clears entire workspace
browser()/debug() Debug leftovers
T/F Ambiguous boolean shortcuts
1:n() Off-by-one risk with empty vectors
options(stringsAsFactors) Deprecated pattern
library() in functions Side effects in functions

Also extends the generic language framework with custom_phases support, allowing any generic
plugin to inject language-specific detector phases without converting to a full plugin.

Files changed

  • desloppify/languages/_framework/generic_support/core.py (+5 lines)
  • desloppify/languages/_framework/generic_support/registration.py (+5 lines)
  • desloppify/languages/r/__init__.py (+7 lines)
  • desloppify/languages/r/detectors/__init__.py (new)
  • desloppify/languages/r/detectors/smells.py (+150 lines)
  • desloppify/languages/r/detectors/smells_catalog.py (+70 lines)
  • desloppify/languages/r/phases_smells.py (+25 lines)
  • desloppify/languages/r/tests/__init__.py (new)
  • desloppify/languages/r/tests/test_r_smells.py (+161 lines)

Testing

Includes comprehensive test coverage for all 10 smell detectors.

Dependencies

None - this is a foundational PR that other R enhancements can build on.

@peteromallet
Copy link
Copy Markdown
Owner

Really like the detector coverage here — the 10 detectors are well-chosen and the test suite is solid for the happy paths. A few things to fix before I can merge:

  1. _detect_library_in_function brace tracking — the current logic increments fn_depth for every { on any line, not just function definitions. This means any bare {} block (if/for/while) outside a function will set fn_depth > 0 and cause false positives for top-level library() calls that follow. Also opens appears to be computed but unused. Could you rework this to only track function-scoped braces?

  2. _strip_r_comments doesn't handle strings# inside string literals (e.g. x <- "value #1") gets stripped. The docstring says 'preserving string literals' but the regex #[^\n]* doesn't account for them.

  3. File discovery bypasses framework_find_r_files rolls its own discovery with hardcoded exclusions instead of using base/source_discovery.py. This means it won't respect project-configured exclusion patterns. Would be great to use the framework's discovery instead.

Happy to help if you want to pair on any of these. The detectors themselves are exactly what R support needs.

@sims1253
Copy link
Copy Markdown
Contributor Author

I'll take a look tomorrow.

@sims1253 sims1253 force-pushed the feat/r-code-smell-detectors branch from 9ef9846 to e1ea153 Compare March 17, 2026 16:14
Add 10 R-specific smell checks: setwd(), <<- global assign, attach(),
rm(list=ls()), browser()/debug() leftovers, T/F ambiguity, 1:n()
off-by-one, options(stringsAsFactors), and library() inside functions.

Also adds custom_phases support to GenericLangOptions so generic plugins
can inject language-specific phases without converting to full plugins.
- Fix _strip_r_comments to properly preserve string literals by using
  placeholder substitution before stripping comments
- Fix _detect_library_in_function to only track function-scoped braces
  by finding function definitions and matching their brace pairs,
  eliminating false positives from if/for/while blocks at top level
- Replace custom _find_r_files with framework's find_source_files to
  respect project-configured exclusion patterns
- Add tests for the fixes: hash in strings, library in non-function
  blocks, nested functions
Replace manual brace tracking with tree-sitter AST parsing for more
accurate detection of library()/require() calls inside function bodies.
Includes fallback to regex-based detection when tree-sitter is unavailable.

Benefits:
- Properly handles nested functions, strings, and edge cases
- Uses existing R_SPEC tree-sitter configuration
- Deduplicates matches in nested function scenarios
@sims1253 sims1253 force-pushed the feat/r-code-smell-detectors branch from 3372d8a to 7062c09 Compare March 18, 2026 20:31
@peteromallet peteromallet merged commit dcca79a into peteromallet:main Mar 19, 2026
7 checks passed
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.

2 participants