Mention the specific operator in lint message for vector_logic_linter()

[MichaelChirico]

Closes #2390

[AshesITR]

We could avoid this map by pasting the op text twice instead.

[MichaelChirico]

that feels over-optimized to me, vector look-up map is very fast in general. If anything I'd do strrep(xml_text(bad_expr), 2L) instead, WDYT?

[MichaelChirico]

In any case, the map approach is way faster than sprintf(), and only slower than strrep() in extreme cases where there's a huge # of lints in the same expression:

op_and = "AND"
op_or = "||"
both = sample(c(op_and, op_or), 1e3, TRUE)
map = c(AND = "&&", or = "||")

str_and = "&"
str_or = "|"
str_both = sample(c(str_or, str_and), 1e3, TRUE)

microbenchmark(times = 1e4,
  map[op_and], sprintf("%1$s%1$s", str_and), strrep(str_and, 2L),
  map[op_or], sprintf("%1$s%1$s", str_or), strrep(str_or, 2L),
  map[both], sprintf("%1$s%1$s", str_both), strrep(str_both, 2L))
Unit: nanoseconds
                          expr    min     lq        mean median       uq      max neval cld
                   map[op_and]    421    521    845.0018    581    721.0    98771 10000 a  
  sprintf("%1$s%1$s", str_and)    762    861   1343.1966    941   1150.0   111471 10000 a  
           strrep(str_and, 2L)    710    812   1307.5081    901   1111.0    99381 10000 a  
                    map[op_or]    591    730   2214.5031    831   1061.0 10363474 10000 a  
   sprintf("%1$s%1$s", str_or)    770    870   1330.1354    941   1150.0    98651 10000 a  
            strrep(str_or, 2L)    710    821   1318.9603    901   1121.0    61772 10000 a  
                     map[both]  45481  49541  68498.7497  52581  58176.0 10830374 10000  b 
 sprintf("%1$s%1$s", str_both) 125141 126591 141180.6610 127491 134721.5 10666224 10000   c
          strrep(str_both, 2L)  52001  55821  64786.3580  56250  60006.5 10300914 10000  b

I find the look-up approach perfectly readable, but maybe strrep() is about the same.

[AshesITR]

Your benchmark is only valid if you copy the full lint message into the map 😉
Anyway, I'd prefer a self-contained approach where all parts of the lint message are assembled in the same place, either outside or inside the linter function.

[MichaelChirico]

Changed to strrep().

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5072724) 98.68% compared to head (cec7266) 98.68%.

❗ Current head cec7266 differs from pull request most recent head f42a474. Consider uploading reports for the commit f42a474 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2398   +/-   ##
=======================================
  Coverage   98.68%   98.68%           
=======================================
  Files         126      126           
  Lines        5713     5714    +1     
=======================================
+ Hits         5638     5639    +1     
  Misses         75       75           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.