Repository navigation
Metrics: robustness lints - #45
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 25.86% 33.41% +7.55%
==========================================
Files 35 37 +2
Lines 1481 1661 +180
==========================================
+ Hits 383 555 +172
- Misses 1098 1106 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Okay, I've made the following changes based on our discussions:
So now we have the following new features:
|
llrs-roche
left a comment
There was a problem hiding this comment.
It looks good, I think it is time we merge this. Just one (last) suggestion about using lintr without checking if the package is present: I think all lint data sources should check that lintr is installed with the suggests = "lintr".
ccdd612 to
aaa83b4
Compare
|
Thanks @llrs-roche ! I added the guard. Additionally, I added lintr to suggest field of metrics to enforce check at higher level as well. |
Summary
This PR adds a new metric,
correctness_lint_count, which uses{lintr}to count code patterns that are likely to cause bugs or incorrect behaviour. The metric counts lints from three lintr tag categories —correctness,common_mistakes, androbustness— and exposes the full lint results as auxiliary data viacorrectness_lints.The intent is to capture functional quality: real programming mistakes, silent bugs, and portability hazards — not code style, naming conventions, or formatting preferences (those are left to
{lintr}'sstyletag, which is intentionally excluded).This closes #38 .
Discussion
Including
correctnessandcommon_mistakestagsThe scope of lints I propose is a bit different from the one discussed in #38. Namely, besides robustness linters I would argue that we should also care about correctness and [https://lintr.r-lib.org/reference/common_mistakes_linters.html](common mistake) lints.
The
correctnesscatches unambiguous bugs like duplicated arguments, missing arguments or mismatched format strings insprintf(). Thecommon_mistakescatches errors like using single&or|in conditional expressions or comparison toNAusing==.Filtering out style linters (or other potentially opinionated linters)
Current implementation is the simplest possible -- we just check lints from each of the categories and report them as is. There are a few caveats like the fact that some of those lints are still classified as style lints as opposed to warning or error types. We could theoretically filter them out -- either by filtering the lints themselves or excluding the linters from the set we are using (so instead of selecting all filters with given tags we could pick them manually). This would exclude e.g. the
undesirable_function_linterwhich just checks usage of functions from given predefined set. And this actually connects us to another question,Configurable linters
Are we happy with the predefined configuration of some of those linters? Namely:
undesirable_function_linter(in robustness): flags functions likesapply,attach,setwd,source,options. Some of these are genuinely hazardous in package code (attach,setwd), while others (sapply) are more of a stylistic preference. I feel like we cannot find one-size-fits-all solution so we should either use base set (as we currently do) or exclude the linter entirely.undesirable_operator_linter(in robustness): flags<<-,:::, and->>. The use of<<-and:::can be legitimate in some contexts (e.g.<<-in{R6}or reference classes,:::when accessing internal functions deliberately; well, we're using them on our own in the{val.meter}). Should these be excluded or configured?backport_linter(in robustness): without an explicitminimum_r_versionconfigured, this linter defaults to the running R version and effectively flags nothing. Should we expose a package option so users can set a minimum R version for their assessment?Counting fraction of lint-free lines
Finally, I would like to have some fraction-like metric accompanying the absolute value provided by the already implemented linter. The one I am imagining is: