[Bioc-devel] Aliasing `]` breaks BiocCheck::BiocCheck() version 1.18.0

[Tierney, Luke]

Modifying findGlobals to return empty variable lists for non-closure
functions was an easy change so an updated codetools is ready to
submit when the CRAN queue opens again in the new year.

Best,

luke

On Thu, 20 Dec 2018, luke-tierney using uiowa.edu wrote:

> That's where the error is signaled, but the issue is in
>
>> 4: lapply(objs, FUN = function(obj) {
>>       value = env[[obj]]
>>       if (is.function(value))
>>           findGlobals(value)
>>       else character(0)
>>   })
>> 3: findLogicalRdir(pkgname, c("T", "F"))
>
> Change is.function(value) to typeof(value) == "closure" and you should be OK.
>
> Best,
>
> luke
>
> On Thu, 20 Dec 2018, Martin Morgan wrote:
>
>> this comes from `findGlobals()`
>> 
>>> foo <- `[`
>>> findGlobals(foo)
>> Error in makeUsageCollector(fun, ...) : only works for closures
>>> traceback()
>> 4: stop("only works for closures")
>> 3: makeUsageCollector(fun, ...)
>> 2: collectUsage(fun, enterGlobal = enter)
>> 1: findGlobals(foo)
>> 
>> In the bigger context it is in code that looks for poor 'coding practice', 
>> in this particular case looking for use of T / F rather than TRUE / FALSE, 
>> where the logic is to parse each function for use of global variables, and 
>> then to search for T / F amongst those.
>> 
>> The full traceback when run on the package at 
>> https://github.com/mtmorgan/PkgA/tree/BiocCheck-sbs
>> 
>> * Checking coding practice...
>> Error in makeUsageCollector(fun, ...) : only works for closures
>>> traceback()
>> 9: stop("only works for closures")
>> 8: makeUsageCollector(fun, ...)
>> 7: collectUsage(fun, enterGlobal = enter)
>> 6: findGlobals(value)
>> 5: FUN(X[[i]], ...)
>> 4: lapply(objs, FUN = function(obj) {
>>       value = env[[obj]]
>>       if (is.function(value))
>>           findGlobals(value)
>>       else character(0)
>>   })
>> 3: findLogicalRdir(pkgname, c("T", "F"))
>> 2: checkCodingPractice(package_dir, parsedCode, package_name)
>> 1: BiocCheck::BiocCheck(".")
>> 
>> Martin
>> 
>> On 12/19/18, 8:32 AM, "Bioc-devel on behalf of Tierney, Luke" 
>> <bioc-devel-bounces using r-project.org on behalf of luke-tierney using uiowa.edu> 
>> wrote:
>>
>>    codetools already checks only closures in checkUsageENv and hande
>>    checkUsagePackage, so this is anissue on the Bioc side.
>>
>>    Best,
>>
>>    luke
>>
>>    On Tue, 18 Dec 2018, Tierney, Luke wrote:
>>
>>    > Codetools should probably be ignoring those. Will have a look
>>    >
>>    > Sent from my iPhone
>>    >
>>    >> On Dec 18, 2018, at 6:54 AM, Shepherd, Lori 
>> <Lori.Shepherd using RoswellPark.org> wrote:
>>    >>
>>    >> Can you please open an issue for this so we don't lose track of it -
>>    >>
>>    >> https://github.com/Bioconductor/BiocCheck/issues
>>    >>
>>    >>
>>    >>
>>    >> Lori Shepherd
>>    >>
>>    >> Bioconductor Core Team
>>    >>
>>    >> Roswell Park Cancer Institute
>>    >>
>>    >> Department of Biostatistics & Bioinformatics
>>    >>
>>    >> Elm & Carlton Streets
>>    >>
>>    >> Buffalo, New York 14263
>>    >>
>>    >> ________________________________
>>    >> From: Bioc-devel <bioc-devel-bounces using r-project.org> on behalf of 
>> Shian Su <su.s using wehi.edu.au>
>>    >> Sent: Monday, December 17, 2018 8:34:10 PM
>>    >> To: bioc-devel
>>    >> Subject: [Bioc-devel] Aliasing `]` breaks BiocCheck::BiocCheck() 
>> version 1.18.0
>>    >>
>>    >> Hi all,
>>    >>
>>    >> If you put
>>    >>
>>    >> foo <- `[`
>>    >>
>>    >> Somewhere in a package, it will trigger
>>    >>
>>    >> Error in makeUsageCollector(fun, ...) : only works for closures
>>    >>
>>    >> In BiocCheck::BiocCheck() (version 1.18.0). This comes from
>>    >>
>>    >> if (typeof(fun) != "closure")
>>    >>        stop("only works for closures")
>>    >>
>>    >> In codetools::makeUsageCollector(), but
>>    >>
>>    >>> typeof(`[`)
>>    >> ## "special"
>>    >>
>>    >> Not that it matters for my use-case because I had discovered 
>> magrittr???s extract alias, but it might be an edge case worth covering, 
>> especially since the error message is so cryptic.
>>    >>
>>    >> Kind regards,
>>    >> Shian Su
>>    >>
>>    >> _______________________________________________
>>    >>
>>    >> The information in this email is confidential and 
>> intend...{{dropped:29}}
>>    >>
>>    >> _______________________________________________
>>    >> Bioc-devel using r-project.org mailing list
>>    >> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>    > _______________________________________________
>>    > Bioc-devel using r-project.org mailing list
>>    > https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>
>>    --
>>    Luke Tierney
>>    Ralph E. Wareham Professor of Mathematical Sciences
>>    University of Iowa                  Phone:             319-335-3386
>>    Department of Statistics and        Fax:               319-335-3017
>>        Actuarial Science
>>    241 Schaeffer Hall                  email:   luke-tierney using uiowa.edu
>>    Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu
>>
>>    _______________________________________________
>>    Bioc-devel using r-project.org mailing list
>>    https://stat.ethz.ch/mailman/listinfo/bioc-devel
>> 
>> 
>
>

-- 
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   luke-tierney using uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu